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: <3179fa38027bdacdd38b4ef34b493bdb5ef7a19a@linux.dev>
Date: Fri, 06 Feb 2026 02:53:52 +0000
From: "Jiayuan Chen" <jiayuan.chen@...ux.dev>
To: "Yosry Ahmed" <yosry.ahmed@...ux.dev>, "SeongJae Park" <sj@...nel.org>
Cc: "SeongJae Park" <sj@...nel.org>, linux-mm@...ck.org, "Jiayuan Chen"
 <jiayuan.chen@...pee.com>, "Johannes Weiner" <hannes@...xchg.org>,
 "Michal Hocko" <mhocko@...nel.org>, "Roman Gushchin"
 <roman.gushchin@...ux.dev>, "Shakeel Butt" <shakeel.butt@...ux.dev>,
 "Muchun Song" <muchun.song@...ux.dev>, "Nhat Pham" <nphamcs@...il.com>,
 "Chengming Zhou" <chengming.zhou@...ux.dev>, "Andrew Morton"
 <akpm@...ux-foundation.org>, "Nick Terrell" <terrelln@...com>, "David 
 Sterba" <dsterba@...e.com>, cgroups@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1] mm: zswap: add per-memcg stat for incompressible
 pages

February 6, 2026 at 10:33, "Yosry Ahmed" <yosry.ahmed@...ux.dev mailto:yosry.ahmed@...ux.dev?to=%22Yosry%20Ahmed%22%20%3Cyosry.ahmed%40linux.dev%3E > wrote:


