On Wed, Dec 10, 2025 at 10:50:11PM +0100, Marco Elver wrote: > On Wed, 10 Dec 2025 at 20:30, Paul E. McKenney <[email protected]> wrote: > > On Thu, Nov 20, 2025 at 04:09:39PM +0100, Marco Elver wrote: > > > Improve the existing annotations to properly support Clang's context > > > analysis. > > > > > > The old annotations distinguished between RCU, RCU_BH, and RCU_SCHED; > > > however, to more easily be able to express that "hold the RCU read lock" > > > without caring if the normal, _bh(), or _sched() variant was used we'd > > > have to remove the distinction of the latter variants: change the _bh() > > > and _sched() variants to also acquire "RCU". > > > > > > When (and if) we introduce context guards to denote more generally that > > > "IRQ", "BH", "PREEMPT" contexts are disabled, it would make sense to > > > acquire these instead of RCU_BH and RCU_SCHED respectively. > > ^
"I can't read!" ;-) > > > The above change also simplified introducing __guarded_by support, where > > > only the "RCU" context guard needs to be held: introduce __rcu_guarded, > > > where Clang's context analysis warns if a pointer is dereferenced > > > without any of the RCU locks held, or updated without the appropriate > > > helpers. > > > > > > The primitives rcu_assign_pointer() and friends are wrapped with > > > context_unsafe(), which enforces using them to update RCU-protected > > > pointers marked with __rcu_guarded. > > > > > > Signed-off-by: Marco Elver <[email protected]> > > > > Good reminder! I had lost track of this series. > > > > My big questions here are: > > > > o What about RCU readers using (say) preempt_disable() instead > > of rcu_read_lock_sched()? > > The infrastructure that is being built up in this series will be able > to support this, it's "just" a matter of enhancing our various > interfaces/macros to use the right annotations, and working out which > kinds of contexts we want to support. There are the obvious > candidates, which this series is being applied to, as a starting > point, but longer-term there are other kinds of context rules that can > be checked with this context analysis. However, I think we have to > start somewhere. > > > o What about RCU readers using local_bh_disable() instead of > > rcu_read_lock_sched()? > > Same as above; this requires adding the necessary annotations to the > BH-disabling/enabling primitives. > > > And keeping in mind that such readers might start in assembly language. > > We can handle this by annotating the C functions invoked from assembly > with attributes like __must_hold_shared(RCU) or > __releases_shared(RCU) (if the callee is expected to release the RCU > read lock / re-enable preemption / etc.) or similar. > > > One reasonable approach is to require such readers to use something like > > rcu_dereference_all() or rcu_dereference_all_check(), which could then > > have special dispensation to instead rely on run-time checks. > > Agree. The current infrastructure encourages run-time checks where the > static analysis cannot be helped sufficiently otherwise (see patch: > "lockdep: Annotate lockdep assertions for context analysis"). OK, very good. > > Another more powerful approach would be to make this facility also > > track preemption, interrupt, NMI, and BH contexts. > > > > Either way could be a significant improvement over what we have now. > > > > Thoughts? > > The current infrastructure is powerful enough to allow for tracking > more contexts, such as interrupt, NMI, and BH contexts, and as I > hinted above, would be nice to eventually get to! But I think this is > also a question of how much do we want to front-load for this to be > useful, and what should incrementally be enhanced while the baseline > infrastructure is already available. > > I think the current series is the baseline required support to be > useful to a large fraction of "normal" code in the kernel. Makes sense to me! > On a whole, my strategy was to get to a point where maintainers and > developers can start using context analysis where appropriate, but at > the same time build up and incrementally add more supported contexts > in parallel. There's also a good chance that, once baseline support > lands, more interested parties contribute and things progress faster > (or so I'd hope :-)). I know that feelling! ;-) OK, for this patch and the SRCU patch based on a quick once-over: Acked-by: Paul E. McKenney <[email protected]> Thanx, Paul
