lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CALTww2_eScuqd4yUtDFhaRUGAK-f8J_L=yOZdTVA9uZ7Tq4bxg@mail.gmail.com>
Date:   Tue, 12 Oct 2021 16:49:01 +0800
From:   Xiao Ni <xni@...hat.com>
To:     Li Feng <fengli@...rtx.com>
Cc:     Song Liu <song@...nel.org>,
        "open list:SOFTWARE RAID (Multiple Disks) SUPPORT" 
        <linux-raid@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH RESEND] md: allow to set the fail_fast on RAID1/RAID10

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 <fengli@...rtx.com> wrote:
>
> Song Liu <song@...nel.org> 于2021年10月12日周二 下午4:17写道:
> >
> > On Tue, Oct 12, 2021 at 1:07 AM Li Feng <fengli@...rtx.com> wrote:
> > >
> > > Xiao Ni <xni@...hat.com> 于2021年10月12日周二 下午2:58写道:
> > > >
> > > > On Mon, Oct 11, 2021 at 5:42 PM Li Feng <fengli@...rtx.com> wrote:
> > > > >
> > > > > Xiao Ni <xni@...hat.com> 于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@...50-1 ~]# cat /sys/block/md127/md/rd1/state
> > in_sync,failfast
> > [root@...50-1 ~]# echo -failfast >  /sys/block/md127/md/rd1/state
> > [root@...50-1 ~]# cat /sys/block/md127/md/rd1/state
> > in_sync
> > [root@...50-1 ~]# mdadm --stop /dev/md*
> > mdadm: /dev/md does not appear to be an md device
> > mdadm: stopped /dev/md127
> > [root@...50-1 ~]# mdadm -As
> > mdadm: /dev/md/0_0 has been started with 4 drives.
> > [root@...50-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
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