Re: [PATCH] md: do not require mddev_lock() for all options

[Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>]

On Mon, 25 Sep 2023 11:05:42 +0800
Yu Kuai <yukuai1@huaweicloud.com> wrote:

> 在 2023/09/23 5:04, Song Liu 写道:
> > Hi Mariusz,
> > 
> > Sorry for the late reply.
> > 
> > On Wed, Sep 13, 2023 at 1:55 AM Mariusz Tkaczyk
> > <mariusz.tkaczyk@linux.intel.com> wrote:  
> >>
> >> We don't need to lock device to reject not supported request
> >> in array_state_store().
> >> Main motivation is to make a room for action does not require lock yet,
> >> like prepare to stop (see md_ioctl()).  
> > 
> > I made some changes to the commit log:
> > 
> >      md: do not require mddev_lock() for all options
> > 
> >      We don't need to lock device to reject not supported request
> >      in array_state_store().
> >      Main motivation is to make a room for action does not require lock yet,
> >      like prepare to stop (see md_ioctl()).
> > 
> > But I am not sure what you meant by "make a room for action does not
> > require lock yet". Could you please explain?  
> 
> Yes, this sounds confusing, if 'action does not require lock', then it
> shound not be blocked by array_state_store() with or without this patch.

In md_ioctl() we do some actions before stopping. We are verifying
how many holders are there (mddev->openers), we are setting MD_CLOSING and
sync_blockdev() is executed. I see that it is omitted in array_state_store().
https://elixir.bootlin.com/linux/latest/source/drivers/md/md.c#L7580

I meant that with this separated switch before locking mddev it is now easy to
add other actions, like mentioned code above for stopping.
> 
> > 
> > Otherwise, the code looks reasonable to me.  
> 
> Changes look good to me, after clarify commit message, feel free to add
> 
> Reviewed-by: Yu Kuai <yukuai3@huawei.com>

Thanks!
I will send v2.

Mariusz