Re: [Intel-gfx] [PATCH 3/8] drm/i915: Disable tracing points on PREEMPT_RT

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Oct 05, 2021 at 05:00:41PM +0200, Sebastian Andrzej Siewior wrote:
> Luca Abeni reported this:
> | BUG: scheduling while atomic: kworker/u8:2/15203/0x00000003
> | CPU: 1 PID: 15203 Comm: kworker/u8:2 Not tainted 4.19.1-rt3 #10
> | Call Trace:
> |  rt_spin_lock+0x3f/0x50
> |  gen6_read32+0x45/0x1d0 [i915]
> |  g4x_get_vblank_counter+0x36/0x40 [i915]
> |  trace_event_raw_event_i915_pipe_update_start+0x7d/0xf0 [i915]
> 
> The tracing events use trace_i915_pipe_update_start() among other events
> use functions acquire spinlock_t locks which are transformed into
> sleeping locks on PREEMPT_RT. A few trace points use
> intel_get_crtc_scanline(), others use ->get_vblank_counter() wich also
> might acquire a sleeping locks on PREEMPT_RT.
> At the time the arguments are evaluated within trace point, preemption
> is disabled and so the locks must not be acquired on PREEMPT_RT.
> 
> Based on this I don't see any other way than disable trace points on
> PREMPT_RT.

I think the correct answer is to make uncore.lock a raw_spinlock.
Without the tracepoints deubgging any of this is stuff pretty much
impossible. We also take that lock a lot.

The horrible truth is that some of the hardware just keels over
if two CPUs access the same mmio cacheline simultanously, so
pretty much all mmio accesses need to be performed under uncore.lock.

The vblank locking situation is a lot more self inflicted. As in
there are something like three different spinlocks in there for
some reason. Not sure what to do about that. I guess one option
would be to skip the vblank timestamp based stuff in the
tracepoints for the time being. Most hardware should have
a hardware vblank counter so the fallback isn't super common.

> 
> Reported-by: Luca Abeni <lucabe72@gmail.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>  drivers/gpu/drm/i915/i915_trace.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
> index 806ad688274bf..773e7362c4442 100644
> --- a/drivers/gpu/drm/i915/i915_trace.h
> +++ b/drivers/gpu/drm/i915/i915_trace.h
> @@ -2,6 +2,10 @@
>  #if !defined(_I915_TRACE_H_) || defined(TRACE_HEADER_MULTI_READ)
>  #define _I915_TRACE_H_
>  
> +#ifdef CONFIG_PREEMPT_RT
> +#define NOTRACE
> +#endif
> +
>  #include <linux/stringify.h>
>  #include <linux/types.h>
>  #include <linux/tracepoint.h>
> -- 
> 2.33.0

-- 
Ville Syrjälä
Intel