Re: [PATCH] bpf: disable lockdep while running BPF on lock_release

On Wed, 05 Aug 2026, Quanye Yang wrote:
> On Tue, Aug 4, 2026 at 4:34 PM NeilBrown <[email protected]> wrote:
> > I know close enough to nothing about BPF or trace points...
> >
> > Can BPF programs ever block waiting for a lock that is held across a
> > tracepoint?
> I investigate this and I think the answer could splits in two:
> 
> Mutex (sleeping locks): no, this is prevented at verification time. A
> BPF program attached to a tracepoint is non-sleepable --
> can_be_sleepable() returns false for BPF_PROG_TYPE_TRACEPOINT, so the
> verifier rejects any sleeping helper; and at runtime it runs under
> rcu_read_lock() with cant_sleep(). It cannot block on a mutex held
> across the tracepoint.
> 
> Spinlock: yes, in principle. Spinlocks busy-wait rather than sleep, so
> the mutex protection does not apply. BPF programs do take spinlocks --
> each map lookup/update/delete takes the map's bucket lock, and
> bpf_spin_lock is a spinlock -- so a program attached to a tracepoint
> held across a spinlock can spin on another spinlock, and if those form a
> cycle (or are the same lock) it is a real deadlock.

Thanks - this is helpful.

> 
> That is exactly the hazard BPF already hardened its own locks against:
> the hashtab bucket lock and bpf_spin_lock detect recursive acquisition
> and return an error instead of deadlocking (rqspinlock, succeeding the
> map_locked approach in 20b6cc34ea74). The rhashtable bucket bitlock has
> no such protection -- which is the gap I think.

I don't think that detecting recursive acquisition is a scalable
solution.  You'll keep finding new locks that you need to enhance.

If I step back a bit, it looks like TP-BPF (BPF attached to tracepoints)
introduces a new execution context, similar to SOFTIRQ and HARDIRQ.
i.e.  it is something that can start running at almost any point and
blocks the currently running code until it completes.
I suspect it would be good to handle it in a similar way.

So I suggest adding a new
  LOCKDEP_STATE(TP_BPF)
to kernel/locking/lockdep_states.h, and teaching lockdep to understand
it.  Then it could help avoid all these problems.

An important part of this would be the ability to temporarily disable
TP_BPF much as we can disable interrupts.  What happens at present if
there is tracepoint that happens in BPF code, and a BPF handler is attached
to that.  Does it get called recursively?
Would there be a problem with disabling new TP_BPF handlers while TP_BPF
code is running?  Maybe this is already done?

Circling back the original patch to rhashtables that was proposed - had
you said that you needed to use rhashtables in a different context
similar to HARDIRQ or SOFTIRQ, then I think I would have had a different
response.  That is a well-specified problem with well-understood
solutions.

I would likely be OK with making is possible for lockdep to see locks
from different tables as different locks, because they need to be taken
in different lockdep contexts.

I would then see if the new lock_class_key could be declared in the
rhashtable_params struct.  This is already passed around everywhere, so
this would be much less intrusive.  We would have to use some cast to
get rid of the 'const' attribute when passing the lock_class_key to
lockdep, but I think that is justifiable.

NeilBrown


> 
> quanyeyang
> > If so, then attaching a BPF program to that trace point could
> > trivially cause a deadlock.
> > If not - then how is that ensured?
> >
> > Thanks,
> > NeilBrown
>