Re: [PATCH 1/3] mm: page_counter: add page_counter_protection struct and init API

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing fixup of `prot.parent` linkage when dynamically creating dmem cgroup pools breaks hierarchical protection.
--

commit 534b5f88933b6e08b308d664bd9c8b28861c0c8c
Author: linuszeng <linuszeng@tencent.com>

mm: page_counter: add page_counter_protection struct and init API

This commit extracts the hierarchical protection state (memory.min and
memory.low) from struct page_counter into a new page_counter_protection
structure. It introduces page_counter_init_protection() to attach this
context, saving space for counters that don't support protection.

> diff --git a/kernel/cgroup/dmem.c b/kernel/cgroup/dmem.c
> index 4683f3d680226..e33d807cdea89 100644
> --- a/kernel/cgroup/dmem.c
> +++ b/kernel/cgroup/dmem.c
> @@ -426,8 +427,9 @@ alloc_pool_single(struct dmemcg_state *dmemcs, struct dmem_cgroup_region *region
>  	if (parent)
>  		ppool = find_cg_pool_locked(parent, region);
>  
> -	page_counter_init(&pool->cnt,
> -			  ppool ? &ppool->cnt : NULL, true);
> +	page_counter_init(&pool->cnt, ppool ? &ppool->cnt : NULL);
> +	page_counter_init_protection(&pool->cnt, &pool->prot,
> +				     ppool ? &ppool->prot : NULL);
>  	reset_all_resource_limits(pool);
>  	refcount_set(&pool->ref, 1);
>  	kref_get(&region->ref);

[Severity: High]
Does get_cg_pool_locked() also need to be updated to fix up the prot.parent
linkage?

When alloc_pool_single() is called during a bottom-up dynamic pool creation,
ppool can be NULL. This sets pool->prot.parent to NULL here. Later in
get_cg_pool_locked(), the missing parent pools are created and a fixup loop
links the counters:

kernel/cgroup/dmem.c:get_cg_pool_locked() {
    ...
		/* ppool was created if it didn't exist by above loop. */
		ppool = find_cg_pool_locked(pp, region);

		/* Fix up parent links, mark as inited. */
		pool->cnt.parent = &ppool->cnt;
		if (ppool && !pool->parent) {
    ...
}

Because pool->prot.parent = &ppool->prot; is missing in this fixup loop,
prot.parent permanently remains NULL. This causes propagate_protected_usage()
to silently abort due to !prot->parent, breaking hierarchical protection
for the dmem cgroup.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909-descriptive-name-v1-0-1828961cb01a@tencent.com?part=1