在 2024/1/18 16:02, Mariusz Tkaczyk 写道:
On Wed, 17 Jan 2024 17:37:07 +0800
linan666@xxxxxxxxxxxxxxx wrote:
From: Li Nan <linan122@xxxxxxxxxx>
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@xxxxxxxxxx>
[...]
+
+ /* 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
