On Fri, Aug 11, 2023 at 4:21 PM Yosry Ahmed <yosryahmed@xxxxxxxxxx> wrote: > > On Fri, Aug 11, 2023 at 2:19 PM Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote: > > > > On Thu, 13 Jul 2023 03:35:25 -0700 Yosry Ahmed <yosryahmed@xxxxxxxxxx> wrote: > > > > > > > > > > I'm experimenting with some other zswap changes - if I have > > > > extra cycles and resources I'll try to apply this patch and see how the > > > > numbers play out. > > > > > > That would be amazing. Looking forward to any numbers you can dig :) > > > > So this patch seems stuck. I can keep it in mm.git until the fog > > clears, but would prefer not to. Can we please revisit and decide on a > > way forward? > > Johannes did not like a config option so I proposed it here as a > constant (like SWAP_CLUSTER_MAX and others we have). This is a value > that we have been using in our data centers for almost a decade, so it > has seen a ton of testing. I was hoping Johannes would get time to > take a look, or Nhat would get time to test it out, but neither of > these things happen. Apologies - finally have some time + freed experiment machine cycles to put in your patch :P And gotta wait a couple of days to obtain sufficient data. Result is quite unexciting - no tremendous gains or significant regression in a bunch of internal metrics I was observing. Of course, it's just one particular workload that I tested on - there could be regression/gains in other workloads (or other metrics). But we can always revisit this when it happens :) With all that said, the code itself looks solid. And while I'm still not in love with the change, I don't have any further objections, as of now. I'll let you (and Johannes) continue from here. Reviewed-by: Nhat Pham <nphamcs@xxxxxxxxx> Tested-by: Nhat Pham <nphamcs@xxxxxxxxx> > > I obviously want it to be merged, but hopefully someone will chime in here :)