All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mel Gorman <mgorman@techsingularity.net>
To: Vlastimil Babka <vbabka@suse.cz>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Jesper Dangaard Brouer <brouer@redhat.com>,
	Linux-MM <linux-mm@kvack.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 0/6] Optimise page alloc/free fast paths followup v2
Date: Tue, 3 May 2016 09:50:39 +0100	[thread overview]
Message-ID: <20160503085039.GS2858@techsingularity.net> (raw)
In-Reply-To: <572715BF.3000003@suse.cz>

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

--
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>

WARNING: multiple messages have this Message-ID (diff)
From: Mel Gorman <mgorman@techsingularity.net>
To: Vlastimil Babka <vbabka@suse.cz>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Jesper Dangaard Brouer <brouer@redhat.com>,
	Linux-MM <linux-mm@kvack.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 0/6] Optimise page alloc/free fast paths followup v2
Date: Tue, 3 May 2016 09:50:39 +0100	[thread overview]
Message-ID: <20160503085039.GS2858@techsingularity.net> (raw)
In-Reply-To: <572715BF.3000003@suse.cz>

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

  reply	other threads:[~2016-05-03  8:50 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-27 14:57 [PATCH 0/6] Optimise page alloc/free fast paths followup v2 Mel Gorman
2016-04-27 14:57 ` Mel Gorman
2016-04-27 14:57 ` [PATCH 1/6] mm, page_alloc: Only check PageCompound for high-order pages -fix Mel Gorman
2016-04-27 14:57   ` Mel Gorman
2016-04-27 14:57 ` [PATCH 2/6] mm, page_alloc: move might_sleep_if check to the allocator slowpath -revert Mel Gorman
2016-04-27 14:57   ` Mel Gorman
2016-04-27 14:57 ` [PATCH 3/6] mm, page_alloc: Check once if a zone has isolated pageblocks -fix Mel Gorman
2016-04-27 14:57   ` Mel Gorman
2016-04-27 14:57 ` [PATCH 4/6] mm, page_alloc: un-inline the bad part of free_pages_check Mel Gorman
2016-04-27 14:57   ` Mel Gorman
2016-04-27 14:57 ` [PATCH 5/6] mm, page_alloc: pull out side effects from free_pages_check Mel Gorman
2016-04-27 14:57   ` Mel Gorman
2016-04-27 14:57 ` [PATCH 6/6] mm, page_alloc: don't duplicate code in free_pcp_prepare Mel Gorman
2016-04-27 14:57   ` Mel Gorman
2016-05-02  8:54 ` [PATCH 0/6] Optimise page alloc/free fast paths followup v2 Vlastimil Babka
2016-05-02  8:54   ` Vlastimil Babka
2016-05-03  8:50   ` Mel Gorman [this message]
2016-05-03  8:50     ` Mel Gorman
2016-05-03 14:33     ` Vlastimil Babka
2016-05-03 14:33       ` Vlastimil Babka

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=20160503085039.GS2858@techsingularity.net \
    --to=mgorman@techsingularity.net \
    --cc=akpm@linux-foundation.org \
    --cc=brouer@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --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: link
Be 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.