From: Brian Foster <bfoster@redhat.com> To: Michal Hocko <mhocko@kernel.org> Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>, Xiong Zhou <xzhou@redhat.com>, Christoph Hellwig <hch@infradead.org>, linux-xfs@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org Subject: Re: mm allocation failure and hang when running xfstests generic/269 on xfs Date: Thu, 2 Mar 2017 08:00:09 -0500 [thread overview] Message-ID: <20170302130009.GC3213@bfoster.bfoster> (raw) In-Reply-To: <20170302124909.GE1404@dhcp22.suse.cz> On Thu, Mar 02, 2017 at 01:49:09PM +0100, Michal Hocko wrote: > On Thu 02-03-17 07:24:27, Brian Foster wrote: > > On Thu, Mar 02, 2017 at 11:35:20AM +0100, Michal Hocko wrote: > > > On Thu 02-03-17 19:04:48, Tetsuo Handa wrote: > > > [...] > > > > So, commit 5d17a73a2ebeb8d1("vmalloc: back off when the current task is > > > > killed") implemented __GFP_KILLABLE flag and automatically applied that > > > > flag. As a result, those who are not ready to fail upon SIGKILL are > > > > confused. ;-) > > > > > > You are right! The function is documented it might fail but the code > > > doesn't really allow that. This seems like a bug to me. What do you > > > think about the following? > > > --- > > > From d02cb0285d8ce3344fd64dc7e2912e9a04bef80d Mon Sep 17 00:00:00 2001 > > > From: Michal Hocko <mhocko@suse.com> > > > Date: Thu, 2 Mar 2017 11:31:11 +0100 > > > Subject: [PATCH] xfs: allow kmem_zalloc_greedy to fail > > > > > > Even though kmem_zalloc_greedy is documented it might fail the current > > > code doesn't really implement this properly and loops on the smallest > > > allowed size for ever. This is a problem because vzalloc might fail > > > permanently. Since 5d17a73a2ebe ("vmalloc: back off when the current > > > task is killed") such a failure is much more probable than it used to > > > be. Fix this by bailing out if the minimum size request failed. > > > > > > This has been noticed by a hung generic/269 xfstest by Xiong Zhou. > > > > > > Reported-by: Xiong Zhou <xzhou@redhat.com> > > > Analyzed-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > > > Signed-off-by: Michal Hocko <mhocko@suse.com> > > > --- > > > fs/xfs/kmem.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/fs/xfs/kmem.c b/fs/xfs/kmem.c > > > index 339c696bbc01..ee95f5c6db45 100644 > > > --- a/fs/xfs/kmem.c > > > +++ b/fs/xfs/kmem.c > > > @@ -34,6 +34,8 @@ kmem_zalloc_greedy(size_t *size, size_t minsize, size_t maxsize) > > > size_t kmsize = maxsize; > > > > > > while (!(ptr = vzalloc(kmsize))) { > > > + if (kmsize == minsize) > > > + break; > > > if ((kmsize >>= 1) <= minsize) > > > kmsize = minsize; > > > } > > > > More consistent with the rest of the kmem code might be to accept a > > flags argument and do something like this based on KM_MAYFAIL. > > Well, vmalloc doesn't really support GFP_NOFAIL semantic right now for > the same reason it doesn't support GFP_NOFS. So I am not sure this is a > good idea. > Not sure I follow..? I'm just suggesting to control the loop behavior based on the KM_ flag, not to do or change anything wrt to GFP_ flags. > > The one > > current caller looks like it would pass it, but I suppose we'd still > > need a mechanism to break out should a new caller not pass that flag. > > Would a fatal_signal_pending() check in the loop as well allow us to > > break out in the scenario that is reproduced here? > > Yes that check would work as well I just thought the break out when the > minsize request fails to be more logical. There might be other reasons > to fail the request and looping here seems just wrong. But whatever you > or other xfs people prefer. There may be higher level reasons for why this code should or should not loop, that just seems like a separate issue to me. My thinking is more that this appears to be how every kmem_*() function operates today and it seems a bit out of place to change behavior of one to fix a bug. Maybe I'm missing something though.. are we subject to the same general problem in any of the other kmem_*() functions that can currently loop indefinitely? Brian > -- > Michal Hocko > SUSE Labs
WARNING: multiple messages have this Message-ID (diff)
From: Brian Foster <bfoster@redhat.com> To: Michal Hocko <mhocko@kernel.org> Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>, Xiong Zhou <xzhou@redhat.com>, Christoph Hellwig <hch@infradead.org>, linux-xfs@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org Subject: Re: mm allocation failure and hang when running xfstests generic/269 on xfs Date: Thu, 2 Mar 2017 08:00:09 -0500 [thread overview] Message-ID: <20170302130009.GC3213@bfoster.bfoster> (raw) In-Reply-To: <20170302124909.GE1404@dhcp22.suse.cz> On Thu, Mar 02, 2017 at 01:49:09PM +0100, Michal Hocko wrote: > On Thu 02-03-17 07:24:27, Brian Foster wrote: > > On Thu, Mar 02, 2017 at 11:35:20AM +0100, Michal Hocko wrote: > > > On Thu 02-03-17 19:04:48, Tetsuo Handa wrote: > > > [...] > > > > So, commit 5d17a73a2ebeb8d1("vmalloc: back off when the current task is > > > > killed") implemented __GFP_KILLABLE flag and automatically applied that > > > > flag. As a result, those who are not ready to fail upon SIGKILL are > > > > confused. ;-) > > > > > > You are right! The function is documented it might fail but the code > > > doesn't really allow that. This seems like a bug to me. What do you > > > think about the following? > > > --- > > > From d02cb0285d8ce3344fd64dc7e2912e9a04bef80d Mon Sep 17 00:00:00 2001 > > > From: Michal Hocko <mhocko@suse.com> > > > Date: Thu, 2 Mar 2017 11:31:11 +0100 > > > Subject: [PATCH] xfs: allow kmem_zalloc_greedy to fail > > > > > > Even though kmem_zalloc_greedy is documented it might fail the current > > > code doesn't really implement this properly and loops on the smallest > > > allowed size for ever. This is a problem because vzalloc might fail > > > permanently. Since 5d17a73a2ebe ("vmalloc: back off when the current > > > task is killed") such a failure is much more probable than it used to > > > be. Fix this by bailing out if the minimum size request failed. > > > > > > This has been noticed by a hung generic/269 xfstest by Xiong Zhou. > > > > > > Reported-by: Xiong Zhou <xzhou@redhat.com> > > > Analyzed-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > > > Signed-off-by: Michal Hocko <mhocko@suse.com> > > > --- > > > fs/xfs/kmem.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/fs/xfs/kmem.c b/fs/xfs/kmem.c > > > index 339c696bbc01..ee95f5c6db45 100644 > > > --- a/fs/xfs/kmem.c > > > +++ b/fs/xfs/kmem.c > > > @@ -34,6 +34,8 @@ kmem_zalloc_greedy(size_t *size, size_t minsize, size_t maxsize) > > > size_t kmsize = maxsize; > > > > > > while (!(ptr = vzalloc(kmsize))) { > > > + if (kmsize == minsize) > > > + break; > > > if ((kmsize >>= 1) <= minsize) > > > kmsize = minsize; > > > } > > > > More consistent with the rest of the kmem code might be to accept a > > flags argument and do something like this based on KM_MAYFAIL. > > Well, vmalloc doesn't really support GFP_NOFAIL semantic right now for > the same reason it doesn't support GFP_NOFS. So I am not sure this is a > good idea. > Not sure I follow..? I'm just suggesting to control the loop behavior based on the KM_ flag, not to do or change anything wrt to GFP_ flags. > > The one > > current caller looks like it would pass it, but I suppose we'd still > > need a mechanism to break out should a new caller not pass that flag. > > Would a fatal_signal_pending() check in the loop as well allow us to > > break out in the scenario that is reproduced here? > > Yes that check would work as well I just thought the break out when the > minsize request fails to be more logical. There might be other reasons > to fail the request and looping here seems just wrong. But whatever you > or other xfs people prefer. There may be higher level reasons for why this code should or should not loop, that just seems like a separate issue to me. My thinking is more that this appears to be how every kmem_*() function operates today and it seems a bit out of place to change behavior of one to fix a bug. Maybe I'm missing something though.. are we subject to the same general problem in any of the other kmem_*() functions that can currently loop indefinitely? Brian > -- > Michal Hocko > SUSE Labs -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2017-03-02 13:55 UTC|newest] Thread overview: 79+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-03-01 4:46 mm allocation failure and hang when running xfstests generic/269 on xfs Xiong Zhou 2017-03-01 4:46 ` Xiong Zhou 2017-03-02 0:37 ` Christoph Hellwig 2017-03-02 0:37 ` Christoph Hellwig 2017-03-02 5:19 ` Xiong Zhou 2017-03-02 5:19 ` Xiong Zhou 2017-03-02 6:41 ` Bob Liu 2017-03-02 6:41 ` Bob Liu 2017-03-02 6:41 ` Bob Liu 2017-03-02 6:41 ` Bob Liu 2017-03-02 6:47 ` Anshuman Khandual 2017-03-02 6:47 ` Anshuman Khandual 2017-03-02 8:42 ` Michal Hocko 2017-03-02 8:42 ` Michal Hocko 2017-03-02 9:23 ` Xiong Zhou 2017-03-02 9:23 ` Xiong Zhou 2017-03-02 10:04 ` Tetsuo Handa 2017-03-02 10:04 ` Tetsuo Handa 2017-03-02 10:35 ` Michal Hocko 2017-03-02 10:35 ` Michal Hocko 2017-03-02 10:35 ` Michal Hocko 2017-03-02 10:53 ` mm allocation failure and hang when running xfstests generic/269on xfs Tetsuo Handa 2017-03-02 10:53 ` Tetsuo Handa 2017-03-02 12:24 ` mm allocation failure and hang when running xfstests generic/269 on xfs Brian Foster 2017-03-02 12:24 ` Brian Foster 2017-03-02 12:49 ` Michal Hocko 2017-03-02 12:49 ` Michal Hocko 2017-03-02 13:00 ` Brian Foster [this message] 2017-03-02 13:00 ` Brian Foster 2017-03-02 13:07 ` Tetsuo Handa 2017-03-02 13:07 ` Tetsuo Handa 2017-03-02 13:27 ` Michal Hocko 2017-03-02 13:27 ` Michal Hocko 2017-03-02 13:41 ` Brian Foster 2017-03-02 13:41 ` Brian Foster 2017-03-02 13:50 ` Michal Hocko 2017-03-02 13:50 ` Michal Hocko 2017-03-02 14:23 ` Brian Foster 2017-03-02 14:23 ` Brian Foster 2017-03-02 14:34 ` Michal Hocko 2017-03-02 14:34 ` Michal Hocko 2017-03-02 14:51 ` Brian Foster 2017-03-02 14:51 ` Brian Foster 2017-03-02 15:14 ` Michal Hocko 2017-03-02 15:14 ` Michal Hocko 2017-03-02 15:30 ` Brian Foster 2017-03-02 15:30 ` Brian Foster 2017-03-02 15:45 ` [PATCH 1/2] xfs: allow kmem_zalloc_greedy to fail Michal Hocko 2017-03-02 15:45 ` Michal Hocko 2017-03-02 15:45 ` Michal Hocko 2017-03-02 15:45 ` Michal Hocko 2017-03-02 15:45 ` [PATCH 2/2] xfs: back off from kmem_zalloc_greedy if the task is killed Michal Hocko 2017-03-02 15:45 ` Michal Hocko 2017-03-02 15:45 ` Michal Hocko 2017-03-02 15:45 ` Michal Hocko 2017-03-02 15:49 ` Christoph Hellwig 2017-03-02 15:49 ` Christoph Hellwig 2017-03-02 15:59 ` Brian Foster 2017-03-02 15:59 ` Brian Foster 2017-03-02 15:49 ` [PATCH 1/2] xfs: allow kmem_zalloc_greedy to fail Christoph Hellwig 2017-03-02 15:49 ` Christoph Hellwig 2017-03-02 15:59 ` Brian Foster 2017-03-02 15:59 ` Brian Foster 2017-03-02 16:16 ` Michal Hocko 2017-03-02 16:16 ` Michal Hocko 2017-03-02 16:44 ` Darrick J. Wong 2017-03-02 16:44 ` Darrick J. Wong 2017-03-03 22:54 ` Dave Chinner 2017-03-03 22:54 ` Dave Chinner 2017-03-03 23:19 ` Darrick J. Wong 2017-03-03 23:19 ` Darrick J. Wong 2017-03-04 4:48 ` Dave Chinner 2017-03-04 4:48 ` Dave Chinner 2017-03-06 13:21 ` Michal Hocko 2017-03-06 13:21 ` Michal Hocko 2017-03-02 15:47 ` mm allocation failure and hang when running xfstests generic/269 on xfs Michal Hocko 2017-03-02 15:47 ` Michal Hocko 2017-03-02 15:47 ` Christoph Hellwig 2017-03-02 15:47 ` Christoph Hellwig
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20170302130009.GC3213@bfoster.bfoster \ --to=bfoster@redhat.com \ --cc=hch@infradead.org \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=linux-xfs@vger.kernel.org \ --cc=mhocko@kernel.org \ --cc=penguin-kernel@I-love.SAKURA.ne.jp \ --cc=xzhou@redhat.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.