Re: [RFC] Storing same-filled pages without a zswap_entry

[Yosry Ahmed <yosryahmed@google.com>]

On Wed, Mar 20, 2024 at 2:19 PM Johannes Weiner <hannes@cmpxchg.org> wrote:
>
> On Wed, Mar 20, 2024 at 05:07:21PM -0400, Johannes Weiner wrote:
> > On Wed, Mar 20, 2024 at 01:49:17PM -0700, Yosry Ahmed wrote:
> > > Hey folks,
> > >
> > > I was looking at cleaning up the same-filled handling code in zswap,
> > > when it hit me that after the xarray conversion, the only member of
> > > struct zwap_entry that is relevant to same-filled pages is now the
> > > objcg pointer.
> > >
> > > The xarray allows a pointer to be tagged by up to two tags (1 and 3),
> > > so we can completely avoid allocating a zswap_entry for same-filled
> > > pages by storing a tagged objcg pointer directly in the xarray
> > > instead.
> > >
> > > Basically the xarray would then either have a pointer to struct
> > > zswap_entry or struct obj_cgroup, where the latter is tagged as
> > > SAME_FILLED_ONE or SAME_FILLED_ZERO.
> > >
> > > There are two benefits of this:
> > > - Saving some memory (precisely 64 bytes per same-filled entry).
> > > - Further separating handling of same-filled pages from compressed
> > > pages, which results in some nice cleanups (especially in
> > > zswap_store()). It also makes further improvements easier (e.g.
> > > skipping limit checking for same-filled entries).
> >
> > This sounds interesting.
> >
> > Where would you store the byte value it's filled with? Or would you
> > limit it to zero-filled only?
>
> The dumb thing about objcg is that for same-filled entries we really
> only need it for bumping ZSWPIN. Nothing else. entry->length is 0 for
> them, so even though we call the charge function, it doesn't actually
> do anything.
>
> Loading them is cheap and doesn't involve decompression. An argument
> could be made to exclude them from ZSWPOUT and ZSWPIN entirely.
>
> Or cheat a little and bump ZSWPIN for current->objcg instead -
> probably good enough to make excessive thrashing discoverable by the
> workload that's directly affected.
>
> Then you could get rid of the objcg pointer and use the xarray slot
> for whatever else you'd want.

Yeah it's only useful for the stats. Using current->objcg would work,
and should be ultimately pointing to the same memcg in *most* cases, I
assume. We still wouldn't be able to store a full word as we do today,
because the xarray needs 1 bit for its own usage. So the same-filled
implementation would still need to change from repeated words (8
bytes) to something smaller -- or we can just allocate a separate
struct for same-filled pages.