Re: [PATCH 07/11] mm/zsmalloc, zswap: Handle objcg charging and lifetime in zsmalloc

Johannes Weiner
2026-03-12T21:42:11Z

On Wed, Mar 11, 2026 at 12:51:44PM -0700, Joshua Hahn wrote:
> @@ -1244,6 +1297,8 @@ void zs_obj_write(struct zs_pool *pool, unsigned long handle,
> if (objcg) {
> WARN_ON_ONCE(!pool->memcg_aware);
> zspage->objcgs[obj_idx] = objcg;
> + obj_cgroup_get(objcg);
> + zs_charge_objcg(pool, objcg, class->size);
> }
>
> if (!ZsHugePage(zspage))

Note that the obj_cgroup_get() reference is for the pointer, not the
charge. I think it all comes out right in the end, but it's a bit
confusing to follow and verify through the series.

IOW, it's better move that obj_cgroup_get() when you add and store
zspage->objcgs[]. If zswap stil has a reference at that point in the
series, then it's fine for there to be two separate obj_cgroup_get()
as well, with later patches deleting the zswap one when its
entry->objcg pointer disappears.

> @@ -711,10 +711,6 @@ static void zswap_entry_free(struct zswap_entry *entry)
> zswap_lru_del(&zswap_list_lru, entry, objcg);
> zs_free(entry->pool->zs_pool, entry->handle);
> zswap_pool_put(entry->pool);
> - if (objcg) {
> - obj_cgroup_uncharge_zswap(objcg, entry->length);
> - obj_cgroup_put(objcg);
> - }
> if (entry->length == PAGE_SIZE)
> atomic_long_dec(&zswap_stored_incompressible_pages);
> zswap_entry_cache_free(entry);

[ I can see that this was misleading. It was really getting a
reference for the entry->objcg = objcg a few lines down, hitching a
ride on that existing `if (objcg)`. ]