Re: [RFC PATCH v2 v2 1/5] mm: swap: introduce swap tier infrastructure

[YoungJun Park <youngjun.park@lge.com>]

On Thu, Feb 12, 2026 at 01:07:45AM -0800, Chris Li wrote:
> > +
> > +       spin_lock(&swap_lock);
> > +       spin_lock(&swap_tier_lock);
> > +
> > +       p = tmp;
> > +       swap_tiers_save(ctx);
> > +
> > +       while (!ret && (token = strsep(&p, ", \t\n")) != NULL) {
> > +               if (!*token)
> > +                       continue;
> > +
> > +               if (token[0] == '-') {
> > +                       ret = swap_tiers_remove(token + 1);
> > +               } else {
> > +
> > +                       name = strsep(&token, ":");
> > +                       if (!token || kstrtos16(token, 10, &prio)) {
> > +                               ret = -EINVAL;
> > +                               goto out;
> > +                       }
> > +
> > +                       if (name[0] == '+')
> > +                               ret = swap_tiers_add(name + 1, prio);
> > +                       else
> > +                               ret = swap_tiers_modify(name, prio);
> > +               }
> > +
> > +               if (ret)
> > +                       goto restore;
> > +       }
> 
> This function can use some simplification to make the indentation flater.

Agreed. I will refactor this to flatten the indentation.


> > +/*
> > + * struct swap_tier - structure representing a swap tier.
> > + *
> > + * @name: name of the swap_tier.
> > + * @prio: starting value of priority.
> > + * @list: linked list of tiers.
> > +*/
> > +static struct swap_tier {
> > +       char name[MAX_TIERNAME];
> > +       short prio;
> > +       struct list_head list;
> > +} swap_tiers[MAX_SWAPTIER];
> 
> We can have a CONFIG option for the MAX_SWAPTIER. I think the default
> should be a small number like 4.

Sounds good. I will add a CONFIG option for it and ensure it doesn't exceed
MAX_SWAPFILE.


> > +
> > +/*
> > + * XXX: Reverting individual operations becomes complex as the number of
> > + * operations grows. Instead, we save the original state beforehand and
> > + * fully restore it if any operation fails.
> > + */
> > +void swap_tiers_save(struct swap_tier_save_ctx ctx[])
> 
> I really hope we don't have to do the save and restore thing. Is there
> another design we can simplify this?

I have given this a lot of thought.

Since the current interface allows mixing add (+), remove (-), and modify
operations, we must either restore from a saved state or reverse the
successful individual operations upon failure.

I implemented both approaches and concluded that reversing individual
operations is error-prone. Also, it could be slow if there are many
operations.

Another approach could be using a "global clone tier" strategy.
(Because operation globally synchronized)

Therefore, I would like to propose restricting the interface to handle a
single operation at a time. What do you think?

> > @@ -76,7 +77,6 @@ atomic_long_t nr_swap_pages;
> >  EXPORT_SYMBOL_GPL(nr_swap_pages);
> >  /* protected with swap_lock. reading in vm_swap_full() doesn't need lock */
> >  long total_swap_pages;
> > -#define DEF_SWAP_PRIO  -1
> >  unsigned long swapfile_maximum_size;
> >  #ifdef CONFIG_MIGRATION
> >  bool swap_migration_ad_supported;
> > @@ -89,7 +89,7 @@ static const char Bad_offset[] = "Bad swap offset entry ";
> >   * all active swap_info_structs
> >   * protected with swap_lock, and ordered by priority.
> >   */
> > -static PLIST_HEAD(swap_active_head);
> > +PLIST_HEAD(swap_active_head);
> 
> One idea is to make each tier have swap_active_head. So different swap
> entry releases on different tiers don't need to be competing on the
> same swap_active_head.
> 
> That will require the swapfile don't jump to another tiers.
>
I agree. With the tier structure, we can limit contention to objects within
the same tier.

I also think swap_avail_list could be optimized in a similar way in the
future.

Youngjun