On Tue, Nov 21 2017, wuzhouhui wrote:
> Hi, I have a suggest about mdadm to set disk faulty. Since commit 1ca69c4bc4b1ef
> (md: avoid taking the mutex on some ioctls) removes lock when set disk faulty,
> so we'd better using ioctl(SET_DISK_FAULTY) to set disk faulty, instead of
> echo faulty > /sys/block/<md>/md/dev-<disk>/state, because caller of state's
> handler would lock mddev. Like following:
>
> Index: mdadm-4.0/Manage.c
> ===================================================================
> --- mdadm-4.0.orig/Manage.c
> +++ mdadm-4.0/Manage.c
> @@ -1662,9 +1662,7 @@ int Manage_subdevs(char *devname, int fd
>
> case 'f': /* set faulty */
> /* FIXME check current member */
> - if ((sysfd >= 0 && write(sysfd, "faulty", 6) != 6) ||
> - (sysfd < 0 && ioctl(fd, SET_DISK_FAULTY,
> - rdev))) {
> + if (ioctl(fd, SET_DISK_FAULTY, rdev)) {
> if (errno == EBUSY)
> busy = 1;
> pr_err("set device faulty failed for %s: %s\n",
>
This isn't just a debatable optimization. It is wrong and would
introduce a regression.
Please make sure you understand code *before* trying to change it.
Please look at the git history for this code and find the patch where
writing "faulty" was added as an alternative. That might help you to
understand what is happening here.
NeilBrown
Attachment:
signature.asc
Description: PGP signature
