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)) {
- 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.0On 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
On Mon, Feb 16, 2026 at 07:20:49AM +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>
> >
> > 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;
>
Agree, I think we can make it even simpler by having:
/* Return empty entry if device is not usable (swapoff or full) */
if (!si || !get_swap_device_info(si))
return entry;
Then the `fail` label is also gone.
I'll post a v4 later today combined with your another suggestion. Thanks!
On Mon, Feb 16, 2026 at 2:21 PM Kairui Song <ryncsn@gmail.com> wrote:
>
> On Mon, Feb 16, 2026 at 07:20:49AM +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>
> > >
> > > 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;
> >
>
> Agree, I think we can make it even simpler by having:
>
> /* Return empty entry if device is not usable (swapoff or full) */
> if (!si || !get_swap_device_info(si))
> return entry;
>
> Then the `fail` label is also gone.
Yes, this looks even nicer to me. :-)
>
> I'll post a v4 later today combined with your another suggestion. Thanks!
© 2016 - 2026 Red Hat, Inc.