From 1d38160ad82aef0dfd231b143a45d873edeb752c Mon Sep 17 00:00:00 2001 From: Douglas Raillard Date: Mon, 11 Sep 2023 10:34:13 +0100 Subject: [PATCH 1/3] lisa._assets.kmodules.lisa: Fail module init if unknown feature is requested FIX If an unknown feature is requested, fail module init instead of silently ignoring that request. --- lisa/_assets/kmodules/lisa/features.c | 43 ++++++++++++++++----------- 1 file changed, 25 insertions(+), 18 deletions(-) diff --git a/lisa/_assets/kmodules/lisa/features.c b/lisa/_assets/kmodules/lisa/features.c index ee253288b..76d4f633e 100644 --- a/lisa/_assets/kmodules/lisa/features.c +++ b/lisa/_assets/kmodules/lisa/features.c @@ -58,28 +58,35 @@ int __disable_feature(struct feature* feature) { typedef int (*feature_process_t)(struct feature*); -static int __select_feature(struct feature* feature, char **selected, size_t selected_len, feature_process_t process) { - size_t i; - if (selected_len) { - for (i=0; i < selected_len; i++) { - if (!strcmp(selected[i], feature->name)) - return process(feature); - } - return 0; - } else if (!feature->__internal) { - return process(feature); - } else { - return 0; - } -} - static int __process_features(char **selected, size_t selected_len, feature_process_t process) { - struct feature *feature; int ret = 0; - for (feature=__lisa_features_start; feature < __lisa_features_stop; feature++) { - ret |= __select_feature(feature, selected, selected_len, process); + if (selected) { + // User asked for a specific set of features + for (size_t i=0; i < selected_len; i++) { + bool found = false; + + for (struct feature *feature=__lisa_features_start; feature < __lisa_features_stop; feature++) { + if (!strcmp(feature->name, selected[i])) { + found = true; + ret |= process(feature); + break; + } + } + if (!found) { + pr_err("Unknown or compile-time disabled feature: %s", selected[i]); + ret |= 1; + } + } + } else { + // User did not ask for any particular feature, so try to enable all non-internal features. + for (struct feature* feature=__lisa_features_start; feature < __lisa_features_stop; feature++) { + if (!feature->__internal) { + ret |= process(feature); + } + } } + return ret; } -- GitLab From 71eba2e302d574be8456dcee9b8169bb57778844 Mon Sep 17 00:00:00 2001 From: Douglas Raillard Date: Mon, 11 Sep 2023 10:39:05 +0100 Subject: [PATCH 2/3] lisa._assets.kmodules.lisa: List features over multiple lines Display feature list over multiple lines. It takes the risk of being interlaced with other dmesg output, but it's easy to filter for anything starting with "lisa:" to filter out the rest. --- lisa/_assets/kmodules/lisa/features.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lisa/_assets/kmodules/lisa/features.c b/lisa/_assets/kmodules/lisa/features.c index 76d4f633e..13bba8fcf 100644 --- a/lisa/_assets/kmodules/lisa/features.c +++ b/lisa/_assets/kmodules/lisa/features.c @@ -93,7 +93,7 @@ static int __process_features(char **selected, size_t selected_len, feature_proc static int __list_feature(struct feature* feature) { if (!feature->__internal) - printk(KERN_CONT "%s, ", feature->name); + pr_info(" %s", feature->name); return 0; } @@ -107,9 +107,8 @@ static int __enable_feature_explicitly(struct feature* feature) { int init_features(char **selected, size_t selected_len) { BUG_ON(MAX_FEATURES < ((__lisa_features_stop - __lisa_features_start) / sizeof(struct feature))); - pr_info("Available features: "); + pr_info("Available features:"); __process_features(NULL, 0, __list_feature); - pr_info("\n"); return __process_features(selected, selected_len, __enable_feature_explicitly); } -- GitLab From 8b4a275f4fed62eb506ffde085ba1c9399278468 Mon Sep 17 00:00:00 2001 From: Douglas Raillard Date: Mon, 11 Sep 2023 13:01:48 +0100 Subject: [PATCH 3/3] lisa._assets.kmodules.lisa: Refcount explicit feature enabling FIX Refcount requests to enable a feature, so that we will reliably disable it when unloading the module. --- lisa/_assets/kmodules/lisa/features.c | 9 +++++---- lisa/_assets/kmodules/lisa/features.h | 8 ++++---- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/lisa/_assets/kmodules/lisa/features.c b/lisa/_assets/kmodules/lisa/features.c index 13bba8fcf..01e5c2c8c 100644 --- a/lisa/_assets/kmodules/lisa/features.c +++ b/lisa/_assets/kmodules/lisa/features.c @@ -99,7 +99,7 @@ static int __list_feature(struct feature* feature) { static int __enable_feature_explicitly(struct feature* feature) { mutex_lock(feature->lock); - feature->__explicitly_enabled = true; + feature->__explicitly_enabled++; mutex_unlock(feature->lock); return __enable_feature(feature); } @@ -113,14 +113,15 @@ int init_features(char **selected, size_t selected_len) { } static int __disable_explicitly_enabled_feature(struct feature* feature) { - bool selected; int ret = 0; mutex_lock(feature->lock); - selected = feature->__explicitly_enabled; + int selected = feature->__explicitly_enabled; mutex_unlock(feature->lock); - if (selected) + while (selected) { ret |= __disable_feature(feature); + selected--; + } return ret; } diff --git a/lisa/_assets/kmodules/lisa/features.h b/lisa/_assets/kmodules/lisa/features.h index b17411f5c..31e322f41 100644 --- a/lisa/_assets/kmodules/lisa/features.h +++ b/lisa/_assets/kmodules/lisa/features.h @@ -34,8 +34,8 @@ struct feature { /* Return code of the enable() function */ int __enable_ret; - /* true if the feature has been explicitly enabled by the user */ - bool __explicitly_enabled; + /* Count of the times the feature has been explicitly enabled by the user */ + int __explicitly_enabled; /* true if the feature is internal, i.e. not exposed to the user. * Internal features are used to share some code between feature, taking * advantage of reference counting to ensure safe setup/teardown. @@ -66,7 +66,7 @@ int __placeholder_deinit(struct feature *feature); .name = #feature_name, \ .data = NULL, \ .enabled = 0, \ - .__explicitly_enabled = false, \ + .__explicitly_enabled = 0, \ .enable = __placeholder_init, \ .disable = __placeholder_deinit, \ .lock = &__lisa_mutex_feature_##feature_name, \ @@ -80,7 +80,7 @@ int __placeholder_deinit(struct feature *feature); .name = #feature_name, \ .data = NULL, \ .enabled = 0, \ - .__explicitly_enabled = false, \ + .__explicitly_enabled = 0, \ .enable = enable_f, \ .disable = disable_f, \ .lock = &__lisa_mutex_feature_##feature_name, \ -- GitLab