Re: [PATCH 1/5] mm: compaction: push watermark into compaction_suitable() callers
From: Zi Yan
Date: Fri Mar 14 2025 - 11:09:06 EST
On 13 Mar 2025, at 17:05, Johannes Weiner wrote:
> compaction_suitable() hardcodes the min watermark, with a boost to the
> low watermark for costly orders. However, compaction_ready() requires
> order-0 at the high watermark. It currently checks the marks twice.
>
> Make the watermark a parameter to compaction_suitable() and have the
> callers pass in what they require:
>
> - compaction_zonelist_suitable() is used by the direct reclaim path,
> so use the min watermark.
>
> - compact_suit_allocation_order() has a watermark in context derived
> from cc->alloc_flags.
>
> The only quirk is that kcompactd doesn't initialize cc->alloc_flags
> explicitly. There is a direct check in kcompactd_do_work() that
> passes ALLOC_WMARK_MIN, but there is another check downstack in
> compact_zone() that ends up passing the unset alloc_flags. Since
> they default to 0, and that coincides with ALLOC_WMARK_MIN, it is
> correct. But it's subtle. Set cc->alloc_flags explicitly.
>
> - should_continue_reclaim() is direct reclaim, use the min watermark.
>
> - Finally, consolidate the two checks in compaction_ready() to a
> single compaction_suitable() call passing the high watermark.
>
> There is a tiny change in behavior: before, compaction_suitable()
> would check order-0 against min or low, depending on costly
> order. Then there'd be another high watermark check.
>
> Now, the high watermark is passed to compaction_suitable(), and the
> costly order-boost (low - min) is added on top. This means
> compaction_ready() sets a marginally higher target for free pages.
>
> In a kernelbuild + THP pressure test, though, this didn't show any
> measurable negative effects on memory pressure or reclaim rates. As
> the comment above the check says, reclaim is usually stopped short
> on should_continue_reclaim(), and this just defines the worst-case
> reclaim cutoff in case compaction is not making any headway.
>
> Signed-off-by: Johannes Weiner <hannes@xxxxxxxxxxx>
> ---
> include/linux/compaction.h | 5 ++--
> mm/compaction.c | 52 ++++++++++++++++++++------------------
> mm/vmscan.c | 26 ++++++++++---------
> 3 files changed, 45 insertions(+), 38 deletions(-)
>
The changes look good to me. Acked-by: Zi Yan <ziy@xxxxxxxxxx>
Best Regards,
Yan, Zi