From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752696AbcGSWgq (ORCPT ); Tue, 19 Jul 2016 18:36:46 -0400 Received: from mail-pa0-f49.google.com ([209.85.220.49]:34172 "EHLO mail-pa0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751701AbcGSWgo (ORCPT ); Tue, 19 Jul 2016 18:36:44 -0400 Date: Tue, 19 Jul 2016 15:36:37 -0700 (PDT) From: David Rientjes X-X-Sender: rientjes@chino.kir.corp.google.com To: Vlastimil Babka cc: Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Michal Hocko , Mel Gorman , Joonsoo Kim , Rik van Riel Subject: Re: [PATCH 3/8] mm, page_alloc: don't retry initial attempt in slowpath In-Reply-To: <20160718112302.27381-4-vbabka@suse.cz> Message-ID: References: <20160718112302.27381-1-vbabka@suse.cz> <20160718112302.27381-4-vbabka@suse.cz> User-Agent: Alpine 2.10 (DEB 1266 2009-07-14) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 18 Jul 2016, Vlastimil Babka wrote: > After __alloc_pages_slowpath() sets up new alloc_flags and wakes up kswapd, it > first tries get_page_from_freelist() with the new alloc_flags, as it may > succeed e.g. due to using min watermark instead of low watermark. It makes > sense to to do this attempt before adjusting zonelist based on > alloc_flags/gfp_mask, as it's still relatively a fast path if we just wake up > kswapd and successfully allocate. > > This patch therefore moves the initial attempt above the retry label and > reorganizes a bit the part below the retry label. We still have to attempt > get_page_from_freelist() on each retry, as some allocations cannot do that > as part of direct reclaim or compaction, and yet are not allowed to fail > (even though they do a WARN_ON_ONCE() and thus should not exist). We can reuse > the call meant for ALLOC_NO_WATERMARKS attempt and just set alloc_flags to > ALLOC_NO_WATERMARKS if the context allows it. As a side-effect, the attempts > from direct reclaim/compaction will also no longer obey watermarks once this > is set, but there's little harm in that. > > Kswapd wakeups are also done on each retry to be safe from potential races > resulting in kswapd going to sleep while a process (that may not be able to > reclaim by itself) is still looping. > > Signed-off-by: Vlastimil Babka > --- > mm/page_alloc.c | 29 ++++++++++++++++++----------- > 1 file changed, 18 insertions(+), 11 deletions(-) > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index eb1968a1041e..30443804f156 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -3541,35 +3541,42 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order, > */ > alloc_flags = gfp_to_alloc_flags(gfp_mask); > > + if (gfp_mask & __GFP_KSWAPD_RECLAIM) > + wake_all_kswapds(order, ac); > + > + /* > + * The adjusted alloc_flags might result in immediate success, so try > + * that first > + */ > + page = get_page_from_freelist(gfp_mask, order, alloc_flags, ac); > + if (page) > + goto got_pg; Any reason to not test gfp_pfmemalloc_allowed() here? For contexts where it returns true, it seems like the above would be an unneeded failure if ALLOC_WMARK_MIN would have failed. No strong opinion. > + > + > retry: > + /* Ensure kswapd doesn't accidentally go to sleep as long as we loop */ > if (gfp_mask & __GFP_KSWAPD_RECLAIM) > wake_all_kswapds(order, ac); > > + if (gfp_pfmemalloc_allowed(gfp_mask)) > + alloc_flags = ALLOC_NO_WATERMARKS; > + > /* > * Reset the zonelist iterators if memory policies can be ignored. > * These allocations are high priority and system rather than user > * orientated. > */ > - if (!(alloc_flags & ALLOC_CPUSET) || gfp_pfmemalloc_allowed(gfp_mask)) { > + if (!(alloc_flags & ALLOC_CPUSET) || (alloc_flags & ALLOC_NO_WATERMARKS)) { Do we need to test ALLOC_NO_WATERMARKS here, or is it just for clarity? Otherwise looks good! > ac->zonelist = node_zonelist(numa_node_id(), gfp_mask); > ac->preferred_zoneref = first_zones_zonelist(ac->zonelist, > ac->high_zoneidx, ac->nodemask); > } > > - /* This is the last chance, in general, before the goto nopage. */ > + /* Attempt with potentially adjusted zonelist and alloc_flags */ > page = get_page_from_freelist(gfp_mask, order, alloc_flags, ac); > if (page) > goto got_pg; > > - /* Allocate without watermarks if the context allows */ > - if (gfp_pfmemalloc_allowed(gfp_mask)) { > - > - page = get_page_from_freelist(gfp_mask, order, > - ALLOC_NO_WATERMARKS, ac); > - if (page) > - goto got_pg; > - } > - > /* Caller is not willing to reclaim, we can't balance anything */ > if (!can_direct_reclaim) { > /*