Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Baoquan He <baoquan.he@linux.dev>
To: Nhat Pham <nphamcs@gmail.com>
Cc: linux-mm@kvack.org, akpm@linux-foundation.org, chrisl@kernel.org,
	kasong@tencent.com, baohua@kernel.org, youngjun.park@lge.com,
	hannes@cmpxchg.org, yosry@kernel.org, david@kernel.org,
	shikemeng@huaweicloud.com, chengming.zhou@linux.dev,
	linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH 00/11] mm, swap: dynamic cluster management for xswap devices
Date: Thu, 30 Jul 2026 00:25:27 +0800	[thread overview]
Message-ID: <amopdxwZErs_1TNP@MiWiFi-R3L-srv> (raw)
In-Reply-To: <CAKEwX=MNhfDr5WbkNAy7kO7z5JFcJT0DE+_hgMoz5Xi7vZ5_yg@mail.gmail.com>

On 07/27/26 at 08:48am, Nhat Pham wrote:
> On Mon, Jul 27, 2026 at 6:50 AM Baoquan He <baoquan.he@linux.dev> wrote:
> >
> > This series implements step 3 of the incremental path proposed in [1]:
> > a virtual swap device with dynamic cluster growth and shrink, backed
> > only by zswap, no writeback yet.
> >
> > During the discussion in [2], Chris and Nhat debated xarray-based vs
> > array-based cluster lookup for virtual swap.  Chris argued for keeping
> > the swap table and cluster_info in the same place with O(1) array
> > indexing, and suggested that the kvmalloc grow approach here should
> > provide the cluster management VS needs.  He further proposed using
> > the same grow technique for the future per-slot backend pointer array
> > (vs_table), keeping a single coherent array layout throughout.
> >
> > This series therefore serves as the foundation: once the dynamic
> > cluster management lands, Nhat's VS series can build the per-slot
> > backend pointer, writeback, and rmap on top of it — without an extra
> > xarray lookup in the cluster access path.
> 
> Thanks Baoquan. I've given it some more thoughts over the past week or
> so, to figure out a way to so that everyone would be happy with the
> final results.
> 
> To be perfectly clear, I'm not married with the xarray data structure.
> I only care about use case aspect (as you have seen, I switch over to
> swap table the moment I figure out a way to realize Kairui's design
> proposal - it's way cleaner) - and I've seen that you have addressed a
> lot of the design concerns I had that would hindered the use cases I
> presented. If the vmalloc data structure outperforms the xarray, it
> should *definitely* be the data structure we use when everything
> settles.
> 
> That just leaves the sequencing of our efforts :)
> 
> My only fear with landing one piece at a time is that that we land
> something that ended up not working for the entire big series in some
> unforeseen cases, and we have to rip it out or rewrite a huge chunks
> of it. Besides, without an actual use case, we can't really test its
> correctness, and purported performance improvement benefits.

Thanks for the thoughtful and honest reply, Nhat.  I really appreciate
the clarity — and the fact that this is a sequencing discussion, not a
technical objection.

Personally, I still lean toward the incremental approach.  The cluster
management layer feels like a well-defined, self-contained foundation
that can be reviewed and tested independently, and landing it first
would let the later change built on it with less effort.

I think the overall direction is converging nicely.  We've reached rough
consensus on the big-picture architecture — decoupling backends, writeback,
dynamic cluster grow/shrink, and per-device xswap/vswap — that's a solid
foundation.  Once we settle on the cluster_info implementation, everything
else should fall into place quickly. From there we can move on to actual
swap-tier usage of xswap/vswap and other extensions on top.

However, if the consensus leans toward merging your xarray-based vswap
series first and then switching the data structure as a follow-up
optimization, I have no problem with that either. One concern: if we
land the xarray-based approach first and then switch to a pointer array
later, the cluster_info type change (e.g to struct swap_cluster_info *
flat array or struct swap_cluster_info ** pointer array) will cause us
touching those codes twice. Just something to weigh against the
"land early" argument.

I'm quite optimistic about how things are shaping up.  It's been a long
discussion but I think we're very close to something that everyone can
get behind.

Thanks
Baoquan

