Messages in this thread | ![/](/images/icornerl.gif) | | Date | Wed, 24 Jan 2024 11:16:48 +0800 | Subject | Re: [PATCH v2 3/3] md: sync blockdev before stopping raid or setting readonly | From | Li Nan <> |
| |
在 2024/1/18 16:02, Mariusz Tkaczyk 写道: > On Wed, 17 Jan 2024 17:37:07 +0800 > linan666@huaweicloud.com wrote: > >> From: Li Nan <linan122@huawei.com> >> >> Commit a05b7ea03d72 ("md: avoid crash when stopping md array races >> with closing other open fds.") added sync_block before stopping raid and >> setting readonly. Later in commit 260fa034ef7a ("md: avoid deadlock when >> dirty buffers during md_stop.") it is moved to ioctl. array_state_store() >> was ignored. Add sync blockdev to array_state_store() now. >> >> Signed-off-by: Li Nan <linan122@huawei.com>
[...]
>> + >> + /* we will call set readonly or stop raid, sync blockdev */ >> + if (st == clear || (mddev->pers && (st == readonly || >> + st == inactive || (st == read_auto && md_is_rdwr(mddev))))) { >> + err = mddev_sync_blockdev(mddev); >> + if (err) >> + return err; >> + clear_md_closing = true; >> + } >> + > > Please reorganize it a little for readability: > I think if no mddev->pers we don't need to consider sync_blockdev at all. If > personality is there we can probably check for read-write. If it is not > read-write then nothing to sync. What about that: > > if (mddev->pers && md_is_rdwr(mddev) && > (st == clear || st == readonly || st == inactive || st == read_auto)) > > Please note that I didn't test it so please let me know if you see issue in > proposed logic. > I think that we may be able to include it in "/* No lock dependent actions */" > switch. Please consider it too: >
Thanks for your review. It is a really good idea. I will test and improve it.
> case clear: > case readonly: > case inactive: > case read_auto: > if(!mddev->pers || !md_is_rdwr(mddev)) > break; > err = mddev_sync_blockdev(mddev); > if (err) > return err; > clear_md_closing = true; >
-- Thanks, Nan
| ![\](/images/icornerr.gif) |