Re: [PATCH v5 01/11] mm, swap: add virtual swap device infrastructure

[Chris Li]

On Thu, Sep 24, 2026 at 6:20 AM Nhat Pham <nphamcs@xxxxxxxxx> wrote:
>
> On Wed, Sep 23, 2026 at 2:18 AM Chris Li <chrisl@xxxxxxxxxx> wrote:
> >
> >
> > There are too many swap_is_vswap() in this series. It fragments the
> > code path, making things harder to reason about. Esepcially around
> > locks. I count 22 in this patch alone.
> >
> > I consider this the biggest drawback of this series. This
> > fragmentation of the code path.
>
> This will mirror my response at [1], but I'm responding here for the
> record and for your convenience.

Thank you. Really appriciate that.

> It is needed because vswap *is* a special device, with its own
> requirements. If you don't special case-it, you'd get undesirable
> behaviors.

Ack, that is why I am against making it a special device. That is the
whole idea behind xswap. It is just normal swap device implemented via
swap_ops just like ext4 is a file system implemnet on top of VFS.

> Let's take Baoquan's patch series as an example.  It treats the xswap
> device as just another swap device, without any special consideration
> for it. Which creates several problem:

That is a careful design choice. Keep in mind that design choice
usually has pros and cons.

I will follow your feedback here.

> 1. At allocation time, cgroups that disable zswap might get an xswap
> slot, because you don't do any check that the device you allocating
> the swap slots for is xswap. At swap_writeout() time, it is *stuck* -
> we already do the unmapping step, so we cannot reclaim the page.
>
> Ironically a physical swapfile backend could have bailed us out here,
> but it's not in that patch series yet :)

I am not sure I follow. Are you referring to the uncompressible pages
that xswap would have to reject?

One way to address that is for xswap to just bite the bullet and store
the uncompressible pages within xswap. That way it will stay out of
the reclaim LRU. Zswap also stores some uncompressible pages. The last
time I checked, it was unfortunately not able to serve my case but
perfect for yours.

> Vswap both has swapfile backend, AND allows you to bypass it if the
> cgroup disables zswap. Both involves a bit of swap_is_vswap() check,
> but I think it's worth it :)

If xswap needs the lower tier it can allocate for one. I don't think
xswap should need to have swap_is_xswap() there. Also xswap does not
have all the baggage of zswap.writeback enable interface.


>
> 2. Per-CPU allocation caching: since all devices share the same cache,
> xswap will invalidate the caching of other devices, and vice versa. It
> would be fine if the these devices are interchangeable, but as noted
> above, it's not, especially without physical swapfile as the backend
> option. So you're forced to taken the slow path more often.

This will be addressed by the swap tiers series if it is not delayed.
This should be addressed in the core swap layer not vswap. Kairui also
has some ideas to improve that. Yes, it is a problem area, but it is
not specific to xswap. We are well aware of it and have plans to
improve it.

>
> Vswap gets around this by adding its own per-cpu allocation caching,
>
> So by being too lazy and not carefully distinguish the swap devices,
> not only does xswap not work for the writeback use case, it cannot
> even work in deployments where some workloads (cgroup) select zswap,
> whereas others select disk swap. It can only work if you intend to
> have a single class of device - either virtual/xswap or disk swap -
> but not both.

Baoquan's writeback series is out. It is by no means perfect. But it
is much closer to what I have in mind. That is a much better base to
work on.
>
> Maybe instead of just grepping for swap_is_vswap() and throw your hand
> at the complexity, read the code and try to understand why it's
> necessary first, then propose simplifications if you have good ideas
> in minds.

Yes, I already read your code I decide that most of the
swap_is_vswap() should not be needed if taking a proper swap_ops
approach. I wanted to offer you the chance to work closely with me to
achieve that, but you did not follow that suggestion.

Quote from my old email:
https://lore.kernel.org/all/CACePvbX+3tO91BmRwaLqf3Xia32GCemt3W8tayErG81BYLgrzA@xxxxxxxxxxxxxx/

"I am happy to spend some time working with you to discuss the generic
adopted version of vswap, if you are open to it. Or if you don't want
to waste time on it. I can have someone else or myself come up with
the generic adopted version of vswap for you to review, which I prefer
less."

Anyway, Baoquan's xswap writeback patch series is out. I suggest
following and improving that series instead.

> > This is questionable. A vswap device does not go through swap on.
> >
> > This changes the behavior where swap devices always go through swap on/off.
> >
> > I wish the swap device went through swap on to be able to start swapping.
> >

>
> What usecase would necessitate the need for swapon/swapoff interface?
> Other than just trying to shoehorn it to an existing interface?

I like universal interfaces. Your vswap will break the /etc/fstab. The
current suggestion for xswap using sysfs will break that as well.
I would like to keep the /etc/fstab interface working, including
support for UUIDs for swap devices, etc.
I am very concerned that vswap will start kicking in without me
explicitly enabling it. That changes the previous user-visible
behavior. It breaks my mental model. Please take this feedback
seriously.

It is a different design philosophy to adopt a swap device to
implement VFS-like file systems for swap, making them swap file
systems.
Just like all file systems have a superblock so you can probe what
file system it is. You don't get a special sysfs interface for each
different file system to create them.
This aligns neatly with swap ops and makes room for future
flash-friendly swap file systems for example.

> I've expressed why I dislike the interface in [1]. Other than the
> sizing aspect, which we are already discussing in another thread,
> priority also makes zero sense. You are literally exposing an
> interface with only one correct answer - what's the point?
>
> Justify your interface choice with real userspace use case, not just by
> conforming to the status quo.

That is why I feel your patch series are really unclean. I dislike
special cases, I try to avoid them as much as possible.

Chris

>
> [1]: https://lore.kernel.org/all/CAKEwX=PEEkYiNRaVhCuM4E6mxB9huPKfkDa4m62VMTzedg1oYw@xxxxxxxxxxxxxx/