From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-f66.google.com (mail-lf1-f66.google.com [209.85.167.66]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 2FF7F46970E for ; Tue, 4 Feb 2020 19:08:03 +0300 (MSK) Received: by mail-lf1-f66.google.com with SMTP id l18so12609429lfc.1 for ; Tue, 04 Feb 2020 08:08:03 -0800 (PST) Date: Tue, 4 Feb 2020 19:08:00 +0300 From: Cyrill Gorcunov Message-ID: <20200204160800.GF12445@uranus> References: <20200115170524.20471-1-gorcunov@gmail.com> <20200115170524.20471-3-gorcunov@gmail.com> <20200204154742.GA32754@atlas> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200204154742.GA32754@atlas> Subject: Re: [Tarantool-patches] [PATCH v2 2/2] fiber: exit with panic if we unable to revert guard page List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Konstantin Osipov Cc: tml On Tue, Feb 04, 2020 at 06:47:42PM +0300, Konstantin Osipov wrote: > > --- a/src/lib/core/fiber.c > > +++ b/src/lib/core/fiber.c > > @@ -1041,13 +1041,17 @@ fiber_stack_destroy(struct fiber *fiber, struct slab_cache *slabc) > > * to setup the original protection back in > > * background. > > * > > + * For now lets exit with panic: if mprotect > > + * failed we must not allow to reuse such slab > > + * with PROT_NONE'ed page somewhere inside. > > + * > > somewhere inside its stack area would be more clear. Thanks! > > * 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(); > > + panic_syserror("fiber: Can't put guard page to slab"); > > While the patch itself is LGTM, we need to nail down the cause of > the failure even at the cost of crash, I suspect what we're getting > here is ENOMEM from the kernel. I suspect we have too many > mprotect regions and the kernel runs out of some internal > resources for them. Yes, seems we might get out of VMA when there are too many fibers and system doesn't have enough memory to continue splitting. > I think adding better diagnostics could help us identify the > issue: the failed address, its slab, the total number of fibers (and by > induction mprotected pages). > > I have also discussed the issue with @xemul, and he suggests that > wrong slab alignment could be causing this. True, and we need investigate it. Once we have the "fix" merged in I'll file a bug to investigate this ideas. Thanks a huge, Kostya! > > Finally, we could try to clear mprotect() first, and if it fails, > avoid destroying the fiber and keep it cached for a while more. > We could retry destroying it when kernel has more memory. Yes. I put FIXME for such intelligent exit (I thought about stack slabs only not the complete fibers though). > > These are of course ideas for follow ups.