linux-kernel - RE: [PATCH v6 2/3] mm: zswap: zswap_store() extended to handle mTHP folios.

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: <SJ0PR11MB56780D9A3812BC08E3B7A461C96C2@SJ0PR11MB5678.namprd11.prod.outlook.com>
Date: Fri, 20 Sep 2024 01:57:39 +0000
From: "Sridhar, Kanchana P" <kanchana.p.sridhar@...el.com>
To: Yosry Ahmed <yosryahmed@...gle.com>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-mm@...ck.org" <linux-mm@...ck.org>, "hannes@...xchg.org"
	<hannes@...xchg.org>, "nphamcs@...il.com" <nphamcs@...il.com>,
	"chengming.zhou@...ux.dev" <chengming.zhou@...ux.dev>,
	"usamaarif642@...il.com" <usamaarif642@...il.com>, "ryan.roberts@....com"
	<ryan.roberts@....com>, "Huang, Ying" <ying.huang@...el.com>,
	"21cnbao@...il.com" <21cnbao@...il.com>, "akpm@...ux-foundation.org"
	<akpm@...ux-foundation.org>, "Zou, Nanhai" <nanhai.zou@...el.com>, "Feghali,
 Wajdi K" <wajdi.k.feghali@...el.com>, "Gopal, Vinodh"
	<vinodh.gopal@...el.com>, "Sridhar, Kanchana P"
	<kanchana.p.sridhar@...el.com>
Subject: RE: [PATCH v6 2/3] mm: zswap: zswap_store() extended to handle mTHP
 folios.

> -----Original Message-----
> From: Yosry Ahmed <yosryahmed@...gle.com>
> Sent: Thursday, August 29, 2024 4:06 PM
> To: Sridhar, Kanchana P <kanchana.p.sridhar@...el.com>
> Cc: linux-kernel@...r.kernel.org; linux-mm@...ck.org;
> hannes@...xchg.org; nphamcs@...il.com; chengming.zhou@...ux.dev;
> usamaarif642@...il.com; ryan.roberts@....com; Huang, Ying
> <ying.huang@...el.com>; 21cnbao@...il.com; akpm@...ux-foundation.org;
> Zou, Nanhai <nanhai.zou@...el.com>; Feghali, Wajdi K
> <wajdi.k.feghali@...el.com>; Gopal, Vinodh <vinodh.gopal@...el.com>
> Subject: Re: [PATCH v6 2/3] mm: zswap: zswap_store() extended to handle
> mTHP folios.
> 
> On Thu, Aug 29, 2024 at 2:27 PM Kanchana P Sridhar
> <kanchana.p.sridhar@...el.com> wrote:
> >
> 
> I think "mm: zswap: support mTHP swapout in zswap_store()" is a better
> subject. We usually use imperative tone for commit logs as much as
> possible.

Sure, this is a much better subject, thanks! I will make this change in v7.

> 
> > zswap_store() will now process and store mTHP and PMD-size THP folios.
> >
> > A new config variable CONFIG_ZSWAP_STORE_THP_DEFAULT_ON (off by
> default)
> > will enable/disable zswap storing of (m)THP.
> >
> > This change reuses and adapts the functionality in Ryan Roberts' RFC
> > patch [1]:
> >
> >   "[RFC,v1] mm: zswap: Store large folios without splitting"
> >
> >   [1] https://lore.kernel.org/linux-mm/20231019110543.3284654-1-
> ryan.roberts@....com/T/#u
> >
> > This patch provides a sequential implementation of storing an mTHP in
> > zswap_store() by iterating through each page in the folio to compress
> > and store it in the zswap zpool.
> >
> > Towards this goal, zswap_compress() is modified to take a page instead
> > of a folio as input.
> >
> > Each page's swap offset is stored as a separate zswap entry.
> >
> > If an error is encountered during the store of any page in the mTHP,
> > all previous pages/entries stored will be invalidated. Thus, an mTHP
> > is either entirely stored in ZSWAP, or entirely not stored in ZSWAP.
> >
> > This forms the basis for building batching of pages during zswap store
> > of large folios, by compressing batches of up to say, 8 pages in an
> > mTHP in parallel in hardware, with the Intel In-Memory Analytics
> > Accelerator (Intel IAA).
> >
> > Also, addressed some of the RFC comments from the discussion in [1].
> >
> > Made a minor edit in the comments for "struct zswap_entry" to delete
> > the comments related to "value" since same-filled page handling has
> > been removed from zswap.
> 
> This commit log is not ordered clearly. Please start by describing
> what we are doing, which is basically making zswap_store() support
> large folios by compressing them page by page. Then mention important
> implementation details and the tunabel and config options added at the
> end. After that, refer to the RFC that this is based on.

Thanks for these comments. Sure, I will incorporate in v7.

> 
> >
> > Co-developed-by: Ryan Roberts
> > Signed-off-by:
> 
> This is probably supposed to be "Signed-off-by: Ryan Roberts". I am
> not sure what the policy is for reusing patches sent earlier on the
> mailing list. Did you talk to Ryan about this?

You're right, this is intended to be "Signed-off-by: Ryan Roberts" once
Ryan has had a chance to review and indicate approval of attribution
as co-author.

I have been following the documentation guidelines for submitting
patches, as pertaining to co-development. Ryan is in the recipients list
and I am hoping he can indicate his approval for the reuse of his original
RFC.

Ryan, I would greatly appreciate your inputs on the reuse of your RFC,
and also any code review comments for improving the patchset!

> 
> > Signed-off-by: Kanchana P Sridhar <kanchana.p.sridhar@...el.com>
> 
> The diff is hard to follow because there is a lot of refactoring mixed
> in with the functional changes. Could you please break this down into
> purely refactoring patches doing the groundwork, and then the actual
> functional change patch(es) on top of them?

Sure, I will do this and submit a v7. Appreciate your comments!

Thanks,
Kanchana

> 
> Thanks!