> 
> > 
> > On Thu, 5 Feb 2026 13:30:12 +0800 Jiayuan Chen <> wrote:
> >  
> >  
> >  From: Jiayuan Chen <jiayuan.chen@...pee.com>
> >  
> >  The global zswap_stored_incompressible_pages counter was added in commit
> >  dca4437a5861 ("mm/zswap: store <PAGE_SIZE compression failed page as-is")
> >  to track how many pages are stored in raw (uncompressed) form in zswap.
> >  However, in containerized environments, knowing which cgroup is
> >  contributing incompressible pages is essential for effective resource
> >  management.
> >  
> >  Add a new memcg stat 'zswpraw' to track incompressible pages per cgroup.
> >  This helps administrators and orchestrators to:
> >  
> >  1. Identify workloads that produce incompressible data (e.g., encrypted
> >  data, already-compressed media, random data) and may not benefit from
> >  zswap.
> >  
> >  2. Make informed decisions about workload placement - moving
> >  incompressible workloads to nodes with larger swap backing devices
> >  rather than relying on zswap.
> >  
> >  3. Debug zswap efficiency issues at the cgroup level without needing to
> >  correlate global stats with individual cgroups.
> >  
> >  While the compression ratio can be estimated from existing stats
> >  (zswap / zswapped * PAGE_SIZE), this doesn't distinguish between
> >  "uniformly poor compression" and "a few completely incompressible pages
> >  mixed with highly compressible ones". The zswpraw stat provides direct
> >  visibility into the latter case.
> >  
> >  Changes
> >  -------
> >  
> >  1. Add zswap_is_raw() helper (include/linux/zswap.h)
> >  - Abstract the PAGE_SIZE comparison logic for identifying raw entries
> >  - Keep the incompressible check in one place for maintainability
> >  
> >  2. Add MEMCG_ZSWAP_RAW stat definition (include/linux/memcontrol.h,
> >  mm/memcontrol.c)
> >  - Add MEMCG_ZSWAP_RAW to memcg_stat_item enum
> >  - Register in memcg_stat_items[] and memory_stats[] arrays
> >  - Export as "zswpraw" in memory.stat
> >  
> >  3. Update statistics accounting (mm/memcontrol.c, mm/zswap.c)
> >  - Track MEMCG_ZSWAP_RAW in obj_cgroup_charge/uncharge_zswap()
> >  - Use zswap_is_raw() helper in zswap.c for consistency
> >  
> >  Test
> >  ----
> >  
> >  I wrote a simple test program[1] that allocates memory and compresses it
> >  with zstd, so kernel zswap cannot compress further.
> >  
> >  $ cgcreate -g memory:test
> >  $ cgexec -g memory:test ./test_zswpraw &
> >  $ cat /sys/fs/cgroup/test/memory.stat | grep zswp
> >  zswpraw 0
> >  zswpin 0
> >  zswpout 0
> >  zswpwb 0
> >  
> >  $ echo "100M" > /sys/fs/cgroup/test/memory.reclaim
> >  $ cat /sys/fs/cgroup/test/memory.stat | grep zswp
> >  zswpraw 104800256
> >  zswpin 0
> >  zswpout 51222
> >  zswpwb 0
> >  
> >  $ pkill test_zswpraw
> >  $ cat /sys/fs/cgroup/test/memory.stat | grep zswp
> >  zswpraw 0
> >  zswpin 1
> >  zswpout 51222
> >  zswpwb 0
> >  
> >  [1] https://gist.github.com/mrpre/00432c6154250326994fbeaf62e0e6f1
> >  
> >  Signed-off-by: Jiayuan Chen <jiayuan.chen@...pee.com>
> >  ---
> >  include/linux/memcontrol.h | 1 +
> >  include/linux/zswap.h | 9 +++++++++
> >  mm/memcontrol.c | 6 ++++++
> >  mm/zswap.c | 6 +++---
> >  4 files changed, 19 insertions(+), 3 deletions(-)
> >  
> >  As others also mentioned, the documentation of the new stat would be needed.
> >  
> >  
> >  diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> >  index b6c82c8f73e1..83d1328f81d1 100644
> >  --- a/include/linux/memcontrol.h
> >  +++ b/include/linux/memcontrol.h
> >  @@ -39,6 +39,7 @@ enum memcg_stat_item {
> >  MEMCG_KMEM,
> >  MEMCG_ZSWAP_B,
> >  MEMCG_ZSWAPPED,
> >  + MEMCG_ZSWAP_RAW,
> >  MEMCG_NR_STAT,
> >  };
> >  
> >  diff --git a/include/linux/zswap.h b/include/linux/zswap.h
> >  index 30c193a1207e..94f84b154b71 100644
> >  --- a/include/linux/zswap.h
> >  +++ b/include/linux/zswap.h
> >  @@ -7,6 +7,15 @@
> >  
> >  struct lruvec;
> >  
> >  +/*
> >  + * Check if a zswap entry is stored in raw (uncompressed) form.
> >  + * This happens when compression doesn't reduce the size.
> >  + */
> >  +static inline bool zswap_is_raw(size_t size)
> >  +{
> >  + return size == PAGE_SIZE;
> >  +}
> >  +
> >  
> >  No strong opinion, but I'm not really sure if the helper is needed, because it
> >  feels quite simple logic:
> >  
> >  "If an object is compressed and the size is same to the original one, the
> >  object is incompressible."
> >  
> >  I also feel the function name bit odd, given the type of the parameter. Based
> >  on the function name and the comment, I'd expect it to receive a zswap_entry
> >  object. I understand it is better to receive a size_t, to be called from
> >  obj_cgroup_[un]charge_zswap(), though. Even in the case, I think the name can
> >  be better (e.g., zswap_compression_failed() or zswap_was_incompressible() ?),
> >  or at least the coment can be more kindly explain the fact that the parameter
> >  is the size of object after the compression attempt.
> > 
> I vote to drop the helper.
>

The reason I introduced the helper is that the incompressible check now lives in two places:

In zswap.c - for the global zswap_stored_incompressible_pages counter
In memcontrol.c - for the per-memcg MEMCG_ZSWAP_INCOMP stat

By extracting a shared helper, both modules use the same logic, which helps with maintainability.

That said, I'm fine with dropping the helper if preferred. I can add a comment in memcontrol.c
explaining the logic. My only concern is that if the incompressible detection logic in zswap
ever changes, someone might forget to update the memcg accounting accordingly.

But perhaps that's an unlikely scenario.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