Re: [PATCH v5 01/11] mm, swap: add virtual swap device infrastructure
From: Chris Li
Date: Thu Sep 24 2026 - 21:18:04 EST
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.
>
> When everything in vswap, we don't need to test for it.
>
I've been there and done that, the result was pretty bad in those
earlier series.
> 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.
Chris
>
> What am I missing?
>
> --
> All Rights Reversed.