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

From: Kairui Song

Date: Mon Feb 16 2026 - 02:54:13 EST


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@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?

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.