linux-kernel - Re: [PATCH 0/2] minimize swapping on zswap store failure

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: <20250402200651.1224617-1-joshua.hahnjy@gmail.com>
Date: Wed,  2 Apr 2025 13:06:49 -0700
From: Joshua Hahn <joshua.hahnjy@...il.com>
To: Yosry Ahmed <yosryahmed@...gle.com>
Cc: Nhat Pham <nphamcs@...il.com>,
	akpm@...ux-foundation.org,
	hannes@...xchg.org,
	cerasuolodomenico@...il.com,
	sjenning@...hat.com,
	ddstreet@...e.org,
	vitaly.wool@...sulko.com,
	hughd@...gle.com,
	corbet@....net,
	konrad.wilk@...cle.com,
	senozhatsky@...omium.org,
	rppt@...nel.org,
	linux-mm@...ck.org,
	kernel-team@...a.com,
	linux-kernel@...r.kernel.org,
	david@...t.cz
Subject: Re: [PATCH 0/2] minimize swapping on zswap store failure

On Mon, 16 Oct 2023 17:57:31 -0700 Yosry Ahmed <yosryahmed@...gle.com> wrote:

> On Mon, Oct 16, 2023 at 5:35 PM Nhat Pham <nphamcs@...il.com> wrote:

> I thought before about having a special list_head that allows us to
> use the lower bits of the pointers as markers, similar to the xarray.
> The markers can be used to place different objects on the same list.
> We can have a list that is a mixture of struct page and struct
> zswap_entry. I never pursued this idea, and I am sure someone will
> scream at me for suggesting it. Maybe there is a less convoluted way
> to keep the LRU ordering intact without allocating memory on the
> reclaim path.

Hi Yosry,

Apologies for reviving an old thread, but I wasn't sure whether opening an
entirely new thread was a better choice : -)

So I've implemented your idea, using the lower 2 bits of the list_head's prev
pointer (last bit indicates whether the list_head belongs to a page or a
zswap_entry, and the second to last bit was repurposed for the second chance
algorithm).

For a very high level overview what I did in the patch:
- When a page fails to compress, I remove the page mapping and tag both the
  xarray entry (tag == set lowest bit to 1) and the page's list_head prev ptr,
  then store the page directly into the zswap LRU.
- In zswap_load, we take the entry out of the xarray and check if it's tagged.
  - If it is tagged, then instead of decompressing, we just copy the page's
    contents to the newly allocated page. 
- (More details about how to teach vmscan / page_io / list iterators how to
  handle this, but we can gloss over those details for now)

I have a working version, but have been holding off because I have only been
seeing regressions. I wasn't really sure where they were coming from, but
after going through some perf traces with Nhat, found out that the regressions
come from the associated page faults that come from initially unmapping the
page, and then re-allocating it for every load. This causes (1) more memcg
flushing, and (2) extra allocations ==> more pressure ==> more reclaim, even
though we only temporarily keep the extra page.

Just wanted to put this here in case you were still thinking about this idea.
What do you think? Ideally, there would be a way to keep the page around in
the zswap LRU, but do not have to re-allocate a new page on a fault, but this
seems like a bigger task.

Ultimately the goal is to prevent an incompressible page from hoarding the
compression algorithm on multiple reclaim attempts, but if we are spending
more time by allocating new pages... maybe this isn't the correct approach :(

Please let me know if you have any thoughts on this : -)
Have a great day!
Joshua

Sent using hkml (https://github.com/sjp38/hackermail)