mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Baoquan He <baoquan.he@linux.dev>
To: Johannes Weiner <hannes@cmpxchg.org>
Cc: "Nhat Pham" <nphamcs@gmail.com>,
	"Kairui Song" <kasong@tencent.com>,
	"Chris Li" <chrisl@kernel.org>,
	"Michal Hocko" <mhocko@kernel.org>,
	"Roman Gushchin" <roman.gushchin@linux.dev>,
	"Shakeel Butt" <shakeel.butt@linux.dev>,
	"Yosry Ahmed" <yosry@kernel.org>,
	"David Hildenbrand" <david@kernel.org>,
	"Muchun Song" <muchun.song@linux.dev>,
	"Kemeng Shi" <shikemeng@huaweicloud.com>,
	"Barry Song" <baohua@kernel.org>,
	"YoungJun Park" <youngjun.park@lge.com>,
	"Chengming Zhou" <chengming.zhou@linux.dev>,
	"Lorenzo Stoakes (Oracle)" <ljs@kernel.org>,
	"Liam R. Howlett" <liam@infradead.org>,
	"Vlastimil Babka (SUSE)" <vbabka@kernel.org>,
	"Mike Rapoport" <rppt@kernel.org>,
	"Suren Baghdasaryan" <surenb@google.com>,
	"Qi Zheng" <qi.zheng@linux.dev>,
	"Axel Rasmussen" <axelrasmussen@google.com>,
	"Yuanchu Xie" <yuanchu@google.com>, "Wei Xu" <weixugc@google.com>,
	"Rik van Riel" <riel@surriel.com>,
	"Gregory Price" <gourry@gourry.net>,
	"Wenchao Hao" <haowenchao22@gmail.com>,
	"Jonathan Corbet" <corbet@lwn.net>,
	"Hugh Dickins" <hughd@google.com>,
	"Baolin Wang" <baolin.wang@linux.alibaba.com>,
	"Tejun Heo" <tj@kernel.org>, "Michal Koutný" <mkoutny@suse.com>,
	"Shuah Khan" <skhan@linuxfoundation.org>,
	"Kunwu Chan" <kunwu.chan@linux.dev>,
	"Meta kernel team" <kernel-team@meta.com>,
	"Linux Memory Management List" <linux-mm@kvack.org>,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
	linux-doc@vger.kernel.org,
	"open list:CONTROL GROUP - MEMORY RESOURCE CONTROLLER (MEMCG)"
	<cgroups@vger.kernel.org>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"Kairui Song" <ryncsn@gmail.com>,
	"Joshua Hahn" <joshua.hahnjy@gmail.com>
Subject: Re: Path forward for Virtualized Swap?
Date: Fri, 11 Sep 2026 20:27:00 +0800	[thread overview]
Message-ID: <aqPzlIVJccbqbaUr@fedora> (raw)
In-Reply-To: <aqLi6cIjD2wJwk0B@cmpxchg.org>

