From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755794AbcECIus (ORCPT ); Tue, 3 May 2016 04:50:48 -0400 Received: from outbound-smtp05.blacknight.com ([81.17.249.38]:55010 "EHLO outbound-smtp05.blacknight.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755774AbcECIun (ORCPT ); Tue, 3 May 2016 04:50:43 -0400 Date: Tue, 3 May 2016 09:50:39 +0100 From: Mel Gorman To: Vlastimil Babka Cc: Andrew Morton , Jesper Dangaard Brouer , Linux-MM , LKML Subject: Re: [PATCH 0/6] Optimise page alloc/free fast paths followup v2 Message-ID: <20160503085039.GS2858@techsingularity.net> References: <1461769043-28337-1-git-send-email-mgorman@techsingularity.net> <572715BF.3000003@suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline In-Reply-To: <572715BF.3000003@suse.cz> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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