On 9/9/26 7:47 PM, [email protected] wrote:
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():
clear_bit is atomic operation. It cannot happen concurrently IMHO.
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:
Even if it happens, it is safe to shut down, rather than continue in weird
state.
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?
[ ... ]
