On Fri, 05 Sep 2014 16:26:13 +0200 Pawel Baldysiak
<pawel.baldysiak@xxxxxxxxx> wrote:
> Sometimes after failure in creation (exp. due to duplicate devices
> in create command) newly created empty md array will not be stopped
> due to openers>1 (create_mddev will not manage to drop lock).
> In this case ioctl() will return error - this needs to be checked
> and if occurs - sending STOP_ARRAY should be repeat after delay
> to make sure that mddev is stopped correctly.
>
> Signed-off-by: Pawel Baldysiak <pawel.baldysiak@xxxxxxxxx>
> ---
> Create.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/Create.c b/Create.c
> index 330c5b4..7c8e53e 100644
> --- a/Create.c
> +++ b/Create.c
> @@ -904,7 +904,12 @@ int Create(struct supertype *st, char *mddev,
> if (st->ss->add_to_super(st, &inf->disk,
> fd, dv->devname,
> dv->data_offset)) {
> - ioctl(mdfd, STOP_ARRAY, NULL);
> + int count = 5;
> + while (count &&
> + (ioctl(mdfd, STOP_ARRAY, NULL) < 0)) {
> + usleep(100000);
> + count--;
> + }
> goto abort_locked;
> }
> st->ss->getinfo_super(st, inf, NULL);
I don't like this. I don't really like any of the other loops like this that
are already in the code either. I wonder if we can avoid the need for it.
Given that the array hasn't been started yet, no other process can actually
be *using* the array. And given that we have an O_EXCL open at this point,
no other process can be trying to stop/start the array.
So it should be safe to change the kernel to not fail in this situation.
If you apply this kernel patch:
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 1294238610df..1bf3fe1ecc79 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -5362,7 +5362,7 @@ static int do_md_stop(struct mddev * mddev, int mode,
mddev_lock_nointr(mddev);
mutex_lock(&mddev->open_mutex);
- if (atomic_read(&mddev->openers) > !!bdev ||
+ if ((mddev->pers && atomic_read(&mddev->openers) > !!bdev) ||
mddev->sysfs_active ||
mddev->sync_thread ||
(bdev && !test_bit(MD_STILL_CLOSED, &mddev->flags))) {
does that fir your problem? Can you see any reason not to allow STOP_ARRAY
to succeed in this situation?
Thanks,
NeilBrown
Attachment:
signature.asc
Description: PGP signature
