Re: [PATCH 0/3] mm/swap: remove plist swap_active_head

[Baoquan He <bhe@redhat.com>]

On 10/01/25 at 11:04pm, Chris Li wrote:
> On Tue, Sep 30, 2025 at 9:34 PM Baoquan He <bhe@redhat.com> wrote:
> >
> 
> Can you please confirm or deny this series is depend on the "[PATCH v3
> 0/2] mm/swapfile.c: select the swap device with default priority round
> robin" series.

Thanks a lot for reviewing this carefully. This series depends on
patchset [PATCH v3 0/2] mm/swapfile.c: select the swap device with default
priority round robin" series.

> 
> If it is depend on that series, you need to declare that in the
> beginning, so Andrew will not try to apply it without the depended
> series.

Sorry, I should have told that clearly at the beginning.

> 
> > In mm/swapfile.c, there are two plist variables, swap_active_head and
> > swap_avail_head. swap_avail_head contains all available (active, not full)
> > swap_info_structs orderred by priority. While swap_active_head contains
> 
> Nit: spelling : orderred -> ordered

Thanks.

> 
> > all active swap_info_structs (active, nor full and full) orderred by
> 
> Same here, ordered.
> 
> > priority. Earlier, it serves three purposes:
> >
> > 1) When swapoff one swap device in the middle, swap devices of priority
> >    lower than the swapped off swap device will be promoted up value one,
> >    e.g, I swapped off zram1 of priority '-3', then zram2 promoted to
> >    have priority '-3', zram3 has priority '-4'. This is done via plist
> >    swap_active_head.
> >    - This code has been taken off in
> >      - [PATCH v3 0/2] mm/swapfile.c: select the swap device with default priority round robin
> 
> See above, if there is patch dependency, declare it early.

Agree, will do in future posting.

> 
> > # swapon
> > NAME       TYPE      SIZE USED PRIO
> > /dev/zram0 partition  16G   0B   -2
> > /dev/zram1 partition  16G   0B   -3
> > /dev/zram2 partition  16G   0B   -4
> > /dev/zram3 partition  16G   0B   -5
> 
> I am a bit confused. If the "[PATCH v3 0/2] mm/swapfile.c: select the
> swap device with default priority round robin" series already change
> the default priority to -1. How does it get the -2 -3 -4 -5? You seems
> describe the behavior before that revert series. Can you please
> clarify.

In the given example, It demonstrates every device will get +1 priority
promotion after the swapped off device. 

I just intended to tell the purposes of swap_active_head in the current
kernel, since my patches haven't been merged. I also had a feeling this
description is messy before posting, diddn't expect it could cause these
confusion.

I should have mentioned this series depends on "[PATCH v3 0/2] mm/swapfile.c:
select the swap device with default priority round robin" series, then I
don't need to mention this purpose of swap_active_head, that could make
the describing easier.

> 
> > # swapoff /dev/zram1
> > # swapon
> > NAME       TYPE      SIZE USED PRIO
> > /dev/zram0 partition  16G   0B   -2
> > /dev/zram2 partition  16G   0B   -3
> > /dev/zram3 partition  16G   0B   -4
> 
> Is that the behavior before your reverting node id for swapfile series?
> Assume it has patch dependency on the earlier series. Wouldn't better
> you describe the behavior after the round robin series and how you can
> do better?

Right, I agree. 
> 
> > 2) Find a swap device in swap_active_head when swap off.
> >    - This can be done through iterating swap_info[] instead. Change is
> >      done in patch 2.
> 
> Can you please clarify what is the benefit of remove plist swap_active_head?
> e.g. Does it simply the code or make swap code perform better.

After series "[PATCH v3 0/2] mm/swapfile.c: select the swap device with
default priority round robin", plist swap_active_head is only used in
swapoff api to scan it to find a swap device according to the device
name. It can be replaced with scanning swap_info[], and there's no
efficiency degradation. And we can remove field "struct plist_node
list;" in struct swap_info_struct, and the relevant handling code which
is unnecessary. 

> 
> > 3) Judge if there's any active swap device via __has_usable_swap().
> >    - This can be done by checking total_swap_pages instead. Change is
> >      done in patch 1.
> >
> > Among them, the purpose 1) is the most important, while it has been
> > taken off in below patchset. So this patchset removing swap_active_head
> > sits on top of it.
> > - [PATCH v3 0/2] mm/swapfile.c: select the swap device with default priority round robin
> >   - https://lore.kernel.org/all/20250930063311.14126-1-bhe@redhat.com/T/#u
> 
> It seems you mean to say this patch series is depend on that PATCH v3 series.

Yeah, I should have mentioned this at the beginning to make the log
clearer.