Re: [PATCH] md:array cannot be opened again after 'md_set_readonly'

[NeilBrown <neilb@suse.com>]

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

On Tue, Mar 28 2017, Zhilong Liu wrote:

> On 03/28/2017 02:22 AM, Shaohua Li wrote:
>> On Mon, Mar 27, 2017 at 03:52:25PM +0800, Zhilong Liu wrote:
>>> This is a bug about array cannot be opened again after 'md_set_readonly',
>>> because the MD_CLOSING bit is still waiting for clear.
>>> MD_CLOSING should only be set for a short period or time to avoid certain
>>> races. After the operation that set it completes, it should be cleared.
>> where is the bit set? Why don't clear it after the operation but clear it in
>> set_readonly?
>>   
>
> it goes two paths after set MD_CLOSING bit, do_md_stop and md_set_readonly,
> the do_md_stop has done the md_clean to clear the mddev->flags, thus I 
> put it in
> md_set_readonly.
> Thanks for your suggestion, is the following changing good for you? here 
> it has
> finished what it needs to do, so clear the MD_CLOSING bit in time.
> if looks good, I would send this in new revision patch.
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index f6ae1d6..e8c1130 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -6868,6 +6868,7 @@ static int md_ioctl(struct block_device *bdev, 
> fmode_t mode,
>                  set_bit(MD_CLOSING, &mddev->flags);
>                  mutex_unlock(&mddev->open_mutex);
>                  sync_blockdev(bdev);
> +               clear_bit(MD_CLOSING, &mddev->flags);

No.

MD_CLOSING is used to prevent a race with md_open().
md_open() blocks on open_mutex(), but we cannot hold that here for
complicated reasons.
So we set MD_CLOSING and need to keep it set at least until
md_set_readonly() or do_md_stop() take that lock again.

Clearing it here just opens up the race again.

It needs to be cleared in md_ioctl, after any call to md_set_readonly()
or do_md_stop().
I've already posted a sample patch which has been tested.  Please don't
do something completely different.

NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]