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

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

[Chen Cheng]

From: Chen Cheng <chencheng@fnnas.com>

md_sync_action() read mddev->recovery in lockless path, use READ_ONCE()
instead of u64 plain read.

Fixes: e792a4c2156a3 ("md: add new helpers for sync_action")

Signed-off-by: Chen Cheng <chencheng@fnnas.com>
---
 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

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

[Abd-Alrhman Masalkhi]

On Sat, Jun 27, 2026 at 18:29 +0800, Chen Cheng wrote:
> From: Chen Cheng <chencheng@fnnas.com>
>
> 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.

> Fixes: e792a4c2156a3 ("md: add new helpers for sync_action")
>
> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
> ---
>  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

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

[Chen Cheng]

在 2026/6/27 18:46, Abd-Alrhman Masalkhi 写道:
> On Sat, Jun 27, 2026 at 18:29 +0800, Chen Cheng wrote:
>> From: Chen Cheng <chencheng@fnnas.com>
>>
>> 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.

> 
>> Fixes: e792a4c2156a3 ("md: add new helpers for sync_action")
>>
>> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
>> ---
>>   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
>>
> 

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

[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@fnnas.com>
>>>
>>> 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@fnnas.com>
>>> ---
>>>   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