Hi,
在 2024/11/22 15:55, Mariusz Tkaczyk 写道:
On Fri, 22 Nov 2024 09:13:18 +0800
Yu Kuai <yukuai1@xxxxxxxxxxxxxxx> wrote:
在 2024/11/21 16:15, Mariusz Tkaczyk 写道:
On Thu, 21 Nov 2024 09:25:50 +0800
Yu Kuai <yukuai1@xxxxxxxxxxxxxxx> wrote:
BitmapUnknown should be used only if we failed to parse bitmap setting in
cmdline. Otherwise first and default value should be always BitmapNone
because data access is always highest priority and dropping bitmap is
always safe. We can print warning in config parse failed or bitmap value
is repeated- it is reasonable. If I'm wrong here, please let me know.
Hi, there is a little difference betewwn BitmapNone and BitmapUnknow, if
user doesn't pass in the "bitmap=xxx", then the BitmapUnkonw will be
used to decide choosing BitmapNone or BimtapInternal based on the disk
size. In Create:
if (!s->bitmap_file &&
┊ !st->ss->external &&
┊ s->level >= 1 &&
┊ st->ss->add_internal_bitmap &&
┊ s->journaldisks == 0 &&
┊ (s->consistency_policy != CONSISTENCY_POLICY_RESYNC &&
┊ s->consistency_policy != CONSISTENCY_POLICY_PPL) &&
┊ (s->write_behind || s->size > 100*1024*1024ULL)) {
if (c->verbose > 0)
pr_err("automatically enabling write-intent
bitmap on large array\n");
s->bitmap_file = "internal";
}
And I realized that I should used BitmapUnknow here, not BimtapNone.
Oh yes.. Looking on that from the interface perspective suggest me that we
should remove it and always let user to decide. If the are not satisfied
with resync times they can enable bitmap in any moment but it may cause
functional regression for users that are used to this auto turning on.
Maybe, we can move it to main() and ask without checking raid size, assuming
that array size <100GB is used mainly for testing nowadays?
Here, proposal based on current code, your change may require some
adjustments:
diff --git a/mdadm.c b/mdadm.c
index 8cb4ba66ac20..2e803d441dd4 100644
--- a/mdadm.c
+++ b/mdadm.c
@@ -1535,6 +1535,13 @@ int main(int argc, char *argv[])
break;
}
+ if (!s->bitmap_file && !c.runstop != 1 && s->level >= 1) {
+ int response = ask("To optimalize resync speed, it
is recommended to enable write-indent bitmap, do you want to enable it
now?"); +
+ if (response)
+ s->bitmap_file = "internal";
+ }
+
rv = Create(ss, &ident, devs_found - 1, devlist->next, &s,
&c); break;
case MISC:
This is more reasonable than auto-forcing bitmap without possibility
to skip it (even for testing). I added c->runstop verification because it is
often used in Create to skip some errors and questions.
What do you think?
I think it's good! I used to be curious why bitmap is not enabled by
default for testing, and have to look into the source code.
One note here (this one is easy to be missed):
If user set --bitmap=None we should not prompt this question, assuming that user
already made his decision. You need to differentiate default BitmapNone
and user defined BitmapNone (boolean is_bitmap_set should be fine, because
adding another enum status is not valuable I think).
TBO, I'll prefer keep the BitmapUnknow for initiazation. Only prompt
this question for BitmapUnknow, and switch to none or internal for
response.
Thanks,
Kuai
Mariusz
.
