From: Li Nan <linan666@huaweicloud.com>
To: Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>,
linan666@huaweicloud.com
Cc: song@kernel.org, shli@fb.com, neilb@suse.com, zlliu@suse.com,
linux-raid@vger.kernel.org, linux-kernel@vger.kernel.org,
yukuai3@huawei.com, yi.zhang@huawei.com, houtao1@huawei.com,
yangerkun@huawei.com
Subject: Re: [PATCH v2 3/3] md: sync blockdev before stopping raid or setting readonly
Date: Wed, 24 Jan 2024 11:16:48 +0800 [thread overview]
Message-ID: <425044a0-95ce-b2a3-e98d-527c31f0313a@huaweicloud.com> (raw)
In-Reply-To: <20240118090205.00000212@linux.intel.com>
在 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
prev parent reply other threads:[~2024-01-24 3:16 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-17 9:37 [PATCH v2 0/3] md: Don't clear MD_CLOSING when the raid is about to stop linan666
2024-01-17 9:37 ` [PATCH v2 1/3] " linan666
2024-01-18 7:35 ` Mariusz Tkaczyk
2024-01-22 2:14 ` Li Nan
2024-01-17 9:37 ` [PATCH v2 2/3] md: factor out a helper mddev_sync_blockdev() to sync mddev linan666
2024-01-18 8:08 ` Mariusz Tkaczyk
2024-01-22 2:15 ` Li Nan
2024-01-17 9:37 ` [PATCH v2 3/3] md: sync blockdev before stopping raid or setting readonly linan666
2024-01-18 8:02 ` Mariusz Tkaczyk
2024-01-24 3:16 ` Li Nan [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=425044a0-95ce-b2a3-e98d-527c31f0313a@huaweicloud.com \
--to=linan666@huaweicloud.com \
--cc=houtao1@huawei.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-raid@vger.kernel.org \
--cc=mariusz.tkaczyk@linux.intel.com \
--cc=neilb@suse.com \
--cc=shli@fb.com \
--cc=song@kernel.org \
--cc=yangerkun@huawei.com \
--cc=yi.zhang@huawei.com \
--cc=yukuai3@huawei.com \
--cc=zlliu@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).