Re: [PATCH 1/5] mm: compaction: push watermark into compaction_suitable() callers

[Zi Yan]

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