All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vlastimil Babka <vbabka@suse.cz>
To: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>, mhocko@kernel.org
Cc: linux-mm@kvack.org, akpm@linux-foundation.org,
	iamjoonsoo.kim@lge.com, riel@redhat.com, rientjes@google.com,
	mgorman@techsingularity.net, hannes@cmpxchg.org,
	linux-kernel@vger.kernel.org, torvalds@linux-foundation.org
Subject: Re: [RFC 02/13] mm, page_alloc: set alloc_flags only once in slowpath
Date: Tue, 10 May 2016 14:30:11 +0200	[thread overview]
Message-ID: <5731D453.8050104@suse.cz> (raw)
In-Reply-To: <201605102028.AAC26596.SMHOQOtLOFFFVJ@I-love.SAKURA.ne.jp>

On 05/10/2016 01:28 PM, Tetsuo Handa wrote:
> Vlastimil Babka wrote:
>> In __alloc_pages_slowpath(), alloc_flags doesn't change after it's initialized,
>> so move the initialization above the retry: label. Also make the comment above
>> the initialization more descriptive.
> 
> Not true. gfp_to_alloc_flags() will include ALLOC_NO_WATERMARKS if current
> thread got TIF_MEMDIE after gfp_to_alloc_flags() was called for the first

Oh, right. Stupid global state.

> time. Do you want to make TIF_MEMDIE threads fail their allocations without
> using memory reserves?

No, thanks for catching this. How about the following version? I think
that's even nicer cleanup, if correct. Note it causes a conflict in
patch 03/13 but it's simple to resolve.

Thanks

----8<----

WARNING: multiple messages have this Message-ID (diff)
From: Vlastimil Babka <vbabka@suse.cz>
To: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>, mhocko@kernel.org
Cc: linux-mm@kvack.org, akpm@linux-foundation.org,
	iamjoonsoo.kim@lge.com, riel@redhat.com, rientjes@google.com,
	mgorman@techsingularity.net, hannes@cmpxchg.org,
	linux-kernel@vger.kernel.org, torvalds@linux-foundation.org
Subject: Re: [RFC 02/13] mm, page_alloc: set alloc_flags only once in slowpath
Date: Tue, 10 May 2016 14:30:11 +0200	[thread overview]
Message-ID: <5731D453.8050104@suse.cz> (raw)
In-Reply-To: <201605102028.AAC26596.SMHOQOtLOFFFVJ@I-love.SAKURA.ne.jp>

On 05/10/2016 01:28 PM, Tetsuo Handa wrote:
> Vlastimil Babka wrote:
>> In __alloc_pages_slowpath(), alloc_flags doesn't change after it's initialized,
>> so move the initialization above the retry: label. Also make the comment above
>> the initialization more descriptive.
> 
> Not true. gfp_to_alloc_flags() will include ALLOC_NO_WATERMARKS if current
> thread got TIF_MEMDIE after gfp_to_alloc_flags() was called for the first

Oh, right. Stupid global state.

> time. Do you want to make TIF_MEMDIE threads fail their allocations without
> using memory reserves?

No, thanks for catching this. How about the following version? I think
that's even nicer cleanup, if correct. Note it causes a conflict in
patch 03/13 but it's simple to resolve.

Thanks

----8<----
>From 68f09f1d4381c7451238b4575557580380d8bf30 Mon Sep 17 00:00:00 2001
From: Vlastimil Babka <vbabka@suse.cz>
Date: Fri, 29 Apr 2016 11:51:17 +0200
Subject: [RFC 02/13] mm, page_alloc: set alloc_flags only once in slowpath

In __alloc_pages_slowpath(), alloc_flags doesn't change after it's initialized,
so move the initialization above the retry: label. Also make the comment above
the initialization more descriptive.

The only exception in the alloc_flags being constant is ALLOC_NO_WATERMARKS,
which may change due to TIF_MEMDIE being set on the allocating thread. We can
fix this, and make the code simpler and a bit more effective at the same time,
by moving the part that determines ALLOC_NO_WATERMARKS from
gfp_to_alloc_flags() to gfp_pfmemalloc_allowed(). This means we don't have to
mask out ALLOC_NO_WATERMARKS in several places in __alloc_pages_slowpath()
anymore.  The only test for the flag can instead call gfp_pfmemalloc_allowed().

Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
 mm/page_alloc.c | 49 ++++++++++++++++++++++++-------------------------
 1 file changed, 24 insertions(+), 25 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index a50184ec6ca0..1b58facf4b5e 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -3216,8 +3216,7 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
 	 */
 	count_vm_event(COMPACTSTALL);
 
