linux-kernel - Re: [PATCH v3] mm: count zeromap read and set for swapout and swapin

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAGsJ_4zYiRzG6mBnW-2wh7YCo_PJQc7u1syd05DNdic7MaE7Zw@mail.gmail.com>
Date: Thu, 7 Nov 2024 09:01:14 +1300
From: Barry Song <21cnbao@...il.com>
To: Johannes Weiner <hannes@...xchg.org>
Cc: akpm@...ux-foundation.org, linux-mm@...ck.org, 
	linux-kernel@...r.kernel.org, Barry Song <v-songbaohua@...o.com>, 
	Nhat Pham <nphamcs@...il.com>, Usama Arif <usamaarif642@...il.com>, 
	Chengming Zhou <chengming.zhou@...ux.dev>, Yosry Ahmed <yosryahmed@...gle.com>, 
	Hailong Liu <hailong.liu@...o.com>, David Hildenbrand <david@...hat.com>, Hugh Dickins <hughd@...gle.com>, 
	Matthew Wilcox <willy@...radead.org>, Shakeel Butt <shakeel.butt@...ux.dev>, 
	Andi Kleen <ak@...ux.intel.com>, Baolin Wang <baolin.wang@...ux.alibaba.com>, 
	Chris Li <chrisl@...nel.org>, "Huang, Ying" <ying.huang@...el.com>, 
	Kairui Song <kasong@...cent.com>, Ryan Roberts <ryan.roberts@....com>
Subject: Re: [PATCH v3] mm: count zeromap read and set for swapout and swapin

On Thu, Nov 7, 2024 at 4:06 AM Johannes Weiner <hannes@...xchg.org> wrote:
>
> On Wed, Nov 06, 2024 at 10:19:34AM +1300, Barry Song wrote:
> > From: Barry Song <v-songbaohua@...o.com>
> >
> > 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@...il.com>
> > Cc: Usama Arif <usamaarif642@...il.com>
> > Cc: Chengming Zhou <chengming.zhou@...ux.dev>
> > Cc: Yosry Ahmed <yosryahmed@...gle.com>
> > Cc: Hailong Liu <hailong.liu@...o.com>
> > Cc: Johannes Weiner <hannes@...xchg.org>
> > Cc: David Hildenbrand <david@...hat.com>
> > Cc: Hugh Dickins <hughd@...gle.com>
> > Cc: Matthew Wilcox (Oracle) <willy@...radead.org>
> > Cc: Shakeel Butt <shakeel.butt@...ux.dev>
> > Cc: Andi Kleen <ak@...ux.intel.com>
> > Cc: Baolin Wang <baolin.wang@...ux.alibaba.com>
> > Cc: Chris Li <chrisl@...nel.org>
> > Cc: "Huang, Ying" <ying.huang@...el.com>
> > Cc: Kairui Song <kasong@...cent.com>
> > Cc: Ryan Roberts <ryan.roberts@....com>
> > Signed-off-by: Barry Song <v-songbaohua@...o.com>
>
> Acked-by: Johannes Weiner <hannes@...xchg.org>

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@mail.gmail.com/

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