Re: [PATCH v2] 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:

Pre-existing issues:
- [Low] Lockless plain reads of `mddev->recovery` will cause KCSAN data race warnings, similar to the ones fixed for `sb_flags`.
--

commit f286787fa23a14cb6ec022eb99574f7a85f9957d
Author: Chen Cheng <[email protected]>

md: use READ_ONCE() for lockless reads of sb_flags

This patch adds READ_ONCE() annotations to lockless reads of mddev->sb_flags
across md, raid1, raid5, and raid10. It aims to fix KCSAN data race warnings
caused by concurrent set_bit() operations on sb_flags.

> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 096bb64e87bd5..c5c50640b6849 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -6832,7 +6832,7 @@ int md_run(struct mddev *mddev)
>  		set_bit(MD_RECOVERY_RECOVER, &mddev->recovery);
>  	set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);

[Severity: Low]
This is a pre-existing issue, but since this patch targets KCSAN warnings for
atomic bitmasks, could plain lockless reads of mddev->recovery trigger similar
data races?

There are places where mddev->recovery is read without locks and without
READ_ONCE(), potentially causing KCSAN noise against set_bit() updates:

drivers/md/md.c:md_sync_action() {
    ...
    unsigned long recovery = mddev->recovery;
    ...
}

drivers/md/md.c:set_bitmap_file() {
    ...
    if (mddev->recovery || mddev->sync_thread)
    ...
}

drivers/md/md.c:update_array_info() {
    ...
    if (mddev->recovery || mddev->sync_thread) {
    ...
}

Should READ_ONCE() also be applied to these reads of mddev->recovery to
provide a more complete fix?

> -	if (mddev->sb_flags)
> +	if (READ_ONCE(mddev->sb_flags))
>  		md_update_sb(mddev, 0);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1