> 
> That's why so far I have bitten the bullet and sent out the whole
> series (even if it's really large) with the core use cases
> implemented. I understand that it is big, and is burdensome for
> reviewers - but at least you know that the core use cases works - and
> if you don't take my work for it, you can test with it and benchmark
> it, etc. And with Kairui's proposal, we still maintain the old
> machinery, which allows us to even merge with something a bit
> suboptimal, then optimize later. FWIW, so far in my testing, the
> numbers have been quite close, at least for zswap backend, and the
> core concern (memory overhead) has been resolved. So it might actually
> already be usable out of the box? :)
> 
> This patch series has reached 700 LoCs, and I still have some
> correctness concerns - I have commented one in another patch. It has
> gotten more complicated now, which is no fault of you - we just need
> more logic to handle the things xarray provided us out of the box,
> with the hope that it would be more optimal. And that's fine. I just
> think it would be better to land the xarray version first to unblock
> users, then just slot your patch series on top as a follow-up
> optimization. It would show the actual win of your data structure over
> xarray data structure with concrete numbers :) Win-win?
> 
> But thanks again for taking a stab at it. Seriously, you're awesome. I
> want to re-iterate again - this is NOT a Nacked-by Nhat. Not with this
> patch series, nor with the direction. I'm just trying to figure out
> the sequencing of our efforts that would make everything work. I'll
> spend sometimes on my own trying to synthesize this data structure
> with vswap design too, to see if I can fish out more gaps, and to see
> if I can clean things up in a way that would make the switch from
> xarray to this new data structure as painless as possible. As well as
> the other design points we have touched on in our last conversation -
> it makes sense to me at the time, but I need to code some prototype
> out and stare at it a bit more.


      reply	other threads:[~2026-07-29 16:25 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-27 13:50 [RFC PATCH 00/11] mm, swap: dynamic cluster management for xswap devices Baoquan He
2026-07-27 13:50 ` [RFC PATCH 01/11] mm: xswap support for zswap Baoquan He
2026-07-27 14:04 ` [RFC PATCH 02/11] mm, swap: add CONFIG_XSWAP and xswap fields to swap_info_struct Baoquan He
2026-07-27 14:04   ` [RFC PATCH 03/11] mm, swap: add xswap cluster grow via VM_SPARSE vmalloc Baoquan He
2026-07-27 15:05     ` Nhat Pham
2026-07-29 15:12       ` Baoquan He
2026-07-27 16:09     ` Chris Li
2026-07-29 14:36       ` Baoquan He
2026-07-27 14:04   ` [RFC PATCH 04/11] mm, swap: add xswap grow trigger on cluster allocation Baoquan He
2026-07-27 14:04   ` [RFC PATCH 05/11] mm, swap: add xswap_try_shrink and shrink trigger on cluster free Baoquan He
2026-07-27 14:04   ` [RFC PATCH 06/11] mm, swap: free backing pages in xswap_unmap_clusters Baoquan He
2026-07-27 14:04   ` [RFC PATCH 07/11] mm, swap: add nr_free_tail for O(1) xswap shrink detection Baoquan He
2026-07-27 14:04   ` [RFC PATCH 08/11] mm, swap: add adjustable runtime ceiling (nr_clusters) for xswap Baoquan He
2026-07-27 14:05   ` [RFC PATCH 09/11] mm, swap: add debugfs knob for xswap per-device cluster limit Baoquan He
2026-07-27 14:05   ` [RFC PATCH 10/11] mm, swap: defer xswap shrink to workqueue to avoid lock recursion Baoquan He
2026-07-27 14:05   ` [RFC PATCH 11/11] mm, swap: serialize xswap map/unmap with a mutex Baoquan He
2026-07-27 15:13     ` Nhat Pham
2026-07-27 15:26   ` [RFC PATCH 02/11] mm, swap: add CONFIG_XSWAP and xswap fields to swap_info_struct Chris Li
2026-07-29 14:26     ` Baoquan He
2026-07-27 15:48 ` [RFC PATCH 00/11] mm, swap: dynamic cluster management for xswap devices Nhat Pham
2026-07-29 16:25   ` Baoquan He [this message]

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=amopdxwZErs_1TNP@MiWiFi-R3L-srv \
    --to=baoquan.he@linux.dev \
    --cc=akpm@linux-foundation.org \
    --cc=baohua@kernel.org \
    --cc=chengming.zhou@linux.dev \
    --cc=chrisl@kernel.org \
    --cc=david@kernel.org \
    --cc=hannes@cmpxchg.org \
    --cc=kasong@tencent.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=nphamcs@gmail.com \
    --cc=shikemeng@huaweicloud.com \
    --cc=yosry@kernel.org \
    --cc=youngjun.park@lge.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