* [PATCH] raid0: fix set_disk_faulty doesn't return -EBUSY
@ 2023-03-21 8:56 Wu Guanghao
2023-03-21 10:18 ` Mariusz Tkaczyk
0 siblings, 1 reply; 5+ messages in thread
From: Wu Guanghao @ 2023-03-21 8:56 UTC (permalink / raw)
To: song, linux-raid; +Cc: liuzhiqiang (I), louhongxiang
The latest kernel version will not report an error through mdadm set_disk_faulty.
$ lsblk
sdb 8:16 0 10G 0 disk
└─md0 9:0 0 19.9G 0 raid0
sdc 8:32 0 10G 0 disk
└─md0 9:0 0 19.9G 0 raid0
old kernel:
...
$ mdadm /dev/md0 -f /dev/sdb
mdadm: set device faulty failed for /dev/sdb: Device or resource busy
...
latest kernel:
...
$ mdadm /dev/md0 -f /dev/sdb
mdadm: set /dev/sdb faulty in /dev/md0
...
The old kernel judges whether the Faulty flag is set in rdev->flags,
and returns -EBUSY if not. And The latest kernel only return -EBUSY
if the MD_BROKEN flag is set in mddev->flags. raid0 doesn't set error_handler,
so MD_BROKEN will not be set, it will return 0.
So if error_handler isn't set for a raid type, also return -EBUSY.
Fixes: 9631abdbf406 ("md: Set MD_BROKEN for RAID1 and RAID10")
Signed-off-by: Wu Guanghao <wuguanghao3@huawei.com>
---
drivers/md/md.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 927a43db5dfb..b1786ff60d97 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -2928,10 +2928,10 @@ state_store(struct md_rdev *rdev, const char *buf, size_t len)
int err = -EINVAL;
bool need_update_sb = false;
- if (cmd_match(buf, "faulty") && rdev->mddev->pers) {
- md_error(rdev->mddev, rdev);
+ if (cmd_match(buf, "faulty") && mddev->pers) {
+ md_error(mddev, rdev);
- if (test_bit(MD_BROKEN, &rdev->mddev->flags))
+ if (!mddev->pers->error_handler || test_bit(MD_BROKEN, &mddev->flags))
err = -EBUSY;
else
err = 0;
@@ -7421,7 +7421,7 @@ static int set_disk_faulty(struct mddev *mddev, dev_t dev)
err = -ENODEV;
else {
md_error(mddev, rdev);
- if (test_bit(MD_BROKEN, &mddev->flags))
+ if (!mddev->pers->error_handler || test_bit(MD_BROKEN, &mddev->flags))
err = -EBUSY;
}
rcu_read_unlock();
--
2.27.0
.
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] raid0: fix set_disk_faulty doesn't return -EBUSY 2023-03-21 8:56 [PATCH] raid0: fix set_disk_faulty doesn't return -EBUSY Wu Guanghao @ 2023-03-21 10:18 ` Mariusz Tkaczyk 2023-03-22 2:24 ` Wu Guanghao 0 siblings, 1 reply; 5+ messages in thread From: Mariusz Tkaczyk @ 2023-03-21 10:18 UTC (permalink / raw) To: Wu Guanghao; +Cc: song, linux-raid, liuzhiqiang (I), louhongxiang On Tue, 21 Mar 2023 16:56:37 +0800 Wu Guanghao <wuguanghao3@huawei.com> wrote: > The latest kernel version will not report an error through mdadm > set_disk_faulty. > > $ lsblk > sdb 8:16 0 10G 0 disk > └─md0 9:0 0 19.9G 0 raid0 > sdc 8:32 0 10G 0 disk > └─md0 9:0 0 19.9G 0 raid0 > > old kernel: > ... > $ mdadm /dev/md0 -f /dev/sdb > mdadm: set device faulty failed for /dev/sdb: Device or resource busy > ... > > latest kernel: > ... > $ mdadm /dev/md0 -f /dev/sdb > mdadm: set /dev/sdb faulty in /dev/md0 > ... > > The old kernel judges whether the Faulty flag is set in rdev->flags, > and returns -EBUSY if not. And The latest kernel only return -EBUSY > if the MD_BROKEN flag is set in mddev->flags. raid0 doesn't set error_handler, > so MD_BROKEN will not be set, it will return 0. > > So if error_handler isn't set for a raid type, also return -EBUSY. Hi, Please test with: https://lore.kernel.org/linux-raid/20230306130317.3418-1-mariusz.tkaczyk@linux.intel.com/ Thanks, Mariusz > > Fixes: 9631abdbf406 ("md: Set MD_BROKEN for RAID1 and RAID10") > Signed-off-by: Wu Guanghao <wuguanghao3@huawei.com> > --- > drivers/md/md.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/md/md.c b/drivers/md/md.c > index 927a43db5dfb..b1786ff60d97 100644 > --- a/drivers/md/md.c > +++ b/drivers/md/md.c > @@ -2928,10 +2928,10 @@ state_store(struct md_rdev *rdev, const char *buf, > size_t len) int err = -EINVAL; > bool need_update_sb = false; > > - if (cmd_match(buf, "faulty") && rdev->mddev->pers) { > - md_error(rdev->mddev, rdev); > + if (cmd_match(buf, "faulty") && mddev->pers) { > + md_error(mddev, rdev); > > - if (test_bit(MD_BROKEN, &rdev->mddev->flags)) > + if (!mddev->pers->error_handler || test_bit(MD_BROKEN, > &mddev->flags)) err = -EBUSY; > else > err = 0; > @@ -7421,7 +7421,7 @@ static int set_disk_faulty(struct mddev *mddev, dev_t > dev) err = -ENODEV; > else { > md_error(mddev, rdev); > - if (test_bit(MD_BROKEN, &mddev->flags)) > + if (!mddev->pers->error_handler || test_bit(MD_BROKEN, > &mddev->flags)) err = -EBUSY; > } > rcu_read_unlock(); > -- > 2.27.0 > . ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] raid0: fix set_disk_faulty doesn't return -EBUSY 2023-03-21 10:18 ` Mariusz Tkaczyk @ 2023-03-22 2:24 ` Wu Guanghao 2023-03-22 7:05 ` Mariusz Tkaczyk 0 siblings, 1 reply; 5+ messages in thread From: Wu Guanghao @ 2023-03-22 2:24 UTC (permalink / raw) To: Mariusz Tkaczyk; +Cc: song, linux-raid, liuzhiqiang (I), louhongxiang 在 2023/3/21 18:18, Mariusz Tkaczyk 写道: > On Tue, 21 Mar 2023 16:56:37 +0800 > Wu Guanghao <wuguanghao3@huawei.com> wrote: > >> The latest kernel version will not report an error through mdadm >> set_disk_faulty. >> >> $ lsblk >> sdb 8:16 0 10G 0 disk >> └─md0 9:0 0 19.9G 0 raid0 >> sdc 8:32 0 10G 0 disk >> └─md0 9:0 0 19.9G 0 raid0 >> >> old kernel: >> ... >> $ mdadm /dev/md0 -f /dev/sdb >> mdadm: set device faulty failed for /dev/sdb: Device or resource busy >> ... >> >> latest kernel: >> ... >> $ mdadm /dev/md0 -f /dev/sdb >> mdadm: set /dev/sdb faulty in /dev/md0 >> ... >> >> The old kernel judges whether the Faulty flag is set in rdev->flags, >> and returns -EBUSY if not. And The latest kernel only return -EBUSY >> if the MD_BROKEN flag is set in mddev->flags. raid0 doesn't set error_handler, >> so MD_BROKEN will not be set, it will return 0. >> >> So if error_handler isn't set for a raid type, also return -EBUSY. > Hi, > Please test with: > https://lore.kernel.org/linux-raid/20230306130317.3418-1-mariusz.tkaczyk@linux.intel.com/ > > Thanks, > Mariusz > Hi, Mariusz Are there other patches? There are other problems with this patch. https://lore.kernel.org/linux-raid/20230306130317.3418-1-mariusz.tkaczyk@linux.intel.com/ md_submit_bio() ... // raid0 set disk faulty failed, but MD_BROKEN flag is set， // write IO will fail. if (unlikely(test_bit(MD_BROKEN, &mddev->flags)) && (rw == WRITE)) { bio_io_error(bio); return; } ... old kernel: ... $ mdadm /dev/md0 -f /dev/sdb mdadm: set device faulty failed for /dev/sdb: Device or resource busy $ mkfs.xfs /dev/md0 log stripe unit (524288 bytes) is too large (maximum is 256KiB) log stripe unit adjusted to 32KiB meta-data=/dev/md0 isize=512 agcount=16, agsize=1800064 blks = sectsz=512 attr=2, projid32bit=1 = crc=1 finobt=1, sparse=1, rmapbt=0 = reflink=1 bigtime=0 inobtcount=0 data = bsize=4096 blocks=28801024, imaxpct=25 = sunit=128 swidth=256 blks naming =version 2 bsize=4096 ascii-ci=0, ftype=1 log =internal log bsize=4096 blocks=14064, version=2 = sectsz=512 sunit=8 blks, lazy-count=1 realtime =none extsz=4096 blocks=0, rtextents=0 Discarding blocks...Done. ... merged patch kernel: ... # mdadm /dev/md0 -f /dev/sdb mdadm: set device faulty failed for /dev/sdb: Device or resource busy mkfs.xfs /dev/md0 log stripe unit (524288 bytes) is too large (maximum is 256KiB) log stripe unit adjusted to 32KiB meta-data=/dev/md0 isize=512 agcount=8, agsize=65408 blks = sectsz=512 attr=2, projid32bit=1 = crc=1 finobt=1, sparse=1, rmapbt=0 = reflink=1 bigtime=0 inobtcount=0 data = bsize=4096 blocks=523264, imaxpct=25 = sunit=128 swidth=256 blks naming =version 2 bsize=4096 ascii-ci=0, ftype=1 log =internal log bsize=4096 blocks=2560, version=2 = sectsz=512 sunit=8 blks, lazy-count=1 realtime =none extsz=4096 blocks=0, rtextents=0 mkfs.xfs: pwrite failed: Input/output error ... Regards, Wu >> >> Fixes: 9631abdbf406 ("md: Set MD_BROKEN for RAID1 and RAID10") >> Signed-off-by: Wu Guanghao <wuguanghao3@huawei.com> >> --- >> drivers/md/md.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/md/md.c b/drivers/md/md.c >> index 927a43db5dfb..b1786ff60d97 100644 >> --- a/drivers/md/md.c >> +++ b/drivers/md/md.c >> @@ -2928,10 +2928,10 @@ state_store(struct md_rdev *rdev, const char *buf, >> size_t len) int err = -EINVAL; >> bool need_update_sb = false; >> >> - if (cmd_match(buf, "faulty") && rdev->mddev->pers) { >> - md_error(rdev->mddev, rdev); >> + if (cmd_match(buf, "faulty") && mddev->pers) { >> + md_error(mddev, rdev); >> >> - if (test_bit(MD_BROKEN, &rdev->mddev->flags)) >> + if (!mddev->pers->error_handler || test_bit(MD_BROKEN, >> &mddev->flags)) err = -EBUSY; >> else >> err = 0; >> @@ -7421,7 +7421,7 @@ static int set_disk_faulty(struct mddev *mddev, dev_t >> dev) err = -ENODEV; >> else { >> md_error(mddev, rdev); >> - if (test_bit(MD_BROKEN, &mddev->flags)) >> + if (!mddev->pers->error_handler || test_bit(MD_BROKEN, >> &mddev->flags)) err = -EBUSY; >> } >> rcu_read_unlock(); >> -- >> 2.27.0 >> . > > > . > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] raid0: fix set_disk_faulty doesn't return -EBUSY 2023-03-22 2:24 ` Wu Guanghao @ 2023-03-22 7:05 ` Mariusz Tkaczyk 2023-03-22 8:38 ` Wu Guanghao 0 siblings, 1 reply; 5+ messages in thread From: Mariusz Tkaczyk @ 2023-03-22 7:05 UTC (permalink / raw) To: Wu Guanghao; +Cc: song, linux-raid, liuzhiqiang (I), louhongxiang On Wed, 22 Mar 2023 10:24:41 +0800 Wu Guanghao <wuguanghao3@huawei.com> wrote: > 在 2023/3/21 18:18, Mariusz Tkaczyk 写道: > > On Tue, 21 Mar 2023 16:56:37 +0800 > > Wu Guanghao <wuguanghao3@huawei.com> wrote: > > > >> The latest kernel version will not report an error through mdadm > >> set_disk_faulty. > >> > >> $ lsblk > >> sdb 8:16 0 10G 0 disk > >> └─md0 9:0 0 19.9G 0 raid0 > >> sdc 8:32 0 10G 0 disk > >> └─md0 9:0 0 19.9G 0 raid0 > >> > >> old kernel: > >> ... > >> $ mdadm /dev/md0 -f /dev/sdb > >> mdadm: set device faulty failed for /dev/sdb: Device or resource busy > >> ... > >> > >> latest kernel: > >> ... > >> $ mdadm /dev/md0 -f /dev/sdb > >> mdadm: set /dev/sdb faulty in /dev/md0 > >> ... > >> > >> The old kernel judges whether the Faulty flag is set in rdev->flags, > >> and returns -EBUSY if not. And The latest kernel only return -EBUSY > >> if the MD_BROKEN flag is set in mddev->flags. raid0 doesn't set > >> error_handler, so MD_BROKEN will not be set, it will return 0. > >> > >> So if error_handler isn't set for a raid type, also return -EBUSY. > > Hi, > > Please test with: > > https://lore.kernel.org/linux-raid/20230306130317.3418-1-mariusz.tkaczyk@linux.intel.com/ > > > > Thanks, > > Mariusz > > > > Hi, Mariusz > > Are there other patches? There are other problems with this patch. > https://lore.kernel.org/linux-raid/20230306130317.3418-1-mariusz.tkaczyk@linux.intel.com/ > > md_submit_bio() > ... > // raid0 set disk faulty failed, but MD_BROKEN flag is set， > // write IO will fail. > if (unlikely(test_bit(MD_BROKEN, &mddev->flags)) && (rw == WRITE)) { > bio_io_error(bio); > return; > } > ... > > old kernel: > ... > $ mdadm /dev/md0 -f /dev/sdb > mdadm: set device faulty failed for /dev/sdb: Device or resource busy > > $ mkfs.xfs /dev/md0 > log stripe unit (524288 bytes) is too large (maximum is 256KiB) > log stripe unit adjusted to 32KiB > meta-data=/dev/md0 isize=512 agcount=16, agsize=1800064 blks > = sectsz=512 attr=2, projid32bit=1 > = crc=1 finobt=1, sparse=1, rmapbt=0 > = reflink=1 bigtime=0 inobtcount=0 > data = bsize=4096 blocks=28801024, imaxpct=25 > = sunit=128 swidth=256 blks > naming =version 2 bsize=4096 ascii-ci=0, ftype=1 > log =internal log bsize=4096 blocks=14064, version=2 > = sectsz=512 sunit=8 blks, lazy-count=1 > realtime =none extsz=4096 blocks=0, rtextents=0 > Discarding blocks...Done. > ... > > > merged patch kernel: > ... > # mdadm /dev/md0 -f /dev/sdb > mdadm: set device faulty failed for /dev/sdb: Device or resource busy > > mkfs.xfs /dev/md0 > log stripe unit (524288 bytes) is too large (maximum is 256KiB) > log stripe unit adjusted to 32KiB > meta-data=/dev/md0 isize=512 agcount=8, agsize=65408 blks > = sectsz=512 attr=2, projid32bit=1 > = crc=1 finobt=1, sparse=1, rmapbt=0 > = reflink=1 bigtime=0 inobtcount=0 > data = bsize=4096 blocks=523264, imaxpct=25 > = sunit=128 swidth=256 blks > naming =version 2 bsize=4096 ascii-ci=0, ftype=1 > log =internal log bsize=4096 blocks=2560, version=2 > = sectsz=512 sunit=8 blks, lazy-count=1 > realtime =none extsz=4096 blocks=0, rtextents=0 > mkfs.xfs: pwrite failed: Input/output error > ... > > Hi Wu, Beside the kernel, there are also patches in mdadm. Please check if you have them all. https://git.kernel.org/pub/scm/utils/mdadm/mdadm.git/commit/?id=b3e7b7eb1dfedd7cbd9a3800e884941f67d94c96 https://git.kernel.org/pub/scm/utils/mdadm/mdadm.git/commit/?id=461fae7e7809670d286cc19aac5bfa861c29f93a https://git.kernel.org/pub/scm/utils/mdadm/mdadm.git/commit/?id=fc6fd4063769f4194c3fb8f77b32b2819e140fb9 Some background: --faulty (-f) is intended to be used by administrators. We cannot rely on kernel answer because if mdadm will try to set device faulty, it results in MD_BROKEN and every new IO will be failed (and that is intended change). Simply, mdadm must check first if it can remove the drive and that was added by the mentioned patches. The first patch (the last one) added verification but brings regression, the next two patches are fixes for omitted scenarios. Thanks, Mariusz ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] raid0: fix set_disk_faulty doesn't return -EBUSY 2023-03-22 7:05 ` Mariusz Tkaczyk @ 2023-03-22 8:38 ` Wu Guanghao 0 siblings, 0 replies; 5+ messages in thread From: Wu Guanghao @ 2023-03-22 8:38 UTC (permalink / raw) To: Mariusz Tkaczyk; +Cc: song, linux-raid, liuzhiqiang (I), louhongxiang 在 2023/3/22 15:05, Mariusz Tkaczyk 写道: > On Wed, 22 Mar 2023 10:24:41 +0800 > Wu Guanghao <wuguanghao3@huawei.com> wrote: > >> 在 2023/3/21 18:18, Mariusz Tkaczyk 写道: >>> On Tue, 21 Mar 2023 16:56:37 +0800 >>> Wu Guanghao <wuguanghao3@huawei.com> wrote: >>> >>>> The latest kernel version will not report an error through mdadm >>>> set_disk_faulty. >>>> >>>> $ lsblk >>>> sdb 8:16 0 10G 0 disk >>>> └─md0 9:0 0 19.9G 0 raid0 >>>> sdc 8:32 0 10G 0 disk >>>> └─md0 9:0 0 19.9G 0 raid0 >>>> >>>> old kernel: >>>> ... >>>> $ mdadm /dev/md0 -f /dev/sdb >>>> mdadm: set device faulty failed for /dev/sdb: Device or resource busy >>>> ... >>>> >>>> latest kernel: >>>> ... >>>> $ mdadm /dev/md0 -f /dev/sdb >>>> mdadm: set /dev/sdb faulty in /dev/md0 >>>> ... >>>> >>>> The old kernel judges whether the Faulty flag is set in rdev->flags, >>>> and returns -EBUSY if not. And The latest kernel only return -EBUSY >>>> if the MD_BROKEN flag is set in mddev->flags. raid0 doesn't set >>>> error_handler, so MD_BROKEN will not be set, it will return 0. >>>> >>>> So if error_handler isn't set for a raid type, also return -EBUSY. >>> Hi, >>> Please test with: >>> https://lore.kernel.org/linux-raid/20230306130317.3418-1-mariusz.tkaczyk@linux.intel.com/ >>> >>> Thanks, >>> Mariusz >>> >> >> Hi, Mariusz >> >> Are there other patches? There are other problems with this patch. >> https://lore.kernel.org/linux-raid/20230306130317.3418-1-mariusz.tkaczyk@linux.intel.com/ >> >> md_submit_bio() >> ... >> // raid0 set disk faulty failed, but MD_BROKEN flag is set， >> // write IO will fail. >> if (unlikely(test_bit(MD_BROKEN, &mddev->flags)) && (rw == WRITE)) { >> bio_io_error(bio); >> return; >> } >> ... >> >> old kernel: >> ... >> $ mdadm /dev/md0 -f /dev/sdb >> mdadm: set device faulty failed for /dev/sdb: Device or resource busy >> >> $ mkfs.xfs /dev/md0 >> log stripe unit (524288 bytes) is too large (maximum is 256KiB) >> log stripe unit adjusted to 32KiB >> meta-data=/dev/md0 isize=512 agcount=16, agsize=1800064 blks >> = sectsz=512 attr=2, projid32bit=1 >> = crc=1 finobt=1, sparse=1, rmapbt=0 >> = reflink=1 bigtime=0 inobtcount=0 >> data = bsize=4096 blocks=28801024, imaxpct=25 >> = sunit=128 swidth=256 blks >> naming =version 2 bsize=4096 ascii-ci=0, ftype=1 >> log =internal log bsize=4096 blocks=14064, version=2 >> = sectsz=512 sunit=8 blks, lazy-count=1 >> realtime =none extsz=4096 blocks=0, rtextents=0 >> Discarding blocks...Done. >> ... >> >> >> merged patch kernel: >> ... >> # mdadm /dev/md0 -f /dev/sdb >> mdadm: set device faulty failed for /dev/sdb: Device or resource busy >> >> mkfs.xfs /dev/md0 >> log stripe unit (524288 bytes) is too large (maximum is 256KiB) >> log stripe unit adjusted to 32KiB >> meta-data=/dev/md0 isize=512 agcount=8, agsize=65408 blks >> = sectsz=512 attr=2, projid32bit=1 >> = crc=1 finobt=1, sparse=1, rmapbt=0 >> = reflink=1 bigtime=0 inobtcount=0 >> data = bsize=4096 blocks=523264, imaxpct=25 >> = sunit=128 swidth=256 blks >> naming =version 2 bsize=4096 ascii-ci=0, ftype=1 >> log =internal log bsize=4096 blocks=2560, version=2 >> = sectsz=512 sunit=8 blks, lazy-count=1 >> realtime =none extsz=4096 blocks=0, rtextents=0 >> mkfs.xfs: pwrite failed: Input/output error >> ... >> >> > Hi Wu, > Beside the kernel, there are also patches in mdadm. Please check if you have > them all. > https://git.kernel.org/pub/scm/utils/mdadm/mdadm.git/commit/?id=b3e7b7eb1dfedd7cbd9a3800e884941f67d94c96 > https://git.kernel.org/pub/scm/utils/mdadm/mdadm.git/commit/?id=461fae7e7809670d286cc19aac5bfa861c29f93a > https://git.kernel.org/pub/scm/utils/mdadm/mdadm.git/commit/?id=fc6fd4063769f4194c3fb8f77b32b2819e140fb9 > Hi, Mariusz Thanks for your reply, I would test with the above patches. > Some background: > --faulty (-f) is intended to be used by administrators. We cannot rely on > kernel answer because if mdadm will try to set device faulty, it results in > MD_BROKEN and every new IO will be failed (and that is intended change). > > Simply, mdadm must check first if it can remove the drive and that was added by > the mentioned patches. The first patch (the last one) added verification but > brings regression, the next two patches are fixes for omitted scenarios. > > Thanks, > Mariusz > . > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-03-22 8:38 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-03-21 8:56 [PATCH] raid0: fix set_disk_faulty doesn't return -EBUSY Wu Guanghao 2023-03-21 10:18 ` Mariusz Tkaczyk 2023-03-22 2:24 ` Wu Guanghao 2023-03-22 7:05 ` Mariusz Tkaczyk 2023-03-22 8:38 ` Wu Guanghao
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.