Re: [PATCH] md/raid10: fix missing wakeup in wait_barrier_nolock
| Newsgroups | org.kernel.vger.linux-raid |
|---|---|
| Message-ID | <[email protected]> |
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] Lockless read of `conf->array_freeze_pending` lacks `READ_ONCE()`, violating kernel concurrency guidelines and potentially triggering KCSAN warnings. -- --- Patch [1]: [PATCH] md/raid10: fix missing wakeup in wait_barrier_nolock --- commit 3470079c0f4271a77d161b3a70575176ea1d322b Author: Zizhi Wo <[email protected]> md/raid10: fix missing wakeup in wait_barrier_nolock This commit fixes a hung task issue in RAID10 by ensuring that wait_barrier_nolock() properly wakes up a pending freeze_array(). It adds a wake-up condition for conf->array_freeze_pending when the pending counter is decremented. > diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c > index 0a3cfdd3f5df8..fad634a444f89 100644 > --- a/drivers/md/raid10.c > +++ b/drivers/md/raid10.c [ ... ] > @@ -996,7 +996,8 @@ static bool wait_barrier_nolock(struct r10conf *conf) > if (!read_seqretry(&conf->resync_lock, seq)) > return true; > > - if (atomic_dec_and_test(&conf->nr_pending)) > + if (atomic_dec_and_test(&conf->nr_pending) || > + conf->array_freeze_pending) [Severity: Low] Does this lockless read of conf->array_freeze_pending need a READ_ONCE() wrapper? Since freeze_array() modifies this variable under the resync_lock seqlock, reading it here locklessly might trigger KCSAN data race warnings or be subject to compiler optimizations. While atomic_dec_and_test() provides a memory barrier, kernel concurrency guidelines generally require READ_ONCE() for plain lockless reads of shared variables to prevent compiler optimizations. It looks like this pattern was copied from allow_barrier(), which also has a lockless read without READ_ONCE(): drivers/md/raid10.c:allow_barrier() { ... if ((atomic_dec_and_test(&conf->nr_pending)) || (conf->array_freeze_pending)) wake_up_barrier(conf); ... } Should both of these locations be updated to use READ_ONCE(conf->array_freeze_pending)? > wake_up_barrier(conf); > > return false; -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1