Re: [PATCH 6/8] raid6: defer implementation selection when built-in
From: sashiko-bot@kernel.org
Date: 2026-07-08 09:23:15
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] The boot-time RAID6 benchmark runs unconditionally when built-in, ignoring the `CONFIG_RAID6_PQ_BENCHMARK` setting and the single-algorithm optimization. --
--- Patch [6]: [PATCH 6/8] raid6: defer implementation selection when built-in ---commit 2c9ec4f0b44515f49715b67c2e70f8628c5abd61 Author: Christoph Hellwig [off-list ref] Subject: raid6: defer implementation selection when built-in This commit defers the RAID6 implementation selection to avoid holding up early boot. It follows the pattern used in the RAID5 XOR code, allowing full use of the timing subsystem for benchmarking.
quoted hunk ↗ jump to hunk
diff --git a/lib/raid/raid6/algos.c b/lib/raid/raid6/algos.c index 6f5c89ab2b17b..e7984bde5157f 100644 --- a/lib/raid/raid6/algos.c +++ b/lib/raid/raid6/algos.c@@ -236,16 +236,6 @@ static int __init raid6_select_algo(void) int i, cycle; int error; - if (!IS_ENABLED(CONFIG_RAID6_PQ_BENCHMARK) || raid6_nr_algos == 1) { - pr_info("raid6: skipped pq benchmark and selected %s\n", - raid6_algos[raid6_nr_algos - 1]->name); - static_call_update(raid6_gen_syndrome_impl, - raid6_algos[raid6_nr_algos - 1]->gen_syndrome); - static_call_update(raid6_xor_syndrome_impl, - raid6_algos[raid6_nr_algos - 1]->xor_syndrome); - return 0; - } - /* prepare the buffer and fill it circularly with gfmul table */ disk_ptr = kmalloc(PAGE_SIZE * RAID6_TEST_DISKS, GFP_KERNEL);
[ ... ]
quoted hunk ↗ jump to hunk
@@ -329,13 +319,40 @@ static int __init raid6_init(void) static_call_update(raid6_recov_datap_impl, raid6_recov_algo->datap); pr_info("raid6: using %s recovery algorithm\n", raid6_recov_algo->name); + if (!IS_ENABLED(CONFIG_RAID6_PQ_BENCHMARK) || raid6_nr_algos == 1) { + pr_info("raid6: skipped pq benchmark and selected %s\n", + raid6_algos[raid6_nr_algos - 1]->name); + goto default_algo; + } + +#ifdef MODULE return raid6_select_algo(); +#else + /* + * Pick the last registered implementation as the temporary default until + * calibration happens. + */ +#endif
[ ... ]
+/* + * When built-in we must register the default implementation before md + * initializes, but we don't want calibration to run that early as that + * would delay the boot process. + */ +#ifndef MODULE +device_initcall(raid6_select_algo); +#endif
[Severity: Medium] Will this unconditional device_initcall cause the benchmark to run even when it is intentionally disabled in the configuration? While raid6_init() checks CONFIG_RAID6_PQ_BENCHMARK and correctly skips the early calibration phase, removing that same check from raid6_select_algo() means this deferred call will unconditionally run the benchmarking loop later in the boot process. It looks like this bypasses both the config setting and the single-algorithm optimization entirely.
subsys_initcall(raid6_init);
module_exit(raid6_exit);
MODULE_LICENSE("GPL");-- Sashiko AI review · https://sashiko.dev/#/patchset/20260708090740.1433685-1-hch@lst.de?part=6