[PATCH] lib/test_kmod: do not hardcode/depend on any filesystem

Herton R. Krzesinski posted 1 patch 8 months ago
There is a newer version of this series
lib/Kconfig.debug |  6 -----
lib/test_kmod.c   | 64 +++++++++++++++++++++++++----------------------
2 files changed, 34 insertions(+), 36 deletions(-)
[PATCH] lib/test_kmod: do not hardcode/depend on any filesystem
Posted by Herton R. Krzesinski 8 months ago
Right now test_kmod has hardcoded dependencies on btrfs/xfs. That
is not optimal since you end up needing to select/build them, but it
is not really required since other fs could be selected for the testing.
Also, we can't change the default/driver module used for testing on
initialization.

Thus make it more generic: introduce two module parameters (start_driver
and start_test_fs), which allow to select which modules/fs to use for
the testing on test_kmod initialization. Then it's up to the user to
select which modules/fs to use for testing based on his config. However,
keep test_module as required default.

This way, config/modules becomes selectable as when the testing is done
from selftests (userspace).

While at it, also change trigger_config_run_type, since at module
initialization we already set the defaults at __kmod_config_init and
should not need to do it again in test_kmod_init(), thus we can
avoid to again set test_driver/test_fs.

Signed-off-by: Herton R. Krzesinski <herton@redhat.com>
---
 lib/Kconfig.debug |  6 -----
 lib/test_kmod.c   | 64 +++++++++++++++++++++++++----------------------
 2 files changed, 34 insertions(+), 36 deletions(-)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 9fe4d8dfe578..1e0f8cbf0365 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2983,13 +2983,7 @@ config TEST_DYNAMIC_DEBUG
 config TEST_KMOD
 	tristate "kmod stress tester"
 	depends on m
-	depends on NETDEVICES && NET_CORE && INET # for TUN
-	depends on BLOCK
-	depends on PAGE_SIZE_LESS_THAN_256KB # for BTRFS
 	select TEST_LKM
-	select XFS_FS
-	select TUN
-	select BTRFS_FS
 	help
 	  Test the kernel's module loading mechanism: kmod. kmod implements
 	  support to load modules using the Linux kernel's usermode helper.
diff --git a/lib/test_kmod.c b/lib/test_kmod.c
index 064ed0fce75a..f0dd092860ea 100644
--- a/lib/test_kmod.c
+++ b/lib/test_kmod.c
@@ -28,14 +28,20 @@
 
 #define TEST_START_NUM_THREADS	50
 #define TEST_START_DRIVER	"test_module"
-#define TEST_START_TEST_FS	"xfs"
 #define TEST_START_TEST_CASE	TEST_KMOD_DRIVER
 
-
 static bool force_init_test = false;
-module_param(force_init_test, bool_enable_only, 0644);
+module_param(force_init_test, bool_enable_only, 0444);
 MODULE_PARM_DESC(force_init_test,
 		 "Force kicking a test immediately after driver loads");
