On Wed, Mar 27, 2024 at 9:41 AM Nhat Pham <nphamcs@xxxxxxxxx> wrote: > > On Mon, Mar 25, 2024 at 4:50 PM Yosry Ahmed <yosryahmed@xxxxxxxxxx> wrote: > > > > The current same-filled pages handling supports pages filled with any > > repeated word-sized pattern. However, in practice, most of these should > > be zero pages anyway. Other patterns should be nearly as common. > > It'd be nice if we can verify this somehow. Maybe hooking bpftrace, > trace_printk, etc. here? I am trying to do that. Unfortunately collecting this data from our fleet is not easy, so it will take some time to figure out. If the data happens to be easy-ish to collect from your fleet that would be awesome :) > > That aside, my intuition is that this is correct too. It's much less > likely to see a non-zero filled page. > > > > > Drop the support for non-zero same-filled pages, but keep the names of > > knobs exposed to userspace as "same_filled", which isn't entirely > > inaccurate. > > > > This yields some nice code simplification and enables a following patch > > that eliminates the need to allocate struct zswap_entry for those pages > > completely. > > > > There is also a very small performance improvement observed over 50 runs > > of kernel build test (kernbench) comparing the mean build time on a > > skylake machine when building the kernel in a cgroup v1 container with a > > 3G limit: > > > > base patched % diff > > real 70.167 69.915 -0.359% > > user 2953.068 2956.147 +0.104% > > sys 2612.811 2594.718 -0.692% > > > > This probably comes from more optimized operations like memchr_inv() and > > clear_highpage(). Note that the percentage of zero-filled pages during > > TIL clear_highpage() is a thing :) > > [..] > > The code itself LGTM, FWIW: > > Reviewed-by: Nhat Pham <nphamcs@xxxxxxxxx> Thanks!
