Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Incomplete implementation of PREEMPT_RT fixes for i915 selftests, leaving preemption-disabled busy-waits and delays that cause deadlocks and latency spikes. -- commit c008dff1b2cefd9ee50ca924f83ff7c237a6fb3a Author: Maarten Lankhorst <[email protected]> drm/i915/gt: Fix selftests on PREEMPT_RT This commit modifies the i915 engine PM selftest to use migrate_disable() instead of preempt_disable() around delay functions. This helps prevent latency issues on PREEMPT_RT kernels. > diff --git a/drivers/gpu/drm/i915/gt/selftest_engine_pm.c > b/drivers/gpu/drm/i915/gt/selftest_engine_pm.c > index 10e556a7eac45..c1eff9edd8a5e 100644 > --- a/drivers/gpu/drm/i915/gt/selftest_engine_pm.c > +++ b/drivers/gpu/drm/i915/gt/selftest_engine_pm.c [ ... ] > @@ -316,11 +316,11 @@ static int live_engine_busy_stats(void *arg) > } > > ENGINE_TRACE(engine, "measuring busy time\n"); > - preempt_disable(); > + migrate_disable(); > de = intel_engine_get_busy_time(engine, &t[0]); > mdelay(100); > de = ktime_sub(intel_engine_get_busy_time(engine, &t[1]), de); > - preempt_enable(); > + migrate_enable(); [Severity: High] This isn't a bug introduced by this patch, but since this series addresses PREEMPT_RT issues in i915 selftests, are there similar preemption-disabled delays and waits remaining in other selftests that should be addressed? For example, looking at live_rps_clock_interval() in drivers/gpu/drm/i915/gt/selftest_rps.c, there is a block that disables preemption for at least 1 millisecond: preempt_disable(); cycles_[i] = -intel_uncore_read_fw(gt->uncore, GEN6_RP_CUR_UP_EI); dt_[i] = ktime_get(); udelay(1000); cycles_[i] += intel_uncore_read_fw(gt->uncore, GEN6_RP_CUR_UP_EI); dt_[i] = ktime_sub(ktime_get(), dt_[i]); preempt_enable(); Could this cause a severe latency spike on PREEMPT_RT? Additionally, in drivers/gpu/drm/i915/selftests/i915_request.c, measure_completion() disables preemption while busy-waiting for a callback: preempt_disable(); semaphore_set(sema, i); while (!READ_ONCE(cb.seen)) cpu_relax(); elapsed[i - 1] = ENGINE_READ_FW(ce->engine, RING_TIMESTAMP); preempt_enable(); If the threaded handler that executes this callback is scheduled on the same CPU on a PREEMPT_RT kernel, would this create a hard deadlock since the busy-wait loop will never yield to the handler? > dt = ktime_sub(t[1], t[0]); > if (100 * de < 95 * dt || 95 * de > 100 * dt) { > pr_err("%s: reported %lldns [%d%%] busyness while > spinning [for %lldns]\n", -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=4
