Re: [PATCH] mm/percpu: clean up callers of pcpu_obj_full_size()

[Joshua Hahn]

On Mon, 19 Oct 2026 18:49:26 +0100 Harry Yoo <harry@xxxxxxxxxx> wrote:

> On Mon, Oct 19, 2026 at 09:28:02AM -0700, Joshua Hahn wrote:
> > The size of a pcpu allocation can be interpreted in two ways:
> > 1. The size of the allocation attributable to a single CPU.
> > 2. The total size of the allocation across all CPUs, including the
> >    allocation overhead (for obj_cgroup membership).
> > 
> > In most accounting paths for percpu memory, we need the second
> > definition (total size). Make this more obvious by coalescing the
> > callers of pcpu_obj_full_size().
> > 
> > No functional changes intended.
> > 
> > Signed-off-by: Joshua Hahn <joshua.hahnjy@xxxxxxxxx>
> > ---
> >  mm/percpu.c | 39 +++++++++++++++++++--------------------
> >  1 file changed, 19 insertions(+), 20 deletions(-)
> > 
> > diff --git a/mm/percpu.c b/mm/percpu.c
> > index d7883507dc151..76848e096caee 100644
> > --- a/mm/percpu.c
> > +++ b/mm/percpu.c
> > @@ -1623,7 +1623,7 @@ static bool pcpu_memcg_pre_alloc_hook(size_t size, gfp_t gfp,
> >  	if (!objcg || obj_cgroup_is_root(objcg))
> >  		return true;
> >  
> > -	if (obj_cgroup_charge(objcg, gfp, pcpu_obj_full_size(size)))
> > +	if (obj_cgroup_charge(objcg, gfp, size))
> >  		return false;
> >  
> >  	*objcgp = objcg;
> > @@ -1642,11 +1642,10 @@ static void pcpu_memcg_post_alloc_hook(struct obj_cgroup *objcg,
> >  		chunk->obj_exts[off >> PCPU_MIN_ALLOC_SHIFT].cgroup = objcg;
> >  
> >  		rcu_read_lock();
> > -		mod_memcg_state(obj_cgroup_memcg(objcg), MEMCG_PERCPU_B,
> > -				pcpu_obj_full_size(size));
> > +		mod_memcg_state(obj_cgroup_memcg(objcg), MEMCG_PERCPU_B, size);
> >  		rcu_read_unlock();
> >  	} else {
> > -		obj_cgroup_uncharge(objcg, pcpu_obj_full_size(size));
> > +		obj_cgroup_uncharge(objcg, size);
> >  	}
> >  }
> >  
> > @@ -1662,11 +1661,10 @@ static void pcpu_memcg_free_hook(struct pcpu_chunk *chunk, int off, size_t size)
> 
> Perhaps it's better to rename 'size' parameter to 'full_size' in those
> memcg hooks to make it clear it's expecting total size?

Hi Harry, thanks for the review! That sounds very reasonable to me.
I'll spin up a v2 to make this change.

> Otherwise LGTM!
> Thanks for cleaning this up.
> 
> And it might be beyond the scope of this patch, but I think
> trace_percpu_alloc_percpu_fail() should take full_size instead of size.

Hmm, I'm not sure. The trace event for percpu_alloc_percpu actually
tracks both size and bytes_alloc separately, but percpu_alloc_percpu_fail
only tracks the "size" paramter. I think it might be a bit confusing to
have the percpu_alloc_percpu_{, fail} have diferent meanings for the
"size" parameter.

Perhaps the right approach here is to add a new field "bytes_alloc"
to trace_percpu_alloc_percpu_fail so that we get full parity with
trace_percpu_alloc_percpu?

( Actually I think we don't even really want size, what we really care
  about to me it would seem is the size of the allocation request that
  failed but alas... it may be too late to change the meaning of the
  field )

wdyt?

Thanks again Harry, have an awesome day : -D

> Wdyt?
> 
> -- 
> Cheers,
> Harry / Hyeonggon