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

[Kairui Song <ryncsn@gmail.com>]

On Mon, Feb 16, 2026 at 03:34:54PM +0800, Barry Song wrote:
> On Mon, Feb 16, 2026 at 3:00 AM Kairui Song via B4 Relay
> <devnull+kasong.tencent.com@kernel.org> wrote:
> >
> > From: Kairui Song <kasong@tencent.com>
> >
> > 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?

Yes, that's fine. alloc_swap_scan_cluster is suppose to update the
percpu offset cache so if the cluster is not usable, writing
SWAP_ENTRY_INVALID to invalidate the cache might even be helpful
for future scan. At lease not harmful, I'll add some explanation,
comments.

> 
> 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.

Thanks for the suggestion, I even thought about renaming the helper
to indicate it will try update the percpu offset and release the lock.
But didn't have a better idea to naming and we also have
alloc_swap_scan_list, leave the name untouched seems more consistent.

I'll just add some comment then.