RE: [PATCH v3 12/13] mm: Add sysctl vm.compress-batching switch for compress batching during swapout.

Sridhar, Kanchana P
2024-11-06T20:39:42Z

Hi Andrew,

> -----Original Message-----
> From: Andrew Morton <akpm@linux-foundation.org>
> Sent: Wednesday, November 6, 2024 12:18 PM
> To: Sridhar, Kanchana P <kanchana.p.sridhar@intel.com>
> Cc: linux-kernel@vger.kernel.org; linux-mm@kvack.org;
> hannes@cmpxchg.org; yosryahmed@google.com; nphamcs@gmail.com;
> chengming.zhou@linux.dev; usamaarif642@gmail.com;
> ryan.roberts@arm.com; Huang, Ying <ying.huang@intel.com>;
> 21cnbao@gmail.com; linux-crypto@vger.kernel.org;
> herbert@gondor.apana.org.au; davem@davemloft.net;
> clabbe@baylibre.com; ardb@kernel.org; ebiggers@google.com;
> surenb@google.com; Accardi, Kristen C <kristen.c.accardi@intel.com>;
> zanussi@kernel.org; Feghali, Wajdi K <wajdi.k.feghali@intel.com>; Gopal,
> Vinodh <vinodh.gopal@intel.com>
> Subject: Re: [PATCH v3 12/13] mm: Add sysctl vm.compress-batching switch
> for compress batching during swapout.
>
> On Wed, 6 Nov 2024 11:21:04 -0800 Kanchana P Sridhar
> <kanchana.p.sridhar@intel.com> wrote:
>
> > extern int sysctl_legacy_va_layout;
> > +extern unsigned int compress_batching;
>
> nit: I suggest calling this "sysctl_compress_batching". See how we
> treated sysctl_legacy_va_layout.

Thanks for the code review comments. Sure, I will incorporate this in v4.

>
> > --- a/mm/swap.c
> > +++ b/mm/swap.c
> > @@ -47,6 +47,9 @@
> > int page_cluster;
> > const int page_cluster_max = 31;
> >
> > +/* Enable/disable compress batching during swapout. */
> > +unsigned int compress_batching;
> > +
> > struct cpu_fbatches {
> > /*
> > * The following folio batches are grouped together because they are
> protected
> > @@ -1074,4 +1077,7 @@ void __init swap_setup(void)
> > * Right now other parts of the system means that we
> > * _really_ don't want to cluster much more
> > */
> > +
> > + /* Disable compress batching during swapout by default. */
> > + compress_batching = 0;
>
> Not really needed? The compiler already did that.

Sure, will address this in v4.

Thanks,
Kanchana

>
> > }