Re: [PATCH 3/8] mm/zsmalloc: Introduce objcgs pointer in struct zpdesc

[Harry Yoo]

On Thu, Feb 26, 2026 at 11:29:26AM -0800, Joshua Hahn wrote:
> Introduce an array of struct obj_cgroup pointers to zpdesc to keep track
> of compressed objects' memcg ownership.
> 
> The 8 bytes required to add the array in struct zpdesc brings its size
> up from 56 bytes to 64 bytes. However, in the current implementation,
> struct zpdesc lays on top of struct page[1]. This allows the increased
> size to remain invisible to the outside, since 64 bytes are used for
> struct zpdesc anyways.
> 
> The newly added obj_cgroup array pointer overlays page->memcg_data,
> which causes problems for functions that try to perform page charging by
> checking the zeroness of page->memcg_data. To make sure that the
> backing zpdesc's obj_cgroup ** is not interpreted as a mem_cgroup *,
> follow SLUB's lead and use the MEMCG_DATA_OBJEXTS bit to tag the pointer.
> 
> Consumers of zsmalloc that do not perform memcg accounting (i.e. zram)
> are completely unaffected by this patch, as the array to track the
> obj_cgroup pointers are only allocated in the zswap path.
> 
> This patch temporarily increases the memory used by zswap by 8 bytes
> per zswap_entry, since the obj_cgroup pointer is duplicated in the
> zpdesc and in zswap_entry. In the following patches, we will redirect
> memory charging operations to use the zpdesc's obj_cgroup instead, and
> remove the pointer from zswap_entry. This will leave no net memory usage
> increase for both zram and zswap.
> 
> In this patch, allocate / free the objcg pointer array for the zswap
> path, and handle partial object migration and full zpdesc migration.
> 
> [1] In the (near) future, struct zpdesc may no longer overlay struct
> page as we shift towards using memdescs. When this happens, the size
> increase of struct zpdesc will no longer free. With that said, the
> difference can be kept minimal.
> 
> All the changes that are being implemented are currently guarded under
> CONFIG_MEMCG. We can optionally minimize the impact on zram users by
> guarding these changes in CONFIG_MEMCG && CONFIG_ZSWAP as well.
> 
> Suggested-by: Johannes Weiner <hannes@xxxxxxxxxxx>
> Signed-off-by: Joshua Hahn <joshua.hahnjy@xxxxxxxxx>
> ---
>  drivers/block/zram/zram_drv.c | 10 ++---
>  include/linux/zsmalloc.h      |  2 +-
>  mm/zpdesc.h                   | 25 +++++++++++-
>  mm/zsmalloc.c                 | 74 +++++++++++++++++++++++++++++------
>  mm/zswap.c                    |  2 +-
>  5 files changed, 93 insertions(+), 20 deletions(-)
> 
> @@ -893,6 +898,43 @@ static void init_zspage(struct size_class *class, struct zspage *zspage)
>  	set_freeobj(zspage, 0);
>  }
>  
> +#ifdef CONFIG_MEMCG
> +static bool alloc_zspage_objcgs(struct size_class *class, gfp_t gfp,
> +				struct zpdesc *zpdescs[])
> +{
> +	/*
> +	 * Add 2 to objcgs_per_zpdesc to account for partial objs that may be
> +	 * stored at the beginning or end of the zpdesc.
> +	 */
> +	int objcgs_per_zpdesc = (PAGE_SIZE / class->size) + 2;
> +	int i;
> +	struct obj_cgroup **objcgs;

Just wondering, perhaps it makes more sense to have an array of
objcg pointers for each zspage (of size objs_per_zspage)?

> +
> +	for (i = 0; i < class->pages_per_zspage; i++) {
> +		objcgs = kcalloc(objcgs_per_zpdesc, sizeof(struct obj_cgroup *),
> +				 gfp & ~__GFP_HIGHMEM);
> +		if (!objcgs) {
> +			while (--i >= 0) {
> +				kfree(zpdesc_objcgs(zpdescs[i]));
> +				zpdescs[i]->objcgs = 0;
> +			}
> +
> +			return false;
> +		}
> +
> +		zpdesc_set_objcgs(zpdescs[i], objcgs);
> +	}
> +
> +	return true;
> +}

-- 
Cheers,
Harry / Hyeonggon