Thread (6 messages) 6 messages, 3 authors, 2017-03-28

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

From: NeilBrown <hidden>
Date: 2017-03-28 21:18:02

On Tue, Mar 28 2017, Zhilong Liu wrote:
quoted hunk ↗ jump to hunk
On 03/28/2017 02:22 AM, Shaohua Li wrote:
quoted
On Mon, Mar 27, 2017 at 03:52:25PM +0800, Zhilong Liu wrote:
quoted
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

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help