Messages in this thread | | | From | Nhat Pham <> | Date | Thu, 17 Aug 2023 15:53:24 -0700 | Subject | Re: [PATCH v3] mm: zswap: multiple zpools support |
| |
On Fri, Aug 11, 2023 at 4:21 PM Yosry Ahmed <yosryahmed@google.com> wrote: > > On Fri, Aug 11, 2023 at 2:19 PM Andrew Morton <akpm@linux-foundation.org> wrote: > > > > On Thu, 13 Jul 2023 03:35:25 -0700 Yosry Ahmed <yosryahmed@google.com> 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@gmail.com> Tested-by: Nhat Pham <nphamcs@gmail.com>
> > I obviously want it to be merged, but hopefully someone will chime in here :)
| |