On Thu, Nov 7, 2024 at 4:06 AM Johannes Weiner <hannes@xxxxxxxxxxx> wrote:
>
> On Wed, Nov 06, 2024 at 10:19:34AM +1300, Barry Song wrote:
> > From: Barry Song <v-songbaohua@xxxxxxxx>
> >
> > When the proportion of folios from the zeromap is small, missing their
> > accounting may not significantly impact profiling. However, it’s easy
> > to construct a scenario where this becomes an issue—for example,
> > allocating 1 GB of memory, writing zeros from userspace, followed by
> > MADV_PAGEOUT, and then swapping it back in. In this case, the swap-out
> > and swap-in counts seem to vanish into a black hole, potentially
> > causing semantic ambiguity.
> >
> > On the other hand, Usama reported that zero-filled pages can exceed 10% in
> > workloads utilizing zswap, while Hailong noted that some app in Android
> > have more than 6% zero-filled pages. Before commit 0ca0c24e3211 ("mm: store
> > zero pages to be swapped out in a bitmap"), both zswap and zRAM implemented
> > similar optimizations, leading to these optimized-out pages being counted
> > in either zswap or zRAM counters (with pswpin/pswpout also increasing for
> > zRAM). With zeromap functioning prior to both zswap and zRAM, userspace
> > will no longer detect these swap-out and swap-in actions.
> >
> > We have three ways to address this:
> >
> > 1. Introduce a dedicated counter specifically for the zeromap.
> > 2. Use pswpin/pswpout accounting, treating the zero map as a standard
> > backend. This approach aligns with zRAM's current handling of
> > same-page fills at the device level. However, it would mean losing
> > the optimized-out page counters previously available in zRAM and
> > would not align with systems using zswap. Additionally, as noted by
> > Nhat Pham, pswpin/pswpout counters apply only to I/O done directly
> > to the backend device.
> > 3. Count zeromap pages under zswap, aligning with system behavior when
> > zswap is enabled. However, this would not be consistent with zRAM,
> > nor would it align with systems lacking both zswap and zRAM.
> >
> > Given the complications with options 2 and 3, this patch selects
> > option 1.
> >
> > We can find these counters from /proc/vmstat (counters for the whole
> > system) and memcg's memory.stat (counters for the interested memcg).
> >
> > For example:
> >
> > $ grep -E 'swpin_zero|swpout_zero' /proc/vmstat
> > swpin_zero 1648
> > swpout_zero 33536
> >
> > $ grep -E 'swpin_zero|swpout_zero' /sys/fs/cgroup/system.slice/memory.stat
> > swpin_zero 3905
> > swpout_zero 3985
> >
> > This patch does not address any specific zeromap bug, but the missing
> > swpout and swpin counts for zero-filled pages can be highly confusing
> > and may mislead user-space agents that rely on changes in these counters
> > as indicators. Therefore, we add a Fixes tag to encourage the inclusion
> > of this counter in any kernel versions with zeromap.
> >
> > Fixes: 0ca0c24e3211 ("mm: store zero pages to be swapped out in a bitmap")
> > Reviewed-by: Nhat Pham <nphamcs@xxxxxxxxx>
> > Cc: Usama Arif <usamaarif642@xxxxxxxxx>
> > Cc: Chengming Zhou <chengming.zhou@xxxxxxxxx>
> > Cc: Yosry Ahmed <yosryahmed@xxxxxxxxxx>
> > Cc: Hailong Liu <hailong.liu@xxxxxxxx>
> > Cc: Johannes Weiner <hannes@xxxxxxxxxxx>
> > Cc: David Hildenbrand <david@xxxxxxxxxx>
> > Cc: Hugh Dickins <hughd@xxxxxxxxxx>
> > Cc: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx>
> > Cc: Shakeel Butt <shakeel.butt@xxxxxxxxx>
> > Cc: Andi Kleen <ak@xxxxxxxxxxxxxxx>
> > Cc: Baolin Wang <baolin.wang@xxxxxxxxxxxxxxxxx>
> > Cc: Chris Li <chrisl@xxxxxxxxxx>
> > Cc: "Huang, Ying" <ying.huang@xxxxxxxxx>
> > Cc: Kairui Song <kasong@xxxxxxxxxxx>
> > Cc: Ryan Roberts <ryan.roberts@xxxxxxx>
> > Signed-off-by: Barry Song <v-songbaohua@xxxxxxxx>
>
> Acked-by: Johannes Weiner <hannes@xxxxxxxxxxx>
Thanks!
Oops, it seems that it depends on Kanchana's 'mm: change count_objcg_event() to
count_objcg_events() for batch event updates,' which also isn't present in 6.12.
Otherwise, it won't build, as reported here:
https://lore.kernel.org/linux-mm/CAGsJ_4whD31+Lk0m2uq-o=ygvkRsw1uXcPeqxBONV-RUXkeEzg@xxxxxxxxxxxxxx/
Hi Andrew,
What’s the best approach here? Should we include Kanchana's patch that extends
the nr argument for count_objcg_events() in 6.12-rc as well?
Thanks
barry
