RE: [PATCH v3 12/13] mm: Add sysctl vm.compress-batching switch for compress batching during swapout.
From: Sridhar, Kanchana P
Date: Wed Nov 06 2024 - 15:39:54 EST
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
>
> > }