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

[David Hildenbrand (Arm)]

On 5/25/26 16:57, Zhang Peng wrote:
> The activate_locked label in shrink_folio_list() reclaims swap cache
> when needed, marks the folio active, and updates activation statistics.
> Extract this block into folio_activate_locked() so it can be reused.
> 
> No functional change.
> 
> Signed-off-by: Zhang Peng <bruzzhang@xxxxxxxxxxx>
> ---
>  mm/vmscan.c | 34 +++++++++++++++++++++++-----------
>  1 file changed, 23 insertions(+), 11 deletions(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index ca4533eba701..886d8b4843aa 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1050,6 +1050,28 @@ static bool may_enter_fs(struct folio *folio, gfp_t gfp_mask)
>  	return !data_race(folio_swap_flags(folio) & SWP_FS_OPS);
>  }
>  
> +/*
> + * Prepare a locked folio to be kept active rather than reclaimed.
> + * Reclaims its swap slot if it will not be swapped, then marks it
> + * active and updates activation statistics.
> + */
> +static void folio_activate_locked(struct folio *folio,
> +		struct reclaim_stat *stat, unsigned int nr_pages)

Passing nr_pages to this helper is rather questionable. Just use
folio_nr_pages(folio) here and make the function less weird.

> +{

Do we want to add a

	VM_WARN_ON_ONCE_FOLIO(!folio_test_locked(folio), folio);

> +	/* Not a candidate for swapping, so reclaim swap space. */
> +	if (folio_test_swapcache(folio) &&
> +	    (mem_cgroup_swap_full(folio) || folio_test_mlocked(folio)))
> +		folio_free_swap(folio);
> +	VM_BUG_ON_FOLIO(folio_test_active(folio), folio);

While at it, do we want to turn this to a VM_WARN_ON_ONCE_FOLIO?

(and can we move that to the beginning of the function?)

In general, LGTM

-- 
Cheers,

David