Re: [PATCH v5 01/11] mm, swap: add virtual swap device infrastructure
From: Chris Li
Date: Sat Sep 26 2026 - 14:38:46 EST
On Thu, Sep 24, 2026 at 3:24 PM Rik van Riel <riel@xxxxxxxxxxx> wrote:
>
> On Thu, 2026-09-24 at 15:17 -1000, Chris Li wrote:
> > On Thu, Sep 24, 2026 at 8:13 AM Rik van Riel <riel@xxxxxxxxxxx>
> > wrote:
> > >
> > > On Tue, 2026-09-22 at 23:18 -1000, Chris Li wrote:
> > > > On Fri, Sep 18, 2026 at 1:03 PM Nhat Pham <nphamcs@xxxxxxxxx>
> > > > wrote:
> > > > >
> > > > >
> > > > > @@ -482,6 +491,11 @@ void swap_read_folio(struct swap_io_ctx
> > > > > *ctx,
> > > > > struct folio *folio)
> > > > > if (zswap_load(folio) != -ENOENT)
> > > > > goto finish;
> > > > >
> > > > > + if (unlikely(swap_is_vswap(sis))) {
> > > >
> > > > 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.
> > >
> > > Could we avoid that by simply always having everything
> > > go through the vswap abstraction layer?
> >
> > Yes, but it will pay the price for going through the unnecessary
> > redirection layer always. It incurs performance and meta data
> > overhead. That was one of my previous objections to the earlier vswap
> > version. Sorry I enjoy micro optimization too much, that is both my
> > strength and weakness.
>
> We really only have two cases here, don't we?
>
> Either the data is in the swap cache, and
> the vswap layer can directly look up the
> swap cache page.
>
> Or the data is in some slower back-end,
> and doing a second lookup is not going to
> introduce noticeable overhead.
>
> If the micro optimization come at the cost
> of less flexibility, and harder to maintain
> code, are they really worth it?
In this case, yes. Because there is an alternative that can both keep
code complexity down, perform well, keep metadata small, and be
flexible.
>
> >
> > > Handling the details of what's behind the vswap would
> > > be handled one layer down.
> > >
> > > Chris, do you think that would be cleaner?
> >
> > If it can wrap below the swap_ops, it is just priviate inernal dedail
> > of implementing its own indirections. That would be much cleaner.
> > That
> > is what I am trying to pitch to Nhat in prevoius email but does not
> > have following actions.
> >
> Can you sketch out the details of what you
> think that should look like?
>
> What would the data structures look like?
>
> What operations should there be at the
> abstraction layer, and at the swap layers
> below?
>
> How should migration of data from one swap
> backend to another be handled?
>
> At this point the best way to make progress
> would be to hash out those details.
>
> If you are unhappy with what you have seen
> from others, what does your ideal design
> look like?
Those are very good questions for the people who want to take that
path. I can work that person to figure those out. It will not be me
just handing out all the answers.
Chris