Received-SPF: pass (google.com: domain of linux-kernel+bounces-122109-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Precedence: bulk
MIME-Version: 1.0
References: <20240325235018.2028408-1-yosryahmed@google.com>
 <20240325235018.2028408-7-yosryahmed@google.com> <CAKEwX=OMtqFKLRXF5v2P-Z5WEq3x0Tb=6dPKta8uGqi3MXsZEA@mail.gmail.com>
In-Reply-To: <CAKEwX=OMtqFKLRXF5v2P-Z5WEq3x0Tb=6dPKta8uGqi3MXsZEA@mail.gmail.com>
From: Yosry Ahmed <yosryahmed@google.com>
Date: Wed, 27 Mar 2024 15:38:40 -0700
Message-ID: <CAJD7tkYmVUhBw5ZXT2yopGOSKRg0Rord1+FFddHWfmb58Z1cgA@mail.gmail.com>
Subject: Re: [RFC PATCH 6/9] mm: zswap: drop support for non-zero same-filled
 pages handling
To: Nhat Pham <nphamcs@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>, Johannes Weiner <hannes@cmpxchg.org>, 
	Chengming Zhou <chengming.zhou@linux.dev>, linux-mm@kvack.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Mar 27, 2024 at 9:41=E2=80=AFAM Nhat Pham <nphamcs@gmail.com> wrote=
:
>
> On Mon, Mar 25, 2024 at 4:50=E2=80=AFPM Yosry Ahmed <yosryahmed@google.co=
m> wrote:
> >
> > The current same-filled pages handling supports pages filled with any
> > repeated word-sized pattern. However, in practice, most of these should
> > be zero pages anyway. Other patterns should be nearly as common.
>
> It'd be nice if we can verify this somehow. Maybe hooking bpftrace,
> trace_printk, etc. here?

I am trying to do that. Unfortunately collecting this data from our
fleet is not easy, so it will take some time to figure out. If the
data happens to be easy-ish to collect from your fleet that would be
awesome :)

>
> That aside, my intuition is that this is correct too. It's much less
> likely to see a non-zero filled page.
>
> >
> > Drop the support for non-zero same-filled pages, but keep the names of
> > knobs exposed to userspace as "same_filled", which isn't entirely
> > inaccurate.
> >
> > This yields some nice code simplification and enables a following patch
> > that eliminates the need to allocate struct zswap_entry for those pages
> > completely.
> >
> > There is also a very small performance improvement observed over 50 run=
s
> > of kernel build test (kernbench) comparing the mean build time on a
> > skylake machine when building the kernel in a cgroup v1 container with =
a
> > 3G limit:
> >
> >                 base            patched         % diff
> > real            70.167          69.915          -0.359%
> > user            2953.068        2956.147        +0.104%
> > sys             2612.811        2594.718        -0.692%
> >
> > This probably comes from more optimized operations like memchr_inv() an=
d
> > clear_highpage(). Note that the percentage of zero-filled pages during
>
> TIL clear_highpage() is a thing :)
>
>
[..]
>
> The code itself LGTM, FWIW:
>
> Reviewed-by: Nhat Pham <nphamcs@gmail.com>

Thanks!