Re: [RFC PATCH 00/10] mm/swap: ghost swapfile with backend switching via Redirect entries

[Chris Li]

On Tue, Jul 21, 2026 at 11:28 AM Nhat Pham <nphamcs@xxxxxxxxx> wrote:
>
> On Tue, Jul 21, 2026 at 11:03 AM Chris Li <chrisl@xxxxxxxxxx> wrote:
>
> I'm writing a full response to Baoquan, but there are a couple points
> I'd like to point out first, to save everyone's from the confusion
> going forward :)

Thanks

> > On Tue, Jul 21, 2026 at 1:06 AM Baoquan He <baoquan.he@xxxxxxxxx> wrote:
> > >
> > > On 07/17/26 at 09:02am, Nhat Pham wrote:
> > > > On Fri, Jul 17, 2026 at 7:27 AM Baoquan He <baoquan.he@xxxxxxxxx> wrote:
> > > > >
> > > > > On 07/16/26 at 09:49am, Nhat Pham wrote:
> > > > >
> > > > > Thanks for the thoughtful reply and the very constructive proposal.
> > > > > Your suggestion about moving forward in small, incremental steps in
> > > > > last mail is really appealing — I think that's the right way to make
> > > > > progress on this. I need a little time to think it through more
> > > > > carefully before giving you a proper response. Will get back to you
> > > > > soon.
> > > >
> > > > Yeah let me know what you think. I'm planning to send out v3 with some
> > > > more cleanups and fixes, but I'm happy to hear what you and Chris
> > > > wants to make it as useful as possible for everyone.
> > >> I agree with that. The current VS patch is a great improvement over
> > the previous version of VS. However it places the cluster info and
> > swap table in different locations when accessing swap table-related
> > information. The code path requires different locks as well, the
> > xarray needs to take the xa lock.  This kind of dual-personality issue
> > will make reasoning harder. Keeping the swap table in the same place
> > is simpler in my mind.
> >
> > The other issue I want to add is that, using xarray as a swap
> > allocator is unverified. It has no size limit which is one of its
> > claimed advantages, but also one of its drawbacks. Because it does not
> > have swap_full() concept. It will never do the normal if swap_full()
> > fail to allocate, let's do swap cache related. That code path cannot
> > trigger because the xarray has no size limit.
>
> 1. If the page is reclaimable, reclaim path will take care of swap cache too.
>
> 2. For non-physical swap users, we actually eagerly attempt to get rid
> of the swap space (and swap cache) once the final reference is
> dropped. Zswap load invalidates the compressed copy, so there's not a
> lot of value in caching. I'm not sure if you missed this or not.

I am aware of that. However my brain is a bit fuzzy on the exact
behavior in zswap because our internal implementation has some minor
differences in how it handles this.

Anyway, I am not sure 1) and 2) is enough to get rid of the
swap_full() and reclaim swap cache test for zswap. If you think that
is sufficient, we should be able to make it as a separate patch to
remove the swap full in this situation and test it on the upstream
kernel before the VS series.

> > There are other considerations as well. It works fine if VS only wants
> > to allocate 4K swap entries. If VS later wants to go into the mthp
> > swap entry allocation. I don't think xarray has similar considerations
> > as the current swap allocator regarding fragment lists, etc. The
> > xarray isn't designed to be used that way, at least not in the common
> > usage situation.  You are likely running into your xarray allocation
> > specific issue.
>
> I don't understand what you're asking here. mTHP swap allocation
> already works with my RFC.
>
> The xarray maps to 512 entry cluster, not individual swap slots. Once
> you get a cluster (either from a partial cluster or a new cluster from
> the xarray), allocation of mTHP works exactly the same as physical
> swap allocator.

Thanks for the clarification. I haven't looked at the patch that
closely. I was mostly thinking about the data structure layout and the
relationship of the newly proposed data structure.
A follow up question. If the free_cluster list able to satisfy the
allocation requirement, there is no extra xarray allocation of the
cluster in your patch series, right? I just want to make sure I
understand it correctly.

> > The xarray allocator also has a fragmentation issue where not all
> > memory released from the xarray returns to the system as free pages.
> > Using kvmalloc grow it will wait some unused cluster info. But let's
> > put it into perspective: cluster info is shared by 512 swap entries.
> > Each swap entry is 4K. So the memory used by swap cluster is pretty
> > small. Much smaller than 1 byte per swap entry. We can also modify the
> > cluster info array, making it an array of pointers. Then the waste
> > will be even smaller (if unused). At the cost of one more pointer per
> > swap_cluster_info. We can consider all this later, not at this current
> > stage.
>
> The xarray also maps each index to a cluster too.

It the cluster slot offset take care of the lower 9 bits (512) entry
and the upper bits of the index is for the xarray look up?

> > > 4. Writeback and rmap as follow-up
> >
> > I agree that adding cluster info growth as the first step. I recall
> > Kairui has an RFC patch series to implement virtual entry lookup
> > (still using the xarray). I think the combination of growth and
> > indirection lookup should provide most of what VS depends on from the
> > core swap point of view.
>
> That is *exactly* what my RFC is using. Did you see the first patch? I
> put Co-developed-by Kairui for a reason.

I saw that. What I mean is that using the xswap (ghostswap) file was
the original purpose of Kairui's patch series. Could you use Kairui's
original patches series with an xswap/ghostswap file and build on top
of that? Now with Baoquan's kvmalloc growing the array, I think VS can
use that instead of the xarray mapping of the cluster. That is what I
mean.

Let me know if you have further questions.


Chris