linux-kernel - Re: [PATCH 5/6] mm/zswap: only support zswap_exclusive_loads

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Zbw_gZIyEHquaxho@google.com>
Date: Fri, 2 Feb 2024 01:04:01 +0000
From: Yosry Ahmed <yosryahmed@...gle.com>
To: Johannes Weiner <hannes@...xchg.org>
Cc: Chengming Zhou <zhouchengming@...edance.com>, Nhat Pham <nphamcs@...il.com>, 
	Andrew Morton <akpm@...ux-foundation.org>, linux-kernel@...r.kernel.org, 
	linux-mm@...ck.org
Subject: Re: [PATCH 5/6] mm/zswap: only support zswap_exclusive_loads_enabled

On Thu, Feb 01, 2024 at 01:12:40PM -0500, Johannes Weiner wrote:
> On Thu, Feb 01, 2024 at 03:49:05PM +0000, Chengming Zhou wrote:
> > The !zswap_exclusive_loads_enabled mode will leave compressed copy in
> > the zswap tree and lru list after the folio swapin.
> > 
> > There are some disadvantages in this mode:
> > 1. It's a waste of memory since there are two copies of data, one is
> >    folio, the other one is compressed data in zswap. And it's unlikely
> >    the compressed data is useful in the near future.
> > 
> > 2. If that folio is dirtied, the compressed data must be not useful,
> >    but we don't know and don't invalidate the trashy memory in zswap.
> > 
> > 3. It's not reclaimable from zswap shrinker since zswap_writeback_entry()
> >    will always return -EEXIST and terminate the shrinking process.
> > 
> > On the other hand, the only downside of zswap_exclusive_loads_enabled
> > is a little more cpu usage/latency when compression, and the same if
> > the folio is removed from swapcache or dirtied.
> > 
> > Not sure if we should accept the above disadvantages in the case of
> > !zswap_exclusive_loads_enabled, so send this out for disscusion.
> > 
> > Signed-off-by: Chengming Zhou <zhouchengming@...edance.com>
> 
> This is interesting.
> 
> First, I will say that I never liked this config option, because it's
> nearly impossible for a user to answer this question. Much better to
> just pick a reasonable default.
> 
> What should the default be?
> 
> Caching "swapout work" is helpful when the system is thrashing. Then
> recently swapped in pages might get swapped out again very soon. It
> certainly makes sense with conventional swap, because keeping a clean
> copy on the disk saves IO work and doesn't cost any additional memory.
> 
> But with zswap, it's different. It saves some compression work on a
> thrashing page. But the act of keeping compressed memory contributes
> to a higher rate of thrashing. And that can cause IO in other places
> like zswap writeback and file memory.

Agreed.

At Google, we have been using exclusive loads for a very long time in
production, so I have no objections to this. The user interface is also
relatively new, so I don't think it will have accumulated users.

> 
> It would be useful to have an A/B test to confirm that not caching is
> better. Can you run your test with and without keeping the cache, and
> in addition to the timings also compare the deltas for pgscan_anon,
> pgscan_file, workingset_refault_anon, workingset_refault_file?

That would be interesting :)