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;
     if (cmd_match(buf, "faulty") && rdev->mddev->pers) {
         md_error(rdev->mddev, rdev);
         if (test_bit(Faulty, &rdev->flags))

@@ -2998,20 +2999,19 @@ state_store(struct md_rdev *rdev, const char

*buf, size_t len)

             if (err == 0) {
                 md_kick_rdev_from_array(rdev);
-                if (mddev->pers) {
-                    set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags);
-                    md_wakeup_thread(mddev->thread);
-                }
+                need_update_sb = 1;
                 md_new_event(mddev);
             }
         }
     } else if (cmd_match(buf, "writemostly")) {
         set_bit(WriteMostly, &rdev->flags);
         mddev_create_serial_pool(rdev->mddev, rdev, false);
+        need_update_sb = 1;
         err = 0;
     } else if (cmd_match(buf, "-writemostly")) {
         mddev_destroy_serial_pool(rdev->mddev, rdev, false);
         clear_bit(WriteMostly, &rdev->flags);
+        need_update_sb = 1;
         err = 0;
     } else if (cmd_match(buf, "blocked")) {
         set_bit(Blocked, &rdev->flags);

@@ -3037,9 +3037,11 @@ state_store(struct md_rdev *rdev, const char

*buf, size_t len)
         err = 0;
     } else if (cmd_match(buf, "failfast")) {
         set_bit(FailFast, &rdev->flags);
+        need_update_sb = 1;
         err = 0;
     } else if (cmd_match(buf, "-failfast")) {
         clear_bit(FailFast, &rdev->flags);
+        need_update_sb = 1;
         err = 0;
     } else if (cmd_match(buf, "-insync") && rdev->raid_disk >= 0 &&
            !test_bit(Journal, &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) {
+            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 [off-list ref] wrote:

Song Liu [off-list ref] 于2021年10月12日周二 下午4:17写道：

quoted

On Tue, Oct 12, 2021 at 1:07 AM Li Feng [off-list ref] wrote:

quoted

Xiao Ni [off-list ref] 于2021年10月12日周二 下午2:58写道：

quoted

On Mon, Oct 11, 2021 at 5:42 PM Li Feng [off-list ref] wrote:

quoted

Xiao Ni [off-list ref] 于2021年10月11日周一 下午3:49写道：

quoted

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.

quoted

Thanks,
Song