[Devel] [PATCH vz10 v2] ve/proc/loadavg: sum nr_unint drift as signed long, not int

[Pavel Tikhomirov]

Reviewed-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>

On 6/11/26 19:24, Konstantin Khorenko wrote:
> cpu_cgroup_proc_loadavg() sums the per-CPU cfs_rq::nr_unint counters
> into a plain int accumulator and prints it with %d:
> 
> 	int nr_running = 0;
> 	...
> 	nr_running += tg->cfs_rq[i]->nr_unint;
> 
> nr_unint is unsigned long and relies on signed reinterpretation to
> fold the migration drift: a task can increment it on one CPU while
> the matching decrement lands on another CPU after migration, so a
> single per-CPU value can be far negative (stored as a huge unsigned
> number) and only the sum over all CPUs is meaningful.
> 
> This is not a wrong-value fix: reading the drift as signed and adding
> it folds to the same small non-negative sum whether the accumulator
> is int or long. The change is for consistency and clarity, not
> correctness:
> 
>  - calc_load_fold_active() and calc_load_ve() fold the very same
>    per-CPU nr_unint in a long with an explicit (long) cast; this site
>    is the odd one out;
>  - an explicit (long) cast states the signed reinterpretation at the
>    read site instead of hiding it in an implicit unsigned-long-to-int
>    narrowing;
>  - a long accumulator prints correctly with %ld and could host a
>    "< 0" sanity check like calc_load_ve() has.
> 
> Accumulate in long, cast each term with (long), print with %ld.
> 
> Fixes: e4a432fe473c ("ve/proc/loadavg: Virtualize /proc/loadavg in Containers")
> 
> Reported-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>
> https://virtuozzo.atlassian.net/browse/VSTOR-132310
> Signed-off-by: Konstantin Khorenko <khorenko at virtuozzo.com>
> ---
>  kernel/sched/core.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index d51d70ff7560..d5b4d8c97a0c 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -10045,7 +10045,7 @@ int cpu_cgroup_proc_loadavg(struct cgroup_subsys_state *css,
>  	struct cgroup *cgrp = css->cgroup;
>  	struct task_group *tg = css_tg(css);
>  	unsigned long avnrun[3];
> -	int nr_running = 0;
> +	long nr_running = 0;
>  	int i;
>  
>  	if (!test_bit(CGRP_VE_ROOT, &cgrp->flags))
> @@ -10064,14 +10064,14 @@ int cpu_cgroup_proc_loadavg(struct cgroup_subsys_state *css,
>  		 * overhead for activate/deactivate operations. So, we
>  		 * don't account child cgroup unint tasks here.
>  		 */
> -		nr_running += tg->cfs_rq[i]->nr_unint;
> +		nr_running += (long)tg->cfs_rq[i]->nr_unint;
>  #endif
>  #ifdef CONFIG_RT_GROUP_SCHED
>  		nr_running += tg->rt_rq[i]->rt_nr_running;
>  #endif
>  	}
>  
> -	seq_printf(p, "%lu.%02lu %lu.%02lu %lu.%02lu %d/%d %d\n",
> +	seq_printf(p, "%lu.%02lu %lu.%02lu %lu.%02lu %ld/%d %d\n",
>  		LOAD_INT(avnrun[0]), LOAD_FRAC(avnrun[0]),
>  		LOAD_INT(avnrun[1]), LOAD_FRAC(avnrun[1]),
>  		LOAD_INT(avnrun[2]), LOAD_FRAC(avnrun[2]),

-- 
Best regards, Pavel Tikhomirov
Senior Software Developer, Virtuozzo.