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

YoungJun Park
2026-02-13T02:18:13Z

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