Re: [PATCH v3 08/11] mm, swap: only charge physical swap entries

[Johannes Weiner]

On Thu, Aug 06, 2026 at 11:42:51AM -0700, Nhat Pham wrote:
> Charge memcg->swap when a vswap entry acquires physical backing rather
> than when it is allocated, so memory.swap.current tracks on-disk swap
> usage. Zswap-backed and zero-filled pages occupy no swap space but were
> charged as though they did.
> 
> memory.swap.current therefore no longer counts them, and a cgroup whose
> pages all land in zswap can now reclaim anon memory with memory.swap.max
> set to 0.
> 
> Direct-mapped physical swap charging is unchanged.
> 
> Signed-off-by: Nhat Pham <nphamcs@xxxxxxxxx>

To head off any uncertainty about this: this is exactly what needs to
happen in terms of cgroup semantics.

memory.swap.* are about physical swap space. They track, control, and
enforce fairness for a finite resource that is separate from memory.

When a user switches on vswsap and a bunch of empty pages are stored
inside the zeromap without consuming swapfile space, these counters
must be 0.

When a user switches on vswap to use zswap without a backing file,
these counters must be 0.

When a user switches on vswap to use zswap with writeback, only the
pages that get written to the swapfile must be tracked and controlled
by these counters.

A few inline comments on the implementation:

> @@ -5701,6 +5702,116 @@ int __mem_cgroup_try_charge_swap(struct folio *folio)
>  	return 0;
>  }
>  
> +/**
> + * __mem_cgroup_record_swap - record memcg for swap without charging
> + * @folio: folio being added to swap
> + *
> + * Pin the memcg private ID ref and record it in the swap cgroup table
> + * without charging memcg->swap; the charge is deferred to physical-backing
> + * allocation (vswap).
> + */
> +void __mem_cgroup_record_swap(struct folio *folio)
> +{
> +	unsigned int nr_pages = folio_nr_pages(folio);
> +	struct swap_cluster_info *ci;
> +	struct mem_cgroup *memcg;
> +	struct obj_cgroup *objcg;
> +
> +	if (do_memsw_account())
> +		return;
> +
> +	objcg = folio_objcg(folio);
> +	VM_WARN_ON_ONCE_FOLIO(!objcg, folio);
> +	if (!objcg)
> +		return;
> +
> +	rcu_read_lock();
> +	memcg = obj_cgroup_memcg(objcg);
> +	if (!folio_test_swapcache(folio)) {
> +		rcu_read_unlock();
> +		return;
> +	}
> +
> +	memcg = mem_cgroup_private_id_get_online(memcg, nr_pages);
> +	rcu_read_unlock();
> +
> +	ci = swap_cluster_get_and_lock(folio);
> +	__swap_cgroup_set(ci, swp_cluster_offset(folio->swap), nr_pages,
> +			  mem_cgroup_private_id(memcg));
> +	swap_cluster_unlock(ci);
> +}
> +
> +/**
> + * __mem_cgroup_charge_backing_phys_swap - charge memcg->swap
> + * @memcg: the mem_cgroup to charge (may be NULL)
> + * @nr_pages: number of physical swap pages to charge
> + *
> + * Charge the swap counter when a vswap entry gains physical backing. The
> + * private ID ref is already held (pinned by __mem_cgroup_record_swap() at
> + * vswap allocation), so this only moves the counter.
> + *
> + * Return: 0 on success, -ENOMEM on failure.
> + */
> +int __mem_cgroup_charge_backing_phys_swap(struct mem_cgroup *memcg,
> +					  unsigned int nr_pages)
> +{
> +	struct page_counter *counter;
> +
> +	if (do_memsw_account())
> +		return 0;
> +	if (!memcg)
> +		return 0;
> +
> +	if (!mem_cgroup_is_root(memcg) &&
> +	    !page_counter_try_charge(&memcg->swap, nr_pages, &counter)) {
> +		memcg_memory_event(memcg, MEMCG_SWAP_MAX);
> +		memcg_memory_event(memcg, MEMCG_SWAP_FAIL);
> +		return -ENOMEM;
> +	}
> +	mod_memcg_state(memcg, MEMCG_SWAP, nr_pages);
> +	return 0;
> +}

