Re: [PATCH] lockdep: Speed up lockdep_unregister_key() with expedited RCU synchronization

[Boqun Feng <boqun.feng@gmail.com>]

On Mon, Mar 31, 2025 at 09:48:34AM -0700, Breno Leitao wrote:
> Hello Boqun, Waimn
> 
> On Wed, Mar 26, 2025 at 11:42:37AM -0700, Boqun Feng wrote:
> > On Wed, Mar 26, 2025 at 10:10:28AM -0700, Paul E. McKenney wrote:
> > > On Wed, Mar 26, 2025 at 01:02:12PM -0400, Waiman Long wrote:
> > [...]
> > > > > > > Thinking about it more, doing it in a lockless way is probably a good
> > > > > > > idea.
> > > > > > > 
> > > > > > If we are using hazard pointer for synchronization, should we also take off
> > > > > > "_rcu" from the list iteration/insertion/deletion macros to avoid the
> > > > > > confusion that RCU is being used?
> > > > > > 
> > > > > We can, but we probably want to introduce a new set of API with suffix
> > > > > "_lockless" or something because they will still need a lockless fashion
> > > > > similar to RCU list iteration/insertion/deletion.
> > > > 
> > > > The lockless part is just the iteration of the list. Insertion and deletion
> > > > is protected by lockdep_lock().
> > > > 
> > > > The current hlist_*_rcu() macros are doing the right things for lockless use
> > > > case too. We can either document that RCU is not being used or have some
> > > > _lockless helpers that just call the _rcu equivalent.
> > > 
> > > We used to have _lockless helper, but we got rid of them.  Not necessarily
> > > meaning that we should not add them back in, but...  ;-)
> > > 
> > 
> > I will probably go with using *_rcu() first with some comments, if this
> > "hazard pointers for hash table" is a good idea in other places, we can
> > add *_hazptr() or pick a better name then.
> 
> I am trying to figure out what are the next steps to get this issue
> solve.
> 

I will send out a serise including introduction of simple hazard
pointers and use it in lockdep for this case, hopefully that can resolve
your issue.

> Would you mind help me to understand what _rcu() fuction you are
> suggesting and what will it replace?
> 

The _rcu() functions we are talking about are:
hlist_for_each_entry_rcu() in is_dynamic_key(), hlist_add_head_rcu() in
lockdep_register_key() and hlist_del_rcu() in lockdep_unregister_key(),
because if we move to hazptr, they are technically not protected by RCU.
But the implementation of these functions is still correct with hazptr,
it's just their names might be confusing, and we may change them later.

Hope this helps.

Regards,
Boqun


> Thank you,
> --breno