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

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

[Joshua Hahn]

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@gmail.com>
---
 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)
 		return;
 	chunk->obj_exts[off >> PCPU_MIN_ALLOC_SHIFT].cgroup = NULL;
 
-	obj_cgroup_uncharge(objcg, pcpu_obj_full_size(size));
+	obj_cgroup_uncharge(objcg, size);
 
 	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();
 
 	obj_cgroup_put(objcg);
@@ -1696,15 +1694,14 @@ static void pcpu_alloc_tag_alloc_hook(struct pcpu_chunk *chunk, int off,
 {
 	if (mem_alloc_profiling_enabled() && likely(chunk->obj_exts)) {
 		alloc_tag_add(&chunk->obj_exts[off >> PCPU_MIN_ALLOC_SHIFT].tag,
-			      current->alloc_tag, pcpu_obj_full_size(size));
+			      current->alloc_tag, size);
 	}
 }
 
 static void pcpu_alloc_tag_free_hook(struct pcpu_chunk *chunk, int off, size_t size)
 {
 	if (mem_alloc_profiling_enabled() && likely(chunk->obj_exts))
-		alloc_tag_sub(&chunk->obj_exts[off >> PCPU_MIN_ALLOC_SHIFT].tag,
-			      pcpu_obj_full_size(size));
+		alloc_tag_sub(&chunk->obj_exts[off >> PCPU_MIN_ALLOC_SHIFT].tag, size);
 }
 #else
 static void pcpu_alloc_tag_alloc_hook(struct pcpu_chunk *chunk, int off,
@@ -1744,7 +1741,7 @@ void __percpu *pcpu_alloc_noprof(size_t size, size_t align, bool reserved,
 	int slot, off, cpu, ret;
 	unsigned long flags;
 	void __percpu *ptr;
-	size_t bits, bit_align;
+	size_t bits, bit_align, full_size;
 
 	gfp = current_gfp_context(gfp);
 	/*
@@ -1773,6 +1770,7 @@ void __percpu *pcpu_alloc_noprof(size_t size, size_t align, bool reserved,
 	size = ALIGN(size, PCPU_MIN_ALLOC_SIZE);
 	bits = size >> PCPU_MIN_ALLOC_SHIFT;
 	bit_align = align >> PCPU_MIN_ALLOC_SHIFT;
+	full_size = pcpu_obj_full_size(size);
 
 	if (unlikely(!size || size > PCPU_MIN_UNIT_SIZE || align > PAGE_SIZE ||
 		     !is_power_of_2(align))) {
@@ -1781,7 +1779,7 @@ void __percpu *pcpu_alloc_noprof(size_t size, size_t align, bool reserved,
 		return NULL;
 	}
 
-	if (unlikely(!pcpu_memcg_pre_alloc_hook(size, gfp, &objcg)))
+	if (unlikely(!pcpu_memcg_pre_alloc_hook(full_size, gfp, &objcg)))
 		return NULL;
 
 	if (!is_atomic) {
@@ -1793,7 +1791,7 @@ void __percpu *pcpu_alloc_noprof(size_t size, size_t align, bool reserved,
 		if (gfp & __GFP_NOFAIL) {
 			mutex_lock(&pcpu_alloc_mutex);
 		} else if (mutex_lock_killable(&pcpu_alloc_mutex)) {
-			pcpu_memcg_post_alloc_hook(objcg, NULL, 0, size);
+			pcpu_memcg_post_alloc_hook(objcg, NULL, 0, full_size);
 			return NULL;
 		}
 	}
@@ -1903,12 +1901,11 @@ void __percpu *pcpu_alloc_noprof(size_t size, size_t align, bool reserved,
 	kmemleak_alloc_percpu(ptr, size, gfp);
 
 	trace_percpu_alloc_percpu(_RET_IP_, reserved, is_atomic, size, align,
-				  chunk->base_addr, off, ptr,
-				  pcpu_obj_full_size(size), gfp);
+				  chunk->base_addr, off, ptr, full_size, gfp);
 
-	pcpu_memcg_post_alloc_hook(objcg, chunk, off, size);
+	pcpu_memcg_post_alloc_hook(objcg, chunk, off, full_size);
 
-	pcpu_alloc_tag_alloc_hook(chunk, off, size);
+	pcpu_alloc_tag_alloc_hook(chunk, off, full_size);
 
 	return ptr;
 
@@ -1938,7 +1935,7 @@ void __percpu *pcpu_alloc_noprof(size_t size, size_t align, bool reserved,
 		mutex_unlock(&pcpu_alloc_mutex);
 	}
 
-	pcpu_memcg_post_alloc_hook(objcg, NULL, 0, size);
+	pcpu_memcg_post_alloc_hook(objcg, NULL, 0, full_size);
 
 	return NULL;
 }
@@ -2242,6 +2239,7 @@ void free_percpu(void __percpu *ptr)
 	struct pcpu_chunk *chunk;
 	unsigned long flags;
 	int size, off;
+	size_t full_size;
 	bool need_balance = false;
 
 	if (!ptr)
@@ -2262,10 +2260,11 @@ void free_percpu(void __percpu *ptr)
 		WARN_ON_ONCE(1);
 		return;
 	}
+	full_size = pcpu_obj_full_size(size);
 
-	pcpu_alloc_tag_free_hook(chunk, off, size);
+	pcpu_alloc_tag_free_hook(chunk, off, full_size);
 
-	pcpu_memcg_free_hook(chunk, off, size);
+	pcpu_memcg_free_hook(chunk, off, full_size);
 
 	/*
 	 * If there are more than one fully free chunks, wake up grim reaper.

base-commit: 2887e63d2e8e0cf55402612d8996ce955a2c3e55
-- 
2.53.0-Meta

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

[Joshua Hahn]

On Mon, 19 Oct 2026 09:28:02 -0700 Joshua Hahn <joshua.hahnjy@gmail.com> 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@gmail.com>

This is a follow-up to Harry's fix [1] and is intended for the next
cycle. Sorry that I did not make this clear in the original patch!

[1] https://lore.kernel.org/linux-mm/20261013-fix-alloc-percpu-accounting-v1-1-03e788a1e41f@kernel.org/

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

[Harry Yoo]

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@gmail.com>
> ---
>  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?

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.

Wdyt?

-- 
Cheers,
Harry / Hyeonggon

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@kernel.org> 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@gmail.com>
> > ---
> >  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

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@kernel.org> 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@gmail.com>
> > > ---
> > > @@ -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