+static char *start_driver;
+module_param(start_driver, charp, 0444);
+MODULE_PARM_DESC(start_driver,
+		 "Module/driver to use for the testing after driver loads");
+static char *start_test_fs;
+module_param(start_test_fs, charp, 0444);
+MODULE_PARM_DESC(start_test_fs,
+		 "File system to use for the testing after driver loads");
 
 /*
  * For device allocation / registration
@@ -508,6 +514,11 @@ static int __trigger_config_run(struct kmod_test_device *test_dev)
 	case TEST_KMOD_DRIVER:
 		return run_test_driver(test_dev);
 	case TEST_KMOD_FS_TYPE:
+		if (!config->test_fs) {
+			dev_warn(test_dev->dev,
+				 "No fs type specified, can't run the test\n");
+			return -EINVAL;
+		}
 		return run_test_fs_type(test_dev);
 	default:
 		dev_warn(test_dev->dev,
@@ -721,26 +732,20 @@ static ssize_t config_test_fs_show(struct device *dev,
 static DEVICE_ATTR_RW(config_test_fs);
 
 static int trigger_config_run_type(struct kmod_test_device *test_dev,
-				   enum kmod_test_case test_case,
-				   const char *test_str)
+				   enum kmod_test_case test_case)
 {
-	int copied = 0;
 	struct test_config *config = &test_dev->config;
 
 	mutex_lock(&test_dev->config_mutex);
 
 	switch (test_case) {
 	case TEST_KMOD_DRIVER:
-		kfree_const(config->test_driver);
-		config->test_driver = NULL;
-		copied = config_copy_test_driver_name(config, test_str,
-						      strlen(test_str));
 		break;
 	case TEST_KMOD_FS_TYPE:
-		kfree_const(config->test_fs);
-		config->test_fs = NULL;
-		copied = config_copy_test_fs(config, test_str,
-					     strlen(test_str));
+		if (!config->test_fs) {
+			mutex_unlock(&test_dev->config_mutex);
+			return 0;
+		}
 		break;
 	default:
 		mutex_unlock(&test_dev->config_mutex);
@@ -751,11 +756,6 @@ static int trigger_config_run_type(struct kmod_test_device *test_dev,
 
 	mutex_unlock(&test_dev->config_mutex);
 
-	if (copied <= 0 || copied != strlen(test_str)) {
-		test_dev->test_is_oom = true;
-		return -ENOMEM;
-	}
-
 	test_dev->test_is_oom = false;
 
 	return trigger_config_run(test_dev);
@@ -800,19 +800,24 @@ static unsigned int kmod_init_test_thread_limit(void)
 static int __kmod_config_init(struct kmod_test_device *test_dev)
 {
 	struct test_config *config = &test_dev->config;
+	const char *test_start_driver = start_driver ? start_driver :
+						       TEST_START_DRIVER;
 	int ret = -ENOMEM, copied;
 
 	__kmod_config_free(config);
 
-	copied = config_copy_test_driver_name(config, TEST_START_DRIVER,
-					      strlen(TEST_START_DRIVER));
-	if (copied != strlen(TEST_START_DRIVER))
+	copied = config_copy_test_driver_name(config, test_start_driver,
+					      strlen(test_start_driver));
+	if (copied != strlen(test_start_driver))
 		goto err_out;
 
-	copied = config_copy_test_fs(config, TEST_START_TEST_FS,
-				     strlen(TEST_START_TEST_FS));
-	if (copied != strlen(TEST_START_TEST_FS))
-		goto err_out;
+
+	if (start_test_fs) {
+		copied = config_copy_test_fs(config, start_test_fs,
+					     strlen(start_test_fs));
+		if (copied != strlen(start_test_fs))
+			goto err_out;
+	}
 
 	config->num_threads = kmod_init_test_thread_limit();
 	config->test_result = 0;
@@ -1178,12 +1183,11 @@ static int __init test_kmod_init(void)
 	 * lowering the init level for more fun.
 	 */
 	if (force_init_test) {
-		ret = trigger_config_run_type(test_dev,
-					      TEST_KMOD_DRIVER, "tun");
+		ret = trigger_config_run_type(test_dev, TEST_KMOD_DRIVER);
 		if (WARN_ON(ret))
 			return ret;
-		ret = trigger_config_run_type(test_dev,
-					      TEST_KMOD_FS_TYPE, "btrfs");
+
+		ret = trigger_config_run_type(test_dev, TEST_KMOD_FS_TYPE);
 		if (WARN_ON(ret))
 			return ret;
 	}