-	page = get_page_from_freelist(gfp_mask, order,
-					alloc_flags & ~ALLOC_NO_WATERMARKS, ac);
+	page = get_page_from_freelist(gfp_mask, order, alloc_flags, ac);
 
 	if (page) {
 		struct zone *zone = page_zone(page);
@@ -3366,8 +3365,7 @@ __alloc_pages_direct_reclaim(gfp_t gfp_mask, unsigned int order,
 		return NULL;
 
 retry:
-	page = get_page_from_freelist(gfp_mask, order,
-					alloc_flags & ~ALLOC_NO_WATERMARKS, ac);
+	page = get_page_from_freelist(gfp_mask, order, alloc_flags, ac);
 
 	/*
 	 * If an allocation failed after direct reclaim, it could be because
@@ -3425,16 +3423,6 @@ gfp_to_alloc_flags(gfp_t gfp_mask)
 	} else if (unlikely(rt_task(current)) && !in_interrupt())
 		alloc_flags |= ALLOC_HARDER;
 
-	if (likely(!(gfp_mask & __GFP_NOMEMALLOC))) {
-		if (gfp_mask & __GFP_MEMALLOC)
-			alloc_flags |= ALLOC_NO_WATERMARKS;
-		else if (in_serving_softirq() && (current->flags & PF_MEMALLOC))
-			alloc_flags |= ALLOC_NO_WATERMARKS;
-		else if (!in_interrupt() &&
-				((current->flags & PF_MEMALLOC) ||
-				 unlikely(test_thread_flag(TIF_MEMDIE))))
-			alloc_flags |= ALLOC_NO_WATERMARKS;
-	}
 #ifdef CONFIG_CMA
 	if (gfpflags_to_migratetype(gfp_mask) == MIGRATE_MOVABLE)
 		alloc_flags |= ALLOC_CMA;
@@ -3444,7 +3432,19 @@ gfp_to_alloc_flags(gfp_t gfp_mask)
 
 bool gfp_pfmemalloc_allowed(gfp_t gfp_mask)
 {
-	return !!(gfp_to_alloc_flags(gfp_mask) & ALLOC_NO_WATERMARKS);
+	if (unlikely(gfp_mask & __GFP_NOMEMALLOC))
+		return false;
+
+	if (gfp_mask & __GFP_MEMALLOC)
+		return true;
+	if (in_serving_softirq() && (current->flags & PF_MEMALLOC))
+		return true;
+	if (!in_interrupt() &&
+			((current->flags & PF_MEMALLOC) ||
+			 unlikely(test_thread_flag(TIF_MEMDIE))))
+		return true;
+
+	return false;
 }
 
 static inline bool is_thp_gfp_mask(gfp_t gfp_mask)
@@ -3579,25 +3579,24 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
 				(__GFP_ATOMIC|__GFP_DIRECT_RECLAIM)))
 		gfp_mask &= ~__GFP_ATOMIC;
 
-retry:
-	if (gfp_mask & __GFP_KSWAPD_RECLAIM)
-		wake_all_kswapds(order, ac);
-
 	/*
-	 * OK, we're below the kswapd watermark and have kicked background
-	 * reclaim. Now things get more complex, so set up alloc_flags according
-	 * to how we want to proceed.
+	 * The fast path uses conservative alloc_flags to succeed only until
+	 * kswapd needs to be woken up, and to avoid the cost of setting up
+	 * alloc_flags precisely. So we do that now.
 	 */
 	alloc_flags = gfp_to_alloc_flags(gfp_mask);
 
+retry:
+	if (gfp_mask & __GFP_KSWAPD_RECLAIM)
+		wake_all_kswapds(order, ac);
+
 	/* This is the last chance, in general, before the goto nopage. */
-	page = get_page_from_freelist(gfp_mask, order,
-				alloc_flags & ~ALLOC_NO_WATERMARKS, ac);
+	page = get_page_from_freelist(gfp_mask, order, alloc_flags, ac);
 	if (page)
 		goto got_pg;
 
 	/* Allocate without watermarks if the context allows */
-	if (alloc_flags & ALLOC_NO_WATERMARKS) {
+	if (gfp_pfmemalloc_allowed(gfp_mask)) {
 		/*
 		 * Ignore mempolicies if ALLOC_NO_WATERMARKS on the grounds
 		 * the allocation is high priority and these type of
-- 
2.8.2

  reply	other threads:[~2016-05-10 12:30 UTC|newest]

Thread overview: 122+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-10  7:35 [RFC 00/13] make direct compaction more deterministic Vlastimil Babka
2016-05-10  7:35 ` Vlastimil Babka
2016-05-10  7:35 ` [RFC 01/13] mm, compaction: don't isolate PageWriteback pages in MIGRATE_SYNC_LIGHT mode Vlastimil Babka
2016-05-10  7:35   ` Vlastimil Babka
2016-05-11 12:40   ` Michal Hocko
2016-05-11 12:40     ` Michal Hocko
2016-05-10  7:35 ` [RFC 02/13] mm, page_alloc: set alloc_flags only once in slowpath Vlastimil Babka
2016-05-10  7:35   ` Vlastimil Babka
2016-05-10 11:28   ` Tetsuo Handa
2016-05-10 11:28     ` Tetsuo Handa
2016-05-10 12:30     ` Vlastimil Babka [this message]
2016-05-10 12:30       ` Vlastimil Babka
2016-05-12 12:41       ` Michal Hocko
2016-05-12 12:41         ` Michal Hocko
2016-05-31  6:20       ` Joonsoo Kim
2016-05-31  6:20         ` Joonsoo Kim
2016-05-31  7:59         ` Vlastimil Babka
2016-05-31  7:59           ` Vlastimil Babka
2016-06-02  1:50           ` Joonsoo Kim
2016-06-02  1:50             ` Joonsoo Kim
2016-05-10  7:35 ` [RFC 03/13] mm, page_alloc: don't retry initial attempt " Vlastimil Babka
2016-05-10  7:35   ` Vlastimil Babka
2016-05-12 12:48   ` Michal Hocko
2016-05-12 12:48     ` Michal Hocko
2016-05-31  6:25   ` Joonsoo Kim
2016-05-31  6:25     ` Joonsoo Kim
2016-05-31 12:03     ` Vlastimil Babka
2016-05-31 12:03       ` Vlastimil Babka
2016-05-10  7:35 ` [RFC 04/13] mm, page_alloc: restructure direct compaction handling " Vlastimil Babka
2016-05-10  7:35   ` Vlastimil Babka
2016-05-12 13:29   ` Michal Hocko
2016-05-12 13:29     ` Michal Hocko
2016-05-13  8:10     ` Vlastimil Babka
2016-05-13  8:10       ` Vlastimil Babka
2016-05-13  8:31       ` Michal Hocko
2016-05-13  8:31         ` Michal Hocko
2016-05-10  7:35 ` [RFC 05/13] mm, page_alloc: make THP-specific decisions more generic Vlastimil Babka
2016-05-10  7:35   ` Vlastimil Babka
2016-05-12 13:43   ` Michal Hocko
2016-05-12 13:43     ` Michal Hocko
2016-05-10  7:35 ` [RFC 06/13] mm, thp: remove __GFP_NORETRY from khugepaged and madvised allocations Vlastimil Babka
2016-05-10  7:35   ` Vlastimil Babka
2016-05-12 16:20   ` Michal Hocko
2016-05-12 16:20     ` Michal Hocko
2016-05-13  8:23     ` Vlastimil Babka
2016-05-13  8:23       ` Vlastimil Babka
2016-05-13 12:05       ` Michal Hocko
2016-05-13 12:05         ` Michal Hocko
2016-05-18 11:59         ` Vlastimil Babka
2016-05-18 11:59           ` Vlastimil Babka
2016-05-18 15:24           ` Michal Hocko
2016-05-18 15:24             ` Michal Hocko
2016-05-20 13:57             ` Vlastimil Babka
2016-05-20 13:57               ` Vlastimil Babka
2016-05-23  8:39               ` Michal Hocko
2016-05-23  8:39                 ` Michal Hocko
2016-05-10  7:35 ` [RFC 07/13] mm, compaction: introduce direct compaction priority Vlastimil Babka
2016-05-10  7:35   ` Vlastimil Babka
2016-05-13 12:37   ` Michal Hocko
2016-05-13 12:37     ` Michal Hocko
2016-05-10  7:35 ` [RFC 08/13] mm, compaction: simplify contended compaction handling Vlastimil Babka
2016-05-10  7:35   ` Vlastimil Babka
2016-05-13 13:09   ` Michal Hocko
2016-05-13 13:09     ` Michal Hocko
2016-05-16  7:10     ` Vlastimil Babka
2016-05-16  7:10       ` Vlastimil Babka
2016-05-10  7:35 ` [RFC 09/13] mm, compaction: make whole_zone flag ignore cached scanner positions Vlastimil Babka
2016-05-10  7:35   ` Vlastimil Babka
2016-05-13 13:23   ` Michal Hocko
2016-05-13 13:23     ` Michal Hocko
2016-05-10  7:36 ` [RFC 10/13] mm, compaction: cleanup unused functions Vlastimil Babka
2016-05-10  7:36   ` Vlastimil Babka
2016-05-10  7:36 ` [RFC 11/13] mm, compaction: add the ultimate direct compaction priority Vlastimil Babka
2016-05-10  7:36   ` Vlastimil Babka
2016-05-13 13:38   ` Michal Hocko
2016-05-13 13:38     ` Michal Hocko
2016-05-16  7:17     ` Vlastimil Babka
2016-05-16  7:17       ` Vlastimil Babka
2016-05-16  8:11       ` Michal Hocko
2016-05-16  8:11         ` Michal Hocko
2016-05-18 12:46       ` Vlastimil Babka
2016-05-18 12:46         ` Vlastimil Babka
2016-05-10  7:36 ` [RFC 12/13] mm, compaction: more reliably increase " Vlastimil Babka
2016-05-10  7:36   ` Vlastimil Babka
2016-05-10 12:55   ` Vlastimil Babka
2016-05-10 12:55     ` Vlastimil Babka
2016-05-13 14:15   ` Michal Hocko
2016-05-13 14:15     ` Michal Hocko
2016-05-16  7:31     ` Vlastimil Babka
2016-05-16  7:31       ` Vlastimil Babka
2016-05-16  8:14       ` Michal Hocko
2016-05-16  8:14         ` Michal Hocko
2016-05-16  9:27         ` Vlastimil Babka
2016-05-16  9:27           ` Vlastimil Babka
2016-05-16  9:52           ` Michal Hocko
2016-05-16  9:52             ` Michal Hocko
2016-05-31  6:37   ` Joonsoo Kim
2016-05-31  6:37     ` Joonsoo Kim
2016-05-31 12:07     ` Vlastimil Babka
2016-05-31 12:07       ` Vlastimil Babka
2016-05-31 12:29       ` Vlastimil Babka
2016-05-31 12:29         ` Vlastimil Babka
2016-06-02  2:50         ` Joonsoo Kim
2016-06-02  2:50           ` Joonsoo Kim
2016-05-10  7:36 ` [RFC 13/13] mm, compaction: fix and improve watermark handling Vlastimil Babka
2016-05-10  7:36   ` Vlastimil Babka
2016-05-16  9:25   ` Michal Hocko
2016-05-16  9:25     ` Michal Hocko
2016-05-16  9:50     ` Vlastimil Babka
2016-05-16  9:50       ` Vlastimil Babka
2016-05-16 12:30       ` Michal Hocko
2016-05-16 12:30         ` Michal Hocko
2016-05-18 13:50     ` Mel Gorman
2016-05-18 13:50       ` Mel Gorman
2016-05-18 14:27       ` Michal Hocko
2016-05-18 14:27         ` Michal Hocko
2016-05-18 14:40         ` Mel Gorman
2016-05-18 14:40           ` Mel Gorman
2016-05-17 20:01 ` [RFC 00/13] make direct compaction more deterministic Michal Hocko
2016-05-17 20:01   ` Michal Hocko
2016-05-18  7:19   ` Vlastimil Babka
2016-05-18  7:19     ` 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=5731D453.8050104@suse.cz \
    --to=vbabka@suse.cz \
    --cc=akpm@linux-foundation.org \
    --cc=hannes@cmpxchg.org \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@techsingularity.net \
    --cc=mhocko@kernel.org \
    --cc=penguin-kernel@I-love.SAKURA.ne.jp \
    --cc=riel@redhat.com \
    --cc=rientjes@google.com \
    --cc=torvalds@linux-foundation.org \
    /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.