Re: [PATCH] mm:zswap: fix zswap entry reclamation failure in two scenarios

From: Chris Li <chrisl@kernel.org>
To: Yosry Ahmed <yosryahmed@google.com>
Cc: Zhongkun He <hezhongkun.hzk@bytedance.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Nhat Pham <nphamcs@gmail.com>,
	Seth Jennings <sjenning@redhat.com>,
	Dan Streetman <ddstreet@ieee.org>,
	Vitaly Wool <vitaly.wool@konsulko.com>,
	linux-mm <linux-mm@kvack.org>,
	LKML <linux-kernel@vger.kernel.org>, Ying <ying.huang@intel.com>
Subject: Re: [PATCH] mm:zswap: fix zswap entry reclamation failure in two scenarios
Date: Fri, 17 Nov 2023 15:30:47 -0800	[thread overview]
Message-ID: <CAF8kJuNAfzkisG3ukYNuWY6CDSKQ7J6bb0xwFpnSHi0wv3mJWg@mail.gmail.com> (raw)
In-Reply-To: <CAJD7tkahj=QkNj1PBfzGRcDyrpE-n5Dbav7VVOVsAFR4vtx9LA@mail.gmail.com>

On Thu, Nov 16, 2023 at 12:46 PM Yosry Ahmed <yosryahmed@google.com> wrote:
>
> On Thu, Nov 16, 2023 at 12:30 PM Chris Li <chriscli@google.com> wrote:
> >
> > On Thu, Nov 16, 2023 at 12:19 PM Yosry Ahmed <yosryahmed@google.com> wrote:
> > >
> > > Not bypassing the swap slot cache, just make the callbacks to
> > > invalidate the zswap entry, do memg uncharging, etc when the slot is
> > > no longer used and is entering the swap slot cache (i.e. when
> > > free_swap_slot() is called), instead of when draining the swap slot
> > > cache (i.e. when swap_range_free() is called). For all parts of MM
> > > outside of swap, the swap entry is freed when free_swap_slot() is
> > > called. We don't free it immediately because of caching, but this
> > > should be transparent to other parts of MM (e.g. zswap, memcg, etc).
> >
> > That will cancel the batching effect on the swap slot free, making the
> > common case for  swapping  faults take longer to complete, righ?
> > If I recall correctly, the uncharge is the expensive part of the swap
> > slot free operation.
> > I just want to figure out what we are trading off against. This is not
> > one side wins all situations.
>
> Interesting. Maybe we can just move the zswap_invalidate() call to
> save memory early, and leave the memcg uncharge call to be batched?
> IIUC we already do some version of this internally at Google.

I would like to see the patch so I can reason about it better.
Do you have the link for the patch you are talking about?
I can also look up the Google internal one if you have a commit hash.

One thing I would like to find out is whether the behavior of  reusing
swap slots without page writing has been changed.
e.g. Previously if the swap slot can be page out again without
writing/compression again, if the page is not dirty. If we change that
behavior, I would like to understand the trade off better.

Chris