-- 
2.47.1
Re: [PATCH] lib/test_kmod: do not hardcode/depend on any filesystem
Posted by Luis Chamberlain 8 months ago
On Thu, Apr 17, 2025 at 09:08:34PM -0300, Herton R. Krzesinski wrote:
> Right now test_kmod has hardcoded dependencies on btrfs/xfs. That
> is not optimal since you end up needing to select/build them, but it
> is not really required since other fs could be selected for the testing.
> Also, we can't change the default/driver module used for testing on
> initialization.
> 
> Thus make it more generic: introduce two module parameters (start_driver
> and start_test_fs), which allow to select which modules/fs to use for
> the testing on test_kmod initialization. Then it's up to the user to
> select which modules/fs to use for testing based on his config. However,
> keep test_module as required default.
> 
> This way, config/modules becomes selectable as when the testing is done
> from selftests (userspace).
> 
> While at it, also change trigger_config_run_type, since at module
> initialization we already set the defaults at __kmod_config_init and
> should not need to do it again in test_kmod_init(), thus we can
> avoid to again set test_driver/test_fs.
> 
> Signed-off-by: Herton R. Krzesinski <herton@redhat.com>
> ---
>  lib/Kconfig.debug |  6 -----
>  lib/test_kmod.c   | 64 +++++++++++++++++++++++++----------------------
>  2 files changed, 34 insertions(+), 36 deletions(-)
> 
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 9fe4d8dfe578..1e0f8cbf0365 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -2983,13 +2983,7 @@ config TEST_DYNAMIC_DEBUG
>  config TEST_KMOD
>  	tristate "kmod stress tester"
>  	depends on m
> -	depends on NETDEVICES && NET_CORE && INET # for TUN
> -	depends on BLOCK
> -	depends on PAGE_SIZE_LESS_THAN_256KB # for BTRFS

Please read the commit log that added PAGE_SIZE_LESS_THAN_256KB, what
do we want to do about those complaining about that? Maybe Cc the
author of that patch.

Other than that, it looks good to me. Did you run the selftest?

Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>

  Luis
Re: [PATCH] lib/test_kmod: do not hardcode/depend on any filesystem
Posted by Herton Krzesinski 7 months, 4 weeks ago
On Thu, Apr 17, 2025 at 9:14 PM Luis Chamberlain <mcgrof@kernel.org> wrote:
>
> On Thu, Apr 17, 2025 at 09:08:34PM -0300, Herton R. Krzesinski wrote:
> > Right now test_kmod has hardcoded dependencies on btrfs/xfs. That
> > is not optimal since you end up needing to select/build them, but it
> > is not really required since other fs could be selected for the testing.
> > Also, we can't change the default/driver module used for testing on
> > initialization.
> >
> > Thus make it more generic: introduce two module parameters (start_driver
> > and start_test_fs), which allow to select which modules/fs to use for
> > the testing on test_kmod initialization. Then it's up to the user to
> > select which modules/fs to use for testing based on his config. However,
> > keep test_module as required default.
> >
> > This way, config/modules becomes selectable as when the testing is done
> > from selftests (userspace).
> >
> > While at it, also change trigger_config_run_type, since at module
> > initialization we already set the defaults at __kmod_config_init and
> > should not need to do it again in test_kmod_init(), thus we can
> > avoid to again set test_driver/test_fs.
> >
> > Signed-off-by: Herton R. Krzesinski <herton@redhat.com>
> > ---
> >  lib/Kconfig.debug |  6 -----
> >  lib/test_kmod.c   | 64 +++++++++++++++++++++++++----------------------
> >  2 files changed, 34 insertions(+), 36 deletions(-)
> >
> > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> > index 9fe4d8dfe578..1e0f8cbf0365 100644
> > --- a/lib/Kconfig.debug
> > +++ b/lib/Kconfig.debug
> > @@ -2983,13 +2983,7 @@ config TEST_DYNAMIC_DEBUG
> >  config TEST_KMOD
> >       tristate "kmod stress tester"
> >       depends on m
> > -     depends on NETDEVICES && NET_CORE && INET # for TUN
> > -     depends on BLOCK
> > -     depends on PAGE_SIZE_LESS_THAN_256KB # for BTRFS
>
> Please read the commit log that added PAGE_SIZE_LESS_THAN_256KB, what
> do we want to do about those complaining about that? Maybe Cc the
> author of that patch.

afaik that problem only happens because of selecting BTRFS. Since I'm removing
selection of BTRFS now it shouldn't be a problem.

>
> Other than that, it looks good to me. Did you run the selftest?

Yes, I did both load the module with force_init_test and without, and
run the kmod.sh selftest and arbitrarily chose vfat module for fs test
(in my config
xfs is loaded/used and ext4 is builtin).

But now I noticed that I forgot to update
tools/testing/selftests/kmod/config, I'll
send a v2 shortly.

>
> Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
>
>   Luis
>