Re: [PATCH] md: protect read mddev->recovery in md_sync_action()

[Chen Cheng]

在 2026/6/27 19:25, Abd-Alrhman Masalkhi 写道:
> 
> Hi Chen,
> 
> On Sat, Jun 27, 2026 at 18:52 +0800, Chen Cheng wrote:
>> 在 2026/6/27 18:46, Abd-Alrhman Masalkhi 写道:
>>> On Sat, Jun 27, 2026 at 18:29 +0800, Chen Cheng wrote:
>>>> From: Chen Cheng <chencheng@xxxxxxxxx>
>>>>
>>>> md_sync_action() read mddev->recovery in lockless path, use READ_ONCE()
>>>> instead of u64 plain read.
>>>>
>>> unisgned long is not always u64...
>>> I can not see what it fixes, is this just to silence KCSAN? If so,
>>> please say so in the commit message.
>>
>> I need to find KCSAN report , I lost this one, but I think it's easy to
>> find locklessly path to call md_sync_action(), so. READ_ONCE is need.
>>
> Thanks. Adding the KCSAN report would be helpful.
> 

hi Abd-Alrhman

Uh... a apologize for my mislead . it's truelly report by review-bot , 
and after double check ,  I think it wouldn't be reproduce in my x86-64 
machine , because 8B in x64-arch load is atomic +align..

thanks,
cheng.

>>>
>>>> Fixes: e792a4c2156a3 ("md: add new helpers for sync_action")
>>>>
>>>> Signed-off-by: Chen Cheng <chencheng@xxxxxxxxx>
>>>> ---
>>>>    drivers/md/md.c | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>>>> index c5c50640b684..f4415c1a79d9 100644
>>>> --- a/drivers/md/md.c
>>>> +++ b/drivers/md/md.c
>>>> @@ -5074,11 +5074,11 @@ static enum sync_action md_get_active_sync_action(struct mddev *mddev)
>>>>    	return is_recover ? ACTION_RECOVER : ACTION_IDLE;
>>>>    }
>>>>    
>>>>    enum sync_action md_sync_action(struct mddev *mddev)
>>>>    {
>>>> -	unsigned long recovery = mddev->recovery;
>>>> +	unsigned long recovery = READ_ONCE(mddev->recovery);
>>>>    	enum sync_action active_action;
>>>>    
>>>>    	/*
>>>>    	 * frozen has the highest priority, means running sync_thread will be
>>>>    	 * stopped immediately, and no new sync_thread can start.
>>>> -- 
>>>> 2.54.0
>>>>
>>>
>>
>