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:
signature.asc
Description: PGP signature
