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

在 2026/6/23 16:29, [email protected] 写道:
> 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?
> 

There isn't data races in strong consistency model arch..
raidXd() would be call periodically, so , steal value is ok..
But I see kcsan report, and I think use read_once to read would be better.

> 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()
> 

raidXd() all call md_check_recovery().

>> 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?
> 

Only add protect for lockless path. __md_stop_writes() already protected 
by mddev_lock.