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

在 2026/6/23 19:25, [email protected] 写道:
> 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);
> 

Only md_sync_action() do lockless u64 plain read, READ_ONCE could be 
safer in here ..