Re: [PATCH v3 2/3] mm, swap: reduce indention for hibernate allocation helper

[Barry Song <21cnbao@gmail.com>]

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>
>
> It doesn't have to check the device flag, as the allocator will also
> check the device flag and refuse to allocate if the device is not
> writable. This might cause a trivial waste of CPU cycles of hibernate
> allocation raced with swapoff, but that is very unlikely to happen.
> Removing the check on the common path should be more helpful.
>
> Signed-off-by: Kairui Song <kasong@tencent.com>
> ---
>  mm/swapfile.c | 38 ++++++++++++++++++--------------------
>  1 file changed, 18 insertions(+), 20 deletions(-)
>
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 32e0e7545ab8..0d1b17c99221 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -1936,27 +1936,25 @@ swp_entry_t swap_alloc_hibernation_slot(int type)
>
>         /* This is called for allocating swap entry, not cache */
>         if (get_swap_device_info(si)) {


I guess we could further reduce indentation by doing:
if (!get_swap_device_info(si))
    goto fail;


> -               if (si->flags & SWP_WRITEOK) {
> -                       /*
> -                        * Try the local cluster first if it matches the device. If
> -                        * not, try grab a new cluster and override local cluster.
> -                        */
> -                       local_lock(&percpu_swap_cluster.lock);
> -                       pcp_si = this_cpu_read(percpu_swap_cluster.si[0]);
> -                       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);
> -                       }
> -                       if (!offset)
> -                               offset = cluster_alloc_swap_entry(si, NULL);
> -                       local_unlock(&percpu_swap_cluster.lock);
> -                       if (offset)
> -                               entry = swp_entry(si->type, offset);
> +               /*
> +                * Try the local cluster first if it matches the device. If
> +                * not, try grab a new cluster and override local cluster.
> +                */
> +               local_lock(&percpu_swap_cluster.lock);
> +               pcp_si = this_cpu_read(percpu_swap_cluster.si[0]);
> +               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);
>                 }
> +               if (!offset)
> +                       offset = cluster_alloc_swap_entry(si, NULL);
> +               local_unlock(&percpu_swap_cluster.lock);
> +               if (offset)
> +                       entry = swp_entry(si->type, offset);
>                 put_swap_device(si);
>         }
>  fail:
>
> --
> 2.52.0
>
>

Thanks
Barry