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

[Sridhar, Kanchana P]

Hi Andrew,

> -----Original Message-----
> From: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> Sent: Wednesday, November 6, 2024 12:18 PM
> To: Sridhar, Kanchana P <kanchana.p.sridhar@xxxxxxxxx>
> Cc: linux-kernel@xxxxxxxxxxxxxxx; linux-mm@xxxxxxxxx;
> hannes@xxxxxxxxxxx; yosryahmed@xxxxxxxxxx; nphamcs@xxxxxxxxx;
> chengming.zhou@xxxxxxxxx; usamaarif642@xxxxxxxxx;
> ryan.roberts@xxxxxxx; Huang, Ying <ying.huang@xxxxxxxxx>;
> 21cnbao@xxxxxxxxx; linux-crypto@xxxxxxxxxxxxxxx;
> herbert@xxxxxxxxxxxxxxxxxxx; davem@xxxxxxxxxxxxx;
> clabbe@xxxxxxxxxxxx; ardb@xxxxxxxxxx; ebiggers@xxxxxxxxxx;
> surenb@xxxxxxxxxx; Accardi, Kristen C <kristen.c.accardi@xxxxxxxxx>;
> zanussi@xxxxxxxxxx; Feghali, Wajdi K <wajdi.k.feghali@xxxxxxxxx>; Gopal,
> Vinodh <vinodh.gopal@xxxxxxxxx>
> 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@xxxxxxxxx> 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

> 
> >  }