There is a report on RT about "BUG: scheduling while atomic" because the sleeping lock is taken in tracing context. This patch simply moves locking operation out of the tracing macro.
Reported-by: Joakim Hernberg jbh@alchemy.lu [C.Emde@osadl.org: pull out seqno and define it so it compiled] Signed-off-by: Sebastian Andrzej Siewior bigeasy@linutronix.de --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 5 ++++- drivers/gpu/drm/i915/i915_trace.h | 1 - drivers/gpu/drm/i915/intel_ringbuffer.h | 2 ++ 3 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 87a3227..fa5b45f 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -844,6 +844,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, struct intel_ring_buffer *ring; u32 ctx_id = i915_execbuffer2_get_context_id(*args); u32 exec_start, exec_len; + u32 seqno; u32 mask, flags; int ret, mode, i; bool need_relocs; @@ -1081,7 +1082,9 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, goto err; }
- trace_i915_gem_ring_dispatch(ring, intel_ring_get_seqno(ring), flags); + seqno = intel_ring_get_seqno(ring); + trace_i915_gem_ring_dispatch(ring, seqno, flags); + i915_trace_irq_get(ring, seqno);
i915_gem_execbuffer_move_to_active(&eb->objects, ring); i915_gem_execbuffer_retire_commands(dev, file, ring, batch_obj); diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h index 3db4a68..29217db 100644 --- a/drivers/gpu/drm/i915/i915_trace.h +++ b/drivers/gpu/drm/i915/i915_trace.h @@ -244,7 +244,6 @@ TRACE_EVENT(i915_gem_ring_dispatch, __entry->ring = ring->id; __entry->seqno = seqno; __entry->flags = flags; - i915_trace_irq_get(ring, seqno); ),
TP_printk("dev=%u, ring=%u, seqno=%u, flags=%x", diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 799f04c..13ffe58 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -262,8 +262,10 @@ static inline u32 intel_ring_get_seqno(struct intel_ring_buffer *ring)
static inline void i915_trace_irq_get(struct intel_ring_buffer *ring, u32 seqno) { +#ifdef CONFIG_TRACEPOINTS if (ring->trace_irq_seqno == 0 && ring->irq_get(ring)) ring->trace_irq_seqno = seqno; +#endif }
/* DRI warts */
On Fri, Jun 21, 2013 at 12:15:53PM +0200, Sebastian Andrzej Siewior wrote:
There is a report on RT about "BUG: scheduling while atomic" because the sleeping lock is taken in tracing context. This patch simply moves locking operation out of the tracing macro.
No. This enables the IRQ, as well as making a number of very expensively serialised read, unconditionally. -Chris
On 06/21/2013 01:08 PM, Chris Wilson wrote:
On Fri, Jun 21, 2013 at 12:15:53PM +0200, Sebastian Andrzej Siewior wrote:
There is a report on RT about "BUG: scheduling while atomic" because the sleeping lock is taken in tracing context. This patch simply moves locking operation out of the tracing macro.
No. This enables the IRQ, as well as making a number of very expensively serialised read, unconditionally.
Ach in case CONFIG_TRACING is enabled but the tracepoint itself is disabled. In that case it is probably best to drop this tracepoint from -RT.
-Chris
Sebastian
On Fri, Jun 21, 2013 at 02:51:07PM +0200, Sebastian Andrzej Siewior wrote:
On 06/21/2013 01:08 PM, Chris Wilson wrote:
On Fri, Jun 21, 2013 at 12:15:53PM +0200, Sebastian Andrzej Siewior wrote:
There is a report on RT about "BUG: scheduling while atomic" because the sleeping lock is taken in tracing context. This patch simply moves locking operation out of the tracing macro.
No. This enables the IRQ, as well as making a number of very expensively serialised read, unconditionally.
Ach in case CONFIG_TRACING is enabled but the tracepoint itself is disabled. In that case it is probably best to drop this tracepoint from -RT.
Iirc when we've last discussed this with Thomas Gleixner he mentioned that there's a setup/teardown hook when enabling/disabling a tracepoint. We could use that to enable/disable interrupts. Needs a notch of logic rework though since the auto-irq disable code needs adjusting ... And on a 2nd look that trace_irq_seqno logic looks a bit broken anyway.
It's somewhere on my todo list, but patches very much welcome. -Daniel
dri-devel@lists.freedesktop.org