From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp36.i.mail.ru (smtp36.i.mail.ru [94.100.177.96]) (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 DFC69469719 for ; Thu, 13 Feb 2020 03:08:46 +0300 (MSK) References: <20200206123114.8010-1-gorcunov@gmail.com> <20200206123114.8010-3-gorcunov@gmail.com> From: Vladislav Shpilevoy Message-ID: <90607bfb-8f31-765f-ae8d-7cdb34598b0c@tarantool.org> Date: Thu, 13 Feb 2020 01:08:45 +0100 MIME-Version: 1.0 In-Reply-To: <20200206123114.8010-3-gorcunov@gmail.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH v6 2/2] fiber: leak slab if unable to bring prots back List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cyrill Gorcunov , tml Thanks for the patch! On 06/02/2020 13:31, Cyrill Gorcunov wrote: > In case if we unable to revert guard page back to > read|write we should never use such slab again. > > Initially I thought of just put panic here and > exit but it is too destructive. I think better > print an error and continue. If node admin ignore > this message then one moment at future there won't > be slab left for use and creating new fibers get > prohibited. > > In future (hopefully near one) we plan to drop > guard pages to prevent VMA fracturing and use > stack marks instead. > > Reviewed-by: Alexander Turenko > Signed-off-by: Cyrill Gorcunov > --- > src/lib/core/fiber.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c > index 57e4cb6ef..a501d846a 100644 > --- a/src/lib/core/fiber.c > +++ b/src/lib/core/fiber.c > @@ -1055,15 +1055,22 @@ fiber_stack_destroy(struct fiber *fiber, struct slab_cache *slabc) > * to setup the original protection back in > * background. > * > + * For now lets keep such slab referenced and > + * leaked: if mprotect failed we must not allow > + * to reuse such slab with PROT_NONE'ed page > + * inside. > + * > * Note that in case if we're called from > * fiber_stack_create() the @mprotect_flags is > * the same as the slab been created with, so > * calling mprotect for VMA with same flags > * won't fail. > */ > - diag_log(); > + say_syserror("fiber: Can't put guard page to slab. " > + "Leak %zu bytes", (size_t)fiber->stack_size); This behaviour actually should be testable, no? You could not test diag, but you should be able to detect a leak. Using slab_cache_used(), for example. Before and after a test. > + } else { > + slab_put(slabc, fiber->stack_slab); > } > - slab_put(slabc, fiber->stack_slab); > } > }