Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) client-ip=2620:137:e000::3:5;
Date:   Thu, 2 Nov 2023 16:50:22 -0400
From:   Johannes Weiner <hannes@cmpxchg.org>
To:     Yosry Ahmed <yosryahmed@google.com>
Cc:     Nhat Pham <nphamcs@gmail.com>, akpm@linux-foundation.org,
        tj@kernel.org, lizefan.x@bytedance.com,
        cerasuolodomenico@gmail.com, sjenning@redhat.com,
        ddstreet@ieee.org, vitaly.wool@konsulko.com, mhocko@kernel.org,
        roman.gushchin@linux.dev, shakeelb@google.com,
        muchun.song@linux.dev, hughd@google.com, corbet@lwn.net,
        konrad.wilk@oracle.com, senozhatsky@chromium.org, rppt@kernel.org,
        linux-mm@kvack.org, kernel-team@meta.com,
        linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
        david@ixit.cz
Subject: Re: [RFC PATCH v2] zswap: memcontrol: implement zswap writeback
 disabling
Message-ID: <20231102205022.GA3265934@cmpxchg.org>
References: <20231102200202.920461-1-nphamcs@gmail.com>
 <CAJD7tkY8iPBo99+1gdsSRMNDu4jkVKz8rb=W+xk9=GE0y=kSuw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAJD7tkY8iPBo99+1gdsSRMNDu4jkVKz8rb=W+xk9=GE0y=kSuw@mail.gmail.com>
Precedence: bulk

On Thu, Nov 02, 2023 at 01:27:24PM -0700, Yosry Ahmed wrote:
> On Thu, Nov 2, 2023 at 1:02 PM Nhat Pham <nphamcs@gmail.com> wrote:
> > @@ -201,6 +201,12 @@ int swap_writepage(struct page *page, struct writeback_control *wbc)
> >                 folio_end_writeback(folio);
> >                 return 0;
> >         }
> > +
> > +       if (!mem_cgroup_zswap_writeback_enabled(folio_memcg(folio))) {
> > +               folio_mark_dirty(folio);
> > +               return AOP_WRITEPAGE_ACTIVATE;
> > +       }
> > +
> 
> I am not a fan of this, because it will disable using disk swap if
> "zswap_writeback" is disabled, even if zswap is disabled or the page
> was never in zswap. The term zswap_writeback makes no sense here tbh.
> 
> I am still hoping someone else will suggest better semantics, because
> honestly I can't think of anything. Perhaps something like
> memory.swap.zswap_only or memory.swap.types which accepts a string
> (e.g. "zswap"/"all",..).

I had suggested the writeback name.

My thinking was this: from a user pov, the way zswap is presented and
described, is a fast writeback cache that sits on top of swap. It's
implemented as this lookaside thing right now, but that's never how it
was sold. And frankly, that's not how it's expected to work, either.

From the docs:

  Zswap is a lightweight compressed cache for swap pages.

  Zswap evicts pages from compressed cache on an LRU basis to the
  backing swap device when the compressed pool reaches its size limit.

When zswap is enabled, IO to the swap device is expected to come from
zswap. Everything else would be a cache failure. There are a few cases
now where zswap rejects and bypasses to swap. It's not a stretch to
call them accelerated writeback or writethrough. But also, these
represent failures and LRU inversions, we're working on fixing them.

So from a user POV it's reasonable to say if I have zswap enabled and
disable writeback, I don't expect swapfile IO.

But yes, if zswap isn't enabled at all, this shouldn't prevent pages
from going to swap.