From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id B4E5445C319 for ; Sat, 28 Mar 2020 23:36:47 +0300 (MSK) Date: Sat, 28 Mar 2020 23:30:28 +0300 From: Igor Munkin Message-ID: <20200328203028.GG22874@tarantool.org> References: <50fe58f83ebb4e4971528641e74f99fe2f9fd8f2.1585304087.git.imun@tarantool.org> <20200328163329.GA328@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20200328163329.GA328@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH luajit 1/2] jit: abort trace recording and execution for C API List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Sergey Ostanevich Cc: Vladislav Shpilevoy , tarantool-patches@dev.tarantool.org Sergos, Thanks for you review! On 28.03.20, Sergey Ostanevich wrote: > Hi! > > Thanks for the patch, see below. > On 27 мар 13:47, Igor Munkin wrote: > > JIT recording semantics assumes FFI calls are atomic regarding the > > LuaJIT VM: if the execution exited Lua world through FFI it is not > > re-entering Lua world again. > > > > However, there are ways to break this atomicity via FFI: one can > > re-enter LuaJIT VM via Lua C API inside the C routine called via FFI. As > > a result the following stack mix is created: > > | Lua-FFI -> С routine -> Lua-C API -> Lua VM Fixed typo (cyrillic С letter) to an ASCII C. > > This sort of re-entrancy is explicitly not supported by LuaJIT compiler > > machinery. @mraleph named such layout an "FFI sandwich" and I'm strictly > > against eating it. > > > > E.g. there is a trigger machinery in Tarantool calling Lua functions > > from C space. As a result of FFI function call with a trigger underneath > > the "FFI sandwich" is created. > > > > This changeset introduces the mechanism for Lua-C API callbacks similar > > to the one existing for Lua-FFI: trace recording is aborted whe the > when Thanks, fixed. > > execution re-enters LuaJIT VM. Sandwich detection while mcode execution > > leads to platform panic. > Could you elaborate here in comment on this a little: it means this > particular Lua call to be aborted, fail for the fiber Lua state, or for > the whole Tarantool? > The whole Tarantool finishes its execution with EXIT_FAILURE code and calls tarantool_panic_handler routine prior to the exit. Should I add this remark into commit message? > > > > Relates to tarantool/tarantool#4427 > > > > Co-authored-by: Vyacheslav Egorov > > Co-authored-by: Sergey Ostanevich > > Signed-off-by: Igor Munkin > > --- > > src/lj_api.c | 35 ++++++++++---- > > src/lj_errmsg.h | 1 + > > test/gh-4427-ffi-sandwich/CMakeLists.txt | 1 + > > test/gh-4427-ffi-sandwich/libsandwich.c | 59 ++++++++++++++++++++++++ > > test/gh-4427-ffi-sandwich/test.lua | 26 +++++++++++ > > 5 files changed, 113 insertions(+), 9 deletions(-) > > create mode 100644 test/gh-4427-ffi-sandwich/CMakeLists.txt > > create mode 100644 test/gh-4427-ffi-sandwich/libsandwich.c > > create mode 100644 test/gh-4427-ffi-sandwich/test.lua > > > > diff --git a/src/lj_api.c b/src/lj_api.c > > index a5e2465..c1f53e0 100644 > > --- a/src/lj_api.c > > +++ b/src/lj_api.c > > @@ -76,6 +76,17 @@ static GCtab *getcurrenv(lua_State *L) > > return fn->c.gct == ~LJ_TFUNC ? tabref(fn->c.env) : tabref(L->env); > > } > > > > +static void call(lua_State *L, TValue *base, int nres) { > I would recommend to name the function in a more descriptive way, like > sane_call. IMHO no_trace_call / no_jit_call looks good to me, but feel free to propose the naming you prefer more. > > > + global_State *g = G(L); > > + if (tvref(g->jit_base)) { > > + setstrV(L, L->top++, lj_err_str(L, LJ_ERR_JITCALL)); > > + if (g->panic) g->panic(L); > > + exit(EXIT_FAILURE); > > + } > > + lj_trace_abort(g); /* Never record across Lua VM entrance */ > > + lj_vm_call(L, base, nres); > > +} > > + > > /* -- Miscellaneous API functions ----------------------------------------- */ > > > > LUA_API int lua_status(lua_State *L) > > Also I have a question on perf impact - have you measured any > benchmarks - Lua only, or as part of Tarantool I compared two sysbench logs ([1] and [2]) via vimdiff and see no critical difference. > > Regards, > Sergos [1]: https://gitlab.com/tarantool/tarantool/-/jobs/489420257/raw [2]: https://gitlab.com/tarantool/tarantool/-/jobs/486707332/raw -- Best regards, IM