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

[Harry Yoo]

On Mon, Oct 19, 2026 at 11:16:27AM -0700, Joshua Hahn wrote:
> 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>
> > > ---
> > > @@ -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.

Thanks!

> > 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.

Ah, right.
 
> 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?

Yeah.

> ( 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...

(looks at fun percpu memory allocator history)

commit df95e795a722 ("percpu: add tracepoint support for percpu memory")
says the tracepoint is for recording & replaying percpu memory
allocation requests re-evaluating the decisions.

I guess for 'replaying' purposes we'd need 'size' rather than
'bytes_alloc'.

> it may be too late to change the meaning of the field )

Agreed.

> wdyt?
 
I guess we can revisit this when it actually bothers us or
somebody then :P

> Thanks again Harry, have an awesome day : -D

No problem, and you too!

-- 
Cheers,
Harry / Hyeonggon