On Mon, 25 Sep 2023 11:05:42 +0800 Yu Kuai <yukuai1@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx> 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@xxxxxxxxxx> Thanks! I will send v2. Mariusz
