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

[Nhat Pham]

On Thu, Sep 24, 2026 at 5:44 PM Chris Li <chrisl@xxxxxxxxxx> wrote:
>
> 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.

Not even that. Say you have two kind of workloads in the same host:
workload enables disk swap but disable zswap (zswap.max = 0), and
workload that wants zswap. Let's even ignore writeback for now.

You add 2 "swap devices" to the system: xswap and NVME swapfile.

At swap allocation time, xswap patch series does not check carefully
which swap device is being used, so you might get an xswap slot for
the cgroup that disables zswap. You're stuck at swap_writeout() time.
You can't undo it anymore, and you don't even support the physical
swapfile backend to fallback.


>
> > 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.

But that's my point. As of now, it does not. Unless you merge the
other RFC series, which brings it to roughly the same size as what I'm
doing here.

>
>
> >
> > 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.


Which is 2 massive patch series away. Also, I don't know how close we
are to reaching a consensus on swap.tier. I defer that to memcg
maintainers.

>
> >
> > 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.

It's *literally* the same design as me, from a metadata design. The
only difference is:

1. It moves the metadata from a new dynamic cluster struct and shoves
it into the existing physical struct.

Like, you can check patch 11 of this series. It's practically the same
thing, with the only difference being ou

> >
> > 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.

"Universal interfaces" is not a use case. None of the thing you list
out here is a use case. It's an interface, but you have not
constructed a single use case to justify all of that.

> 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.

I mean, there's a knob for it. How can it start kicking in without
your enablement?


>
> > 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.

Again, your disliking something is not a use case.