diff mbox series

[eoan:linux-gcp,1/2] UBUNTU: SAUCE: raid6: Add option for default PQ algorithm

Message ID 20191106185742.5835-2-marcelo.cerri@canonical.com
State New
Headers show
Series [eoan:linux-gcp,1/2] UBUNTU: SAUCE: raid6: Add option for default PQ algorithm | expand

Commit Message

Marcelo Henrique Cerri Nov. 6, 2019, 6:57 p.m. UTC 
BugLink: https://bugs.launchpad.net/bugs/1812728
BugLink: https://bugs.launchpad.net/bugs/1851538

Add a new config ("CONFIG_RAID6_PQ_DEFAULT_ALG_BOOL") to enable
specifying a default gen() algorithm for RAID_PQ via the kernel
"raid6_pq_default_alg" cmdline option. When a default algorithm is
given, the raid6_pq module will skip the performance round of tests.

The config CONFIG_RAID6_PQ_DEFAULT_ALG can be used to define a default
value for the kernel option.

Signed-off-by: Marcelo Henrique Cerri <marcelo.cerri@canonical.com>
---
 lib/Kconfig       | 17 +++++++++++++++++
 lib/raid6/algos.c | 26 ++++++++++++++++++++++++++
 2 files changed, 43 insertions(+)

Comments

Sultan Alsawaf Nov. 6, 2019, 9:47 p.m. UTC | #1 
On Wed, Nov 06, 2019 at 03:57:38PM -0300, Marcelo Henrique Cerri wrote:
> +	if (strlen(raid6_pq_default_alg)) {

The strlen() can be replaced with raid6_pq_default_alg[0].

> +		for (algo = raid6_algos; *algo; algo++) {
> +			if (!strncmp(raid6_pq_default_alg, (*algo)->name, sizeof(raid6_pq_default_alg))) {

(*algo)->name is a statically initialized kernel string, which is terminated.
The module parameter string set function also ensures that raid6_pq_default_alg
will be terminated. You can safely use strcmp() here instead of strncmp().

> +				if ((*algo)->valid && !(*algo)->valid()) {
> +					pr_info("raid6: default alg \"%s\" is invalid.\n",
> +						raid6_pq_default_alg);
> +					continue;
> +				}

Why continue if the default algorithm is invalid?

> +				pr_info("raid6: using default algorithm %s gen() without performace tests.\n",
> +					(*algo)->name);

Misspelling: s/performace/performance/
Also, as per the kernel coding style guide, kernel messages do not need to be
terminated with a period.

Other than those nits, this seems fine.

Sultan
diff mbox series

Patch

diff --git a/lib/Kconfig b/lib/Kconfig
index 4e6b1c3e4c98..dae35c80dfda 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -19,6 +19,23 @@  config RAID6_PQ_BENCHMARK
 	  Benchmark all available RAID6 PQ functions on init and choose the
 	  fastest one.
 
+config RAID6_PQ_DEFAULT_ALG_BOOL
+	bool "Default RAID6 PQ algorithm"
+	default n
+	depends on RAID6_PQ
+	help
+	  Allow for specifying a default algorithm via the kernel
+	  parameter "raid6_pq_default_alg", which forces the performance
+	  tests to be skipped. This can save between 500ms to 2s
+	  during boot.
+
+config RAID6_PQ_DEFAULT_ALG
+	string "Default RAID6 PQ algorithm name"
+	default ""
+	depends on RAID6_PQ_DEFAULT_ALG_BOOL
+	help
+	  The default algorithm name to be used by default.
+
 config PACKING
 	bool "Generic bitfield packing and unpacking"
 	default n
diff --git a/lib/raid6/algos.c b/lib/raid6/algos.c
index 608b648261bd..67af502eada3 100644
--- a/lib/raid6/algos.c
+++ b/lib/raid6/algos.c
@@ -25,6 +25,12 @@  EXPORT_SYMBOL(raid6_empty_zero_page);
 #endif
 #endif
 
+#ifdef CONFIG_RAID6_PQ_DEFAULT_ALG_BOOL
+static char raid6_pq_default_alg[32] = CONFIG_RAID6_PQ_DEFAULT_ALG;
+module_param_string(raid6_pq_default_alg, raid6_pq_default_alg, sizeof(raid6_pq_default_alg), 0444);
+MODULE_PARM_DESC(raid6_pq_default_alg, "Default gen/xor() algorithm");
+#endif
+
 struct raid6_calls raid6_call;
 EXPORT_SYMBOL_GPL(raid6_call);
 
@@ -153,6 +159,26 @@  static inline const struct raid6_calls *raid6_choose_gen(
 	const struct raid6_calls *const *algo;
 	const struct raid6_calls *best;
 
+#ifdef CONFIG_RAID6_PQ_DEFAULT_ALG_BOOL
+	if (strlen(raid6_pq_default_alg)) {
+		for (algo = raid6_algos; *algo; algo++) {
+			if (!strncmp(raid6_pq_default_alg, (*algo)->name, sizeof(raid6_pq_default_alg))) {
+				if ((*algo)->valid && !(*algo)->valid()) {
+					pr_info("raid6: default alg \"%s\" is invalid.\n",
+						raid6_pq_default_alg);
+					continue;
+				}
+				pr_info("raid6: using default algorithm %s gen() without performace tests.\n",
+					(*algo)->name);
+				raid6_call = **algo;
+				return *algo;
+			}
+		}
+		pr_info("raid6: default alg \"%s\" not found. Choosing the best alg as fallback...\n",
+			raid6_pq_default_alg);
+	}
+#endif
+
 	for (bestgenperf = 0, bestxorperf = 0, best = NULL, algo = raid6_algos; *algo; algo++) {
 		if (!best || (*algo)->prefer >= best->prefer) {
 			/* 2 ^ (RAID6_TIME_JIFFIES_LG2 - 0.5) */