On Thu, Feb 20, 2025 at 11:11:04PM +0100, Marco Elver wrote: > On Thu, 20 Feb 2025 at 23:00, Paul E. McKenney <paulmck@xxxxxxxxxx> wrote: > > > > On Thu, Feb 06, 2025 at 07:10:09PM +0100, Marco Elver wrote: > > > Improve the existing annotations to properly support Clang's capability > > > analysis. > > > > > > The old annotations distinguished between RCU, RCU_BH, and RCU_SCHED. > > > However, it does not make sense to acquire rcu_read_lock_bh() after > > > rcu_read_lock() - annotate the _bh() and _sched() variants to also > > > acquire 'RCU', so that Clang (and also Sparse) can warn about it. > > > > You lost me on this one. What breaks if rcu_read_lock_bh() is invoked > > while rcu_read_lock() is in effect? > > I thought something like this does not make sense: > > rcu_read_lock_bh(); > .. > rcu_read_lock(); > .. > rcu_read_unlock(); > .. > rcu_read_unlock_bh(); If you have the choice, it is often better to do the rcu_read_lock() first and the rcu_read_lock_bh() second. > However, the inverse may well be something we might find somewhere in > the kernel? Suppose that one function walks an RCU-protected list, calling some function from some other subsystem on each element. Suppose that each element has another RCU protected list. It would be good if the two subsystems could just choose their desired flavor of RCU reader, without having to know about each other. > Another problem was that if we want to indicate that "RCU" read lock > is held, then we should just be able to write > "__must_hold_shared(RCU)", and it shouldn't matter if rcu_read_lock() > or rcu_read_lock_bh() was used. Previously each of them acquired their > own capability "RCU" and "RCU_BH" respectively. But rather, we're > dealing with one acquiring a superset of the other, and expressing > that is also what I attempted to solve. > Let me rethink this... Would it work to have just one sort of RCU reader, relying on a separate BH-disable capability for the additional semantics of rcu_read_lock_bh()? Thanx, Paul
