Re: [PATCH] md: use READ_ONCE() for lockless reads of sb_flags

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 <[email protected]>

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/[email protected]?part=1