RE: [PATCH v1 2/2] mm: zswap: zswap_store_pages() simplifications for batching.
From: Sridhar, Kanchana P
Date: Mon Dec 02 2024 - 20:15:59 EST
> -----Original Message-----
> From: Nhat Pham <nphamcs@xxxxxxxxx>
> Sent: Monday, December 2, 2024 4:17 PM
> To: Chengming Zhou <chengming.zhou@xxxxxxxxx>
> Cc: Sridhar, Kanchana P <kanchana.p.sridhar@xxxxxxxxx>; linux-
> kernel@xxxxxxxxxxxxxxx; linux-mm@xxxxxxxxx; hannes@xxxxxxxxxxx;
> yosryahmed@xxxxxxxxxx; usamaarif642@xxxxxxxxx;
> ryan.roberts@xxxxxxx; 21cnbao@xxxxxxxxx; akpm@xxxxxxxxxxxxxxxxxxxx;
> Feghali, Wajdi K <wajdi.k.feghali@xxxxxxxxx>; Gopal, Vinodh
> <vinodh.gopal@xxxxxxxxx>
> Subject: Re: [PATCH v1 2/2] mm: zswap: zswap_store_pages() simplifications
> for batching.
>
> On Wed, Nov 27, 2024 at 11:00 PM Chengming Zhou
> <chengming.zhou@xxxxxxxxx> wrote:
> >
> > How about introducing a `zswap_compress_folio()` interface which
> > can be used by `zswap_store()`?
> > ```
> > zswap_store()
> > nr_pages = folio_nr_pages(folio)
> >
> > entries = zswap_alloc_entries(nr_pages)
> >
> > ret = zswap_compress_folio(folio, entries, pool)
> >
> > // store entries into xarray and LRU list
> > ```
> >
> > And this version `zswap_compress_folio()` is very simple for now:
> > ```
> > zswap_compress_folio()
> > nr_pages = folio_nr_pages(folio)
> >
> > for (index = 0; index < nr_pages; ++index) {
> > struct page *page = folio_page(folio, index);
> >
> > if (!zswap_compress(page, &entries[index], pool))
> > return false;
> > }
> >
> > return true;
> > ```
> > This can be easily extended to support your "batched" version.
> >
> > Then the old `zswap_store_page()` could be removed.
> >
> > The good point is simplicity, that we don't need to slice folio
> > into multiple batches, then repeat the common operations for each
> > batch, like preparing entries, storing into xarray and LRU list...
> >
>
> +1.
Thanks Nhat. I replied with some potential considerations in my reply
to Chengming's and Yosry's comments. Would appreciate it if you can
add follow-up suggestions to that reply.
Thanks,
Kanchana