Hi Heinz On Thu, Sep 18, 2025 at 9:55 PM Heinz Mauelshagen <[email protected]> wrote: > > The dm-raid code was using hardcoded integer values to represent the > read-only/read-write state > of RAID arrays instead of the proper enumeration constants defined in the > md_ro_state enumerator type. > > Changes: > > - Replace hardcoded integers with the appropriate md_ro_state enumerator > values > > - Add the missing MD_RDONLY setting in the post_suspend function
It should be the most important change in this patch. Mind adding the reproducer in the commit message? And explain why you say missing it? Best Regards Xiao > > This improves code clarity and maintainability by using the defined > enumeration constants > rather than magic numbers, ensuring the code properly conforms to the > established API interface. > > Signed-off-by: Heinz Mauelshagen <[email protected]> > --- > drivers/md/dm-raid.c | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c > index 79ea85d18e24..6cdfd7ca998c 100644 > --- a/drivers/md/dm-raid.c > +++ b/drivers/md/dm-raid.c > @@ -3247,7 +3247,7 @@ static int raid_ctr(struct dm_target *ti, unsigned int > argc, char **argv) > rs_reset_inconclusive_reshape(rs); > > /* Start raid set read-only and assumed clean to change in > raid_resume() */ > - rs->md.ro = 1; > + rs->md.ro = MD_RDONLY; > rs->md.in_sync = 1; > > /* Has to be held on running the array */ > @@ -3385,7 +3385,7 @@ static enum sync_state decipher_sync_action(struct > mddev *mddev, unsigned long r > /* The MD sync thread can be done with io or be interrupted but still > be running */ > if (!test_bit(MD_RECOVERY_DONE, &recovery) && > (test_bit(MD_RECOVERY_RUNNING, &recovery) || > - (!mddev->ro && test_bit(MD_RECOVERY_NEEDED, &recovery)))) { > + (md_is_rdwr(mddev) && test_bit(MD_RECOVERY_NEEDED, &recovery)))) > { > if (test_bit(MD_RECOVERY_RESHAPE, &recovery)) > return st_reshape; > > @@ -3775,11 +3775,11 @@ static int raid_message(struct dm_target *ti, > unsigned int argc, char **argv, > } else > return -EINVAL; > } > - if (mddev->ro == 2) { > + if (mddev->ro == MD_AUTO_READ) { > /* A write to sync_action is enough to justify > * canceling read-auto mode > */ > - mddev->ro = 0; > + mddev->ro = MD_RDWR; > if (!mddev->suspended) > md_wakeup_thread(mddev->sync_thread); > } > @@ -3858,6 +3858,7 @@ static void raid_postsuspend(struct dm_target *ti) > */ > md_stop_writes(&rs->md); > mddev_suspend(&rs->md, false); > + rs->md.ro = MD_RDONLY; > } > } > > @@ -3968,7 +3969,7 @@ static void rs_update_sbs(struct raid_set *rs) > int ro = mddev->ro; > > set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags); > - mddev->ro = 0; > + mddev->ro = MD_RDWR; > md_update_sb(mddev, 1); > mddev->ro = ro; > } > @@ -4125,7 +4126,7 @@ static void raid_resume(struct dm_target *ti) > WARN_ON_ONCE(rcu_dereference_protected(mddev->sync_thread, > > lockdep_is_held(&mddev->reconfig_mutex))); > clear_bit(RT_FLAG_RS_FROZEN, &rs->runtime_flags); > - mddev->ro = 0; > + mddev->ro = MD_RDWR; > mddev->in_sync = 0; > md_unfrozen_sync_thread(mddev); > mddev_unlock_and_resume(mddev); > -- > 2.51.0 > >
