Linux RAID subsystem development
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Chen Cheng" <chencheng@fnnas.com>
Cc: linux-raid@vger.kernel.org, yukuai@fygo.io
Subject: Re: [PATCH] md: use READ_ONCE() for lockless reads of sb_flags
Date: Tue, 23 Jun 2026 08:29:14 +0000	[thread overview]
Message-ID: <20260623082915.1B8A71F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260623081942.2488045-1-chencheng@fnnas.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Incomplete fix: The patch claims to resolve KCSAN data races on `sb_flags` in md, raid1, raid5, and raid10, but completely omits the necessary `READ_ONCE()` updates to the `raid1`, `raid5`, and `raid10` subsystems.
--

commit adcaa558925c24a70a377065ea735febac939f0b
Author: Chen Cheng <chencheng@fnnas.com>

md: use READ_ONCE() for lockless reads of sb_flags

This commit adds READ_ONCE() wrappers around lockless reads of mddev->sb_flags
to prevent KCSAN data race warnings. The commit message states it addresses
issues across md, raid1, raid5, and raid10.

>     sb_flags is checked without a lock in md, raid1, raid5, and raid10.
>     KCSAN reports these reads as data races.
>     
>     The write side uses atomic bit ops.
>     The read side still has plain loads in a few places.
>     
>     Use READ_ONCE() for the lockless reads of sb_flags.

[Severity: Medium]
Does this patch completely address the data races mentioned in the commit
message?

The commit message indicates that sb_flags is checked without a lock in md,
raid1, raid5, and raid10, and includes KCSAN traces for raid1d, raid5d, and
raid10d. However, the provided diff only includes changes to drivers/md/md.c.

Are there additional READ_ONCE() updates missing for the raid1, raid5, and
raid10 subsystems? For example:
- drivers/md/raid1.c:raid1d()
- drivers/md/raid5.c:raid5d(), ops_run_io(), and reshape_request()
- drivers/md/raid10.c:raid10d() and reshape_request()

> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 096bb64e87bd5..f83cca895f85f 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
[ ... ]
> @@ -10425,7 +10425,7 @@ void md_check_recovery(struct mddev *mddev)
>  			spin_unlock(&mddev->lock);
>  		}
>  
> -		if (mddev->sb_flags)
> +		if (READ_ONCE(mddev->sb_flags))
>  			md_update_sb(mddev, 0);
>  
>  		/*

[Severity: Medium]
Are there other plain reads of sb_flags remaining in this file?

Looking at drivers/md/md.c:__md_stop_writes(), there appears to be another
plain lockless read of mddev->sb_flags. Should that also be wrapped with
READ_ONCE() to fully eliminate the KCSAN warnings?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260623081942.2488045-1-chencheng@fnnas.com?part=1

  reply	other threads:[~2026-06-23  8:29 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-23  8:19 [PATCH] md: use READ_ONCE() for lockless reads of sb_flags Chen Cheng
2026-06-23  8:29 ` sashiko-bot [this message]
2026-06-23  8:44   ` Chen Cheng
  -- strict thread matches above, loose matches on Subject: below --
2026-07-11  2:51 Chen Cheng
2026-07-11 18:16 ` yu kuai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260623082915.1B8A71F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=chencheng@fnnas.com \
    --cc=linux-raid@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=yukuai@fygo.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox