Re: [PATCH v13 12/13] virt/steal_governor: Implement steal_governor policy loop
From: sashiko-bot@kernel.org
Date: 2026-09-09 14:16:33
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Lockless access to `cpu_active_mask` in `preferred_cpus_valid()` can lead to a spurious failure of `cpumask_subset()`, causing the driver to permanently shut down. -- commit 77aa795bc87999089d7a60bf5b5985b2c580cbec Author: Shrikanth Hegde [off-list ref] virt/steal_governor: Implement steal_governor policy loop This patch implements the core policy loop for the steal_governor driver using a delayed workqueue. It periodically calculates the steal time ratio and dynamically adjusts the preferred CPU mask based on high or low contention thresholds.
quoted hunk ↗ jump to hunk
diff --git a/drivers/virt/steal_governor.c b/drivers/virt/steal_governor.c index 27f53ea16498b..6e31f9923dea4 100644 --- a/drivers/virt/steal_governor.c +++ b/drivers/virt/steal_governor.c
[ ... ]
+static bool preferred_cpus_valid(void)
+{
+ if (cpumask_empty(cpu_preferred_mask)) {
+ pr_err("empty preferred mask. stopping\n");
+ return false;
+ }
+
+ if (!cpumask_subset(cpu_preferred_mask, cpu_active_mask)) {[Severity: High] Can this lockless check spuriously fail and cause the driver to incorrectly shut down? Because cpus_read_lock() is not held here, there appears to be a race condition with CPU hotplug operations (such as sched_cpu_deactivate()). When a CPU is taken offline, it is removed from the relevant masks sequentially. If cpumask_subset() reads cpu_preferred_mask while the dying CPU's bit is still set, and then reads cpu_active_mask after the CPU's bit has been cleared, the subset evaluation will spuriously return false.
+ pr_err("preferred: %*pbl is not subset of active: %*pbl, stopping\n",
+ cpumask_pr_args(cpu_preferred_mask),
+ cpumask_pr_args(cpu_active_mask));
+ return false;
+ }
+
+ return true;
+}
+
+static void steal_governor_loop(struct work_struct *work)
+{[ ... ]
+ if (steal_ratio > sg_ctx.high_threshold)
+ decrease_preferred_cpus();
+ else if (steal_ratio <= sg_ctx.low_threshold)
+ increase_preferred_cpus();
+ /*
+ * else: steal ratio is within bounds. Still do design checks so that
+ * module restores to active if CPU hotplug breaks those assumptions.
+ */
+ if (!preferred_cpus_valid()) {
+ restore_preferred_to_active();
+ return;[Severity: High] If preferred_cpus_valid() spuriously fails due to the lockless race described above, does this early return permanently disable the driver? By returning here without requeueing the work via schedule_delayed_work(), the periodic loop is halted completely, leaving the driver non-functional until the module is manually reloaded.
+ } + +requeue_work: + schedule_delayed_work(&sg_ctx.work, sg_ctx.delay); +}
-- Sashiko AI review · https://sashiko.dev/#/patchset/20260909135617.871006-1-sshegde@linux.ibm.com?part=12