Re: [PATCH v5 1/5] mm/vmscan: introduce folio_activate_locked() helper

[Zhang Peng]

On Mon, Aug 10, 2026 at 4:36 PM Barry Song <baohua@xxxxxxxxxx> wrote:
>
> Reviewed-by: Barry Song <baohua@xxxxxxxxxx>

Thanks for the review!

> > + * Prepare a locked folio to be kept active rather than reclaimed.
> > + * Reclaims its swap slot if it will not be swapped, then marks it
>
> I'm not quite sure whether this should be "if". Because, it seems
> to always be true up to this point. BTW, if we really want to use
> "if", shouldn't we use it to check whether swap is full?

You're right, the wording was misleading: the condition that actually
guards folio_free_swap() is the swapcache/swap-full/mlocked test right
below it, not anything about whether the folio "will be swapped".

Rather than try to restate that in prose, I dropped the paragraph and
left a single line, since the code below is already explicit:

	/* Activate an isolated, locked folio and account the activation. */
	static void folio_activate_locked(struct folio *folio,
			struct reclaim_stat *stat)

One thing I should flag, since it is not just a comment change: the
VM_BUG_ON_FOLIO(folio_test_active(folio)) that used to sit at the
activate_locked label is now a VM_WARN_ON_ONCE_FOLIO(), so a caller
that gets this wrong is reported rather than taking the machine down.
Both are CONFIG_DEBUG_VM-only, and no non-debug behaviour changes, but
it is a deliberate change rather than a pure move, and the changelog
now says so.

I kept your Reviewed-by on that basis - please let me know if you'd
rather I dropped it, or if you'd prefer the BUG_ON left alone.

This patch is now part of a smaller cleanup-only series, see my reply
on 5/5.

Thanks
Zhang Peng