Re: [RFT] Revert "lockdep: Enable PROVE_RAW_LOCK_NESTING with PROVE_LOCKING."

[Peter Zijlstra <peterz@infradead.org>]

On Tue, Dec 10, 2024 at 06:00:33PM -0600, Lucas De Marchi wrote:

> 	raw_spin_lock_init(&guc->timestamp.lock);
> 	raw_spin_lock_init(&pmu->lock);
> 	raw_spin_lock_init(&i915->mmio_debug.lock);
> 	raw_spin_lock_init(&uncore->lock);
> 
> And it's still not sufficient, because intel_ref_tracker tries to
> allocate while holding one of those and I'm not confident on making that
> pass GFP_ATOMIC.

You cannot allocate memory under raw_spinlock_t at all, ever. Nor free
for that matter. The allocators use spinlock internally.


> [  204.835058]  __kmalloc_cache_noprof+0x277/0x480
> [  204.839554]  ? __kmalloc_cache_noprof+0x277/0x480
> [  204.844221]  ref_tracker_alloc+0xd7/0x270
> [  204.848206]  ? ref_tracker_alloc+0xd7/0x270
> [  204.852357]  guc_engine_busyness+0x122/0x2c0 [i915]
> 
> 
> > 
> > > 
> > > It's a real problem only for PREEMPT_RT since otherwise there's
> > > no difference between the 2 lock types. However fixing this may involve
> > > quite a few changes: if we convert the lock to raw we may need to
> > > cascade the conversions to additional locks.  The ones I identified are:
> > > pmu->lock, which would also need to have uncore->lock converted, which
> > > would then probably cascade to quite a few others :-/. I'm not sure
> > > converting uncore->lock will actually be a good thing.
> > 
> > hmm raw_spinlocks for the lowlevel might not be a bad idea, but perhaps
> > we need to convert the other way around the upper levels?
> 
> that would mean:
> 
> <4> [96.733606]  #0: ffff88885f432038 (&cpuctx_lock){....}-{2:2}, at: __perf_install_in_context+0x3f/0x360
> 
> so inside the perf event infra, that has been using raw_spinlock_t
> since forever. I'm surprised we got this only 10 years later :-/.
> I don't think perf can sleep in that context, but Cc'ing a few people
> and lkml for that question.

You very much cannot sleep here. This is hardirq context.