| Message ID | 1371809753-16052-1-git-send-email-bigeasy@linutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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 */
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(-)