Re: [PATCH] Create.c: Try few more times to stop array after failed creation

[NeilBrown <neilb@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 2334 bytes --]

On Fri, 05 Sep 2014 16:26:13 +0200 Pawel Baldysiak
<pawel.baldysiak@intel.com> 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@intel.com>
> ---
>  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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]