linux-kernel - Re: [PATCH v5 00/21] mm/zsmalloc: add zpdesc memory descriptor for zswap.zpool

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: <66ce5eed.170a0220.387c4d.276d@mx.google.com>
Date: Tue, 27 Aug 2024 16:19:06 -0700
From: Vishal Moola <vishal.moola@...il.com>
To: Sergey Senozhatsky <senozhatsky@...omium.org>
Cc: Matthew Wilcox <willy@...radead.org>,
	Yosry Ahmed <yosryahmed@...gle.com>,
	Andrew Morton <akpm@...ux-foundation.org>, alexs@...nel.org,
	Vitaly Wool <vitaly.wool@...sulko.com>,
	Miaohe Lin <linmiaohe@...wei.com>, linux-kernel@...r.kernel.org,
	linux-mm@...ck.org, minchan@...nel.org, david@...hat.com,
	42.hyeyoo@...il.com, nphamcs@...il.com
Subject: Re: [PATCH v5 00/21] mm/zsmalloc: add zpdesc memory descriptor for
 zswap.zpool

On Wed, Aug 14, 2024 at 03:03:54PM +0900, Sergey Senozhatsky wrote:
> On (24/08/08 04:37), Matthew Wilcox wrote:
> [..]
> > > So I guess if we have something
> > > 
> > > struct zspage {
> > > 	..
> > > 	struct zpdesc *first_desc;
> > > 	..
> > > }
> > > 
> > > and we "chain" zpdesc-s to form a zspage, and make each of them point to
> > > a corresponding struct page (memdesc -> *page), then it'll resemble current
> > > zsmalloc and should work for everyone? I also assume for zspdesc-s zsmalloc
> > > will need to maintain a dedicated kmem_cache?
> > 
> > Right, we could do that.  Each memdesc has to be a multiple of 16 bytes,
> > sp we'd be doing something like allocating 32 bytes for each page.
> > Is there really 32 bytes of information that we want to store for
> > each page?  Or could we store all of the information in (a somewhat
> > larger) zspage?  Assuming we allocate 3 pages per zspage, if we allocate
> > an extra 64 bytes in the zspage, we've saved 32 bytes per zspage.
> 
> I certainly like (and appreciate) the approach that saves us
> some bytes here and there.  zsmalloc page can consist of 1 to
> up to CONFIG_ZSMALLOC_CHAIN_SIZE (max 16) physical pages.  I'm
> trying to understand (in pseudo-C code) what does a "somewhat larger
> zspage" mean.  A fixed size array (given that we know the max number
> of physical pages) per-zspage?

I haven't had the opportunity to respond until now as I was on vacation.

With the current approach in a memdesc world, we would do the following:

1) kmem_cache_alloc() every single Zpdesc
2) Allocate a memdesc/page that points to its own Zpdesc
3) Access/Track Zpdescs directly
4) Use those Zpdescs to build a Zspage

An alternative approach would move more metadata storage from a Zpdesc
into a Zspage instead. That extreme would leave us with:

1) kmem_cache_alloc() once for a Zspage
2) Allocate a memdesc/page that points to the Zspage
3) Use the Zspage to access/track its own subpages (through some magic
we would have to figure out)
4) Zpdescs are just Zspages (since all the information would be in a Zspage)

IMO, we should introduce zpdescs first, then start to shift
metadata from "struct zpdesc" into "struct zspage" until we no longer
need "struct zpdesc". My big concern is whether or not this patchset works
towards those goals. Will it make consolidating the metadata easier? And are
these goals feasible (while maintaining the wins of zsmalloc)? Or should we
aim to leave zsmalloc as it is currently implemented?