public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Johannes Weiner <hannes@cmpxchg.org>
To: Vlastimil Babka <vbabka@suse.cz>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Mel Gorman <mgorman@techsingularity.net>,
	Michal Hocko <mhocko@suse.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	kernel-team@fb.com
Subject: Re: [PATCH 2/5] mm: compaction: simplify should_compact_retry()
Date: Fri, 2 Jun 2023 10:47:05 -0400	[thread overview]
Message-ID: <20230602144705.GB161817@cmpxchg.org> (raw)
In-Reply-To: <20230529163805.GA84971@cmpxchg.org>

On Mon, May 29, 2023 at 12:38:07PM -0400, Johannes Weiner wrote:
> On Mon, May 29, 2023 at 03:03:52PM +0200, Vlastimil Babka wrote:
> > I think you simplified this part too much, so now once it runs out of
> > retries, it will return false, while previously it would increase the priority.

Here is the delta fix. If this looks good to everybody, can you please
fold this into the patch you have in tree? Thanks!

---
From 4b9429f9ef04fcb7bb5ffae0db8ea113b26d097b Mon Sep 17 00:00:00 2001
From: Johannes Weiner <hannes@cmpxchg.org>
Date: Fri, 2 Jun 2023 16:02:37 +0200
Subject: [PATCH] mm: compaction: simplify should_compact_retry() fix

Vlastimil points out an unintended change. Previously when hitting
max_retries we'd bump the priority level and restart the loop. Now we
bail out and fail instead. Restore the original behavior.

Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
---
 mm/page_alloc.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 72660e924b95..e7d7db36582b 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -3768,6 +3768,15 @@ should_compact_retry(struct alloc_context *ac, int order, int alloc_flags,
 	if (fatal_signal_pending(current))
 		return false;
 
+	/*
+	 * Compaction was skipped due to a lack of free order-0
+	 * migration targets. Continue if reclaim can help.
+	 */
+	if (compact_result == COMPACT_SKIPPED) {
+		ret = compaction_zonelist_suitable(ac, order, alloc_flags);
+		goto out;
+	}
+
 	/*
 	 * Compaction managed to coalesce some page blocks, but the
 	 * allocation failed presumably due to a race. Retry some.
@@ -3785,17 +3794,10 @@ should_compact_retry(struct alloc_context *ac, int order, int alloc_flags,
 		if (order > PAGE_ALLOC_COSTLY_ORDER)
 			max_retries /= 4;
 
-		ret = ++(*compaction_retries) <= max_retries;
-		goto out;
-	}
-
-	/*
-	 * Compaction was skipped due to a lack of free order-0
-	 * migration targets. Continue if reclaim can help.
-	 */
-	if (compact_result == COMPACT_SKIPPED) {
-		ret = compaction_zonelist_suitable(ac, order, alloc_flags);
-		goto out;
+		if (++(*compaction_retries) <= max_retries) {
+			ret = true;
+			goto out;
+		}
 	}
 
 	/*
-- 
2.40.1


  reply	other threads:[~2023-06-02 14:47 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-19 12:39 [PATCH 0/5] mm: compaction: cleanups & simplifications Johannes Weiner
2023-05-19 12:39 ` [PATCH 1/5] mm: compaction: remove compaction result helpers Johannes Weiner
2023-05-29  9:58   ` Vlastimil Babka
2023-05-19 12:39 ` [PATCH 2/5] mm: compaction: simplify should_compact_retry() Johannes Weiner
2023-05-29 13:03   ` Vlastimil Babka
2023-05-29 16:38     ` Johannes Weiner
2023-06-02 14:47       ` Johannes Weiner [this message]
2023-06-06 12:58         ` Vlastimil Babka
2023-05-19 12:39 ` [PATCH 3/5] mm: compaction: refactor __compaction_suitable() Johannes Weiner
2023-05-29 17:11   ` Vlastimil Babka
2023-06-02 14:49     ` Johannes Weiner
2023-05-19 12:39 ` [PATCH 4/5] mm: compaction: remove unnecessary is_via_compact_memory() checks Johannes Weiner
2023-05-29 17:12   ` Vlastimil Babka
2023-05-19 12:39 ` [PATCH 5/5] mm: compaction: drop redundant watermark check in compaction_zonelist_suitable() Johannes Weiner
2023-05-29 17:12   ` Vlastimil Babka
2023-06-02 15:12 ` [PATCH 6/5] mm: compaction: have compaction_suitable() return bool Johannes Weiner
2023-06-06 13:04   ` 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=20230602144705.GB161817@cmpxchg.org \
    --to=hannes@cmpxchg.org \
    --cc=akpm@linux-foundation.org \
    --cc=kernel-team@fb.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@techsingularity.net \
    --cc=mhocko@suse.com \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox