Hi,
在 2023/12/07 1:24, Song Liu 写道:
On Wed, Dec 6, 2023 at 3:36 AM Yu Kuai <yukuai1@xxxxxxxxxxxxxxx> wrote:
Hi,
在 2023/12/06 16:30, Song Liu 写道:
On Sun, Dec 3, 2023 at 7:18 PM Yu Kuai <yukuai1@xxxxxxxxxxxxxxx> wrote:
From: Yu Kuai <yukuai3@xxxxxxxxxx>
New mddev_resume() calls are added to synchroniza IO with array
reconfiguration, however, this introduce a regression while adding it in
md_start_sync():
1) someone set MD_RECOVERY_NEEDED first;
2) daemon thread grab reconfig_mutex, then clear MD_RECOVERY_NEEDED and
queue a new sync work;
3) daemon thread release reconfig_mutex;
4) in md_start_sync
a) check that there are spares that can be added/removed, then suspend
the array;
b) remove_and_add_spares may not be called, or called without really
add/remove spares;
c) resume the array, then set MD_RECOVERY_NEEDED again!
Loop between 2 - 4, then mddev_suspend() will be called quite often, for
consequence, normal IO will be quite slow.
Fix this problem by spliting MD_RECOVERY_NEEDED out of mddev_resume(), so
that md_start_sync() won't set such flag and hence the loop will be broken.
I hope we don't leak set_bit MD_RECOVERY_NEEDED to all call
sites of mddev_resume().
There are also some other mddev_resume() that is added later and don't
need recovery, so md_start_sync() is not the only place:
- md_setup_drive
- rdev_attr_store
- suspend_lo_store
- suspend_hi_store
- autorun_devices
- md_ioct
- r5c_disable_writeback_async
- error path from new_dev_store(), ...
I'm not sure add a new helper is a good idea, because all above apis
should use new helper as well.
I think for most of these call sites, it is OK to set MD_RECOVERY_NEEDED
(although it is not needed), and md_start_sync() is the only one that may
trigger "loop between 2 - 4" scenario. Did I miss something?
Yes, it's the only problematic one. I'll send v2.
Thanks,
Kuai
It is already rc4, so we need to send the fix soon.
Thanks,
Song
.
