On Tue, Oct 12, 2021 at 1:49 AM Xiao Ni <xni@xxxxxxxxxx> wrote:
>
> Hi all
>
> How about this patch? Now writemostly flag doesn't be stored in
> superblock too. So this patch fix this problem too.
> If this patch is ok, I'll send the patch.
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 6c0c3d0d905a..9e8a8c5c7758 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -2977,6 +2977,7 @@ state_store(struct md_rdev *rdev, const char
> *buf, size_t len)
> * {,-}failfast - set/clear FailFast
> */
> int err = -EINVAL;
> + int need_update_sb = 0;
Please use bool for need_update_sb.
> if (cmd_match(buf, "faulty") && rdev->mddev->pers) {
> md_error(rdev->mddev, rdev);
> if (test_bit(Faulty, &rdev->flags))
[...]
> @@ -3120,6 +3122,11 @@ state_store(struct md_rdev *rdev, const char
> *buf, size_t len)
> }
> if (!err)
> sysfs_notify_dirent_safe(rdev->sysfs_state);
> + if (need_update_sb)
> + if (mddev->pers) {
We can merge the two conditions in in one line
if (need_update_sb && mddev->pers) {
...
}
> + set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags);
> + md_wakeup_thread(mddev->thread);
> + }
> return err ? err : len;
> }
> static struct rdev_sysfs_entry rdev_state =
>
> On Tue, Oct 12, 2021 at 4:44 PM Li Feng <fengli@xxxxxxxxxx> wrote:
> >
> > Song Liu <song@xxxxxxxxxx> 于2021年10月12日周二 下午4:17写道：
> > >
> > > On Tue, Oct 12, 2021 at 1:07 AM Li Feng <fengli@xxxxxxxxxx> wrote:
> > > >
> > > > Xiao Ni <xni@xxxxxxxxxx> 于2021年10月12日周二 下午2:58写道：
> > > > >
> > > > > On Mon, Oct 11, 2021 at 5:42 PM Li Feng <fengli@xxxxxxxxxx> wrote:
> > > > > >
> > > > > > Xiao Ni <xni@xxxxxxxxxx> 于2021年10月11日周一 下午3:49写道：
> > > > > > >
> > > > > > > Hi all
> > > > > > >
> > > > > > > Now the per device sysfs interface file state can change failfast. Do
> > > > > > > we need a new file for failfast?
> > > > > > >
> > > > > > > I did a test. The steps are:
> > > > > > >
> > > > > > > mdadm -CR /dev/md0 -l1 -n2 /dev/sdb /dev/sdc --assume-clean
> > > > > > > cd /sys/block/md0/md/dev-sdb
> > > > > > > echo failfast > state
> > > > > > > cat state
> > > > > > > in_sync,failfast
> > > > > >
> > > > > > This works, will it be persisted to disk?
> > > > > >
> > > > >
> > > > > mdadm --detail /dev/md0 can show the failfast information. So it
> > > > > should be written in superblock.
> > > > > But I don't find how md does this. I'm looking at this.
> > > > >
> > > > Yes, I have tested that it has been persisted, but don't understand who does it.
> > >
> > > I think this is not guaranteed to be persistent:
> > >
> > > [root@eth50-1 ~]# cat /sys/block/md127/md/rd1/state
> > > in_sync,failfast
> > > [root@eth50-1 ~]# echo -failfast > /sys/block/md127/md/rd1/state
> > > [root@eth50-1 ~]# cat /sys/block/md127/md/rd1/state
> > > in_sync
> > > [root@eth50-1 ~]# mdadm --stop /dev/md*
> > > mdadm: /dev/md does not appear to be an md device
> > > mdadm: stopped /dev/md127
> > > [root@eth50-1 ~]# mdadm -As
> > > mdadm: /dev/md/0_0 has been started with 4 drives.
> > > [root@eth50-1 ~]# cat /sys/block/md127/md/rd1/state
> > > in_sync,failfast
> > >
> > > How about we fix state_store to make sure it is always persistent?
> > >
> > I agree with you.
> >
> > > Thanks,
> > > Song
> >
>
