On Tue, Mar 28, 2017 at 09:19:12PM +0800, 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);
> }
> err = mddev_lock(mddev);
> if (err) {
It would be better done at the end of set_readonly to guarantee set ro done.
Thanks,
Shaohua
> Thanks,
> -Zhilong
> > > Reviewed-by: NeilBrown <neilb@xxxxxxxx>
> > > Cc: Guoqing Jiang <gqjiang@xxxxxxxx>
> > > Signed-off-by: Zhilong Liu <zlliu@xxxxxxxx>
> > > ---
> > > drivers/md/md.c | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/md/md.c b/drivers/md/md.c
> > > index f6ae1d6..7f2db7c 100644
> > > --- a/drivers/md/md.c
> > > +++ b/drivers/md/md.c
> > > @@ -5588,6 +5588,7 @@ static int md_set_readonly(struct mddev *mddev, struct block_device *bdev)
> > > int err = 0;
> > > int did_freeze = 0;
> > > + test_and_clear_bit(MD_CLOSING, &mddev->flags);
> > I don't understand why this must be a test_and_clear.
> >
> > Thanks,
> > Shaohua
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
>
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
