Re: [PATCH] rcu: Use wrapper for lockdep asserts
On Wed, Jan 17, 2018 at 04:41:43PM -0800, Matthew Wilcox wrote: > On Wed, Jan 17, 2018 at 03:29:13PM -0800, Paul E. McKenney wrote: > > On Wed, Jan 17, 2018 at 06:24:30AM -0800, Matthew Wilcox wrote: > > > > > > From: Matthew Wilcox> > > > > > Commits c0b334c5bfa9 and ea9b0c8a26a2 introduced new sparse warnings > > > by accessing rcu_node->lock directly and ignoring the __private > > > marker. Introduce a new wrapper and use it. Also fix a similar problem > > > in srcutree.c introduced by a3883df3935e. > > > > > > Signed-off-by: Matthew Wilcox > > > > Good catch! Applied for review and testing. > > > > For some reason, I was expecting 0day to catch this sort of thing... > > Funny you should say, it was 0day which pointed it out to me! I changed > lockdep to take a const struct lockdep_map *lock instead of a plain > struct lockdep_map *lock and I got a whinge from 0day that I'd changed > some of the warnings, so I looked into it in case it was my fault. Nary a peep at me. Regardless, glad you caught it, and thank you for the fix! Thanx, Paul
Re: [PATCH] rcu: Use wrapper for lockdep asserts
On Wed, Jan 17, 2018 at 04:41:43PM -0800, Matthew Wilcox wrote: > On Wed, Jan 17, 2018 at 03:29:13PM -0800, Paul E. McKenney wrote: > > On Wed, Jan 17, 2018 at 06:24:30AM -0800, Matthew Wilcox wrote: > > > > > > From: Matthew Wilcox > > > > > > Commits c0b334c5bfa9 and ea9b0c8a26a2 introduced new sparse warnings > > > by accessing rcu_node->lock directly and ignoring the __private > > > marker. Introduce a new wrapper and use it. Also fix a similar problem > > > in srcutree.c introduced by a3883df3935e. > > > > > > Signed-off-by: Matthew Wilcox > > > > Good catch! Applied for review and testing. > > > > For some reason, I was expecting 0day to catch this sort of thing... > > Funny you should say, it was 0day which pointed it out to me! I changed > lockdep to take a const struct lockdep_map *lock instead of a plain > struct lockdep_map *lock and I got a whinge from 0day that I'd changed > some of the warnings, so I looked into it in case it was my fault. Nary a peep at me. Regardless, glad you caught it, and thank you for the fix! Thanx, Paul
Re: [PATCH] rcu: Use wrapper for lockdep asserts
On Wed, Jan 17, 2018 at 03:29:13PM -0800, Paul E. McKenney wrote: > On Wed, Jan 17, 2018 at 06:24:30AM -0800, Matthew Wilcox wrote: > > > > From: Matthew Wilcox> > > > Commits c0b334c5bfa9 and ea9b0c8a26a2 introduced new sparse warnings > > by accessing rcu_node->lock directly and ignoring the __private > > marker. Introduce a new wrapper and use it. Also fix a similar problem > > in srcutree.c introduced by a3883df3935e. > > > > Signed-off-by: Matthew Wilcox > > Good catch! Applied for review and testing. > > For some reason, I was expecting 0day to catch this sort of thing... Funny you should say, it was 0day which pointed it out to me! I changed lockdep to take a const struct lockdep_map *lock instead of a plain struct lockdep_map *lock and I got a whinge from 0day that I'd changed some of the warnings, so I looked into it in case it was my fault.
Re: [PATCH] rcu: Use wrapper for lockdep asserts
On Wed, Jan 17, 2018 at 03:29:13PM -0800, Paul E. McKenney wrote: > On Wed, Jan 17, 2018 at 06:24:30AM -0800, Matthew Wilcox wrote: > > > > From: Matthew Wilcox > > > > Commits c0b334c5bfa9 and ea9b0c8a26a2 introduced new sparse warnings > > by accessing rcu_node->lock directly and ignoring the __private > > marker. Introduce a new wrapper and use it. Also fix a similar problem > > in srcutree.c introduced by a3883df3935e. > > > > Signed-off-by: Matthew Wilcox > > Good catch! Applied for review and testing. > > For some reason, I was expecting 0day to catch this sort of thing... Funny you should say, it was 0day which pointed it out to me! I changed lockdep to take a const struct lockdep_map *lock instead of a plain struct lockdep_map *lock and I got a whinge from 0day that I'd changed some of the warnings, so I looked into it in case it was my fault.
Re: [PATCH] rcu: Use wrapper for lockdep asserts
On Wed, Jan 17, 2018 at 06:24:30AM -0800, Matthew Wilcox wrote: > > From: Matthew Wilcox> > Commits c0b334c5bfa9 and ea9b0c8a26a2 introduced new sparse warnings > by accessing rcu_node->lock directly and ignoring the __private > marker. Introduce a new wrapper and use it. Also fix a similar problem > in srcutree.c introduced by a3883df3935e. > > Signed-off-by: Matthew Wilcox Good catch! Applied for review and testing. For some reason, I was expecting 0day to catch this sort of thing... Thanx, Paul > diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h > index 59c471de342a..30db0a581628 100644 > --- a/kernel/rcu/rcu.h > +++ b/kernel/rcu/rcu.h > @@ -360,7 +360,7 @@ do { > \ > } while (0) > > #define raw_spin_unlock_irqrestore_rcu_node(p, flags) > \ > - raw_spin_unlock_irqrestore(_PRIVATE(p, lock), flags) \ > + raw_spin_unlock_irqrestore(_PRIVATE(p, lock), flags) > > #define raw_spin_trylock_rcu_node(p) \ > ({ \ > @@ -371,6 +371,9 @@ do { > \ > ___locked; \ > }) > > +#define raw_lockdep_assert_held_rcu_node(p) \ > + lockdep_assert_held(_PRIVATE(p, lock)) > + > #endif /* #if defined(SRCU) || !defined(TINY_RCU) */ > > #ifdef CONFIG_TINY_RCU > diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c > index 6d5880089ff6..b7bdd3ff4611 100644 > --- a/kernel/rcu/srcutree.c > +++ b/kernel/rcu/srcutree.c > @@ -412,7 +412,7 @@ static void srcu_gp_start(struct srcu_struct *sp) > struct srcu_data *sdp = this_cpu_ptr(sp->sda); > int state; > > - lockdep_assert_held(>lock); > + lockdep_assert_held(_PRIVATE(sp, lock)); > WARN_ON_ONCE(ULONG_CMP_GE(sp->srcu_gp_seq, sp->srcu_gp_seq_needed)); > rcu_segcblist_advance(>srcu_cblist, > rcu_seq_current(>srcu_gp_seq)); > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c > index f9c0ca2ccf0c..aba8dd0cc1f8 100644 > --- a/kernel/rcu/tree.c > +++ b/kernel/rcu/tree.c > @@ -1245,7 +1245,7 @@ static int rcu_is_cpu_rrupt_from_idle(void) > */ > static void rcu_gpnum_ovf(struct rcu_node *rnp, struct rcu_data *rdp) > { > - lockdep_assert_held(>lock); > + raw_lockdep_assert_held_rcu_node(rnp); > if (ULONG_CMP_LT(READ_ONCE(rdp->gpnum) + ULONG_MAX / 4, rnp->gpnum)) > WRITE_ONCE(rdp->gpwrap, true); > if (ULONG_CMP_LT(rdp->rcu_iw_gpnum + ULONG_MAX / 4, rnp->gpnum)) > @@ -1712,7 +1712,7 @@ void rcu_cpu_stall_reset(void) > static unsigned long rcu_cbs_completed(struct rcu_state *rsp, > struct rcu_node *rnp) > { > - lockdep_assert_held(>lock); > + raw_lockdep_assert_held_rcu_node(rnp); > > /* >* If RCU is idle, we just wait for the next grace period. > @@ -1759,7 +1759,7 @@ rcu_start_future_gp(struct rcu_node *rnp, struct > rcu_data *rdp, > bool ret = false; > struct rcu_node *rnp_root = rcu_get_root(rdp->rsp); > > - lockdep_assert_held(>lock); > + raw_lockdep_assert_held_rcu_node(rnp); > > /* >* Pick up grace-period number for new callbacks. If this > @@ -1887,7 +1887,7 @@ static bool rcu_accelerate_cbs(struct rcu_state *rsp, > struct rcu_node *rnp, > { > bool ret = false; > > - lockdep_assert_held(>lock); > + raw_lockdep_assert_held_rcu_node(rnp); > > /* If no pending (not yet ready to invoke) callbacks, nothing to do. */ > if (!rcu_segcblist_pend_cbs(>cblist)) > @@ -1927,7 +1927,7 @@ static bool rcu_accelerate_cbs(struct rcu_state *rsp, > struct rcu_node *rnp, > static bool rcu_advance_cbs(struct rcu_state *rsp, struct rcu_node *rnp, > struct rcu_data *rdp) > { > - lockdep_assert_held(>lock); > + raw_lockdep_assert_held_rcu_node(rnp); > > /* If no pending (not yet ready to invoke) callbacks, nothing to do. */ > if (!rcu_segcblist_pend_cbs(>cblist)) > @@ -1955,7 +1955,7 @@ static bool __note_gp_changes(struct rcu_state *rsp, > struct rcu_node *rnp, > bool ret; > bool need_gp; > > - lockdep_assert_held(>lock); > + raw_lockdep_assert_held_rcu_node(rnp); > > /* Handle the ends of any preceding grace periods first. */ > if (rdp->completed == rnp->completed && > @@ -2380,7 +2380,7 @@ static bool > rcu_start_gp_advanced(struct rcu_state *rsp, struct rcu_node *rnp, > struct rcu_data *rdp) > { > - lockdep_assert_held(>lock); > + raw_lockdep_assert_held_rcu_node(rnp); > if (!rsp->gp_kthread || !cpu_needs_another_gp(rsp, rdp)) { > /* >* Either we have not
Re: [PATCH] rcu: Use wrapper for lockdep asserts
On Wed, Jan 17, 2018 at 06:24:30AM -0800, Matthew Wilcox wrote: > > From: Matthew Wilcox > > Commits c0b334c5bfa9 and ea9b0c8a26a2 introduced new sparse warnings > by accessing rcu_node->lock directly and ignoring the __private > marker. Introduce a new wrapper and use it. Also fix a similar problem > in srcutree.c introduced by a3883df3935e. > > Signed-off-by: Matthew Wilcox Good catch! Applied for review and testing. For some reason, I was expecting 0day to catch this sort of thing... Thanx, Paul > diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h > index 59c471de342a..30db0a581628 100644 > --- a/kernel/rcu/rcu.h > +++ b/kernel/rcu/rcu.h > @@ -360,7 +360,7 @@ do { > \ > } while (0) > > #define raw_spin_unlock_irqrestore_rcu_node(p, flags) > \ > - raw_spin_unlock_irqrestore(_PRIVATE(p, lock), flags) \ > + raw_spin_unlock_irqrestore(_PRIVATE(p, lock), flags) > > #define raw_spin_trylock_rcu_node(p) \ > ({ \ > @@ -371,6 +371,9 @@ do { > \ > ___locked; \ > }) > > +#define raw_lockdep_assert_held_rcu_node(p) \ > + lockdep_assert_held(_PRIVATE(p, lock)) > + > #endif /* #if defined(SRCU) || !defined(TINY_RCU) */ > > #ifdef CONFIG_TINY_RCU > diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c > index 6d5880089ff6..b7bdd3ff4611 100644 > --- a/kernel/rcu/srcutree.c > +++ b/kernel/rcu/srcutree.c > @@ -412,7 +412,7 @@ static void srcu_gp_start(struct srcu_struct *sp) > struct srcu_data *sdp = this_cpu_ptr(sp->sda); > int state; > > - lockdep_assert_held(>lock); > + lockdep_assert_held(_PRIVATE(sp, lock)); > WARN_ON_ONCE(ULONG_CMP_GE(sp->srcu_gp_seq, sp->srcu_gp_seq_needed)); > rcu_segcblist_advance(>srcu_cblist, > rcu_seq_current(>srcu_gp_seq)); > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c > index f9c0ca2ccf0c..aba8dd0cc1f8 100644 > --- a/kernel/rcu/tree.c > +++ b/kernel/rcu/tree.c > @@ -1245,7 +1245,7 @@ static int rcu_is_cpu_rrupt_from_idle(void) > */ > static void rcu_gpnum_ovf(struct rcu_node *rnp, struct rcu_data *rdp) > { > - lockdep_assert_held(>lock); > + raw_lockdep_assert_held_rcu_node(rnp); > if (ULONG_CMP_LT(READ_ONCE(rdp->gpnum) + ULONG_MAX / 4, rnp->gpnum)) > WRITE_ONCE(rdp->gpwrap, true); > if (ULONG_CMP_LT(rdp->rcu_iw_gpnum + ULONG_MAX / 4, rnp->gpnum)) > @@ -1712,7 +1712,7 @@ void rcu_cpu_stall_reset(void) > static unsigned long rcu_cbs_completed(struct rcu_state *rsp, > struct rcu_node *rnp) > { > - lockdep_assert_held(>lock); > + raw_lockdep_assert_held_rcu_node(rnp); > > /* >* If RCU is idle, we just wait for the next grace period. > @@ -1759,7 +1759,7 @@ rcu_start_future_gp(struct rcu_node *rnp, struct > rcu_data *rdp, > bool ret = false; > struct rcu_node *rnp_root = rcu_get_root(rdp->rsp); > > - lockdep_assert_held(>lock); > + raw_lockdep_assert_held_rcu_node(rnp); > > /* >* Pick up grace-period number for new callbacks. If this > @@ -1887,7 +1887,7 @@ static bool rcu_accelerate_cbs(struct rcu_state *rsp, > struct rcu_node *rnp, > { > bool ret = false; > > - lockdep_assert_held(>lock); > + raw_lockdep_assert_held_rcu_node(rnp); > > /* If no pending (not yet ready to invoke) callbacks, nothing to do. */ > if (!rcu_segcblist_pend_cbs(>cblist)) > @@ -1927,7 +1927,7 @@ static bool rcu_accelerate_cbs(struct rcu_state *rsp, > struct rcu_node *rnp, > static bool rcu_advance_cbs(struct rcu_state *rsp, struct rcu_node *rnp, > struct rcu_data *rdp) > { > - lockdep_assert_held(>lock); > + raw_lockdep_assert_held_rcu_node(rnp); > > /* If no pending (not yet ready to invoke) callbacks, nothing to do. */ > if (!rcu_segcblist_pend_cbs(>cblist)) > @@ -1955,7 +1955,7 @@ static bool __note_gp_changes(struct rcu_state *rsp, > struct rcu_node *rnp, > bool ret; > bool need_gp; > > - lockdep_assert_held(>lock); > + raw_lockdep_assert_held_rcu_node(rnp); > > /* Handle the ends of any preceding grace periods first. */ > if (rdp->completed == rnp->completed && > @@ -2380,7 +2380,7 @@ static bool > rcu_start_gp_advanced(struct rcu_state *rsp, struct rcu_node *rnp, > struct rcu_data *rdp) > { > - lockdep_assert_held(>lock); > + raw_lockdep_assert_held_rcu_node(rnp); > if (!rsp->gp_kthread || !cpu_needs_another_gp(rsp, rdp)) { > /* >* Either we have not yet spawned the grace-period > @@ -2442,7