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

From: Abd-Alrhman Masalkhi

Date: Sat Jun 27 2026 - 07:25:55 EST



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.

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

--
Best Regards,
Abd-Alrhman