Re: [PATCH 06/11] memcg: register per-area usage counters in cgroupfs

On Fri, Aug 21, 2026 at 07:17:09PM +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] CMA pages and memcg charges will leak and trigger a `bad_page()` BUG if `memory_cma_accounting` is disabled at runtime via remount.
> - [Medium] Potential 32-bit integer overflow when calculating CMA memory usage in `memory_cma_current_show`.
> --
> 
> --- Patch [6]: [PATCH 06/11] memcg: register per-area usage counters in cgroupfs ---
> commit 7a745023362168cdad80f7d03d5d5225a5b34c22
> Author: Eric Chanudet <[email protected]>
> 
> memcg: register per-area usage counters in cgroupfs
> 
> Expose memory.cma.<area>.{current,max} files under each non-root cgroup
> when cgroupfs memory_cma_accounting option is enabled.
> memory.cma.<area>.current reports the cgroup's CMA usage for that area.
> memory.cma.<area>.max is a read-write limit (default: max).
> 
> > diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
> > index 3dfc13f31a0c0..39871a7a08a8d 100644
> > --- a/Documentation/admin-guide/cgroup-v2.rst
> > +++ b/Documentation/admin-guide/cgroup-v2.rst
> 
> [ ... ]
> 
> > @@ -1928,6 +1928,21 @@ The following nested keys are defined.
> >  	This setting has no effect if zswap is disabled, and swapping
> >  	is allowed unless memory.swap.max is set to 0.
> >  
> > +  memory.cma.<area>.current
> > +        A read-only single value file which exists on non-root cgroups. <area>
> > +        is the CMA area name (e.g. "reserved" for the default DMA CMA area).
> > +
> > +        The total amount of CMA memory currently charged to the cgroup for the
> > +        given CMA area. Only accounted when memory_cma_accounting is enabled.
> 
> [Severity: High]
> What happens if memory_cma_accounting is disabled at runtime via remount?
> 
> If an administrator mounts cgroupv2 with memory_cma_accounting enabled,
> workloads allocate CMA memory, and then the administrator remounts without
> the option, CGRP_ROOT_MEMORY_CMA_ACCOUNTING is cleared dynamically in
> kernel/cgroup/cgroup.c:apply_cgroup_root_flags().
> 
> When workloads later free their CMA memory, will mem_cgroup_uncharge_cma()
> return early because memcg_accounts_cma() is false? This would bypass the
> memcg uncharge while folio->memcg_data is still set.
> 
> Could this cause the pages to leak and trigger a bad_page BUG trace when
> they hit the buddy allocator?
> 

Same as Patch 4.

> > +
> > +  memory.cma.<area>.max
> > +        A read-write single value file which exists on non-root cgroups. The
> > +        default is "max".
> 
> [ ... ]
> 
> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > index 8eca00d30fc50..f28d610a59ce7 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> 
> [ ... ]
> 
> > @@ -6182,3 +6182,92 @@ void mem_cgroup_show_protected_memory(struct mem_cgroup *memcg)
> >  		K(atomic_long_read(&memcg->memory.children_min_usage)),
> >  		K(atomic_long_read(&memcg->memory.children_low_usage)));
> >  }
> > +
> > +#ifdef CONFIG_CMA
> > +static int memory_cma_current_show(struct seq_file *m, void *v)
> > +{
> > +	struct mem_cgroup *memcg = mem_cgroup_from_seq(m);
> > +	int idx = seq_cft(m)->private;
> > +	unsigned long cur = page_counter_read(&memcg->cma_counters[idx]);
> > +
> > +	seq_printf(m, "%lu\n", cur * PAGE_SIZE);
> 
> [Severity: Medium]
> Could this calculation overflow on 32-bit architectures?
> 
> Both cur and PAGE_SIZE are 32-bit unsigned longs on 32-bit systems. If the
> CMA area usage exceeds 4GB, the multiplication will wrap around before being
> passed to seq_printf().
> 
> Should this value be cast to u64 before multiplying, similar to how it is
> handled in seq_puts_memcg_tunable()?
> 

Yes, it be u64.

> > +
> > +	return 0;
> > +}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6
> 

-- 
Eric Chanudet