Re: [PATCH v5 01/11] mm, swap: add virtual swap device infrastructure
From: Nhat Pham
Date: Thu Sep 24 2026 - 12:51:21 EST
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.
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.
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:
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 :)
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 :)
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.
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.
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.
>
> 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.
>
> Chris
What usecase would necessitate the need for swapon/swapoff interface?
Other than just trying to shoehorn it to an existing interface?
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.
[1]: https://lore.kernel.org/all/CAKEwX=PEEkYiNRaVhCuM4E6mxB9huPKfkDa4m62VMTzedg1oYw@xxxxxxxxxxxxxx/