On Tue, Mar 06, 2018 at 09:47:38AM +0100, Ingo Molnar wrote:
>
> * Paul E. McKenney <paul...@linux.vnet.ibm.com> wrote:
>
> > > > But if we look at the bigger API picture:
> > > >
> > > > !PREEMPT_RCU PREEMPT_RCU=y
> > > > rcu_read_lock(): atomic preemptible
> > > > rcu_read_lock_sched(): atomic atomic
> > > > srcu_read_lock(): preemptible preemptible
> > > >
> > > > Then we could maintain full read side API flexibility by making
> > > > PREEMPT_RCU=y the
> > > > only model, merging it with SRCU and using these main read side APIs:
> > > >
> > > > rcu_read_lock_preempt_disable(): atomic
> > > > rcu_read_lock(): preemptible
> >
> > One issue with merging SRCU into rcu_read_lock() is the general blocking
> > within
> > SRCU readers. Once merged in, these guys block everyone. We should focus
> > initially on the non-SRCU variants.
> >
> > On the other hand, Linus's suggestion of merging rcu_read_lock_sched()
> > into rcu_read_lock() just might be feasible. If that really does pan
> > out, we end up with the following:
> >
> > !PREEMPT PREEMPT=y
> > rcu_read_lock(): atomic preemptible
> > srcu_read_lock(): preemptible preemptible
> >
> > In this model, rcu_read_lock_sched() maps to preempt_disable() and (as
> > you say above) rcu_read_lock_bh() maps to local_bh_disable(). The way
> > this works is that in PREEMPT=y kernels, synchronize_rcu() waits not
> > only for RCU read-side critical sections, but also for regions of code
> > with preemption disabled. The main caveat seems to be that there be an
> > assumed point of preemptibility between each interrupt and each softirq
> > handler, which should be OK.
> >
> > There will be some adjustments required for lockdep-RCU, but that should
> > be reasonably straightforward.
> >
> > Seem reasonable?
>
> Yes, that approach sounds very reasonable to me: it is similar to what we do
> on
> the locking side as well, where we have 'atomic' variants (spinlocks/rwlocks)
> and
> 'sleeping' variants (mutexes, rwsems, etc.).
>
> ( This means there will be more automatic coupling between BH and preempt
> critical
> sections and RCU models not captured via explicit RCU-namespace APIs, but
> that
> should be OK I think. )
Thus far, I have been unable to prove that it cannot work, which is about
as good as it gets at this stage. So here is hoping! ;-)
I will look at your later corrected message, but will gratefully accept
your offer of help with the naming transition.
Thanx, Paul
> A couple of small side notes:
>
> - Could we please also clean up the namespace of the synchronization APIs
> and
> change them all to an rcu_ prefix, like all the other RCU APIs are? Right
> now
> have a mixture like rcu_read_lock() but synchronize_rcu(), while I'd reall
> love
> to be able to do:
>
> git grep '\<rcu_' ...
>
> ... to see RCU API usage within a particular kernel area. This would also
> clean
> up some of the internal inconsistencies like having 'struct
> rcu_synchronize'.
>
> - If we are cleaning up the write side APIs, could we move over to a _wait
> nomenclature, i.e. rcu_wait*()?
>
> I.e. the new RCU namespace would be something like:
>
> rcu_read_lock => rcu_read_lock # unchanged
> rcu_read_unlock => rcu_read_unlock # unchanged
>
> call_rcu => rcu_call_rcu
> call_rcu_bh => rcu_call_bh
> call_rcu_sched => rcu_call_sched
>
> synchronize_rcu => rcu_wait_
> synchronize_rcu_bh => rcu_wait_bh
> synchronize_rcu_bh_expedited => rcu_wait_expedited_bh
> synchronize_rcu_expedited => rcu_wait_expedited
> synchronize_rcu_mult => rcu_wait_mult
> synchronize_rcu_sched => rcu_wait_sched
> synchronize_rcu_tasks => rcu_wait_tasks
>
> srcu_read_lock => srcu_read_lock # unchanged
> srcu_read_unlock => srcu_read_unlock # unchanged
>
> synchronize_srcu => srcu_wait
> synchronize_srcu_expedited => srcu_wait_expedited
>
> Note that due to the prefix approach we gain various new patterns:
>
> git grep rcu_wait # matches both rcu and srcu
> git grep rcu_wait # matches all RCU waiting variants
> git grep wait_expedited # matches all expedited variants
>
> ... which all increase the organization of the namespace.
>
> - While we are at it, the two RCU-state API variants, while rarely used, are
> named in a pretty obscure, disconnected fashion as well. A much better
> naming
> would be:
>
> get_state_synchronize_rcu => rcu_get_state
> cond_synchronize_rcu => rcu_wait_state
>
> ... or so. This would also move them into the new, unified rcu_ prefix
> namespace.
>
> Note how consistent and hierarchical the new RCU API namespace is:
>
> <subsystem-prefix>_<verb>[_<qualifier[s]>]
>
> If you agree with the overall concept of this I'd be glad to help out with
> scripting & testing the RCU namespace transition safely in an unintrusive
> fashion
> once you've done the model unification work, with compatibility defines to
> not
> create conflicts, churn and pain, etc.
>
> Thanks,
>
> Ingo
>
