On 2024/4/9 22:46, Zi Yan wrote:
On 9 Apr 2024, at 5:31, Baolin Wang wrote:
On 2024/4/8 22:23, Johannes Weiner wrote:
On Mon, Apr 08, 2024 at 09:38:20AM +0200, Vlastimil Babka wrote:
On 4/7/24 12:19 PM, Baolin Wang wrote:
On 2024/3/21 02:02, Johannes Weiner wrote:
+ account_freepages(page, zone, 1 << order, migratetype);
+
while (order < MAX_PAGE_ORDER) {
- if (compaction_capture(capc, page, order, migratetype)) {
- __mod_zone_freepage_state(zone, -(1 << order),
- migratetype);
+ int buddy_mt = migratetype;
+
+ if (compaction_capture(capc, page, order, migratetype))
return;
- }
IIUC, if the released page is captured by compaction, then the
statistics for free pages should be correspondingly decreased,
otherwise, there will be a slight regression for my thpcompact benchmark.
thpcompact Percentage Faults Huge
k6.9-rc2-base base + patch10 + 2 fixes
Percentage huge-1 78.18 ( 0.00%) 71.92 ( -8.01%)
Percentage huge-3 86.70 ( 0.00%) 86.07 ( -0.73%)
Percentage huge-5 90.26 ( 0.00%) 78.02 ( -13.57%)
Percentage huge-7 92.34 ( 0.00%) 78.67 ( -14.81%)
Percentage huge-12 91.18 ( 0.00%) 81.04 ( -11.12%)
Percentage huge-18 89.00 ( 0.00%) 79.57 ( -10.60%)
Percentage huge-24 90.52 ( 0.00%) 80.07 ( -11.54%)
Percentage huge-30 94.44 ( 0.00%) 96.28 ( 1.95%)
Percentage huge-32 93.09 ( 0.00%) 99.39 ( 6.77%)
I add below fix based on your fix 2, then the thpcompact Percentage
looks good. How do you think for the fix?
Yeah another well spotted, thanks. "slight regression" is an understatement,
this affects not just a "statistics" but very important counter
NR_FREE_PAGES which IIUC would eventually become larger than reality, make
the watermark checks false positive and result in depleted reserves etc etc.
Actually wondering why we're not seeing -next failures already (or maybe I
just haven't noticed).
Good catch indeed.
Trying to understand why I didn't notice this during testing, and I
think it's because I had order-10 pageblocks in my config. There is
this in compaction_capture():
if (order < pageblock_order && migratetype == MIGRATE_MOVABLE)
return false;
Most compaction is for order-9 THPs on movable blocks, so I didn't get
much capturing in practice in order for that leak to be noticable.
This makes me wonder why not use 'cc->migratetype' for migratetype comparison, so that low-order (like mTHP) compaction can directly get the released pages, which could avoid some compaction scans without mixing the migratetype?
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 2facf844ef84..7a64020f8222 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -622,7 +622,7 @@ compaction_capture(struct capture_control *capc, struct page *page,
* and vice-versa but no more than normal fallback logic which can
* have trouble finding a high-order free page.
*/
- if (order < pageblock_order && migratetype == MIGRATE_MOVABLE)
+ if (order < pageblock_order && capc->cc->migratetype != migratetype)
return false;
capc->page = page;
It is worth trying, since at the original patch time mTHP was not present and
not capturing any MIGRATE_MOVABLE makes sense. But with your change, the capture
will lose the opportunity of letting an unmovable request use a reclaimable
pageblock and vice-versa, like the comment says. Please change the comment
as well and we should monitor potential unmovable and reclaimable regression.
Yes, but I think this case is easy to solve. Anyway let me try to do
some measurement for mTHP.