On 09/10/26 at 01:03pm, Johannes Weiner wrote:
> [This reply was not LLM-generated.]
> 
> On Thu, Sep 10, 2026 at 03:09:59PM +0800, Baoquan He wrote:
> > Hi Nhat,
> > 
> > On 09/04/26 at 02:14pm, Nhat Pham wrote:
> > .....snip...
> > > Now, on xswap. Baoquan's working on a series [15] that covers some of the
> > > same ground, and the VM_SPARSE cluster_info idea in it is genuinely good.
> > > I've been reviewing that lineage since July [16] and I'd like whatever
> > > lands to end up with the best parts of both. From my perspective the
> > > differences are:
> > > 
> > > 1. Userspace knobs. xswap asks the admin for a size (a percent of RAM) plus
> > >    a per-device limit to tune afterwards. I'm not aware of any use case
> > >    that needs those, and I don't think users have a good way to answer the
> > >    question anyway - sizing swap for compressed memory depends on memory
> > >    size, workload, and compression ratio all at once. That's precisely the
> > >    provisioning problem vswap exists to remove. The kernel should be as
> > >    transparent and dynamic as possible here, and not add knobs unless
> > >    there's a use case for them.
> > > 
> > > 2. Writeback support. Writeback is core functionality for zswap, not an
> > >    add-on, and a design needs to account for it from the start. This came
> > >    up before, in the discussion around Chris' ghost swapfile RFC [17]: for
> > >    a solution here to be acceptable, it has to work with the primary
> > >    usecase and support disk writeback. Without it, whatever zswap won't
> > >    take (incompressible pages especially) has nowhere to go, and cold
> > >    compressed data can never leave RAM.
> > > 
> > > 3. Cgroup charging behavior. vswap/xswap shouldn't be charged against the
> > >    swap usage counter. It's fundamentally a different resource from
> > >    physical swapfile space, and memory.swap.* should read 0 when nothing is
> > >    on disk [18]. I made the longer argument for this in [19].
> > > 
> > > 4. Data structure (xarray vs sparse vmalloc array). Even with xarray, vswap
> > >    is already on par with or beating baseline. I like the sparse array
> > >    idea, but why are we landing an optimization before the feature itself,
> > >    without any A/B data showing the difference matters?
> > 
> > 
> > Thanks for laying this out, and for the honest push to converge. Let me
> > be equally direct about the ordering: I think the xswap base should land
> > first, and the things vswap demonstrates -  writeback, rmap lookup, the
> > charging semantics, later THP -- should be built on top of it. Because
> > it is the foundation that keeps the swap core simpler, and the first thing
> > to merge should be the one that doesn't have to be redone.
> > 
> > The VM_SPARSE array is not an optimization to bolt on later; it is a
> > structural choice, and the code reflects it. In vswap, the cluster
> > metadata lives in a dynamically-allocated xarray.
> > 
> > struct swap_cluster_info_dynamic {
> >         struct swap_cluster_info ci;
> >         unsigned int index;             /* for cluster_index() */
> >         struct rcu_head rcu;
> >         atomic_long_t *virtual_table;   /* Backing pointers for vswap slots */
> > };
> > 
> > To support dynamic growth and shrink, vswap stores its cluster metadata
> > in an xarray, and that forces two things the plain swap_cluster_info[]
> > array never needed:
> > 
> > 1. Every cluster has to carry an extra index and an rcu_head —
> > 24 bytes per cluster — purely so the xarray can locate it and free
> > it safely.
> > 2. To keep that bookkeeping from leaking into the normal-swap code, the
> > cluster had to be wrapped in a container, swap_cluster_info_dynamic,
> > so the xarray holds a pointer to the wrapper instead of an inline
> > array element.
> > 
> > So in vswap, every cluster access in the shared hot path has to answer
> > "is this a vswap device?" and take a separate branch:
> > 
> > - swap_is_vswap() is checked in 36 places across page_io.c, swapfile.c,
> >   zswap.c and swap.h;
> > - __swap_offset_to_cluster() branches into xa_load() for vswap vs the
> >   flat array otherwise, and the xarray path can return NULL (a cluster
> >   can be torn down);
> > - __swap_cluster_lock() branches into __vswap_cluster_lock(), which
> >   wraps every access in rcu_read_lock() and a CLUSTER_FLAG_DEAD check,
> >   plus kfree_rcu()/container_of()/rcu_head plumbing for node lifetime.
> 
> Well to state the obvious: the reason it does all that is to make the
> compression space transparent to the user.
> 
> The user can answer a simple boolean question: whether they want
> compression or not. And it will work on tiny machines, on humongous
> machines, and everything in between. That's a simple policy question
> with a clear answer.
> 
> What you're doing, asking the user for a static size, is much more
> difficult and has usability issues.
> 
> You're comparing implementations that don't accomplish the same thing.
> 
> The problem we're trying to solve is implementing a clean compression
> space abstraction. I'm arguing that vswap does, and xswap does not.
> 
> While they're both using parts of the swap device code to implement a
> compression space, xswap actually PRESENTS IT TO THE USER as a swap
> device, and then makes optimizations BASED ON BAKED IN LIMITATIONS.
> 
> But a conventional, statically sized swap device is a bad abstraction
> for the compression space. Here is why:
> 
> In conventional swap space, one memory page translates to one swap
> page. Compression space doesn't act this way: a memory page can
> consume anything between a few bytes to a full page in compression
> space. It depends on memory contents and compression algorithm. So
> right off the bat, this is a hard question to answer at the host level
> which could run all kinds of workloads.
> 
> In conventional swap space, the resource consumed is a different
> one. You're offloading memory by consuming disk space. This eats into
> the space available to the filesystem, which is totally unrelated.
> Asking the user for this tradeoff is a legitimate policy question.
> 
> Compression space is not a separate resource. It's page tables,
> backing pages, and swap descriptors. It's just MEMORY. There isn't a
> size tradeoff, because moving pages from memory space into compression
> space DOES NOT CONSUME A NEW RESOURCE. It's still just memory. All you
> need for containment already exists: rlimits, OOM killer, cgroup
> memory controls.
> 
> By making this a user-visible virtual swap device, you're sending
> users down the wrong path. You're asking them to set a new limit on a
> resource that's already limited by other means. You're framing the
> question as conventional swap which behaves completely differently.
> 
> If you ask them "how much swap space", they WILL reference this to
> available RAM capacity. Maybe half of ram, maybe twice the RAM.
> 
> But when compression space is referenced to RAM, it's trivial to fill
> it up with zeroed pages or easily compressible data LONG BEFORE the
> process or container would hit any of its MEMORY limits.
> 
> This creates an artificial resource shortages. It forces a competition
> where there shouldn't be one. And then you need new controls to manage
> a competition that doesn't have to exist.
> 
> Like I said before, including compression space (which is memory) in
> memory.swap.* (which is for disk space) is not going to be acceptable
> from the cgroup side. We can talk about that if you want.
> 
> But asking the user questions they shouldn't have to answer, or
> already answered elsewhere, is weak interface design. Allowing, let
> alone encouraging, answers that create a whole new host of
> organizational issues is outright bad interface design.
> 
> So if you want to compare implementations, you first have to actually
> implement the same thing:
> 
> Stop asking user "how large". Let compression space expand towards
> existing memory limits, such that it doesn't create an awkward and
> artificial new resource competition.
> 
> Then we can compare implementations.
> 
> If the optimizations still apply under those constraints, great.
> 
> Until then, there is little point in discussing differences that, by
> your own admission, have little to no impact on real world performance.

