Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] preferred_cpus_valid() reads cpu_preferred_mask and cpu_active_mask 
without holding cpus_read_lock(), creating a data race with CPU hotplug that 
causes the governor to permanently shut down.
--

commit b0d791ea69ac4f874448d3baf935e4825cba622f
Author: Shrikanth Hegde <[email protected]>

virt/steal_governor: Enable the driver

This commit provides a configuration option to enable the steal_governor driver
for paravirtualized environments. It enforces PARAVIRT and SMP dependencies and
selects CONFIG_PREFERRED_CPU for core scheduler mechanisms.

> diff --git a/drivers/virt/Kconfig b/drivers/virt/Kconfig
> index 52eb7e4ba71f6..eeb84e578ddf9 100644
> --- a/drivers/virt/Kconfig
> +++ b/drivers/virt/Kconfig
> @@ -41,6 +41,23 @@ config FSL_HV_MANAGER
>            4) A kernel interface for receiving callbacks when a managed
>            partition shuts down.
>  
> +config STEAL_GOVERNOR
> +     tristate "Dynamic vCPU management based on steal time"
> +     depends on PARAVIRT && SMP
> +     select PREFERRED_CPU

[Severity: High]
While this patch enables the driver, can the background loop in
steal_governor_loop() permanently terminate during a concurrent CPU hotplug
operation?

If a CPU is offlined, sched_cpu_deactivate() clears the target CPU from
cpu_preferred_mask and then cpu_active_mask without memory barriers. If this
happens concurrently, preferred_cpus_valid() in drivers/virt/steal_governor.c
might observe a torn state because it evaluates the masks without holding
cpus_read_lock():

drivers/virt/steal_governor.c:preferred_cpus_valid()
        if (!cpumask_subset(cpu_preferred_mask, cpu_active_mask)) {
                pr_err("preferred: %*pbl is not subset of active: ...\n",
                       ...);
                return false;
        }

If the check fails due to this race, the governor loop permanently shuts down
since it returns instead of requeuing the work:

drivers/virt/steal_governor.c:steal_governor_loop()
        if (!preferred_cpus_valid()) {
                restore_preferred_to_active();
                return;
        }

Would holding cpus_read_lock() during these mask checks prevent this race and
keep the loop running correctly?

[ ... ]

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

Reply via email to