Re: [PATCH v3 02/12] mm, swap: move common swap cache operations into standalone helpers

[Chris Li]

On Tue, Apr 21, 2026 at 8:16 AM Kairui Song via B4 Relay
<devnull+kasong.tencent.com@xxxxxxxxxx> wrote:
>
> From: Kairui Song <kasong@xxxxxxxxxxx>
>
> Move a few swap cache checking, adding, and deletion operations
> into standalone helpers to be used later. And while at it, add
> proper kernel doc.
>
> No feature or behavior change.
>
> Signed-off-by: Kairui Song <kasong@xxxxxxxxxxx>

Acked-by: Chris Li <chrisl@xxxxxxxxxx>


> ---
>  mm/swap_state.c | 141 ++++++++++++++++++++++++++++++++++++++------------------
>  1 file changed, 95 insertions(+), 46 deletions(-)
>
> diff --git a/mm/swap_state.c b/mm/swap_state.c
> index 204a9499d50c..3da285a891b2 100644
> --- a/mm/swap_state.c
> +++ b/mm/swap_state.c
> @@ -137,8 +137,42 @@ void *swap_cache_get_shadow(swp_entry_t entry)
>         return NULL;
>  }
>
> -void __swap_cache_add_folio(struct swap_cluster_info *ci,
> -                           struct folio *folio, swp_entry_t entry)
> +/**
> + * __swap_cache_add_check - Check if a range is suitable for adding a folio.
> + * @ci: The locked swap cluster.
> + * @ci_off: Range start offset.
> + * @nr: Number of slots to check.
> + * @shadow: Returns the shadow value if one exists in the range.
> + *
> + * Check if all slots covered by given range have a swap count >= 1.
> + * Retrieves the shadow if there is one.
> + *
> + * Context: Caller must lock the cluster.
> + */
> +static int __swap_cache_add_check(struct swap_cluster_info *ci,
> +                                 unsigned int ci_off, unsigned int nr,
> +                                 void **shadow)
> +{
> +       unsigned int ci_end = ci_off + nr;
> +       unsigned long old_tb;
> +

Nitpick: Can add lockdep_assert_held(&ci->lock);

Can check ci_end < SWAPFILE_CLUSTER and bail out on error.

> +       if (unlikely(!ci->table))
> +               return -ENOENT;
> +       do {
> +               old_tb = __swap_table_get(ci, ci_off);
> +               if (unlikely(swp_tb_is_folio(old_tb)))
> +                       return -EEXIST;
> +               if (unlikely(!__swp_tb_get_count(old_tb)))
> +                       return -ENOENT;
> +               if (swp_tb_is_shadow(old_tb))
> +                       *shadow = swp_tb_to_shadow(old_tb);

Nitpick: You can create a local variable for the shadow and assign it
at the end. Because it is a pointer, the compiler can't optimize the
store away.

Chris