These functions are just __mem_cgroup_try_charge_swap() in two acts :-)

Please refactor this properly:

__mem_cgroup_swap_record()
__mem_cgroup_swap_charge()

> + * __mem_cgroup_uncharge_backing_phys_swap - uncharge memcg->swap counter
> + * @memcg: the mem_cgroup to uncharge (may be NULL)
> + * @nr_pages: number of physical swap pages to uncharge
> + *
> + * Uncharge the swap counter on physical backing release for a vswap entry.
> + * The private ID ref is dropped separately via __mem_cgroup_id_put_swap() when
> + * the vswap entry is freed.
> + */
> +void __mem_cgroup_uncharge_backing_phys_swap(struct mem_cgroup *memcg,
> +					     unsigned int nr_pages)

Same on the uncharge side...

__mem_cgroup_swap_uncharge()

> +{
> +	if (!memcg)
> +		return;
> +
> +	if (!mem_cgroup_is_root(memcg)) {
> +		if (do_memsw_account())
> +			page_counter_uncharge(&memcg->memsw, nr_pages);
> +		else
> +			page_counter_uncharge(&memcg->swap, nr_pages);
> +	}
> +	mod_memcg_state(memcg, MEMCG_SWAP, -nr_pages);
> +}
> +
> +/**
> + * __mem_cgroup_id_put_swap - drop memcg private ID ref without uncharging
> + * @id: cgroup private id
> + * @nr_pages: number of refs to drop
> + */
> +void __mem_cgroup_id_put_swap(unsigned short id, unsigned int nr_pages)
> +{
> +	struct mem_cgroup *memcg;
> +
> +	rcu_read_lock();
> +	memcg = mem_cgroup_from_private_id(id);
> +	if (memcg)
> +		mem_cgroup_private_id_put(memcg, nr_pages);
> +	rcu_read_unlock();
> +}

__mem_cgroup_swap_put()

and then remove __mem_cgroup_uncharge_swap(). Handle this split the
same way as on the charge path.

> @@ -2116,8 +2117,16 @@ int folio_alloc_swap(struct folio *folio)
>  			goto again;
>  	}
>  
> -	/* Need to call this even if allocation failed, for MEMCG_SWAP_FAIL. */
> -	if (unlikely(mem_cgroup_try_charge_swap(folio)))
> +	/*
> +	 * A vswap entry has no physical swap yet, so only record the memcg;
> +	 * folio_realloc_swap() charges once backing is allocated.
> +	 *
> +	 * Need to call this even if allocation failed, for MEMCG_SWAP_FAIL.
> +	 */
> +	if (folio_test_swapcache(folio) &&
> +	    is_vswap_entry(folio->swap))
> +		mem_cgroup_record_swap(folio);
> +	else if (unlikely(mem_cgroup_try_charge_swap(folio)))
>  		swap_cache_del_folio(folio);

This becomes:

	if (!vswap && mem_cgroup_swap_try_charge())
		abort
	mem_cgroup_swap_record()

> @@ -2614,18 +2685,28 @@ void __swap_cluster_free_entries(struct swap_info_struct *si,
>  		/*
>  		 * Uncharge swap slots by memcg in batches. Consecutive
>  		 * slots with the same cgroup id are uncharged together.
> +		 * For vswap, only drop the ID ref - physical swap was
> +		 * already uncharged in __vswap_release_backing above.
>  		 */
>  		id_cur = __swap_cgroup_clear(ci, ci_off, 1);
>  		if (batch_id != id_cur) {
> -			if (batch_id)
> -				mem_cgroup_uncharge_swap(batch_id, ci_off - batch_off);
> +			if (batch_id) {
> +				if (is_vswap)
> +					mem_cgroup_id_put_swap(batch_id, ci_off - batch_off);
> +				else
> +					mem_cgroup_uncharge_swap(batch_id, ci_off - batch_off);
> +			}

And this becomes:

	if (!vswap)
		mem_cgroup_swap_uncharge()
	mem_cgroup_swap_put()