On Tue, Dec 14, 2021 at 12:24:34PM +0100, Frederic Weisbecker wrote:
> On Mon, Dec 06, 2021 at 10:55:29AM -0800, Paul E. McKenney wrote:
> > Hello, Frederic!
> >
> > KCSAN complains about the following when augmented by Marco's latest patch
> > series:
> >
> > [ 15.432187] ==================================================================
> > [ 15.440802] BUG: KCSAN: data-race in rcu_nocb_cb_kthread / rcu_nocb_gp_kthread
> > [ 15.441715]
> > [ 15.441895] read (marked) to 0xffff8a05df5acb50 of 1 bytes by task 153 on cpu 7:
> > [ 15.443781] rcu_nocb_gp_kthread+0x237/0x1180
> > [ 15.444272] kthread+0x29b/0x2b0
> > [ 15.444617] ret_from_fork+0x22/0x30
> > [ 15.445123]
> > [ 15.445280] no locks held by rcuog/12/153.
> > [ 15.445694] irq event stamp: 7379
> > [ 15.446063] hardirqs last enabled at (7379): [<ffffffffa8b1b23a>] _raw_spin_unlock_irqrestore+0x3a/0x70
> > [ 15.447870] hardirqs last disabled at (7378): [<ffffffffa75b14c2>] rcu_nocb_gp_kthread+0x2d2/0x1180
> > [ 15.449478] softirqs last enabled at (7232): [<ffffffffa74bf844>] __irq_exit_rcu+0x64/0xc0
> > [ 15.451430] softirqs last disabled at (7225): [<ffffffffa74bf844>] __irq_exit_rcu+0x64/0xc0
> > [ 15.452259]
> > [ 15.452418] write to 0xffff8a05df5acb50 of 1 bytes by task 169 on cpu 10:
> > [ 15.454395] rcu_nocb_cb_kthread+0x4b0/0x760
> > [ 15.454835] kthread+0x29b/0x2b0
> > [ 15.458271] ret_from_fork+0x22/0x30
> > [ 15.458657]
> > [ 15.458817] 1 lock held by rcuop/14/169:
> > [ 15.459220] #0: ffff8a05df5acc70 (&rdp->nocb_lock){-.-.}-{2:2}, at: rcu_nocb_cb_kthread+0x2ff/0x760
> > [ 15.460127] irq event stamp: 62
> > [ 15.460441] hardirqs last enabled at (61): [<ffffffffa74bf40a>] __local_bh_enable_ip+0xca/0x120
> > [ 15.461305] hardirqs last disabled at (62): [<ffffffffa75b2657>] rcu_nocb_cb_kthread+0x2e7/0x760
> > [ 15.462169] softirqs last enabled at (60): [<ffffffffa75adbed>] local_bh_enable+0xd/0x30
> > [ 15.462973] softirqs last disabled at (58): [<ffffffffa75ad35d>] local_bh_disable+0xd/0x30
> >
> > And gdb fingers these two accesses:
> >
> > (gdb) l*rcu_nocb_gp_kthread+0x237
> > 0xffffffff811b1427 is in rcu_nocb_gp_kthread (kernel/rcu/rcu_segcblist.h:71).
> > 66 }
> > 67
> > 68 static inline bool rcu_segcblist_test_flags(struct rcu_segcblist *rsclp,
> > 69 int flags)
> > 70 {
> > 71 return READ_ONCE(rsclp->flags) & flags;
> > 72 }
> > 73
> > 74 /*
> > 75 * Is the specified rcu_segcblist enabled, for example, not corresponding
> > (gdb) l*rcu_nocb_cb_kthread+0x4b0
> > 0xffffffff811b2820 is in rcu_nocb_cb_kthread (kernel/rcu/rcu_segcblist.h:59).
> > 54 }
> > 55
> > 56 static inline void rcu_segcblist_set_flags(struct rcu_segcblist *rsclp,
> > 57 int flags)
> > 58 {
> > 59 rsclp->flags |= flags;
> > 60 }
> > 61
> > 62 static inline void rcu_segcblist_clear_flags(struct rcu_segcblist *rsclp,
> > 63 int flags)
> >
> > Any reason not to turn that "rsclp->flags |= flags" into a WRITE_ONCE()?
> > Maybe a READ_ONCE() as well, if multiple CPUs can be updating this field
> > (but I hope not!).
>
> The write side update should be taken care of by locking and/or irqs
> disabling. But I'll check that more thoroughly for the next nocb update. I
> have a few related items to take care of after Neeraj's and your reviews.
Looking forward to seeing what you come up with!
Thanx, Paul
