Received-SPF: pass (google.com: domain of linux-kernel+bounces-214635-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Precedence: bulk
MIME-Version: 1.0
References: <20240610121820.328876-1-usamaarif642@gmail.com>
 <CAJD7tkZzHhvVNjECqMiFKEu988eteiFk4aGpE=f=gXE4tnF4NA@mail.gmail.com> <52df55b7-2e1d-4a95-85bf-19f6680e3fec@gmail.com>
In-Reply-To: <52df55b7-2e1d-4a95-85bf-19f6680e3fec@gmail.com>
From: Yosry Ahmed <yosryahmed@google.com>
Date: Fri, 14 Jun 2024 02:28:47 -0700
Message-ID: <CAJD7tkYRU-8_fxco6B+--9wVCKzHWOWM19AwfzJTLpVcJ_1P0Q@mail.gmail.com>
Subject: Re: [PATCH v3 0/2] mm: store zero pages to be swapped out in a bitmap
To: Usama Arif <usamaarif642@gmail.com>
Cc: akpm@linux-foundation.org, hannes@cmpxchg.org, david@redhat.com, 
	ying.huang@intel.com, hughd@google.com, willy@infradead.org, 
	nphamcs@gmail.com, chengming.zhou@linux.dev, linux-mm@kvack.org, 
	linux-kernel@vger.kernel.org, kernel-team@meta.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Fri, Jun 14, 2024 at 2:22=E2=80=AFAM Usama Arif <usamaarif642@gmail.com>=
 wrote:
>
>
> On 13/06/2024 22:21, Yosry Ahmed wrote:
> > On Mon, Jun 10, 2024 at 5:18=E2=80=AFAM Usama Arif <usamaarif642@gmail.=
com> wrote:
> >> Going back to the v1 implementation of the patchseries. The main reaso=
n
> >> is that a correct version of v2 implementation requires another rmap
> >> walk in shrink_folio_list to change the ptes from swap entry to zero p=
ages to
> >> work (i.e. more CPU used) [1], is more complex to implement compared t=
o v1
> >> and is harder to verify correctness compared to v1, where everything i=
s
> >> handled by swap.
> >>
> >> ---
> >> As shown in the patchseries that introduced the zswap same-filled
> >> optimization [2], 10-20% of the pages stored in zswap are same-filled.
> >> This is also observed across Meta's server fleet.
> >> By using VM counters in swap_writepage (not included in this
> >> patchseries) it was found that less than 1% of the same-filled
> >> pages to be swapped out are non-zero pages.
> >>
> >> For conventional swap setup (without zswap), rather than reading/writi=
ng
> >> these pages to flash resulting in increased I/O and flash wear, a bitm=
ap
> >> can be used to mark these pages as zero at write time, and the pages c=
an
> >> be filled at read time if the bit corresponding to the page is set.
> >>
> >> When using zswap with swap, this also means that a zswap_entry does no=
t
> >> need to be allocated for zero filled pages resulting in memory savings
> >> which would offset the memory used for the bitmap.
> >>
> >> A similar attempt was made earlier in [3] where zswap would only track
> >> zero-filled pages instead of same-filled.
> >> This patchseries adds zero-filled pages optimization to swap
> >> (hence it can be used even if zswap is disabled) and removes the
> >> same-filled code from zswap (as only 1% of the same-filled pages are
> >> non-zero), simplifying code.
> >>
> >> This patchseries is based on mm-unstable.
> > Aside from saving swap/zswap space and simplifying the zswap code
> > (thanks for that!), did you observe any performance benefits from not
> > having to go into zswap code for zero-filled pages?
> >
> > In [3], I observed ~1.5% improvement in kernbench just by optimizing
> > zswap's handling of zero-filled pages, and that benchmark only
> > produced around 1.5% zero-filled pages. I imagine avoiding the zswap
> > code entirely, and for workloads that have 10-20% zero-filled pages,
> > the performance improvement should be more pronounced.
> >
> > When zswap is not being used and all swap activity translates to IO, I
> > imagine the benefits will be much more significant.
> >
> > I am curious if you have any numbers with or without zswap :)
>
> Apart from tracking zero-filled pages (using inaccurate counters not in
> this series) which had the same pattern to zswap_same_filled_pages, the
> nvme writes went down around 5-10% during stable points in the
> production experiment. The performance improved by 2-3% at some points,
> but this is comparing 2 sets of machines running production workloads
> (which can vary between machine sets), so I would take those numbers
> cautiously and which is why I didnt include them in the cover letter.
>

Yeah this makes sense, thanks. It would have been great if we had
comparable numbers with and without this series. But this shouldn't be
a big deal, the advantage of the series should be self-explanatory.
It's just a shame you don't get to brag about it :)