linux-kernel - Re: [PATCH 0/3] mm: zswap: trivial folio conversions

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 for Android: free password hash cracker in your pocket

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

Message-ID: <ZlAQo0P4Z-dgVHn6@casper.infradead.org>
Date: Fri, 24 May 2024 04:59:31 +0100
From: Matthew Wilcox <willy@...radead.org>
To: Yosry Ahmed <yosryahmed@...gle.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
	Johannes Weiner <hannes@...xchg.org>, Nhat Pham <nphamcs@...il.com>,
	Chengming Zhou <chengming.zhou@...ux.dev>, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 0/3] mm: zswap: trivial folio conversions

On Fri, May 24, 2024 at 03:38:15AM +0000, Yosry Ahmed wrote:
> Some trivial folio conversions in zswap code.

The three patches themselves look good.

> The mean reason I included a cover letter is that I wanted to get
> feedback on what other trivial conversions can/should be done in
> mm/zswap.c (keeping in mind that only order-0 folios are supported
> anyway).  These are the things I came across while searching for 'page'
> in mm/zswap.c, and chose not to do anything about for now:

I think there's a deeper question to answer before answering these
questions, which is what we intend to do with large folios and zswap in
the future.  Do we intend to split them?  Compress them as a large
folio?  Compress each page in a large folio separately?  I can see an
argument for choices 2 and 3, but I think choice 1 is going to be
increasingly untenable.

> 1. zswap_max_pages(), zswap_accept_thr_pages(), zswap_total_pages():
>   - We can use 'size' instead of 'pages' and shift the return by
>     PAGE_SHIFT. This adds an unnecessary shift, but I doubt it matters
>     at all. The motivation is to get rid of 'page' to find things that
>     should be converted more easily.
> 
> 2. Counters names: zswap_stored_pages, zswap_written_back_pages, etc.
> 
> 3. Comments all over the place reference 'page' instead of 'folio'.
> 
> 4. shrink_memcg_cb(), zswap_shrinker_scan():
>   - Rename encountered_page_in_swap_cache to
>     encounterd_folio_in_swap_cache, or even better: folio_eexist or
>     hit_swap_cache.
> 
> 5. entry_to_nid():
>   - It's tempting to try to use folio_to_nid(virt_to_folio()), but I
>     think this adds an unnecessary call to compound_head(). It may not
>     matter in practice though because the page is always a head page.
> 
> Yosry Ahmed (3):
>   mm: zswap: use sg_set_folio() in zswap_{compress/decompress}()
>   mm :zswap: use kmap_local_folio() in zswap_load()
>   mm: zswap: make same_filled functions folio-friendly
> 
>  mm/zswap.c | 41 ++++++++++++++++++-----------------------
>  1 file changed, 18 insertions(+), 23 deletions(-)
> 
> -- 
> 2.45.1.288.g0e0cd299f1-goog
> 
>