* [PATCH] md: use READ_ONCE() for lockless reads of sb_flags
@ 2026-06-23 8:19 Chen Cheng
2026-06-23 8:29 ` sashiko-bot
0 siblings, 1 reply; 5+ messages in thread
From: Chen Cheng @ 2026-06-23 8:19 UTC (permalink / raw)
To: linux-raid, yukuai, yukuai; +Cc: chencheng, linux-kernel
From: Chen Cheng <chencheng@fnnas.com>
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.
KCSAN reports #1:
======================================
BUG: KCSAN: data-race in md_check_recovery / md_write_start
write (marked) to 0xffff8e39f897f030 of 8 bytes by task 248146 on cpu 8:
md_write_start+0x5dd/0x910
raid10_make_request+0x9b/0x1080
md_handle_request+0x4a2/0xa40
[........]
read to 0xffff8e39f897f030 of 8 bytes by task 250445 on cpu 11:
md_check_recovery+0x574/0x900
raid10d+0xb7/0x2950
[........]
KCSAN reports #2:
======================================
BUG: KCSAN: data-race in md_check_recovery / md_write_start
write (marked) to 0xffff8e39e953f030 of 8 bytes by task 540091 on cpu 11:
md_write_start+0x5dd/0x910
raid1_make_request+0x141/0x1990
[........]
read to 0xffff8e39e953f030 of 8 bytes by task 580822 on cpu 0:
md_check_recovery+0x574/0x900
raid1d+0xcc/0x3840
[........]
value changed: 0x0000000000000002 -> 0x0000000000000006
KCSAN reports #3:
======================================
BUG: KCSAN: data-race in md_check_recovery / md_do_sync.cold
write (marked) to 0xffff8e39e9404030 of 8 bytes by task 492473 on cpu 6:
md_do_sync.cold+0x3f6/0x1686
[........]
read to 0xffff8e39e9404030 of 8 bytes by task 492402 on cpu 3:
md_check_recovery+0x16d/0x900
raid1d+0xcc/0x3840
[........]
value changed: 0x0000000000000000 -> 0x0000000000000002
KCSAN reports #4:
======================================
BUG: KCSAN: data-race in md_do_sync.cold / raid5d
write (marked) to 0xffff8e39c35cb030 of 8 bytes by task 192196 on cpu 10:
md_do_sync.cold+0x3f6/0x1686
md_thread+0x15a/0x2d0
[........]
read to 0xffff8e39c35cb030 of 8 bytes by task 190759 on cpu 5:
raid5d+0x7f9/0xba0
md_thread+0x15a/0x2d0
[........]
value changed: 0x0000000000000000 -> 0x0000000000000002
Signed-off-by: Chen Cheng <chencheng@fnnas.com>
---
drivers/md/md.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 096bb64e87bd..f83cca895f85 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -6830,11 +6830,11 @@ int md_run(struct mddev *mddev)
* via sysfs - until a lack of spares is confirmed.
*/
set_bit(MD_RECOVERY_RECOVER, &mddev->recovery);
set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
- if (mddev->sb_flags)
+ if (READ_ONCE(mddev->sb_flags))
md_update_sb(mddev, 0);
if (IS_ENABLED(CONFIG_MD_BITMAP) && !mddev->bitmap_info.file &&
!mddev->bitmap_info.offset)
md_bitmap_set_none(mddev);
@@ -10294,11 +10294,11 @@ static bool md_should_do_recovery(struct mddev *mddev)
/*
* MD_SB_CHANGE_PENDING indicates that the array is switching from clean to
* active, and no action is needed for now.
* All other MD_SB_* flags require to update the superblock.
*/
- if (mddev->sb_flags & ~ (1<<MD_SB_CHANGE_PENDING))
+ if (READ_ONCE(mddev->sb_flags) & ~ (1<<MD_SB_CHANGE_PENDING))
return true;
/*
* If the array is not using external metadata and there has been no data
* written for some time, then the array's status needs to be set to
@@ -10423,11 +10423,11 @@ void md_check_recovery(struct mddev *mddev)
spin_lock(&mddev->lock);
set_in_sync(mddev);
spin_unlock(&mddev->lock);
}
- if (mddev->sb_flags)
+ if (READ_ONCE(mddev->sb_flags))
md_update_sb(mddev, 0);
/*
* Never start a new sync thread if MD_RECOVERY_RUNNING is
* still set.
--
2.54.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] md: use READ_ONCE() for lockless reads of sb_flags
2026-06-23 8:19 [PATCH] md: use READ_ONCE() for lockless reads of sb_flags Chen Cheng
@ 2026-06-23 8:29 ` sashiko-bot
2026-06-23 8:44 ` Chen Cheng
0 siblings, 1 reply; 5+ messages in thread
From: sashiko-bot @ 2026-06-23 8:29 UTC (permalink / raw)
To: Chen Cheng; +Cc: linux-raid, yukuai
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 <chencheng@fnnas.com>
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?
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()
> 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?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260623081942.2488045-1-chencheng@fnnas.com?part=1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] md: use READ_ONCE() for lockless reads of sb_flags
2026-06-23 8:29 ` sashiko-bot
@ 2026-06-23 8:44 ` Chen Cheng
0 siblings, 0 replies; 5+ messages in thread
From: Chen Cheng @ 2026-06-23 8:44 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-raid, yukuai
在 2026/6/23 16:29, sashiko-bot@kernel.org 写道:
> 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 <chencheng@fnnas.com>
>
> 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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] md: use READ_ONCE() for lockless reads of sb_flags
@ 2026-07-11 2:51 Chen Cheng
2026-07-11 18:16 ` yu kuai
0 siblings, 1 reply; 5+ messages in thread
From: Chen Cheng @ 2026-07-11 2:51 UTC (permalink / raw)
To: linux-raid, yukuai; +Cc: chencheng, linux-kernel
From: Chen Cheng <chencheng@fnnas.com>
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.
KCSAN reports #1:
======================================
BUG: KCSAN: data-race in md_check_recovery / md_write_start
write (marked) to 0xffff8e39f897f030 of 8 bytes by task 248146 on cpu 8:
md_write_start+0x5dd/0x910
raid10_make_request+0x9b/0x1080
md_handle_request+0x4a2/0xa40
[........]
read to 0xffff8e39f897f030 of 8 bytes by task 250445 on cpu 11:
md_check_recovery+0x574/0x900
raid10d+0xb7/0x2950
[........]
KCSAN reports #2:
======================================
BUG: KCSAN: data-race in md_check_recovery / md_write_start
write (marked) to 0xffff8e39e953f030 of 8 bytes by task 540091 on cpu 11:
md_write_start+0x5dd/0x910
raid1_make_request+0x141/0x1990
[........]
read to 0xffff8e39e953f030 of 8 bytes by task 580822 on cpu 0:
md_check_recovery+0x574/0x900
raid1d+0xcc/0x3840
[........]
value changed: 0x0000000000000002 -> 0x0000000000000006
KCSAN reports #3:
======================================
BUG: KCSAN: data-race in md_check_recovery / md_do_sync.cold
write (marked) to 0xffff8e39e9404030 of 8 bytes by task 492473 on cpu 6:
md_do_sync.cold+0x3f6/0x1686
[........]
read to 0xffff8e39e9404030 of 8 bytes by task 492402 on cpu 3:
md_check_recovery+0x16d/0x900
raid1d+0xcc/0x3840
[........]
value changed: 0x0000000000000000 -> 0x0000000000000002
KCSAN reports #4:
======================================
BUG: KCSAN: data-race in md_do_sync.cold / raid5d
write (marked) to 0xffff8e39c35cb030 of 8 bytes by task 192196 on cpu 10:
md_do_sync.cold+0x3f6/0x1686
md_thread+0x15a/0x2d0
[........]
read to 0xffff8e39c35cb030 of 8 bytes by task 190759 on cpu 5:
raid5d+0x7f9/0xba0
md_thread+0x15a/0x2d0
[........]
value changed: 0x0000000000000000 -> 0x0000000000000002
Fixes: 2953079c692d ("md: separate flags for superblock changes")
Signed-off-by: Chen Cheng <chencheng@fnnas.com>
---
v1 -> v2
- pass checkpatch.pl check.
---
drivers/md/md.c | 8 ++++----
drivers/md/raid1.c | 2 +-
drivers/md/raid10.c | 4 ++--
drivers/md/raid5.c | 7 ++++---
4 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 096bb64e87bd..e72bf2010287 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -6830,11 +6830,11 @@ int md_run(struct mddev *mddev)
* via sysfs - until a lack of spares is confirmed.
*/
set_bit(MD_RECOVERY_RECOVER, &mddev->recovery);
set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
- if (mddev->sb_flags)
+ if (READ_ONCE(mddev->sb_flags))
md_update_sb(mddev, 0);
if (IS_ENABLED(CONFIG_MD_BITMAP) && !mddev->bitmap_info.file &&
!mddev->bitmap_info.offset)
md_bitmap_set_none(mddev);
@@ -7024,11 +7024,11 @@ static void __md_stop_writes(struct mddev *mddev)
mddev->bitmap_ops->flush(mddev);
}
if (md_is_rdwr(mddev) &&
((!mddev->in_sync && !mddev_is_clustered(mddev)) ||
- mddev->sb_flags)) {
+ READ_ONCE(mddev->sb_flags))) {
/* mark array as shutdown cleanly */
if (!mddev_is_clustered(mddev))
mddev->in_sync = 1;
md_update_sb(mddev, 1);
}
@@ -10294,11 +10294,11 @@ static bool md_should_do_recovery(struct mddev *mddev)
/*
* MD_SB_CHANGE_PENDING indicates that the array is switching from clean to
* active, and no action is needed for now.
* All other MD_SB_* flags require to update the superblock.
*/
- if (mddev->sb_flags & ~ (1<<MD_SB_CHANGE_PENDING))
+ if (READ_ONCE(mddev->sb_flags) & ~(1<<MD_SB_CHANGE_PENDING))
return true;
/*
* If the array is not using external metadata and there has been no data
* written for some time, then the array's status needs to be set to
@@ -10423,11 +10423,11 @@ void md_check_recovery(struct mddev *mddev)
spin_lock(&mddev->lock);
set_in_sync(mddev);
spin_unlock(&mddev->lock);
}
- if (mddev->sb_flags)
+ if (READ_ONCE(mddev->sb_flags))
md_update_sb(mddev, 0);
/*
* Never start a new sync thread if MD_RECOVERY_RUNNING is
* still set.
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 29b58583e381..bd6808656edb 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -2738,11 +2738,11 @@ static void raid1d(struct md_thread *thread)
handle_read_error(conf, r1_bio);
else
WARN_ON_ONCE(1);
cond_resched();
- if (mddev->sb_flags & ~(1<<MD_SB_CHANGE_PENDING))
+ if (READ_ONCE(mddev->sb_flags) & ~(1 << MD_SB_CHANGE_PENDING))
md_check_recovery(mddev);
}
blk_finish_plug(&plug);
}
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index adaf9e432e25..3ffa5a19964d 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -3030,11 +3030,11 @@ static void raid10d(struct md_thread *thread)
handle_read_error(mddev, r10_bio);
else
WARN_ON_ONCE(1);
cond_resched();
- if (mddev->sb_flags & ~(1<<MD_SB_CHANGE_PENDING))
+ if (READ_ONCE(mddev->sb_flags) & ~(1 << MD_SB_CHANGE_PENDING))
md_check_recovery(mddev);
}
blk_finish_plug(&plug);
}
@@ -4698,11 +4698,11 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr,
else
mddev->curr_resync_completed = conf->reshape_progress;
conf->reshape_checkpoint = jiffies;
set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags);
md_wakeup_thread(mddev->thread);
- wait_event(mddev->sb_wait, mddev->sb_flags == 0 ||
+ wait_event(mddev->sb_wait, READ_ONCE(mddev->sb_flags) == 0 ||
test_bit(MD_RECOVERY_INTR, &mddev->recovery));
if (test_bit(MD_RECOVERY_INTR, &mddev->recovery)) {
allow_barrier(conf);
return sectors_done;
}
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index ded6a69f7795..cb58b4353995 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -1219,11 +1219,11 @@ static void ops_run_io(struct stripe_head *sh, struct stripe_head_state *s)
break;
if (bad < 0) {
set_bit(BlockedBadBlocks, &rdev->flags);
if (!conf->mddev->external &&
- conf->mddev->sb_flags) {
+ READ_ONCE(conf->mddev->sb_flags)) {
/* It is very unlikely, but we might
* still need to write out the
* bad block log - better give it
* a chance*/
md_check_recovery(conf->mddev);
@@ -6469,11 +6469,11 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr, int *sk
rdev->recovery_offset = sector_nr;
conf->reshape_checkpoint = jiffies;
set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags);
md_wakeup_thread(mddev->thread);
- wait_event(mddev->sb_wait, mddev->sb_flags == 0 ||
+ wait_event(mddev->sb_wait, READ_ONCE(mddev->sb_flags) == 0 ||
test_bit(MD_RECOVERY_INTR, &mddev->recovery));
if (test_bit(MD_RECOVERY_INTR, &mddev->recovery))
return 0;
spin_lock_irq(&conf->device_lock);
conf->reshape_safe = mddev->reshape_position;
@@ -6913,11 +6913,12 @@ static void raid5d(struct md_thread *thread)
conf->temp_inactive_list);
if (!batch_size && !released)
break;
handled += batch_size;
- if (mddev->sb_flags & ~(1 << MD_SB_CHANGE_PENDING)) {
+ if (READ_ONCE(mddev->sb_flags) &
+ ~(1 << MD_SB_CHANGE_PENDING)) {
spin_unlock_irq(&conf->device_lock);
md_check_recovery(mddev);
spin_lock_irq(&conf->device_lock);
}
}
--
2.54.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] md: use READ_ONCE() for lockless reads of sb_flags
2026-07-11 2:51 Chen Cheng
@ 2026-07-11 18:16 ` yu kuai
0 siblings, 0 replies; 5+ messages in thread
From: yu kuai @ 2026-07-11 18:16 UTC (permalink / raw)
To: Chen Cheng, linux-raid; +Cc: linux-kernel, yu kuai
Hi,
在 2026/7/11 10:51, Chen Cheng 写道:
> From: Chen Cheng <chencheng@fnnas.com>
>
> 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.
>
> KCSAN reports #1:
> ======================================
>
> BUG: KCSAN: data-race in md_check_recovery / md_write_start
>
> write (marked) to 0xffff8e39f897f030 of 8 bytes by task 248146 on cpu 8:
> md_write_start+0x5dd/0x910
> raid10_make_request+0x9b/0x1080
> md_handle_request+0x4a2/0xa40
> [........]
>
> read to 0xffff8e39f897f030 of 8 bytes by task 250445 on cpu 11:
> md_check_recovery+0x574/0x900
> raid10d+0xb7/0x2950
> [........]
>
> KCSAN reports #2:
> ======================================
> BUG: KCSAN: data-race in md_check_recovery / md_write_start
>
> write (marked) to 0xffff8e39e953f030 of 8 bytes by task 540091 on cpu 11:
> md_write_start+0x5dd/0x910
> raid1_make_request+0x141/0x1990
> [........]
>
> read to 0xffff8e39e953f030 of 8 bytes by task 580822 on cpu 0:
> md_check_recovery+0x574/0x900
> raid1d+0xcc/0x3840
> [........]
>
> value changed: 0x0000000000000002 -> 0x0000000000000006
>
> KCSAN reports #3:
> ======================================
> BUG: KCSAN: data-race in md_check_recovery / md_do_sync.cold
>
> write (marked) to 0xffff8e39e9404030 of 8 bytes by task 492473 on cpu 6:
> md_do_sync.cold+0x3f6/0x1686
> [........]
>
> read to 0xffff8e39e9404030 of 8 bytes by task 492402 on cpu 3:
> md_check_recovery+0x16d/0x900
> raid1d+0xcc/0x3840
> [........]
>
> value changed: 0x0000000000000000 -> 0x0000000000000002
>
> KCSAN reports #4:
> ======================================
> BUG: KCSAN: data-race in md_do_sync.cold / raid5d
>
> write (marked) to 0xffff8e39c35cb030 of 8 bytes by task 192196 on cpu 10:
> md_do_sync.cold+0x3f6/0x1686
> md_thread+0x15a/0x2d0
> [........]
>
> read to 0xffff8e39c35cb030 of 8 bytes by task 190759 on cpu 5:
> raid5d+0x7f9/0xba0
> md_thread+0x15a/0x2d0
> [........]
>
> value changed: 0x0000000000000000 -> 0x0000000000000002
>
> Fixes: 2953079c692d ("md: separate flags for superblock changes")
>
> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
> ---
> v1 -> v2
> - pass checkpatch.pl check.
> ---
> drivers/md/md.c | 8 ++++----
> drivers/md/raid1.c | 2 +-
> drivers/md/raid10.c | 4 ++--
> drivers/md/raid5.c | 7 ++++---
> 4 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 096bb64e87bd..e72bf2010287 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -6830,11 +6830,11 @@ int md_run(struct mddev *mddev)
> * via sysfs - until a lack of spares is confirmed.
> */
> set_bit(MD_RECOVERY_RECOVER, &mddev->recovery);
> set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
>
> - if (mddev->sb_flags)
> + if (READ_ONCE(mddev->sb_flags))
> md_update_sb(mddev, 0);
>
> if (IS_ENABLED(CONFIG_MD_BITMAP) && !mddev->bitmap_info.file &&
> !mddev->bitmap_info.offset)
> md_bitmap_set_none(mddev);
> @@ -7024,11 +7024,11 @@ static void __md_stop_writes(struct mddev *mddev)
> mddev->bitmap_ops->flush(mddev);
> }
>
> if (md_is_rdwr(mddev) &&
> ((!mddev->in_sync && !mddev_is_clustered(mddev)) ||
> - mddev->sb_flags)) {
> + READ_ONCE(mddev->sb_flags))) {
> /* mark array as shutdown cleanly */
> if (!mddev_is_clustered(mddev))
> mddev->in_sync = 1;
> md_update_sb(mddev, 1);
> }
> @@ -10294,11 +10294,11 @@ static bool md_should_do_recovery(struct mddev *mddev)
> /*
> * MD_SB_CHANGE_PENDING indicates that the array is switching from clean to
> * active, and no action is needed for now.
> * All other MD_SB_* flags require to update the superblock.
> */
> - if (mddev->sb_flags & ~ (1<<MD_SB_CHANGE_PENDING))
> + if (READ_ONCE(mddev->sb_flags) & ~(1<<MD_SB_CHANGE_PENDING))
> return true;
So this should be a v3, and the above line still not fixed. Please check checkpatch
warnings.
>
> /*
> * If the array is not using external metadata and there has been no data
> * written for some time, then the array's status needs to be set to
> @@ -10423,11 +10423,11 @@ void md_check_recovery(struct mddev *mddev)
> spin_lock(&mddev->lock);
> set_in_sync(mddev);
> spin_unlock(&mddev->lock);
> }
>
> - if (mddev->sb_flags)
> + if (READ_ONCE(mddev->sb_flags))
> md_update_sb(mddev, 0);
>
> /*
> * Never start a new sync thread if MD_RECOVERY_RUNNING is
> * still set.
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index 29b58583e381..bd6808656edb 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -2738,11 +2738,11 @@ static void raid1d(struct md_thread *thread)
> handle_read_error(conf, r1_bio);
> else
> WARN_ON_ONCE(1);
>
> cond_resched();
> - if (mddev->sb_flags & ~(1<<MD_SB_CHANGE_PENDING))
> + if (READ_ONCE(mddev->sb_flags) & ~(1 << MD_SB_CHANGE_PENDING))
> md_check_recovery(mddev);
> }
> blk_finish_plug(&plug);
> }
>
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index adaf9e432e25..3ffa5a19964d 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -3030,11 +3030,11 @@ static void raid10d(struct md_thread *thread)
> handle_read_error(mddev, r10_bio);
> else
> WARN_ON_ONCE(1);
>
> cond_resched();
> - if (mddev->sb_flags & ~(1<<MD_SB_CHANGE_PENDING))
> + if (READ_ONCE(mddev->sb_flags) & ~(1 << MD_SB_CHANGE_PENDING))
> md_check_recovery(mddev);
> }
> blk_finish_plug(&plug);
> }
>
> @@ -4698,11 +4698,11 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr,
> else
> mddev->curr_resync_completed = conf->reshape_progress;
> conf->reshape_checkpoint = jiffies;
> set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags);
> md_wakeup_thread(mddev->thread);
> - wait_event(mddev->sb_wait, mddev->sb_flags == 0 ||
> + wait_event(mddev->sb_wait, READ_ONCE(mddev->sb_flags) == 0 ||
> test_bit(MD_RECOVERY_INTR, &mddev->recovery));
> if (test_bit(MD_RECOVERY_INTR, &mddev->recovery)) {
> allow_barrier(conf);
> return sectors_done;
> }
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index ded6a69f7795..cb58b4353995 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -1219,11 +1219,11 @@ static void ops_run_io(struct stripe_head *sh, struct stripe_head_state *s)
> break;
>
> if (bad < 0) {
> set_bit(BlockedBadBlocks, &rdev->flags);
> if (!conf->mddev->external &&
> - conf->mddev->sb_flags) {
> + READ_ONCE(conf->mddev->sb_flags)) {
> /* It is very unlikely, but we might
> * still need to write out the
> * bad block log - better give it
> * a chance*/
> md_check_recovery(conf->mddev);
> @@ -6469,11 +6469,11 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr, int *sk
> rdev->recovery_offset = sector_nr;
>
> conf->reshape_checkpoint = jiffies;
> set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags);
> md_wakeup_thread(mddev->thread);
> - wait_event(mddev->sb_wait, mddev->sb_flags == 0 ||
> + wait_event(mddev->sb_wait, READ_ONCE(mddev->sb_flags) == 0 ||
> test_bit(MD_RECOVERY_INTR, &mddev->recovery));
> if (test_bit(MD_RECOVERY_INTR, &mddev->recovery))
> return 0;
> spin_lock_irq(&conf->device_lock);
> conf->reshape_safe = mddev->reshape_position;
> @@ -6913,11 +6913,12 @@ static void raid5d(struct md_thread *thread)
> conf->temp_inactive_list);
> if (!batch_size && !released)
> break;
> handled += batch_size;
>
> - if (mddev->sb_flags & ~(1 << MD_SB_CHANGE_PENDING)) {
> + if (READ_ONCE(mddev->sb_flags) &
> + ~(1 << MD_SB_CHANGE_PENDING)) {
> spin_unlock_irq(&conf->device_lock);
> md_check_recovery(mddev);
> spin_lock_irq(&conf->device_lock);
> }
> }
--
Thanks,
Kuai
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-07-11 18:16 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-23 8:19 [PATCH] md: use READ_ONCE() for lockless reads of sb_flags Chen Cheng
2026-06-23 8:29 ` sashiko-bot
2026-06-23 8:44 ` Chen Cheng
-- strict thread matches above, loose matches on Subject: below --
2026-07-11 2:51 Chen Cheng
2026-07-11 18:16 ` yu kuai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox