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

["Sridhar, Kanchana P" <kanchana.p.sridhar@intel.com>]

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

> 
> >  }