On Fri, Mar 09, 2018 at 08:51:54AM +0800, Boqun Feng wrote:
> On Thu, Mar 08, 2018 at 04:34:43PM -0800, Paul E. McKenney wrote:
> > On Fri, Mar 09, 2018 at 06:52:34AM +0800, Boqun Feng wrote:
> > > On Fri, Mar 09, 2018 at 05:41:27AM +0800, kbuild test robot wrote:
> > > > tree:   
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git 
> > > > rcu/dev
> > > > head:   b8909ec707bb5beba94e7c7d62cc6b3115ceae50
> > > > commit: b8909ec707bb5beba94e7c7d62cc6b3115ceae50 [39/39] rcu: Protect 
> > > > all sync_rcu_preempt_exp_done() with rcu_node lock
> > > > reproduce:
> > > >         # apt-get install sparse
> > > >         git checkout b8909ec707bb5beba94e7c7d62cc6b3115ceae50
> > > >         make ARCH=x86_64 allmodconfig
> > > >         make C=1 CF=-D__CHECK_ENDIAN__
> > > > 
> > > > 
> > > > sparse warnings: (new ones prefixed by >>)
> > > > 
> > > [...]
> > > >    kernel/rcu/tree.c:345:6: sparse: symbol 
> > > > 'rcu_dynticks_curr_cpu_in_eqs' was not declared. Should it be static?
> > > >    kernel/rcu/tree.c:3953:21: sparse: incorrect type in argument 1 
> > > > (different modifiers) @@    expected int ( *threadfn )( ... ) @@    got 
> > > > int ( [noreint ( *threadfn )( ... ) @@
> > > >    kernel/rcu/tree.c:3953:21:    expected int ( *threadfn )( ... )
> > > >    kernel/rcu/tree.c:3953:21:    got int ( [noreturn] *<noident> )( ... 
> > > > )
> > > > >> kernel/rcu/tree_exp.h:163:9: sparse: incorrect type in argument 1 
> > > > >> (different modifiers) @@    expected struct lockdep_map const *lock 
> > > > >> @@    got strustruct lockdep_map const *lock @@
> > > >    kernel/rcu/tree_exp.h:163:9:    expected struct lockdep_map const 
> > > > *lock
> > > >    kernel/rcu/tree_exp.h:163:9:    got struct lockdep_map [noderef] 
> > > > *<noident>
> > > >    kernel/rcu/tree.c:1752:9: sparse: context imbalance in 
> > > > 'rcu_start_future_gp' - different lock contexts for basic block
> > > >    kernel/rcu/tree.c:2786:9: sparse: context imbalance in 
> > > > 'force_qs_rnp' - different lock contexts for basic block
> > > >    kernel/rcu/tree.c:2849:25: sparse: context imbalance in 
> > > > 'force_quiescent_state' - unexpected unlock
> > > >    kernel/rcu/tree_exp.h:203:9: sparse: too many warnings
> > > > 
> > > > vim +163 kernel/rcu/tree_exp.h
> > > > 
> > > >    151  
> > > >    152  /*
> > > >    153   * Return non-zero if there is no RCU expedited grace period in 
> > > > progress
> > > >    154   * for the specified rcu_node structure, in other words, if all 
> > > > CPUs and
> > > >    155   * tasks covered by the specified rcu_node structure have done 
> > > > their bit
> > > >    156   * for the current expedited grace period.  Works only for 
> > > > preemptible
> > > >    157   * RCU -- other RCU implementation use other means.
> > > >    158   *
> > > >    159   * Caller must hold the specificed rcu_node structure's ->lock
> > > >    160   */
> > > >    161  static bool sync_rcu_preempt_exp_done(struct rcu_node *rnp)
> > > >    162  {
> > > >  > 163          lockdep_assert_held(&rnp->lock);
> > > 
> > > OK, so we need ACCESS_PRIVATE() to visit ->lock in rcu_node. I will
> > > introduce something like:
> > > 
> > >   #define rcu_node_lock_assert_held(rnp) 
> > > lockdep_assert_held(&ACCESS_PRIVATE(rnp, lock))
> > > 
> > > in v3.
> > 
> > Or use this, which is in kernel/rcu/rcu.h:
> > 
> > #define raw_lockdep_assert_held_rcu_node(p)                         \
> >     lockdep_assert_held(&ACCESS_PRIVATE(p, lock))
> 
> Good point, thank you for pointing this out ;-)

"It is a service that I provide."  ;-)

I removed your earlier patch in anticipation of the next version.

                                                        Thanx, Paul

Reply via email to