Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

drm/i915: Don't disable interrupts and pretend a lock as been acquired in __timeline_mark_lock().

This is a revert of commits
d67739268cf0e ("drm/i915/gt: Mark up the nested engine-pm timeline lock as irqsafe")
6c69a45445af9 ("drm/i915/gt: Mark context->active_count as protected by timeline->mutex")
6dcb85a0ad990 ("drm/i915: Hold irq-off for the entire fake lock period")

The existing code leads to a different behaviour depending on whether
lockdep is enabled or not. Any following lock that is acquired without
disabling interrupts (but needs to) will not be noticed by lockdep.

This it not just a lockdep annotation but is used but an actual mutex_t
that is properly used as a lock but in case of __timeline_mark_lock()
lockdep is only told that it is acquired but no lock has been acquired.

It appears that its purpose is just satisfy the lockdep_assert_held()
check in intel_context_mark_active(). The other problem with disabling
interrupts is that on PREEMPT_RT interrupts are also disabled which
leads to problems for instance later during memory allocation.

Add a CONTEXT_IS_PARKING bit to intel_engine_cs and set_bit/clear_bit it
instead of mutex_acquire/mutex_release. Use test_bit in the two
identified spots which relied on the lockdep annotation.

Cc: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/YbO8Ie1Nj7XcQPNQ@linutronix.de

authored by

Sebastian Andrzej Siewior and committed by
Tvrtko Ursulin
bce45c26 76aee865

+7 -38
+2 -1
drivers/gpu/drm/i915/gt/intel_context.h
··· 211 211 212 212 static inline void intel_context_mark_active(struct intel_context *ce) 213 213 { 214 - lockdep_assert_held(&ce->timeline->mutex); 214 + lockdep_assert(lockdep_is_held(&ce->timeline->mutex) || 215 + test_bit(CONTEXT_IS_PARKING, &ce->flags)); 215 216 ++ce->active_count; 216 217 } 217 218
+1
drivers/gpu/drm/i915/gt/intel_context_types.h
··· 118 118 #define CONTEXT_LRCA_DIRTY 9 119 119 #define CONTEXT_GUC_INIT 10 120 120 #define CONTEXT_PERMA_PIN 11 121 + #define CONTEXT_IS_PARKING 12 121 122 122 123 struct { 123 124 u64 timeout_us;
+2 -36
drivers/gpu/drm/i915/gt/intel_engine_pm.c
··· 80 80 return 0; 81 81 } 82 82 83 - #if IS_ENABLED(CONFIG_LOCKDEP) 84 - 85 - static unsigned long __timeline_mark_lock(struct intel_context *ce) 86 - { 87 - unsigned long flags; 88 - 89 - local_irq_save(flags); 90 - mutex_acquire(&ce->timeline->mutex.dep_map, 2, 0, _THIS_IP_); 91 - 92 - return flags; 93 - } 94 - 95 - static void __timeline_mark_unlock(struct intel_context *ce, 96 - unsigned long flags) 97 - { 98 - mutex_release(&ce->timeline->mutex.dep_map, _THIS_IP_); 99 - local_irq_restore(flags); 100 - } 101 - 102 - #else 103 - 104 - static unsigned long __timeline_mark_lock(struct intel_context *ce) 105 - { 106 - return 0; 107 - } 108 - 109 - static void __timeline_mark_unlock(struct intel_context *ce, 110 - unsigned long flags) 111 - { 112 - } 113 - 114 - #endif /* !IS_ENABLED(CONFIG_LOCKDEP) */ 115 - 116 83 static void duration(struct dma_fence *fence, struct dma_fence_cb *cb) 117 84 { 118 85 struct i915_request *rq = to_request(fence); ··· 126 159 { 127 160 struct intel_context *ce = engine->kernel_context; 128 161 struct i915_request *rq; 129 - unsigned long flags; 130 162 bool result = true; 131 163 132 164 /* ··· 180 214 * engine->wakeref.count, we may see the request completion and retire 181 215 * it causing an underflow of the engine->wakeref. 182 216 */ 183 - flags = __timeline_mark_lock(ce); 217 + set_bit(CONTEXT_IS_PARKING, &ce->flags); 184 218 GEM_BUG_ON(atomic_read(&ce->timeline->active_count) < 0); 185 219 186 220 rq = __i915_request_create(ce, GFP_NOWAIT); ··· 212 246 213 247 result = false; 214 248 out_unlock: 215 - __timeline_mark_unlock(ce, flags); 249 + clear_bit(CONTEXT_IS_PARKING, &ce->flags); 216 250 return result; 217 251 } 218 252
+2 -1
drivers/gpu/drm/i915/i915_request.h
··· 657 657 { 658 658 /* Valid only while the request is being constructed (or retired). */ 659 659 return rcu_dereference_protected(rq->timeline, 660 - lockdep_is_held(&rcu_access_pointer(rq->timeline)->mutex)); 660 + lockdep_is_held(&rcu_access_pointer(rq->timeline)->mutex) || 661 + test_bit(CONTEXT_IS_PARKING, &rq->context->flags)); 661 662 } 662 663 663 664 static inline struct i915_gem_context *