Re: [PATCH v3 3/3] mm, swap: merge common convention and simplify allocation helper

[Barry Song]

On Mon, Feb 16, 2026 at 3:00 AM Kairui Song via B4 Relay
<devnull+kasong.tencent.com@xxxxxxxxxx> wrote:
>
> From: Kairui Song <kasong@xxxxxxxxxxx>
>
> Almost all callers of the cluster scan helper require the: lock -> check
> usefulness/emptiness check -> allocate -> unlock routine. So merge them
> into the same helper to simplify the code.

Previously, when !cluster_is_usable(ci, order), we only called
swap_cluster_unlock(). Now we do more work in this path:


out:
        relocate_cluster(si, ci);
        swap_cluster_unlock(ci);
        if (si->flags & SWP_SOLIDSTATE) {
                this_cpu_write(percpu_swap_cluster.offset[order], next);
                this_cpu_write(percpu_swap_cluster.si[order], si);
        } else {
                si->global_cluster->next[order] = next;
        }
        return found;

I assume this is what you want to do as well, but can we add
some explanation here?

Also, it would be better to add a comment that
alloc_swap_scan_cluster() expects ci->lock to be held on
entry and releases ci->lock before returning.


>
> Signed-off-by: Kairui Song <kasong@xxxxxxxxxxx>
> ---
>  mm/swapfile.c | 30 ++++++++----------------------
>  1 file changed, 8 insertions(+), 22 deletions(-)
>
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 0d1b17c99221..68dbbbd0dd24 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -923,11 +923,14 @@ static unsigned int alloc_swap_scan_cluster(struct swap_info_struct *si,
>         bool need_reclaim, ret, usable;
>
>         lockdep_assert_held(&ci->lock);
> -       VM_WARN_ON(!cluster_is_usable(ci, order));
>
> -       if (end < nr_pages || ci->count + nr_pages > SWAPFILE_CLUSTER)
> +       if (!cluster_is_usable(ci, order) || end < nr_pages ||
> +           ci->count + nr_pages > SWAPFILE_CLUSTER)
>                 goto out;
>
> +       if (cluster_is_empty(ci))
> +               offset = cluster_offset(si, ci);
> +
>         for (end -= nr_pages; offset <= end; offset += nr_pages) {
>                 need_reclaim = false;
>                 if (!cluster_scan_range(si, ci, offset, nr_pages, &need_reclaim))
> @@ -1060,14 +1063,7 @@ static unsigned long cluster_alloc_swap_entry(struct swap_info_struct *si,
>                         goto new_cluster;
>
>                 ci = swap_cluster_lock(si, offset);
> -               /* Cluster could have been used by another order */
> -               if (cluster_is_usable(ci, order)) {
> -                       if (cluster_is_empty(ci))
> -                               offset = cluster_offset(si, ci);
> -                       found = alloc_swap_scan_cluster(si, ci, folio, offset);
> -               } else {
> -                       swap_cluster_unlock(ci);
> -               }
> +               found = alloc_swap_scan_cluster(si, ci, folio, offset);
>                 if (found)
>                         goto done;
>         }
> @@ -1332,14 +1328,7 @@ static bool swap_alloc_fast(struct folio *folio)
>                 return false;
>
>         ci = swap_cluster_lock(si, offset);
> -       if (cluster_is_usable(ci, order)) {
> -               if (cluster_is_empty(ci))
> -                       offset = cluster_offset(si, ci);
> -               alloc_swap_scan_cluster(si, ci, folio, offset);
> -       } else {
> -               swap_cluster_unlock(ci);
> -       }
> -
> +       alloc_swap_scan_cluster(si, ci, folio, offset);
>         put_swap_device(si);
>         return folio_test_swapcache(folio);
>  }
> @@ -1945,10 +1934,7 @@ swp_entry_t swap_alloc_hibernation_slot(int type)
>                 pcp_offset = this_cpu_read(percpu_swap_cluster.offset[0]);
>                 if (pcp_si == si && pcp_offset) {
>                         ci = swap_cluster_lock(si, pcp_offset);
> -                       if (cluster_is_usable(ci, 0))
> -                               offset = alloc_swap_scan_cluster(si, ci, NULL, pcp_offset);
> -                       else
> -                               swap_cluster_unlock(ci);
> +                       offset = alloc_swap_scan_cluster(si, ci, NULL, pcp_offset);
>                 }
>                 if (!offset)
>                         offset = cluster_alloc_swap_entry(si, NULL);
>
> --
> 2.52.0
>
>

Thanks
Barry