[PATCH] mm/memcontrol: hoist pstatc_pcpu assignment out of CPU loop

Hui Zhu posted 1 patch 1 month, 2 weeks ago
mm/memcontrol.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
[PATCH] mm/memcontrol: hoist pstatc_pcpu assignment out of CPU loop
Posted by Hui Zhu 1 month, 2 weeks ago
From: Hui Zhu <zhuhui@kylinos.cn>

In mem_cgroup_alloc(), the assignment of pstatc_pcpu is invariant
with respect to the for_each_possible_cpu() loop: both the 'parent'
pointer and 'parent->vmstats_percpu' remain constant throughout all
iterations.

The original code redundantly re-evaluated the 'if (parent)'
condition and reassigned pstatc_pcpu on every CPU iteration, then
repeated the same ternary check 'parent ? pstatc_pcpu : NULL' when
storing into statc->parent_pcpu.

Move the single conditional assignment of pstatc_pcpu to before the
loop, resolving both the loop-invariant placement issue and the
duplicated null check. On systems with a large number of possible
CPUs, this eliminates repeated branch evaluation with no functional
change.

No functional change intended.

Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>
---
 mm/memcontrol.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index c3d98ab41f1f..4f4a60e57a08 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3993,11 +3993,10 @@ static struct mem_cgroup *mem_cgroup_alloc(struct mem_cgroup *parent)
 	if (!memcg1_alloc_events(memcg))
 		goto fail;
 
+	pstatc_pcpu = parent ? parent->vmstats_percpu : NULL;
 	for_each_possible_cpu(cpu) {
-		if (parent)
-			pstatc_pcpu = parent->vmstats_percpu;
 		statc = per_cpu_ptr(memcg->vmstats_percpu, cpu);
-		statc->parent_pcpu = parent ? pstatc_pcpu : NULL;
+		statc->parent_pcpu = pstatc_pcpu;
 		statc->vmstats = memcg->vmstats;
 	}
 
-- 
2.43.0
Re: [PATCH] mm/memcontrol: hoist pstatc_pcpu assignment out of CPU loop
Posted by Shakeel Butt 1 month, 2 weeks ago
On Wed, Apr 29, 2026 at 04:42:16PM +0800, Hui Zhu wrote:
> From: Hui Zhu <zhuhui@kylinos.cn>
> 
> In mem_cgroup_alloc(), the assignment of pstatc_pcpu is invariant
> with respect to the for_each_possible_cpu() loop: both the 'parent'
> pointer and 'parent->vmstats_percpu' remain constant throughout all
> iterations.
> 
> The original code redundantly re-evaluated the 'if (parent)'
> condition and reassigned pstatc_pcpu on every CPU iteration, then
> repeated the same ternary check 'parent ? pstatc_pcpu : NULL' when
> storing into statc->parent_pcpu.
> 
> Move the single conditional assignment of pstatc_pcpu to before the
> loop, resolving both the loop-invariant placement issue and the
> duplicated null check. On systems with a large number of possible
> CPUs, this eliminates repeated branch evaluation with no functional
> change.
> 
> No functional change intended.
> 
> Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>

Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
Re: [PATCH] mm/memcontrol: hoist pstatc_pcpu assignment out of CPU loop
Posted by SeongJae Park 1 month, 2 weeks ago
On Wed, 29 Apr 2026 16:42:16 +0800 Hui Zhu <hui.zhu@linux.dev> wrote:

> From: Hui Zhu <zhuhui@kylinos.cn>
> 
> In mem_cgroup_alloc(), the assignment of pstatc_pcpu is invariant
> with respect to the for_each_possible_cpu() loop: both the 'parent'
> pointer and 'parent->vmstats_percpu' remain constant throughout all
> iterations.
> 
> The original code redundantly re-evaluated the 'if (parent)'
> condition and reassigned pstatc_pcpu on every CPU iteration, then
> repeated the same ternary check 'parent ? pstatc_pcpu : NULL' when
> storing into statc->parent_pcpu.
> 
> Move the single conditional assignment of pstatc_pcpu to before the
> loop, resolving both the loop-invariant placement issue and the
> duplicated null check. On systems with a large number of possible
> CPUs, this eliminates repeated branch evaluation with no functional
> change.
> 
> No functional change intended.

Makes sense and looks good to me.

> 
> Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>

Reviewed-by: SeongJae Park <sj@kernel.org>


Thanks,
SJ

[...]
Re: [PATCH] mm/memcontrol: hoist pstatc_pcpu assignment out of CPU loop
Posted by Andrew Morton 1 month, 2 weeks ago
On Wed, 29 Apr 2026 16:42:16 +0800 Hui Zhu <hui.zhu@linux.dev> wrote:

> From: Hui Zhu <zhuhui@kylinos.cn>
> 
> In mem_cgroup_alloc(), the assignment of pstatc_pcpu is invariant
> with respect to the for_each_possible_cpu() loop: both the 'parent'
> pointer and 'parent->vmstats_percpu' remain constant throughout all
> iterations.
> 
> The original code redundantly re-evaluated the 'if (parent)'
> condition and reassigned pstatc_pcpu on every CPU iteration, then
> repeated the same ternary check 'parent ? pstatc_pcpu : NULL' when
> storing into statc->parent_pcpu.
> 
> Move the single conditional assignment of pstatc_pcpu to before the
> loop, resolving both the loop-invariant placement issue and the
> duplicated null check. On systems with a large number of possible
> CPUs, this eliminates repeated branch evaluation with no functional
> change.
> 
> No functional change intended.
> 
> ...
>
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -3993,11 +3993,10 @@ static struct mem_cgroup *mem_cgroup_alloc(struct mem_cgroup *parent)
>  	if (!memcg1_alloc_events(memcg))
>  		goto fail;
>  
> +	pstatc_pcpu = parent ? parent->vmstats_percpu : NULL;
>  	for_each_possible_cpu(cpu) {
> -		if (parent)
> -			pstatc_pcpu = parent->vmstats_percpu;
>  		statc = per_cpu_ptr(memcg->vmstats_percpu, cpu);
> -		statc->parent_pcpu = parent ? pstatc_pcpu : NULL;
> +		statc->parent_pcpu = pstatc_pcpu;
>  		statc->vmstats = memcg->vmstats;
>  	}

lgtm.

I expected this to make no change to generated code but it actually
reduces memcontrol.o text by nearly 300 bytes (x86_64 allmodconfig).