RE: [PATCH v7 6/8] mm: zswap: Support mTHP swapout in zswap_store().

[Sridhar, Kanchana P]

> -----Original Message-----
> From: Nhat Pham <nphamcs@xxxxxxxxx>
> Sent: Tuesday, September 24, 2024 3:16 PM
> To: Yosry Ahmed <yosryahmed@xxxxxxxxxx>
> Cc: Sridhar, Kanchana P <kanchana.p.sridhar@xxxxxxxxx>; linux-
> kernel@xxxxxxxxxxxxxxx; linux-mm@xxxxxxxxx; hannes@xxxxxxxxxxx;
> chengming.zhou@xxxxxxxxx; usamaarif642@xxxxxxxxx;
> shakeel.butt@xxxxxxxxx; ryan.roberts@xxxxxxx; Huang, Ying
> <ying.huang@xxxxxxxxx>; 21cnbao@xxxxxxxxx; akpm@xxxxxxxxxxxxxxxxxxxx;
> Zou, Nanhai <nanhai.zou@xxxxxxxxx>; Feghali, Wajdi K
> <wajdi.k.feghali@xxxxxxxxx>; Gopal, Vinodh <vinodh.gopal@xxxxxxxxx>
> Subject: Re: [PATCH v7 6/8] mm: zswap: Support mTHP swapout in
> zswap_store().
> 
> On Tue, Sep 24, 2024 at 2:34 PM Yosry Ahmed <yosryahmed@xxxxxxxxxx>
> wrote:
> >
> >
> > Why can't we just handle it the same way as we handle zswap
> > disablement? If it is disabled, we invalidate any old entries for the
> > offsets and return false to swapout to disk.
> 
> I think that was the suggestion.
> 
> >
> > Taking a step back, why do we need the runtime knob and config option?
> > Are there cases where we think zswapout of mTHPs will perform badly,
> > or is it just due to lack of confidence in the feature?
> 
> Fair point. I think the reason why I suggested this knob was because
> we observe so much regressions in earlier benchmarks, and especially
> on the software compressor column.
> 
> But now that we've reworked the benchmark + use zstd for software
> compressor, I think we can get rid of this knob/config option, and
> simplify things.

I agree, thanks Nhat! Will fix this in v8.

Thanks,
Kanchana