All of lore.kernel.org
 help / color / mirror / Atom feed
From: Barry Song <21cnbao@gmail.com>
To: akpm@linux-foundation.org, baolin.wang@linux.alibaba.com,
	linux-mm@kvack.org
Cc: david@redhat.com, hannes@cmpxchg.org, huzhanyuan@oppo.com,
	linux-kernel@vger.kernel.org, mgorman@techsingularity.net,
	shikemeng@huaweicloud.com, v-songbaohua@oppo.com,
	willy@infradead.org
Subject: [PATCH] mm: compaction: avoid fast_isolate_freepages blindly choose improper pageblock
Date: Thu,  7 Dec 2023 00:00:54 +1300	[thread overview]
Message-ID: <20231206110054.61617-1-v-songbaohua@oppo.com> (raw)

Testing shows fast_isolate_freepages can blindly choose an unsuitable
pageblock from time to time particularly while the min mark is used
from XXX path:
 if (!page) {
         cc->fast_search_fail++;
         if (scan_start) {
                 /*
                  * Use the highest PFN found above min. If one was
                  * not found, be pessimistic for direct compaction
                  * and use the min mark.
                  */
                 if (highest >= min_pfn) {
                         page = pfn_to_page(highest);
                         cc->free_pfn = highest;
                 } else {
                         if (cc->direct_compaction && pfn_valid(min_pfn)) { /* XXX */
                                 page = pageblock_pfn_to_page(min_pfn,
                                         min(pageblock_end_pfn(min_pfn),
                                             zone_end_pfn(cc->zone)),
                                         cc->zone);
                                 cc->free_pfn = min_pfn;
                         }
                 }
         }
 }

The reason is that no code is doing any check on the min_pfn
 min_pfn = pageblock_start_pfn(cc->free_pfn - (distance >> 1));

In contrast, slow path of isolate_freepages() is always skipping unsuitable
pageblocks in a decent way.

This issue doesn't happen quite often. When running 25 machines with 16GiB
memory for one night, most of them can hit this unexpected code path.
However the frequency isn't like many times per second. It might be one
time in a couple of hours. Thus, it is very hard to measure the visible
performance impact in my machines though the affection of choosing the
unsuitable migration_target should be negative in theory.

I feel it's still worth fixing this to at least make the code theoretically
self-explanatory as it is quite odd an unsuitable migration_target can be
still migration_target.

Reported-by: Zhanyuan Hu <huzhanyuan@oppo.com>
Signed-off-by: Barry Song <v-songbaohua@oppo.com>
---
 v1:
    move the fix to the specific min_pfn path with respect to Baolin's comment
 rfc:
    https://lore.kernel.org/linux-mm/20231129104530.63787-1-v-songbaohua@oppo.com/#t

 mm/compaction.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/mm/compaction.c b/mm/compaction.c
index 01ba298739dd..de15a2ef0af5 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -1611,6 +1611,9 @@ static void fast_isolate_freepages(struct compact_control *cc)
 						min(pageblock_end_pfn(min_pfn),
 						    zone_end_pfn(cc->zone)),
 						cc->zone);
+					if (page && !suitable_migration_target(cc, page))
+						page = NULL;
+
 					cc->free_pfn = min_pfn;
 				}
 			}
-- 
2.34.1


             reply	other threads:[~2023-12-06 11:03 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-06 11:00 Barry Song [this message]
2023-12-07  1:57 ` [PATCH] mm: compaction: avoid fast_isolate_freepages blindly choose improper pageblock Baolin Wang
2023-12-13 13:02 ` Mel Gorman
2023-12-14  1:34   ` Barry Song
2023-12-14 22:17     ` Andrew Morton

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=20231206110054.61617-1-v-songbaohua@oppo.com \
    --to=21cnbao@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=baolin.wang@linux.alibaba.com \
    --cc=david@redhat.com \
    --cc=hannes@cmpxchg.org \
    --cc=huzhanyuan@oppo.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@techsingularity.net \
    --cc=shikemeng@huaweicloud.com \
    --cc=v-songbaohua@oppo.com \
    --cc=willy@infradead.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.