Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] s390/hiperdispatch: TOCTOU race in hd_calculate_steal_percentage() 
causes artificial steal percentage spikes
--

commit 543d41ec158ad38d026cd1a0e58754cc9393d347
Author: Shrikanth Hegde <[email protected]>

sched/cputime: Add kcpustat_field_total helper

This patch introduces a new helper function to sum up a given cpustat type
over a specified cpumask. It replaces the open-coded steal calculation in
s390 hiperdispatch and idle time calculation in proc uptime to simplify
the callers.

> diff --git a/arch/s390/kernel/hiperdispatch.c 
> b/arch/s390/kernel/hiperdispatch.c
> index 2172065222661..c21496f0a141a 100644
> --- a/arch/s390/kernel/hiperdispatch.c
> +++ b/arch/s390/kernel/hiperdispatch.c

[ ... ]

> @@ -207,16 +207,12 @@ static unsigned long hd_calculate_steal_percentage(void)
>  {
>       unsigned long time_delta, steal_delta, steal, percentage;
>       static ktime_t prev;
> -     int cpus, cpu;
> +     int cpus;
>       ktime_t now;
>  
> -     cpus = 0;
> -     steal = 0;
>       percentage = 0;
> -     for_each_cpu(cpu, &hd_vmvl_cpumask) {
> -             steal += kcpustat_cpu(cpu).cpustat[CPUTIME_STEAL];
> -             cpus++;
> -     }
> +     steal = kcpustat_field_total(CPUTIME_STEAL, &hd_vmvl_cpumask);
> +     cpus = cpumask_weight(&hd_vmvl_cpumask);

[Severity: High]
Could separating the summation from the divisor count open a race window
against concurrent mask updates?

In the previous single-loop calculation, the total steal time and CPU count
were evaluated together on a stable view of the mask. With these changes,
if a concurrent CPU hotplug event calls update_cpu_masks() while
hd_calculate_steal_percentage() is running, hd_vmvl_cpumask could be cleared
and repopulated asynchronously.

If the mask changes from multiple CPUs to 1 CPU between the
kcpustat_field_total calculation and the cpumask_weight call, steal will
reflect the delta of the larger number of CPUs, but cpus will evaluate to 1.

Because the percentage is computed as steal_delta / cpus, dividing the
larger steal_delta by 1 would artificially inflate the steal percentage.
Can this erroneous spike trip the hd_steal_threshold, causing the scheduler
topology to wrongly disable Hiperdispatch and downgrade high-capacity cores?

[ ... ]

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=1

Reply via email to