On 15/02/2018 11:47, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2018-02-15 11:13:33)From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> We can convert engine stats from a spinlock to seqlock to ensure interrupt processing is never even a tiny bit delayed by parallel readers. There is a smidgen bit more cost on the write lock side, and an extremely unlikely chance that readers will have to retry a few times in face of heavy interrupt load.Bbut it should be extremely unlikely given how lightweight read side section is compared to the interrupt processing side, and also compared to the rest of the code paths which can lead into it. Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> Suggested-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> --- @@ -2028,12 +2028,13 @@ static ktime_t __intel_engine_get_busy_time(struct intel_engine_cs *engine) */ ktime_t intel_engine_get_busy_time(struct intel_engine_cs *engine) { + unsigned int seq; ktime_t total; - unsigned long flags;- spin_lock_irqsave(&engine->stats.lock, flags);- total = __intel_engine_get_busy_time(engine); - spin_unlock_irqrestore(&engine->stats.lock, flags); + do { + seq = read_seqbegin(&engine->stats.lock); + total = __intel_engine_get_busy_time(engine); + } while (read_seqretry(&engine->stats.lock, seq));Hmm, only the reader is inside a hard-irq context right? (Thanks perf!) I was hoping we could avoid disabling irqs around the writer. Now that requires some coffee...
Oh yeah.. it was irqsave only because of perf. I am pretty sure you're right and I can downgrade write locking to _bh - after the recent changes we got a writer in process context (enable/disable), and a writer in tasklet context (context in/out).
Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
