Messages in this thread | | | Subject | Re: [PATCH -next v2 3/7] md: delay choosing sync direction to md_start_sync() | From | Yu Kuai <> | Date | Sun, 20 Aug 2023 10:04:23 +0800 |
| |
Hi,
在 2023/08/16 14:38, Xiao Ni 写道: > On Tue, Aug 15, 2023 at 11:13 AM Yu Kuai <yukuai1@huaweicloud.com> wrote: >> >> From: Yu Kuai <yukuai3@huawei.com> >> >> Before this patch, for read-write array: >> >> 1) md_check_recover() found that something need to be done, and it'll >> try to grab 'reconfig_mutex'. The case that md_check_recover() need >> to do something: >> - array is not suspend; >> - super_block need to be updated; >> - 'MD_RECOVERY_NEEDED' or ''MD_RECOVERY_DONE' is set; >> - unusual case related to safemode; >> >> 2) if 'MD_RECOVERY_RUNNING' is not set, and 'MD_RECOVERY_NEEDED' is set, >> md_check_recover() will try to choose a sync direction, and then >> queue a work md_start_sync(). >> >> 3) md_start_sync() register sync_thread; >> >> After this patch, >> >> 1) is the same; >> 2) if 'MD_RECOVERY_RUNNING' is not set, and 'MD_RECOVERY_NEEDED' is set, >> queue a work md_start_sync() directly; >> 3) md_start_sync() will try to choose a sync direction, and then >> register sync_thread(); >> >> Because 'MD_RECOVERY_RUNNING' is cleared when sync_thread is done, 2) >> and 3) is always ran in serial and they can never concurrent, this >> change should not introduce any behavior change for now. >> >> Also fix a problem that md_start_sync() can clear 'MD_RECOVERY_RUNNING' >> without protection in error path, which might affect the logical in >> md_check_recovery(). >> >> The advantage to change this is that array reconfiguration is >> independent from daemon now, and it'll be much easier to synchronize it >> with io, consider that io may rely on daemon thread to be done. > > Hi Kuai > > What's the meaning of "array reconfiguration" here? "mdadm -f/-r/-a" > something like this, right?. Because before and after this patch, only > one sync thread can be running, so If we don't do this change, are > there bugs or performance problems?
As we discussed([1]), and explained in patch 0, the purpose of this change is to prepare to synchronize io with array reconfiguration(add or remove rdev from array, for example, modify conf->mirrors[].rdev/replacement for raid10).
Without this change, normal io can rely on daemon thread, while daemone thread can change array configuration. raid1/raid10 record such io as 'io_queued', and can use freeze_array() to do synchronization in daemon thread. However, other personalities have to implement such logical as well, and I found it quite complicated, at least for raid456.
[1] https://lore.kernel.org/all/cb390b39-1b1e-04e1-55ad-2ff8afc47e1b@huawei.com/
Thanks, Kuai
> > If it's only a patch that wants to make md_check_recovery more clearer > and easier, I'm good with this idea too. >
| |