Re: [Question] srcu: is it making sense to recursively invoke srcu_read_lock?

["Paul E. McKenney" <paulmck@kernel.org>]

On Fri, Apr 22, 2022 at 08:52:12AM +0800, Hillf Danton wrote:
> On Thu, 21 Apr 2022 06:34:14 -0700 Paul E. McKenney wrote:
> > On Thu, Apr 21, 2022 at 12:22:11PM +0800, Hillf Danton wrote:
> > > Given rcu_lock_acquire() in srcu_read_lock(),
> > > 
> > > 	iA = srcu_read_lock(foo);
> > > 	iB = srcu_read_lock(foo); // not bar
> > > 	...
> > > 	srcu_read_unlock(foo, iB);
> > > 	srcu_read_unlock(foo, iA);
> > > 
> > > can the call sequence above trigger warning with CONFIG_DEBUG_LOCK_ALLOC enabled?
> > 
> > I hope not!  After all, nesting SRCU read-side critical sections is
> > perfectly legal.  But why not just try it and see?
> 
> Thanks for shedding light on nested SRCUs - it cures my pain working out
> the reason for how detecting nested SRCUs is added [1]. Now I see why it
> is out of kernel/rcu.

Just to be clear...  If the KVM guys want to impose a design rule that
SRCU read-side critical sections never be nested within their code,
that is a perfectly reasonable thing for them to do.

							Thanx, Paul

> Hillf
> 
> [1] https://lore.kernel.org/lkml/20220415004343.2203171-4-seanjc@google.com/
> 
> > > Does it make sense to add srcu_lock_acquire() in line with rwsem_acquire_read() if
> > > warning is expected but not triggered?
> > 
> > Please understand that while SRCU can often be used where an rwsem
> > might otherwise be used, SRCU is not an rwsem.  For one thing, rwsem
> > readers can deadlock in ways that SRCU reader cannot.
> > 
> > Now, I don't yet know of a non-destructive use case for partially
> > overlapping SRCU read-side critical sections, for example, if you
> > switched the two srcu_read_unlock() calls above.  But at the same
> > time, I cannot prove that there is no valid use case, not yet,
> > anyway.
> > 
> > 						Thanx, Paul
> > 
> > > Thanks
> > > Hillf
> > > 
> > > static inline void rcu_lock_acquire(struct lockdep_map *map)
> > > {
> > > 	lock_acquire(map, 0, 0, 2, 0, NULL, _THIS_IP_);
> > > }
> > > 
> > > static inline void srcu_lock_acquire(struct lockdep_map *map)
> > > {
> > > 	lock_acquire(map, 0, 0, 1, 0, NULL, _THIS_IP_);
> > > }
> >