From: Mel Gorman <mgorman@suse.de> To: David Rientjes <rientjes@google.com> Cc: Andrew Morton <akpm@linux-foundation.org>, James Bottomley <James.Bottomley@hansenpartnership.com>, Colin King <colin.king@canonical.com>, Raghavendra D Prabhu <raghu.prabhu13@gmail.com>, Jan Kara <jack@suse.cz>, Chris Mason <chris.mason@oracle.com>, Christoph Lameter <cl@linux.com>, Pekka Enberg <penberg@kernel.org>, Rik van Riel <riel@redhat.com>, Johannes Weiner <hannes@cmpxchg.org>, linux-fsdevel <linux-fsdevel@vger.kernel.org>, linux-mm <linux-mm@kvack.org>, linux-kernel <linux-kernel@vger.kernel.org>, linux-ext4 <linux-ext4@vger.kernel.org> Subject: Re: [PATCH 3/4] mm: slub: Do not take expensive steps for SLUBs speculative high-order allocations Date: Tue, 17 May 2011 09:42:27 +0100 [thread overview] Message-ID: <20110517084227.GI5279@suse.de> (raw) In-Reply-To: <alpine.DEB.2.00.1105161411440.4353@chino.kir.corp.google.com> On Mon, May 16, 2011 at 02:16:46PM -0700, David Rientjes wrote: > On Fri, 13 May 2011, Mel Gorman wrote: > > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > > index 9f8a97b..057f1e2 100644 > > --- a/mm/page_alloc.c > > +++ b/mm/page_alloc.c > > @@ -1972,6 +1972,7 @@ gfp_to_alloc_flags(gfp_t gfp_mask) > > { > > int alloc_flags = ALLOC_WMARK_MIN | ALLOC_CPUSET; > > const gfp_t wait = gfp_mask & __GFP_WAIT; > > + const gfp_t can_wake_kswapd = !(gfp_mask & __GFP_NO_KSWAPD); > > > > /* __GFP_HIGH is assumed to be the same as ALLOC_HIGH to save a branch. */ > > BUILD_BUG_ON(__GFP_HIGH != (__force gfp_t) ALLOC_HIGH); > > @@ -1984,7 +1985,7 @@ gfp_to_alloc_flags(gfp_t gfp_mask) > > */ > > alloc_flags |= (__force int) (gfp_mask & __GFP_HIGH); > > > > - if (!wait) { > > + if (!wait && can_wake_kswapd) { > > /* > > * Not worth trying to allocate harder for > > * __GFP_NOMEMALLOC even if it can't schedule. > > diff --git a/mm/slub.c b/mm/slub.c > > index 98c358d..c5797ab 100644 > > --- a/mm/slub.c > > +++ b/mm/slub.c > > @@ -1170,7 +1170,8 @@ static struct page *allocate_slab(struct kmem_cache *s, gfp_t flags, int node) > > * Let the initial higher-order allocation fail under memory pressure > > * so we fall-back to the minimum order allocation. > > */ > > - alloc_gfp = (flags | __GFP_NOWARN | __GFP_NORETRY | __GFP_NO_KSWAPD) & ~__GFP_NOFAIL; > > + alloc_gfp = (flags | __GFP_NOWARN | __GFP_NO_KSWAPD) & > > + ~(__GFP_NOFAIL | __GFP_WAIT | __GFP_REPEAT); > > > > page = alloc_slab_page(alloc_gfp, node, oo); > > if (unlikely(!page)) { > > It's unnecessary to clear __GFP_REPEAT, these !__GFP_NOFAIL allocations > will immediately fail. > We can enter enter direct compaction or direct reclaim at least once. If compaction is enabled and we enter reclaim/compaction, the presense of __GFP_REPEAT makes a difference in should_continue_reclaim(). With compaction disabled, the presense of the flag is relevant in should_alloc_retry() with it being possible to loop in the allocator instead of failing the SLUB allocation and dropping back. Maybe you meant !__GFP_WAIT instead of !__GFP_NOFAIL which makes more sense. In that case, we clear both flags because __GFP_REPEAT && !_GFP_WAIT is a senseless combination of flags. If for whatever reason the __GFP_WAIT was re-added, the presense of __GFP_REPEAT could cause problems in reclaim that would be hard to spot again. > alloc_gfp would probably benefit from having a comment about why > __GFP_WAIT should be masked off here: that we don't want to do compaction > or direct reclaim or retry the allocation more than once (so both > __GFP_NORETRY and __GFP_REPEAT are no-ops). That would have been helpful all right. I should have caught that and explained it properly. In the event there is a new version of the patch, I'll add one. For the moment, I'm dropping this patch entirely. Christoph wants to maintain historic behaviour of SLUB to maximise the number of high-order pages it uses and at the end of the day, which option performs better depends entirely on the workload and machine configuration. -- Mel Gorman SUSE Labs
WARNING: multiple messages have this Message-ID (diff)
From: Mel Gorman <mgorman@suse.de> To: David Rientjes <rientjes@google.com> Cc: Andrew Morton <akpm@linux-foundation.org>, James Bottomley <James.Bottomley@hansenpartnership.com>, Colin King <colin.king@canonical.com>, Raghavendra D Prabhu <raghu.prabhu13@gmail.com>, Jan Kara <jack@suse.cz>, Chris Mason <chris.mason@oracle.com>, Christoph Lameter <cl@linux.com>, Pekka Enberg <penberg@kernel.org>, Rik van Riel <riel@redhat.com>, Johannes Weiner <hannes@cmpxchg.org>, linux-fsdevel <linux-fsdevel@vger.kernel.org>, linux-mm <linux-mm@kvack.org>, linux-kernel <linux-kernel@vger.kernel.org>, linux-ext4 <linux-ext4@vger.kernel.org> Subject: Re: [PATCH 3/4] mm: slub: Do not take expensive steps for SLUBs speculative high-order allocations Date: Tue, 17 May 2011 09:42:27 +0100 [thread overview] Message-ID: <20110517084227.GI5279@suse.de> (raw) In-Reply-To: <alpine.DEB.2.00.1105161411440.4353@chino.kir.corp.google.com> On Mon, May 16, 2011 at 02:16:46PM -0700, David Rientjes wrote: > On Fri, 13 May 2011, Mel Gorman wrote: > > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > > index 9f8a97b..057f1e2 100644 > > --- a/mm/page_alloc.c > > +++ b/mm/page_alloc.c > > @@ -1972,6 +1972,7 @@ gfp_to_alloc_flags(gfp_t gfp_mask) > > { > > int alloc_flags = ALLOC_WMARK_MIN | ALLOC_CPUSET; > > const gfp_t wait = gfp_mask & __GFP_WAIT; > > + const gfp_t can_wake_kswapd = !(gfp_mask & __GFP_NO_KSWAPD); > > > > /* __GFP_HIGH is assumed to be the same as ALLOC_HIGH to save a branch. */ > > BUILD_BUG_ON(__GFP_HIGH != (__force gfp_t) ALLOC_HIGH); > > @@ -1984,7 +1985,7 @@ gfp_to_alloc_flags(gfp_t gfp_mask) > > */ > > alloc_flags |= (__force int) (gfp_mask & __GFP_HIGH); > > > > - if (!wait) { > > + if (!wait && can_wake_kswapd) { > > /* > > * Not worth trying to allocate harder for > > * __GFP_NOMEMALLOC even if it can't schedule. > > diff --git a/mm/slub.c b/mm/slub.c > > index 98c358d..c5797ab 100644 > > --- a/mm/slub.c > > +++ b/mm/slub.c > > @@ -1170,7 +1170,8 @@ static struct page *allocate_slab(struct kmem_cache *s, gfp_t flags, int node) > > * Let the initial higher-order allocation fail under memory pressure > > * so we fall-back to the minimum order allocation. > > */ > > - alloc_gfp = (flags | __GFP_NOWARN | __GFP_NORETRY | __GFP_NO_KSWAPD) & ~__GFP_NOFAIL; > > + alloc_gfp = (flags | __GFP_NOWARN | __GFP_NO_KSWAPD) & > > + ~(__GFP_NOFAIL | __GFP_WAIT | __GFP_REPEAT); > > > > page = alloc_slab_page(alloc_gfp, node, oo); > > if (unlikely(!page)) { > > It's unnecessary to clear __GFP_REPEAT, these !__GFP_NOFAIL allocations > will immediately fail. > We can enter enter direct compaction or direct reclaim at least once. If compaction is enabled and we enter reclaim/compaction, the presense of __GFP_REPEAT makes a difference in should_continue_reclaim(). With compaction disabled, the presense of the flag is relevant in should_alloc_retry() with it being possible to loop in the allocator instead of failing the SLUB allocation and dropping back. Maybe you meant !__GFP_WAIT instead of !__GFP_NOFAIL which makes more sense. In that case, we clear both flags because __GFP_REPEAT && !_GFP_WAIT is a senseless combination of flags. If for whatever reason the __GFP_WAIT was re-added, the presense of __GFP_REPEAT could cause problems in reclaim that would be hard to spot again. > alloc_gfp would probably benefit from having a comment about why > __GFP_WAIT should be masked off here: that we don't want to do compaction > or direct reclaim or retry the allocation more than once (so both > __GFP_NORETRY and __GFP_REPEAT are no-ops). That would have been helpful all right. I should have caught that and explained it properly. In the event there is a new version of the patch, I'll add one. For the moment, I'm dropping this patch entirely. Christoph wants to maintain historic behaviour of SLUB to maximise the number of high-order pages it uses and at the end of the day, which option performs better depends entirely on the workload and machine configuration. -- Mel Gorman 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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2011-05-17 8:42 UTC|newest] Thread overview: 119+ messages / expand[flat|nested] mbox.gz Atom feed top 2011-05-13 14:03 [PATCH 0/4] Reduce impact to overall system of SLUB using high-order allocations V2 Mel Gorman 2011-05-13 14:03 ` Mel Gorman 2011-05-13 14:03 ` [PATCH 1/4] mm: vmscan: Correct use of pgdat_balanced in sleeping_prematurely Mel Gorman 2011-05-13 14:03 ` Mel Gorman 2011-05-13 14:28 ` Johannes Weiner 2011-05-13 14:28 ` Johannes Weiner 2011-05-14 16:30 ` Minchan Kim 2011-05-14 16:30 ` Minchan Kim 2011-05-16 14:30 ` Rik van Riel 2011-05-16 14:30 ` Rik van Riel 2011-05-13 14:03 ` [PATCH 2/4] mm: slub: Do not wake kswapd for SLUBs speculative high-order allocations Mel Gorman 2011-05-13 14:03 ` Mel Gorman 2011-05-16 21:10 ` David Rientjes 2011-05-16 21:10 ` David Rientjes 2011-05-18 6:09 ` Pekka Enberg 2011-05-18 6:09 ` Pekka Enberg 2011-05-18 17:21 ` Christoph Lameter 2011-05-18 17:21 ` Christoph Lameter 2011-05-13 14:03 ` [PATCH 3/4] mm: slub: Do not take expensive steps " Mel Gorman 2011-05-13 14:03 ` Mel Gorman 2011-05-16 21:16 ` David Rientjes 2011-05-16 21:16 ` David Rientjes 2011-05-17 8:42 ` Mel Gorman [this message] 2011-05-17 8:42 ` Mel Gorman 2011-05-17 13:51 ` Christoph Lameter 2011-05-17 13:51 ` Christoph Lameter 2011-05-17 16:22 ` Mel Gorman 2011-05-17 16:22 ` Mel Gorman 2011-05-17 17:52 ` Christoph Lameter 2011-05-17 17:52 ` Christoph Lameter 2011-05-17 19:35 ` David Rientjes 2011-05-17 19:35 ` David Rientjes 2011-05-17 19:31 ` David Rientjes 2011-05-17 19:31 ` David Rientjes 2011-05-13 14:03 ` [PATCH 4/4] mm: vmscan: If kswapd has been running too long, allow it to sleep Mel Gorman 2011-05-13 14:03 ` Mel Gorman 2011-05-15 10:27 ` KOSAKI Motohiro 2011-05-15 10:27 ` KOSAKI Motohiro 2011-05-16 4:21 ` James Bottomley 2011-05-16 4:21 ` James Bottomley 2011-05-16 5:04 ` Minchan Kim 2011-05-16 5:04 ` Minchan Kim 2011-05-16 8:45 ` Mel Gorman 2011-05-16 8:45 ` Mel Gorman 2011-05-16 8:45 ` Mel Gorman 2011-05-16 8:58 ` Minchan Kim 2011-05-16 8:58 ` Minchan Kim 2011-05-16 8:58 ` Minchan Kim 2011-05-16 10:27 ` Mel Gorman 2011-05-16 10:27 ` Mel Gorman 2011-05-16 10:27 ` Mel Gorman 2011-05-16 23:50 ` Minchan Kim 2011-05-16 23:50 ` Minchan Kim 2011-05-17 0:48 ` Minchan Kim 2011-05-17 0:48 ` Minchan Kim 2011-05-17 0:48 ` Minchan Kim 2011-05-17 10:38 ` Mel Gorman 2011-05-17 10:38 ` Mel Gorman 2011-05-17 10:38 ` Mel Gorman 2011-05-17 13:50 ` Colin Ian King 2011-05-17 13:50 ` Colin Ian King 2011-05-17 16:15 ` [PATCH] mm: vmscan: Correctly check if reclaimer should schedule during shrink_slab Mel Gorman 2011-05-17 16:15 ` Mel Gorman 2011-05-18 0:45 ` KOSAKI Motohiro 2011-05-18 0:45 ` KOSAKI Motohiro 2011-05-19 0:03 ` Minchan Kim 2011-05-19 0:03 ` Minchan Kim 2011-05-19 0:03 ` Minchan Kim 2011-05-19 0:09 ` Minchan Kim 2011-05-19 0:09 ` Minchan Kim 2011-05-19 0:09 ` Minchan Kim 2011-05-19 11:36 ` Colin Ian King 2011-05-19 11:36 ` Colin Ian King 2011-05-20 0:06 ` Minchan Kim 2011-05-20 0:06 ` Minchan Kim 2011-05-20 0:06 ` Minchan Kim 2011-05-18 4:19 ` [PATCH 4/4] mm: vmscan: If kswapd has been running too long, allow it to sleep Minchan Kim 2011-05-18 4:19 ` Minchan Kim 2011-05-18 7:39 ` Colin Ian King 2011-05-18 7:39 ` Colin Ian King 2011-05-18 4:09 ` James Bottomley 2011-05-18 4:09 ` James Bottomley 2011-05-18 1:05 ` KOSAKI Motohiro 2011-05-18 1:05 ` KOSAKI Motohiro 2011-05-18 5:44 ` Minchan Kim 2011-05-18 5:44 ` Minchan Kim 2011-05-18 5:44 ` Minchan Kim 2011-05-18 6:05 ` KOSAKI Motohiro 2011-05-18 6:05 ` KOSAKI Motohiro 2011-05-18 9:58 ` Mel Gorman 2011-05-18 9:58 ` Mel Gorman 2011-05-18 9:58 ` Mel Gorman 2011-05-18 22:55 ` Minchan Kim 2011-05-18 22:55 ` Minchan Kim 2011-05-18 23:54 ` KOSAKI Motohiro 2011-05-18 23:54 ` KOSAKI Motohiro 2011-05-18 0:26 ` KOSAKI Motohiro 2011-05-18 0:26 ` KOSAKI Motohiro 2011-05-18 9:57 ` Mel Gorman 2011-05-18 9:57 ` Mel Gorman 2011-05-16 8:45 ` Mel Gorman 2011-05-16 8:45 ` Mel Gorman 2011-05-16 14:30 ` Rik van Riel 2011-05-16 14:30 ` Rik van Riel 2011-05-13 15:19 ` [PATCH 0/4] Reduce impact to overall system of SLUB using high-order allocations V2 James Bottomley 2011-05-13 15:19 ` James Bottomley 2011-05-13 15:19 ` James Bottomley 2011-05-13 15:52 ` Mel Gorman 2011-05-13 15:52 ` Mel Gorman 2011-05-13 15:21 ` Christoph Lameter 2011-05-13 15:21 ` Christoph Lameter 2011-05-13 15:43 ` Mel Gorman 2011-05-13 15:43 ` Mel Gorman 2011-05-14 8:34 ` Colin Ian King 2011-05-14 8:34 ` Colin Ian King 2011-05-16 8:37 ` Mel Gorman 2011-05-16 8:37 ` Mel Gorman 2011-05-16 11:24 ` Colin Ian King 2011-05-16 11:24 ` Colin Ian King
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=20110517084227.GI5279@suse.de \ --to=mgorman@suse.de \ --cc=James.Bottomley@hansenpartnership.com \ --cc=akpm@linux-foundation.org \ --cc=chris.mason@oracle.com \ --cc=cl@linux.com \ --cc=colin.king@canonical.com \ --cc=hannes@cmpxchg.org \ --cc=jack@suse.cz \ --cc=linux-ext4@vger.kernel.org \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=penberg@kernel.org \ --cc=raghu.prabhu13@gmail.com \ --cc=riel@redhat.com \ --cc=rientjes@google.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.