Re: PREEMPT_RT vs i915

Sebastian Andrzej Siewior
2025-07-10T06:41:39Z

On 2025-07-09 23:09:22 [+0300], Ville Syrjälä wrote:
> On Wed, Jul 09, 2025 at 09:44:43PM +0200, Sebastian Andrzej Siewior wrote:
> > On 2025-07-09 20:30:26 [+0300], Ville Syrjälä wrote:
> > > >
> > > > It seems like the critical uncore lock is currently held in a lot of
> > > > places and potentially for a long time.
> > >
> > > It shouldn't be held for that long. I think it should just be
> > > a raw spinlock.
> >
> > What about I resubmit the series and we look again? I don't think the
> > lock should be made raw just to be done with it.
>
> Until someone actually does the work to confirm the thing is working
> reliably there's no point in posting anything.

Well it works on my machine and this machine dose not pass the code
paths that I patch.

Every patch made was done because someone reported an error/ warning and
confirmed afterwards that the patch fixes it for them and they can use
the machine and don't observe anything.

> And IIRC the other remaining problem with RT was the spinlocks used
> inside tracepoints (which is uncore lock, and probably some vblank
> locks). So that too needs some kind of solution because it's going to
> very hard to debug the timing sensitive parts without the tracepoints.

no, not just that. Making the lock raw led to latency spikes in simple
spikes and I just disabled trace points. It could be worked around by
taking the lock if the tracepoint is enabled and then invoking the
tracepoint unconditionally and not taking the lock anymore. Steven made
a suggestion a while ago how to put this in macro as far as I remember.

Looking at series there are things like execlists_dequeue_irq() which do
local_irq_disable() follwed by spin_lock() which is a no-no and
explained in Documentation/locking/locktypes.rst. There is also
intel_guc_send_busy_loop() no considering RCU read-locks for their
atomic detection. I have 8 patches in total and one in the pipe.

I have patches with Acks by Tvrtko Ursulin but I remain caring them.
I can repost it but usually the bot complains, the bot gets retriggered.
The first patch in series is vblank detection and I've been told this is
old code and scares people. So that might be why the remaining part is
ignored.

Sebastian