Re: [PATCH v13 13/13] virt/steal_governor: Enable the driver

[sashiko-bot@kernel.org]

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 <sshegde@linux.ibm.com>

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/20260909135617.871006-1-sshegde@linux.ibm.com?part=13