From: Michal Hocko <mhocko@kernel.org> To: Wei Yang <richard.weiyang@gmail.com> Cc: linux-mm@kvack.org, Vlastimil Babka <vbabka@suse.cz>, Johannes Weiner <hannes@cmpxchg.org>, Mel Gorman <mgorman@suse.de>, Andrew Morton <akpm@linux-foundation.org>, LKML <linux-kernel@vger.kernel.org> Subject: Re: [RFC PATCH 2/4] mm, tree wide: replace __GFP_REPEAT by __GFP_RETRY_MAYFAIL with more useful semantic Date: Mon, 5 Jun 2017 08:43:43 +0200 [thread overview] Message-ID: <20170605064343.GE9248@dhcp22.suse.cz> (raw) In-Reply-To: <20170603022440.GA11080@WeideMacBook-Pro.local> On Sat 03-06-17 10:24:40, Wei Yang wrote: > Hi, Michal > > Just go through your patch. > > I have one question and one suggestion as below. > > One suggestion: > > This patch does two things to me: > 1. Replace __GFP_REPEAT with __GFP_RETRY_MAYFAIL > 2. Adjust the logic in page_alloc to provide the middle semantic > > My suggestion is to split these two task into two patches, so that readers > could catch your fundamental logic change easily. Well, the rename and the change is intentionally tight together. My previous patches have removed all __GFP_REPEAT users for low order requests which didn't have any implemented semantic. So as of now we should only have those users which semantic will not change. I do not add any new low order user in this patch so it in fact doesn't change any existing semnatic. > > On Tue, Mar 07, 2017 at 04:48:41PM +0100, Michal Hocko wrote: > >From: Michal Hocko <mhocko@suse.com> [...] > >@@ -3776,9 +3784,9 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order, > > > > /* > > * Do not retry costly high order allocations unless they are > >- * __GFP_REPEAT > >+ * __GFP_RETRY_MAYFAIL > > */ > >- if (order > PAGE_ALLOC_COSTLY_ORDER && !(gfp_mask & __GFP_REPEAT)) > >+ if (order > PAGE_ALLOC_COSTLY_ORDER && !(gfp_mask & __GFP_RETRY_MAYFAIL)) > > goto nopage; > > One question: > > From your change log, it mentions will provide the same semantic for !costly > allocations. While the logic here is the same as before. > > For a !costly allocation with __GFP_REPEAT flag, the difference after this > patch is no OOM will be invoked, while it will still continue in the loop. Not really. There are two things. The above will shortcut retrying if there is _no_ __GFP_RETRY_MAYFAIL. If the flags _is_ specified we will back of in __alloc_pages_may_oom. > Maybe I don't catch your point in this message: > > __GFP_REPEAT was designed to allow retry-but-eventually-fail semantic to > the page allocator. This has been true but only for allocations requests > larger than PAGE_ALLOC_COSTLY_ORDER. It has been always ignored for > smaller sizes. This is a bit unfortunate because there is no way to > express the same semantic for those requests and they are considered too > important to fail so they might end up looping in the page allocator for > ever, similarly to GFP_NOFAIL requests. > > I thought you will provide the same semantic to !costly allocation, or I > misunderstand? yes and that is the case. __alloc_pages_may_oom will back off before OOM killer is invoked and the allocator slow path will fail because did_some_progress == 0; -- Michal Hocko SUSE Labs
WARNING: multiple messages have this Message-ID (diff)
From: Michal Hocko <mhocko@kernel.org> To: Wei Yang <richard.weiyang@gmail.com> Cc: linux-mm@kvack.org, Vlastimil Babka <vbabka@suse.cz>, Johannes Weiner <hannes@cmpxchg.org>, Mel Gorman <mgorman@suse.de>, Andrew Morton <akpm@linux-foundation.org>, LKML <linux-kernel@vger.kernel.org> Subject: Re: [RFC PATCH 2/4] mm, tree wide: replace __GFP_REPEAT by __GFP_RETRY_MAYFAIL with more useful semantic Date: Mon, 5 Jun 2017 08:43:43 +0200 [thread overview] Message-ID: <20170605064343.GE9248@dhcp22.suse.cz> (raw) In-Reply-To: <20170603022440.GA11080@WeideMacBook-Pro.local> On Sat 03-06-17 10:24:40, Wei Yang wrote: > Hi, Michal > > Just go through your patch. > > I have one question and one suggestion as below. > > One suggestion: > > This patch does two things to me: > 1. Replace __GFP_REPEAT with __GFP_RETRY_MAYFAIL > 2. Adjust the logic in page_alloc to provide the middle semantic > > My suggestion is to split these two task into two patches, so that readers > could catch your fundamental logic change easily. Well, the rename and the change is intentionally tight together. My previous patches have removed all __GFP_REPEAT users for low order requests which didn't have any implemented semantic. So as of now we should only have those users which semantic will not change. I do not add any new low order user in this patch so it in fact doesn't change any existing semnatic. > > On Tue, Mar 07, 2017 at 04:48:41PM +0100, Michal Hocko wrote: > >From: Michal Hocko <mhocko@suse.com> [...] > >@@ -3776,9 +3784,9 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order, > > > > /* > > * Do not retry costly high order allocations unless they are > >- * __GFP_REPEAT > >+ * __GFP_RETRY_MAYFAIL > > */ > >- if (order > PAGE_ALLOC_COSTLY_ORDER && !(gfp_mask & __GFP_REPEAT)) > >+ if (order > PAGE_ALLOC_COSTLY_ORDER && !(gfp_mask & __GFP_RETRY_MAYFAIL)) > > goto nopage; > > One question: > > From your change log, it mentions will provide the same semantic for !costly > allocations. While the logic here is the same as before. > > For a !costly allocation with __GFP_REPEAT flag, the difference after this > patch is no OOM will be invoked, while it will still continue in the loop. Not really. There are two things. The above will shortcut retrying if there is _no_ __GFP_RETRY_MAYFAIL. If the flags _is_ specified we will back of in __alloc_pages_may_oom. > Maybe I don't catch your point in this message: > > __GFP_REPEAT was designed to allow retry-but-eventually-fail semantic to > the page allocator. This has been true but only for allocations requests > larger than PAGE_ALLOC_COSTLY_ORDER. It has been always ignored for > smaller sizes. This is a bit unfortunate because there is no way to > express the same semantic for those requests and they are considered too > important to fail so they might end up looping in the page allocator for > ever, similarly to GFP_NOFAIL requests. > > I thought you will provide the same semantic to !costly allocation, or I > misunderstand? yes and that is the case. __alloc_pages_may_oom will back off before OOM killer is invoked and the allocator slow path will fail because did_some_progress == 0; -- 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-06-05 6:43 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 [this message] 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 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=20170605064343.GE9248@dhcp22.suse.cz \ --to=mhocko@kernel.org \ --cc=akpm@linux-foundation.org \ --cc=hannes@cmpxchg.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=mgorman@suse.de \ --cc=richard.weiyang@gmail.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.