Re: [PATCH 0/6] Optimise page alloc/free fast paths followup v2

[Mel Gorman <mgorman@techsingularity.net>]

On Mon, May 02, 2016 at 10:54:23AM +0200, Vlastimil Babka wrote:
> On 04/27/2016 04:57 PM, Mel Gorman wrote:
> > as the patch "mm, page_alloc: inline the fast path of the zonelist iterator"
> > is fine. The nodemask pointer is the same between cpuset retries. If the
> > zonelist changes due to ALLOC_NO_WATERMARKS *and* it races with a cpuset
> > change then there is a second harmless pass through the page allocator.
> 
> True. But I just realized (while working on direct compaction priorities)
> that there's another subtle issue with the ALLOC_NO_WATERMARKS part.
> According to the comment it should be ignoring mempolicies, but it still
> honours ac.nodemask, and your patch is replacing NULL ac.nodemask with the
> mempolicy one.
> 
> I think it's possibly easily fixed outside the fast path like this. If
> you agree, consider it has my s-o-b:
> 

While I see your point, I don't necessarily see why this fixes it as the
original nodemask may also be a restricted set that ALLOC_NO_WATERMARKS
should ignore. How about this?

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 79100583b9de..dbb08d102d41 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -3432,9 +3432,13 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
 		/*
 		 * Ignore mempolicies if ALLOC_NO_WATERMARKS on the grounds
 		 * the allocation is high priority and these type of
-		 * allocations are system rather than user orientated
+		 * allocations are system rather than user orientated. If a
+		 * cpuset retry occurs then these values persist across the
+		 * retry but that's ok for a context ignoring watermarks.
 		 */
 		ac->zonelist = node_zonelist(numa_node_id(), gfp_mask);
+		ac->high_zoneidx = MAX_NR_ZONES - 1;
+		ac->nodemask = NULL;
 		page = get_page_from_freelist(gfp_mask, order,
 						ALLOC_NO_WATERMARKS, ac);
 		if (page)

-- 
Mel Gorman
SUSE Labs