Message-ID: <etjzs45r7derztzgrb5b4tub4c72vobo6esc3ndksyke6kpiov@qj3ar7xwh6ee>
Date: Thu, 13 Jun 2024 15:41:10 -0700
From: Shakeel Butt <shakeel.butt@...ux.dev>
To: Yosry Ahmed <yosryahmed@...gle.com>
Cc: Usama Arif <usamaarif642@...il.com>, akpm@...ux-foundation.org, 
	hannes@...xchg.org, david@...hat.com, ying.huang@...el.com, hughd@...gle.com, 
	willy@...radead.org, nphamcs@...il.com, chengming.zhou@...ux.dev, linux-mm@...ck.org, 
	linux-kernel@...r.kernel.org, kernel-team@...a.com, Minchan Kim <minchan@...nel.org>, 
	Sergey Senozhatsky <senozhatsky@...omium.org>
Subject: Re: [PATCH v3 0/2] mm: store zero pages to be swapped out in a bitmap

On Thu, Jun 13, 2024 at 02:50:31PM GMT, Yosry Ahmed wrote:
> On Mon, Jun 10, 2024 at 5:18 AM Usama Arif <usamaarif642@...il.com> wrote:
> >
> > Going back to the v1 implementation of the patchseries. The main reason
> > 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 pages to
> > work (i.e. more CPU used) [1], is more complex to implement compared to v1
> > and is harder to verify correctness compared to v1, where everything is
> > 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/writing
> > these pages to flash resulting in increased I/O and flash wear, a bitmap
> > can be used to mark these pages as zero at write time, and the pages can
> > 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 not
> > 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.
> 
> There is also code to handle same-filled pages in zram, should we
> remove this as well? It is worth noting that the handling in zram was
> initially for zero-filled pages only, but it was extended to cover
> same-filled pages as well by commit 8e19d540d107 ("zram: extend zero
> pages to same element pages"). Apparently in a test on Android, about
> 2.5% of the swapped out pages were non-zero same-filled pages.
> 
> However, the leap from handling zero-filled pages to handling all
> same-filled pages in zram wasn't a stretch. But now that zero-filled
> pages handling in zram is redundant with this series, I wonder if it's
> still worth keeping the same-filled pages handling.

Please correct me if I am wrong but zram same-filled page handling is
not just limited to swap-on-zram use-case and any zram as block device
user can benefit from it. Also zram might not see any simplification
similar to zswap in this patch series. I would say motivation behind
zswap changes seems quite different from possible zram changes. I would
recommed to evaluate these cases independently.

Powered by blists - more mailing lists