On 9/9/26 7:46 PM, [email protected] wrote:
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 <[email protected]>
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.
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.
CPU Hotplug race is taken care. order of udpates are expected not to cause the
above issue.
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.
That's intentional.
+ }
+
+requeue_work:
+ schedule_delayed_work(&sg_ctx.work, sg_ctx.delay);
+}
