On 13/06/2024 11:20, Sebastian Andrzej Siewior wrote:
The !in_atomic() check in _wait_for_atomic() triggers on PREEMPT_RT
because the uncore::lock is a spinlock_t and does not disable
preemption or interrupts.
Changing the uncore:lock to a raw_spinlock_t doubles the worst case
latency on an otherwise idle testbox during testing. Therefore I'm
currently unsure about changing this.
Link: https://lore.kernel.org/all/20211006164628.s2mtsdd2jdbfyf7g@xxxxxxxxxxxxx/
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
---
drivers/gpu/drm/i915/i915_utils.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h
index 06ec6ceb61d57..2ca54bc235925 100644
--- a/drivers/gpu/drm/i915/i915_utils.h
+++ b/drivers/gpu/drm/i915/i915_utils.h
@@ -274,7 +274,7 @@ wait_remaining_ms_from_jiffies(unsigned long timestamp_jiffies, int to_wait_ms)
#define wait_for(COND, MS) _wait_for((COND), (MS) * 1000, 10, 1000)
/* If CONFIG_PREEMPT_COUNT is disabled, in_atomic() always reports false. */
-#if defined(CONFIG_DRM_I915_DEBUG) && defined(CONFIG_PREEMPT_COUNT)
+#if defined(CONFIG_DRM_I915_DEBUG) && defined(CONFIG_PREEMPT_COUNT) && !defined(CONFIG_PREEMPT_RT)
# define _WAIT_FOR_ATOMIC_CHECK(ATOMIC) WARN_ON_ONCE((ATOMIC) && !in_atomic())
#else
# define _WAIT_FOR_ATOMIC_CHECK(ATOMIC) do { } while (0)
I think this could be okay-ish in principle, but the commit text is not
entirely accurate because there is no direct coupling between the wait
helpers and the uncore lock. They can be used from any atomic context.
Okay-ish in principle because there is sufficient testing in Intel's CI
on non-PREEMPT_RT kernels to catch any conceptual misuses.
But see also the caller in skl_pcode_request. It is a bit harder to hit
since it is the fallback path. Or gen5_rps_enable which nests under a
different lock.
Hmm would there be a different helper, or combination of helpers, which
could replace in_atomic() which would do the right thing on both
kernels? Something to tell us we are neither under a spin_lock, nor
preempt_disable, nor interrupts disabled, nor bottom-half. On either
stock or PREEMPT_RT.
WARN_ON_ONCE((ATOMIC) && !(!preemptible() || in_hardirq() ||
in_serving_softirq())
Would this work?
Regards,
Tvrtko
