* 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. )

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

Reply via email to