From: Michal Hocko <mhocko@kernel.org> To: NeilBrown <neilb@suse.com> Cc: Vlastimil Babka <vbabka@suse.cz>, linux-mm@kvack.org, Johannes Weiner <hannes@cmpxchg.org>, Mel Gorman <mgorman@suse.de>, Andrew Morton <akpm@linux-foundation.org>, LKML <linux-kernel@vger.kernel.org>, "Darrick J. Wong" <darrick.wong@oracle.com>, Heiko Carstens <heiko.carstens@de.ibm.com>, NeilBrown <neilb@suse.de>, Jonathan Corbet <corbet@lwn.net>, Paolo Bonzini <pbonzini@redhat.com>, "Eric W. Biederman" <ebiederm@xmission.com> Subject: Re: [RFC PATCH 0/4 v2] mm: give __GFP_REPEAT a better semantic Date: Wed, 24 May 2017 09:34:03 +0200 [thread overview] Message-ID: <20170524073403.GA14733@dhcp22.suse.cz> (raw) In-Reply-To: <87shjvhxmr.fsf@notabene.neil.brown.name> On Wed 24-05-17 11:06:04, NeilBrown wrote: > On Tue, May 23 2017, Vlastimil Babka wrote: > > > On 05/16/2017 11:10 AM, Michal Hocko wrote: > >> So, is there some interest in this? I am not going to push this if there > >> is a general consensus that we do not need to do anything about the > >> current situation or need a different approach. > > > > After the recent LWN article [1] I think that we should really support > > marking allocations as failable, without making them too easily failable > > via __GFP_NORETRY. The __GFP_RETRY_MAY_FAIL flag sounds like a good way > > to do that without introducing a new __GFP_MAYFAIL. We could also > > introduce a wrapper such as GFP_KERNEL_MAYFAIL. > > > > [1] https://lwn.net/Articles/723317/ > > Yes please!!! > > I particularly like: > > > - GFP_KERNEL | __GFP_NORETRY - overrides the default allocator behavior and > > all allocation requests fail early rather than cause disruptive > > reclaim (one round of reclaim in this implementation). The OOM killer > > is not invoked. > > - GFP_KERNEL | __GFP_RETRY_MAYFAIL - overrides the default allocator behavior > > and all allocation requests try really hard. The request will fail if the > > reclaim cannot make any progress. The OOM killer won't be triggered. > > - GFP_KERNEL | __GFP_NOFAIL - overrides the default allocator behavior > > and all allocation requests will loop endlessly until they > > succeed. This might be really dangerous especially for larger orders. > > There seems to be a good range here, and the two end points are good > choices. > I like that only __GFP_NOFAIL triggers the OOM. > I would like the middle option to be the default. I think that is what > many people thought the default was. I appreciate that making the > transition might be awkward. Yeah, turning GFP_KERNEL int GFP_KERNEL | __GFP_RETRY_MAYFAIL would be hard if possible at all. One of the problems with the current code is that error paths are checked but there is rarely a sane error handling strategy implemented on top. So we mostly check for the failure and return -ENOMEM up the call chain without having a great clue what will happen up there. And the result might be really unexpected. Say that some allocation fails on the sys_close() path and returns to the userspace. a) this syscall is not supposed to return -ENOMEM b) there is no _transaction_ rollback to have the fd in a sane state to retry later. Therefore I assume that __GFP_RETRY_MAYFAIL will be slowly added to those places where the error path strategy is clear. > Maybe create GFP_DEFAULT which matches the middle option and encourage > that in new code?? > > We would probably want guidelines on when __GFP_NOFAIL is acceptable. > I assume: > - no locks held This is of course preferable but hard to demand in general. I think that requiring "no locks which can block oom victim exit" would be more appropriate, albeit much more fuzzy. But in general locks should be much smaller problem these days with the async OOM reclaim (oom_reaper) and with __GFP_NOFAIL gaining access to a part of memory reserves when hitting the OOM path. > - small allocations OK, large allocation need clear justification. yes > - error would be exposed to systemcall Not only. There are some FS transaction code paths where failure basically means RO remount and such. This would be acceptable as well. > ??? > > I think it is important to give kernel developers clear options and make > it easy for them to choose the best option. This helps to do that. Yes, I completely agree here. Does the updated documentation in the patch helps or would you suggest som improvements? -- Michal Hocko SUSE Labs
WARNING: multiple messages have this Message-ID (diff)
From: Michal Hocko <mhocko@kernel.org> To: NeilBrown <neilb@suse.com> Cc: Vlastimil Babka <vbabka@suse.cz>, linux-mm@kvack.org, Johannes Weiner <hannes@cmpxchg.org>, Mel Gorman <mgorman@suse.de>, Andrew Morton <akpm@linux-foundation.org>, LKML <linux-kernel@vger.kernel.org>, "Darrick J. Wong" <darrick.wong@oracle.com>, Heiko Carstens <heiko.carstens@de.ibm.com>, NeilBrown <neilb@suse.de>, Jonathan Corbet <corbet@lwn.net>, Paolo Bonzini <pbonzini@redhat.com>, "Eric W. Biederman" <ebiederm@xmission.com> Subject: Re: [RFC PATCH 0/4 v2] mm: give __GFP_REPEAT a better semantic Date: Wed, 24 May 2017 09:34:03 +0200 [thread overview] Message-ID: <20170524073403.GA14733@dhcp22.suse.cz> (raw) In-Reply-To: <87shjvhxmr.fsf@notabene.neil.brown.name> On Wed 24-05-17 11:06:04, NeilBrown wrote: > On Tue, May 23 2017, Vlastimil Babka wrote: > > > On 05/16/2017 11:10 AM, Michal Hocko wrote: > >> So, is there some interest in this? I am not going to push this if there > >> is a general consensus that we do not need to do anything about the > >> current situation or need a different approach. > > > > After the recent LWN article [1] I think that we should really support > > marking allocations as failable, without making them too easily failable > > via __GFP_NORETRY. The __GFP_RETRY_MAY_FAIL flag sounds like a good way > > to do that without introducing a new __GFP_MAYFAIL. We could also > > introduce a wrapper such as GFP_KERNEL_MAYFAIL. > > > > [1] https://lwn.net/Articles/723317/ > > Yes please!!! > > I particularly like: > > > - GFP_KERNEL | __GFP_NORETRY - overrides the default allocator behavior and > > all allocation requests fail early rather than cause disruptive > > reclaim (one round of reclaim in this implementation). The OOM killer > > is not invoked. > > - GFP_KERNEL | __GFP_RETRY_MAYFAIL - overrides the default allocator behavior > > and all allocation requests try really hard. The request will fail if the > > reclaim cannot make any progress. The OOM killer won't be triggered. > > - GFP_KERNEL | __GFP_NOFAIL - overrides the default allocator behavior > > and all allocation requests will loop endlessly until they > > succeed. This might be really dangerous especially for larger orders. > > There seems to be a good range here, and the two end points are good > choices. > I like that only __GFP_NOFAIL triggers the OOM. > I would like the middle option to be the default. I think that is what > many people thought the default was. I appreciate that making the > transition might be awkward. Yeah, turning GFP_KERNEL int GFP_KERNEL | __GFP_RETRY_MAYFAIL would be hard if possible at all. One of the problems with the current code is that error paths are checked but there is rarely a sane error handling strategy implemented on top. So we mostly check for the failure and return -ENOMEM up the call chain without having a great clue what will happen up there. And the result might be really unexpected. Say that some allocation fails on the sys_close() path and returns to the userspace. a) this syscall is not supposed to return -ENOMEM b) there is no _transaction_ rollback to have the fd in a sane state to retry later. Therefore I assume that __GFP_RETRY_MAYFAIL will be slowly added to those places where the error path strategy is clear. > Maybe create GFP_DEFAULT which matches the middle option and encourage > that in new code?? > > We would probably want guidelines on when __GFP_NOFAIL is acceptable. > I assume: > - no locks held This is of course preferable but hard to demand in general. I think that requiring "no locks which can block oom victim exit" would be more appropriate, albeit much more fuzzy. But in general locks should be much smaller problem these days with the async OOM reclaim (oom_reaper) and with __GFP_NOFAIL gaining access to a part of memory reserves when hitting the OOM path. > - small allocations OK, large allocation need clear justification. yes > - error would be exposed to systemcall Not only. There are some FS transaction code paths where failure basically means RO remount and such. This would be acceptable as well. > ??? > > I think it is important to give kernel developers clear options and make > it easy for them to choose the best option. This helps to do that. Yes, I completely agree here. Does the updated documentation in the patch helps or would you suggest som improvements? -- 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-05-24 7:34 UTC|newest] Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-03-07 15:48 [RFC PATCH 0/4 v2] mm: give __GFP_REPEAT a better semantic Michal Hocko 2017-03-07 15:48 ` Michal Hocko 2017-03-07 15:48 ` [PATCH 1/4] s390: get rid of superfluous __GFP_REPEAT Michal Hocko 2017-03-07 15:48 ` Michal Hocko 2017-03-08 8:23 ` Heiko Carstens 2017-03-08 8:23 ` Heiko Carstens 2017-03-08 14:11 ` Michal Hocko 2017-03-08 14:11 ` Michal Hocko 2017-03-09 8:27 ` Heiko Carstens 2017-03-09 8:27 ` Heiko Carstens 2017-03-07 15:48 ` [RFC PATCH 2/4] mm, tree wide: replace __GFP_REPEAT by __GFP_RETRY_MAYFAIL with more useful semantic Michal Hocko 2017-03-07 15:48 ` Michal Hocko 2017-05-25 1:21 ` NeilBrown 2017-05-31 11:42 ` Michal Hocko 2017-05-31 11:42 ` Michal Hocko 2017-06-03 2:24 ` Wei Yang 2017-06-05 6:43 ` Michal Hocko 2017-06-05 6:43 ` Michal Hocko 2017-06-06 3:04 ` Wei Yang 2017-06-06 12:03 ` Michal Hocko 2017-06-06 12:03 ` Michal Hocko 2017-06-07 2:10 ` Wei Yang 2017-06-09 7:32 ` Michal Hocko 2017-06-09 7:32 ` Michal Hocko 2017-03-07 15:48 ` [RFC PATCH 3/4] xfs: map KM_MAYFAIL to __GFP_RETRY_MAYFAIL Michal Hocko 2017-03-07 15:48 ` Michal Hocko 2017-03-07 17:05 ` Darrick J. Wong 2017-03-07 17:05 ` Darrick J. Wong 2017-03-08 9:35 ` Michal Hocko 2017-03-08 9:35 ` Michal Hocko 2017-03-08 11:23 ` Tetsuo Handa 2017-03-08 11:23 ` Tetsuo Handa 2017-03-08 12:54 ` Michal Hocko 2017-03-08 12:54 ` Michal Hocko 2017-03-08 15:06 ` Christoph Hellwig 2017-03-08 15:06 ` Christoph Hellwig 2017-03-09 9:16 ` Michal Hocko 2017-03-09 9:16 ` Michal Hocko 2017-03-07 15:48 ` [RFC PATCH 4/4] mm: kvmalloc support __GFP_RETRY_MAYFAIL for all sizes Michal Hocko 2017-03-07 15:48 ` Michal Hocko 2017-05-16 9:10 ` [RFC PATCH 0/4 v2] mm: give __GFP_REPEAT a better semantic Michal Hocko 2017-05-16 9:10 ` Michal Hocko 2017-05-23 8:12 ` Vlastimil Babka 2017-05-23 8:12 ` Vlastimil Babka 2017-05-24 1:06 ` NeilBrown 2017-05-24 7:34 ` Michal Hocko [this message] 2017-05-24 7:34 ` Michal Hocko
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=20170524073403.GA14733@dhcp22.suse.cz \ --to=mhocko@kernel.org \ --cc=akpm@linux-foundation.org \ --cc=corbet@lwn.net \ --cc=darrick.wong@oracle.com \ --cc=ebiederm@xmission.com \ --cc=hannes@cmpxchg.org \ --cc=heiko.carstens@de.ibm.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=mgorman@suse.de \ --cc=neilb@suse.com \ --cc=neilb@suse.de \ --cc=pbonzini@redhat.com \ --cc=vbabka@suse.cz \ /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.