Thanks for your sharing with deliberate thought.

Agreed, and I want to be clear that the size knob is my implementation
choice, not something the design needs.

Now in v2, xswap's create() already takes no size, only an optional
priority: the device's address space is set by the kernel to the machine's
memory and cluster_info is mapped lazily, so nothing is allocated up front.
The only knob left is an optional per-device ceiling. If we really want to
remove it, that's quite easy thing, we can just remove the runtime
growth ceiling and the shrink machinery that serves it.

When I asked why shrink is needed, Nhat told on system, memory pressure
could reach a peak, than later may not reach it again for a long time. I
don't like the continuous automatic growing/shrinking, I think it
doesn't make much sense just for saving that memory serving struct
swap_cluster_info. But it's not bad to provide a mechanism for
admin/users to tune it.

But as I said, xswap/vswap both claim to solve the problem of zswap
physical disk slot and swap slot coupling, and meantime extend
functionality to make it more flexible than zswap/zram. While Nhat's
vswap is boot-time per-device swap. And Nhat's own description of it
in this thread is "vswap is just a normal swap device, no?". If I didn't
apply Nhat's code and test I couldn't realize it. I executed swapon but
can't see any output. I was shocked.

I really appreciated your patient and detailed sharing, while it takes
you so long words to explain it. IMHO, it deserves a separate patch
posting to justify it so that anyone can know why it is.

Thanks
Baoquan

  reply	other threads:[~2026-09-11 12:27 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-04 21:14 Nhat Pham
2026-09-07  5:51 ` Kairui Song
2026-09-08 16:36   ` Nhat Pham
2026-09-11 16:09     ` Kairui Song
2026-09-11 16:57       ` Nhat Pham
2026-09-11 18:14         ` Kairui Song
2026-09-11 19:03           ` Nhat Pham
2026-09-12  8:47             ` Kairui Song
2026-09-08 18:30   ` Johannes Weiner
2026-09-09 16:41     ` Nhat Pham
2026-09-09 17:47       ` Nhat Pham
2026-09-12  9:00       ` Kairui Song
2026-09-12 11:51         ` Johannes Weiner
2026-09-10 23:27   ` Nhat Pham
2026-09-07 11:30 ` David Hildenbrand (Arm)
2026-09-08 16:45   ` Nhat Pham
2026-09-10 10:56     ` David Hildenbrand (Arm)
2026-09-10 16:22       ` Nhat Pham
2026-09-10 17:57         ` David Hildenbrand (Arm)
2026-09-11 16:20         ` Kairui Song
2026-09-11 16:56           ` David Hildenbrand (Arm)
2026-09-10  7:09 ` Baoquan He
2026-09-10 16:39   ` Shakeel Butt
2026-09-11 13:06     ` Baoquan He
2026-09-11 16:45       ` Shakeel Butt
2026-09-10 17:03   ` Johannes Weiner
2026-09-11 12:27     ` Baoquan He [this message]
2026-09-11 16:21       ` Johannes Weiner
2026-09-10 17:16   ` Nhat Pham

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aqPzlIVJccbqbaUr@fedora \
    --to=baoquan.he@linux.dev \
    --cc=akpm@linux-foundation.org \
    --cc=axelrasmussen@google.com \
    --cc=baohua@kernel.org \
    --cc=baolin.wang@linux.alibaba.com \
    --cc=cgroups@vger.kernel.org \
    --cc=chengming.zhou@linux.dev \
    --cc=chrisl@kernel.org \
    --cc=corbet@lwn.net \
    --cc=david@kernel.org \
    --cc=gourry@gourry.net \
    --cc=hannes@cmpxchg.org \
    --cc=haowenchao22@gmail.com \
    --cc=hughd@google.com \
    --cc=joshua.hahnjy@gmail.com \
    --cc=kasong@tencent.com \
    --cc=kernel-team@meta.com \
    --cc=kunwu.chan@linux.dev \
    --cc=liam@infradead.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=ljs@kernel.org \
    --cc=mhocko@kernel.org \
    --cc=mkoutny@suse.com \
    --cc=muchun.song@linux.dev \
    --cc=nphamcs@gmail.com \
    --cc=qi.zheng@linux.dev \
    --cc=riel@surriel.com \
    --cc=roman.gushchin@linux.dev \
    --cc=rppt@kernel.org \
    --cc=ryncsn@gmail.com \
    --cc=shakeel.butt@linux.dev \
    --cc=shikemeng@huaweicloud.com \
    --cc=skhan@linuxfoundation.org \
    --cc=surenb@google.com \
    --cc=tj@kernel.org \
    --cc=vbabka@kernel.org \
    --cc=weixugc@google.com \
    --cc=yosry@kernel.org \
    --cc=youngjun.park@lge.com \
    --cc=yuanchu@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®