All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/8] Add --wait to modprobe -r
@ 2022-06-03 21:50 Lucas De Marchi
  2022-06-03 21:50 ` [PATCH 1/8] modprobe: Move -R to "Query options" Lucas De Marchi
                   ` (8 more replies)
  0 siblings, 9 replies; 11+ messages in thread
From: Lucas De Marchi @ 2022-06-03 21:50 UTC (permalink / raw)
  To: linux-modules; +Cc: Lucas De Marchi

Keep trying to remove the module if it's failing with EAGAIN. This is an
alternative to

v1: https://lore.kernel.org/linux-modules/20210803202417.462197-1-mcgrof@kernel.org/
v2: https://lore.kernel.org/linux-modules/20210810051602.3067384-1-mcgrof@kernel.org/

The v2 above or variand thereof would be probably a better way, but
unfortunately we can't poll the refcnt file in sysfs to get notified
when it reaches 0. The alternative in v1, with sleep(), uses an arbitrary
interval/maximum. It's not something I'm very  confortable to add to the
library side. So, add a quick implementation in modprobe to allow it
to remove the module and wait up until a maximum timeout. Intention is
to later migrate part of the logic in modprobe to libkmod, including for
example the holders removal recently fixed.

Lucas De Marchi (8):
  modprobe: Move -R to "Query options"
  libkmod: Allow to ignore log message on module removal
  module-playground: Add debugfs entry in mod-simple
  util: Add time-related functions from testsuite
  util: Add msec variants for time-related functions
  util: Add exponential backoff sleep
  testsuite: Add tests for sleep calculation
  modprobe: Add --wait

 libkmod/libkmod-module.c                 | 13 ++--
 libkmod/libkmod.h                        |  2 +
 man/modprobe.xml                         | 17 ++++++
 shared/util.c                            | 72 ++++++++++++++++++++++
 shared/util.h                            | 17 ++++++
 testsuite/module-playground/mod-simple.c | 18 +++++-
 testsuite/test-util.c                    | 41 +++++++++++++
 testsuite/testsuite.c                    | 14 +----
 tools/modprobe.c                         | 76 ++++++++++++++++++++----
 9 files changed, 240 insertions(+), 30 deletions(-)

-- 
2.36.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/8] modprobe: Move -R to "Query options"
  2022-06-03 21:50 [PATCH 0/8] Add --wait to modprobe -r Lucas De Marchi
@ 2022-06-03 21:50 ` Lucas De Marchi
  2022-06-03 21:50 ` [PATCH 2/8] libkmod: Allow to ignore log message on module removal Lucas De Marchi
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Lucas De Marchi @ 2022-06-03 21:50 UTC (permalink / raw)
  To: linux-modules; +Cc: Lucas De Marchi

Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
---
 tools/modprobe.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/modprobe.c b/tools/modprobe.c
index a825fb5..caaf87f 100644
--- a/tools/modprobe.c
+++ b/tools/modprobe.c
@@ -110,7 +110,6 @@ static void help(void)
 		"\t-r, --remove                Remove modules instead of inserting\n"
 		"\t    --remove-dependencies   Deprecated: use --remove-holders\n"
 		"\t    --remove-holders        Also remove module holders (use together with -r)\n"
-		"\t-R, --resolve-alias         Only lookup and print alias and exit\n"
 		"\t    --first-time            Fail if module already inserted or removed\n"
 		"\t-i, --ignore-install        Ignore install commands\n"
 		"\t-i, --ignore-remove         Ignore remove commands\n"
@@ -122,6 +121,7 @@ static void help(void)
 		"\t    --force-vermagic        Ignore module's version magic\n"
 		"\n"
 		"Query Options:\n"
+		"\t-R, --resolve-alias         Only lookup and print alias and exit\n"
 		"\t-D, --show-depends          Only print module dependencies and exit\n"
 		"\t-c, --showconfig            Print out known configuration and exit\n"
 		"\t-c, --show-config           Same as --showconfig\n"
@@ -800,9 +800,6 @@ static int do_modprobe(int argc, char **orig_argv)
 		case 5:
 			remove_holders = 1;
 			break;
-		case 'R':
-			lookup_only = 1;
-			break;
 		case 3:
 			first_time = 1;
 			break;
@@ -826,6 +823,9 @@ static int do_modprobe(int argc, char **orig_argv)
 			dry_run = 1;
 			do_show = 1;
 			break;
+		case 'R':
+			lookup_only = 1;
+			break;
 		case 'c':
 			do_show_config = 1;
 			break;
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/8] libkmod: Allow to ignore log message on module removal
  2022-06-03 21:50 [PATCH 0/8] Add --wait to modprobe -r Lucas De Marchi
  2022-06-03 21:50 ` [PATCH 1/8] modprobe: Move -R to "Query options" Lucas De Marchi
@ 2022-06-03 21:50 ` Lucas De Marchi
  2022-06-03 21:50 ` [PATCH 3/8] module-playground: Add debugfs entry in mod-simple Lucas De Marchi
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Lucas De Marchi @ 2022-06-03 21:50 UTC (permalink / raw)
  To: linux-modules; +Cc: Lucas De Marchi

Caller may want to handle retries, in which case the log message is not
appropriate.

Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
---
 libkmod/libkmod-module.c | 13 +++++++++----
 libkmod/libkmod.h        |  2 ++
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/libkmod/libkmod-module.c b/libkmod/libkmod-module.c
index efdd679..12d8ed1 100644
--- a/libkmod/libkmod-module.c
+++ b/libkmod/libkmod-module.c
@@ -823,11 +823,13 @@ extern long delete_module(const char *name, unsigned int flags);
 /**
  * kmod_module_remove_module:
  * @mod: kmod module
- * @flags: flags to pass to Linux kernel when removing the module. The only valid flag is
+ * @flags: flags used when removing the module.
  * KMOD_REMOVE_FORCE: force remove module regardless if it's still in
- * use by a kernel subsystem or other process;
- * KMOD_REMOVE_NOWAIT is always enforced, causing us to pass O_NONBLOCK to
+ * use by a kernel subsystem or other process; passed directly to Linux kernel
+ * KMOD_REMOVE_NOWAIT: is always enforced, causing us to pass O_NONBLOCK to
  * delete_module(2).
+ * KMOD_REMOVE_NOLOG: when module removal fails, do not log anything as the
+ * caller may want to handle retries and log when appropriate.
  *
  * Remove a module from Linux kernel.
  *
@@ -836,6 +838,8 @@ extern long delete_module(const char *name, unsigned int flags);
 KMOD_EXPORT int kmod_module_remove_module(struct kmod_module *mod,
 							unsigned int flags)
 {
+	unsigned int libkmod_flags = flags & 0xff;
+
 	int err;
 
 	if (mod == NULL)
@@ -848,7 +852,8 @@ KMOD_EXPORT int kmod_module_remove_module(struct kmod_module *mod,
 	err = delete_module(mod->name, flags);
 	if (err != 0) {
 		err = -errno;
-		ERR(mod->ctx, "could not remove '%s': %m\n", mod->name);
+		if (!(libkmod_flags & KMOD_REMOVE_NOLOG))
+			ERR(mod->ctx, "could not remove '%s': %m\n", mod->name);
 	}
 
 	return err;
diff --git a/libkmod/libkmod.h b/libkmod/libkmod.h
index fed216b..7251aa7 100644
--- a/libkmod/libkmod.h
+++ b/libkmod/libkmod.h
@@ -145,6 +145,8 @@ struct kmod_module *kmod_module_get_module(const struct kmod_list *entry);
 enum kmod_remove {
 	KMOD_REMOVE_FORCE = O_TRUNC,
 	KMOD_REMOVE_NOWAIT = O_NONBLOCK, /* always set */
+	/* libkmod-only defines, not passed to kernel */
+	KMOD_REMOVE_NOLOG = 1,
 };
 
 /* Insertion flags */
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/8] module-playground: Add debugfs entry in mod-simple
  2022-06-03 21:50 [PATCH 0/8] Add --wait to modprobe -r Lucas De Marchi
  2022-06-03 21:50 ` [PATCH 1/8] modprobe: Move -R to "Query options" Lucas De Marchi
  2022-06-03 21:50 ` [PATCH 2/8] libkmod: Allow to ignore log message on module removal Lucas De Marchi
@ 2022-06-03 21:50 ` Lucas De Marchi
  2022-06-03 21:50 ` [PATCH 4/8] util: Add time-related functions from testsuite Lucas De Marchi
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Lucas De Marchi @ 2022-06-03 21:50 UTC (permalink / raw)
  To: linux-modules; +Cc: Lucas De Marchi

Add a debugfs file in mod-simple for manual tests: insert the module and
open the file to have its refcount increased.

Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
---
 testsuite/module-playground/mod-simple.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/testsuite/module-playground/mod-simple.c b/testsuite/module-playground/mod-simple.c
index cb38580..503e4d8 100644
--- a/testsuite/module-playground/mod-simple.c
+++ b/testsuite/module-playground/mod-simple.c
@@ -1,16 +1,32 @@
+#include <linux/debugfs.h>
 #include <linux/init.h>
 #include <linux/module.h>
 
+static struct dentry *debugfs_dir;
+
+static int test_show(struct seq_file *s, void *data)
+{
+	seq_puts(s, "test");
+	return 0;
+}
+
+DEFINE_SHOW_ATTRIBUTE(test);
+
 static int __init test_module_init(void)
 {
+	debugfs_dir = debugfs_create_dir(KBUILD_MODNAME, NULL);
+	debugfs_create_file("test", 0444, debugfs_dir, NULL, &test_fops);
+
 	return 0;
 }
 
 static void test_module_exit(void)
 {
+	debugfs_remove_recursive(debugfs_dir);
 }
+
 module_init(test_module_init);
 module_exit(test_module_exit);
 
 MODULE_AUTHOR("Lucas De Marchi <lucas.demarchi@intel.com>");
-MODULE_LICENSE("LGPL");
+MODULE_LICENSE("GPL");
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/8] util: Add time-related functions from testsuite
  2022-06-03 21:50 [PATCH 0/8] Add --wait to modprobe -r Lucas De Marchi
                   ` (2 preceding siblings ...)
  2022-06-03 21:50 ` [PATCH 3/8] module-playground: Add debugfs entry in mod-simple Lucas De Marchi
@ 2022-06-03 21:50 ` Lucas De Marchi
  2022-06-03 21:50 ` [PATCH 5/8] util: Add msec variants for time-related functions Lucas De Marchi
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Lucas De Marchi @ 2022-06-03 21:50 UTC (permalink / raw)
  To: linux-modules; +Cc: Lucas De Marchi

This will be useful in future not only to testsuite, but also to tools
and library.

Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
---
 shared/util.c         | 10 ++++++++++
 shared/util.h         |  8 ++++++++
 testsuite/testsuite.c | 14 +-------------
 3 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/shared/util.c b/shared/util.c
index b487b5f..aeb171f 100644
--- a/shared/util.c
+++ b/shared/util.c
@@ -466,6 +466,16 @@ unsigned long long ts_usec(const struct timespec *ts)
 	       (unsigned long long) ts->tv_nsec / NSEC_PER_USEC;
 }
 
+unsigned long long now_usec(void)
+{
+	struct timespec ts;
+
+	if (clock_gettime(CLOCK_MONOTONIC, &ts) != 0)
+		return 0;
+
+	return ts_usec(&ts);
+}
+
 unsigned long long stat_mstamp(const struct stat *st)
 {
 #ifdef HAVE_STRUCT_STAT_ST_MTIM
diff --git a/shared/util.h b/shared/util.h
index c6a31df..734a523 100644
--- a/shared/util.h
+++ b/shared/util.h
@@ -7,6 +7,7 @@
 #include <stdio.h>
 #include <sys/types.h>
 #include <sys/stat.h>
+#include <time.h>
 
 #include <shared/macro.h>
 
@@ -42,7 +43,14 @@ char *path_make_absolute_cwd(const char *p) _must_check_ __attribute__((nonnull(
 int mkdir_p(const char *path, int len, mode_t mode);
 int mkdir_parents(const char *path, mode_t mode);
 unsigned long long stat_mstamp(const struct stat *st);
+
+/* time-related functions
+ * ************************************************************************ */
+#define USEC_PER_SEC	1000000ULL
+#define USEC_PER_MSEC	1000ULL
+
 unsigned long long ts_usec(const struct timespec *ts);
+unsigned long long now_usec(void);
 
 /* endianess and alignments                                                 */
 /* ************************************************************************ */
diff --git a/testsuite/testsuite.c b/testsuite/testsuite.c
index 05df553..6a2d296 100644
--- a/testsuite/testsuite.c
+++ b/testsuite/testsuite.c
@@ -51,6 +51,7 @@ static const struct option options[] = {
 };
 
 #define OVERRIDE_LIBDIR ABS_TOP_BUILDDIR "/testsuite/.libs/"
+#define TEST_TIMEOUT_USEC 2 * USEC_PER_SEC
 
 struct _env_config {
 	const char *key;
@@ -62,19 +63,6 @@ struct _env_config {
 	[TC_DELETE_MODULE_RETCODES] = { S_TC_DELETE_MODULE_RETCODES, OVERRIDE_LIBDIR "delete_module.so" },
 };
 
-#define USEC_PER_SEC  1000000ULL
-#define USEC_PER_MSEC  1000ULL
-#define TEST_TIMEOUT_USEC 2 * USEC_PER_SEC
-static unsigned long long now_usec(void)
-{
-	struct timespec ts;
-
-	if (clock_gettime(CLOCK_MONOTONIC, &ts) != 0)
-		return 0;
-
-	return ts_usec(&ts);
-}
-
 static void help(void)
 {
 	const struct option *itr;
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 5/8] util: Add msec variants for time-related functions
  2022-06-03 21:50 [PATCH 0/8] Add --wait to modprobe -r Lucas De Marchi
                   ` (3 preceding siblings ...)
  2022-06-03 21:50 ` [PATCH 4/8] util: Add time-related functions from testsuite Lucas De Marchi
@ 2022-06-03 21:50 ` Lucas De Marchi
  2022-06-03 21:50 ` [PATCH 6/8] util: Add exponential backoff sleep Lucas De Marchi
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Lucas De Marchi @ 2022-06-03 21:50 UTC (permalink / raw)
  To: linux-modules; +Cc: Lucas De Marchi

Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
---
 shared/util.c | 16 ++++++++++++++++
 shared/util.h |  4 ++++
 2 files changed, 20 insertions(+)

diff --git a/shared/util.c b/shared/util.c
index aeb171f..d4452eb 100644
--- a/shared/util.c
+++ b/shared/util.c
@@ -466,6 +466,12 @@ unsigned long long ts_usec(const struct timespec *ts)
 	       (unsigned long long) ts->tv_nsec / NSEC_PER_USEC;
 }
 
+unsigned long long ts_msec(const struct timespec *ts)
+{
+	return (unsigned long long) ts->tv_sec * MSEC_PER_SEC +
+	       (unsigned long long) ts->tv_nsec / NSEC_PER_MSEC;
+}
+
 unsigned long long now_usec(void)
 {
 	struct timespec ts;
@@ -476,6 +482,16 @@ unsigned long long now_usec(void)
 	return ts_usec(&ts);
 }
 
+unsigned long long now_msec(void)
+{
+	struct timespec ts;
+
+	if (clock_gettime(CLOCK_MONOTONIC, &ts) != 0)
+		return 0;
+
+	return ts_msec(&ts);
+}
+
 unsigned long long stat_mstamp(const struct stat *st)
 {
 #ifdef HAVE_STRUCT_STAT_ST_MTIM
diff --git a/shared/util.h b/shared/util.h
index 734a523..bedafa3 100644
--- a/shared/util.h
+++ b/shared/util.h
@@ -48,9 +48,13 @@ unsigned long long stat_mstamp(const struct stat *st);
  * ************************************************************************ */
 #define USEC_PER_SEC	1000000ULL
 #define USEC_PER_MSEC	1000ULL
+#define MSEC_PER_SEC	1000ULL
+#define NSEC_PER_MSEC	1000000ULL
 
 unsigned long long ts_usec(const struct timespec *ts);
+unsigned long long ts_msec(const struct timespec *ts);
 unsigned long long now_usec(void);
+unsigned long long now_msec(void);
 
 /* endianess and alignments                                                 */
 /* ************************************************************************ */
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 6/8] util: Add exponential backoff sleep
  2022-06-03 21:50 [PATCH 0/8] Add --wait to modprobe -r Lucas De Marchi
                   ` (4 preceding siblings ...)
  2022-06-03 21:50 ` [PATCH 5/8] util: Add msec variants for time-related functions Lucas De Marchi
@ 2022-06-03 21:50 ` Lucas De Marchi
  2022-06-03 21:50 ` [PATCH 7/8] testsuite: Add tests for sleep calculation Lucas De Marchi
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Lucas De Marchi @ 2022-06-03 21:50 UTC (permalink / raw)
  To: linux-modules; +Cc: Lucas De Marchi

Add simple functions to put the current thread to sleep using
exponential backoff to split the interval in smaller pieces.

Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
---
 shared/util.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 shared/util.h |  5 +++++
 2 files changed, 51 insertions(+)

diff --git a/shared/util.c b/shared/util.c
index d4452eb..4b547ff 100644
--- a/shared/util.c
+++ b/shared/util.c
@@ -472,6 +472,52 @@ unsigned long long ts_msec(const struct timespec *ts)
 	       (unsigned long long) ts->tv_nsec / NSEC_PER_MSEC;
 }
 
+static struct timespec msec_ts(unsigned long long msec)
+{
+	struct timespec ts = {
+		.tv_sec = msec / MSEC_PER_SEC,
+		.tv_nsec = (msec % MSEC_PER_SEC) * NSEC_PER_MSEC,
+	};
+
+	return ts;
+}
+
+int sleep_until_msec(unsigned long long msec)
+{
+	struct timespec ts = msec_ts(msec);
+
+	if (clock_nanosleep(CLOCK_MONOTONIC, TIMER_ABSTIME, &ts, NULL) < 0 &&
+	    errno != EINTR)
+		return -errno;
+
+	return 0;
+}
+
+/*
+ * Exponential retry backoff with tail
+ */
+unsigned long long get_backoff_delta_msec(unsigned long long t0,
+					  unsigned long long tend,
+					  unsigned long long *delta)
+{
+	unsigned long long t;
+
+	t = now_msec();
+
+	if (!*delta)
+		*delta = 1;
+	else
+		*delta <<= 1;
+
+	while (t + *delta > tend)
+		*delta >>= 1;
+
+	if (!*delta && tend > t)
+		*delta = tend - t;
+
+	return t + *delta;
+}
+
 unsigned long long now_usec(void)
 {
 	struct timespec ts;
diff --git a/shared/util.h b/shared/util.h
index bedafa3..7030653 100644
--- a/shared/util.h
+++ b/shared/util.h
@@ -55,6 +55,11 @@ unsigned long long ts_usec(const struct timespec *ts);
 unsigned long long ts_msec(const struct timespec *ts);
 unsigned long long now_usec(void);
 unsigned long long now_msec(void);
+int sleep_until_msec(unsigned long long msec);
+unsigned long long get_backoff_delta_msec(unsigned long long t0,
+					  unsigned long long tend,
+					  unsigned long long *delta);
+
 
 /* endianess and alignments                                                 */
 /* ************************************************************************ */
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 7/8] testsuite: Add tests for sleep calculation
  2022-06-03 21:50 [PATCH 0/8] Add --wait to modprobe -r Lucas De Marchi
                   ` (5 preceding siblings ...)
  2022-06-03 21:50 ` [PATCH 6/8] util: Add exponential backoff sleep Lucas De Marchi
@ 2022-06-03 21:50 ` Lucas De Marchi
  2022-06-03 21:50 ` [PATCH 8/8] modprobe: Add --wait Lucas De Marchi
  2022-06-15 16:31 ` [PATCH 0/8] Add --wait to modprobe -r Lucas De Marchi
  8 siblings, 0 replies; 11+ messages in thread
From: Lucas De Marchi @ 2022-06-03 21:50 UTC (permalink / raw)
  To: linux-modules; +Cc: Lucas De Marchi

Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
---
 testsuite/test-util.c | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/testsuite/test-util.c b/testsuite/test-util.c
index 621446b..fb8c9ef 100644
--- a/testsuite/test-util.c
+++ b/testsuite/test-util.c
@@ -229,4 +229,45 @@ DEFINE_TEST(test_addu64_overflow,
 	);
 
 
+static int test_backoff_time(const struct test *t)
+{
+	unsigned long long delta;
+
+	/* Check exponential increments */
+	get_backoff_delta_msec(now_msec(), now_msec() + 10, &delta);
+	assert_return(delta == 1, EXIT_FAILURE);
+	get_backoff_delta_msec(now_msec(), now_msec() + 10, &delta);
+	assert_return(delta == 2, EXIT_FAILURE);
+	get_backoff_delta_msec(now_msec(), now_msec() + 10, &delta);
+	assert_return(delta == 4, EXIT_FAILURE);
+	get_backoff_delta_msec(now_msec(), now_msec() + 10, &delta);
+	assert_return(delta == 8, EXIT_FAILURE);
+
+	{
+		unsigned long long t0, tend;
+
+		/* Check tail */
+		delta = 4;
+		tend = now_msec() + 3;
+		t0 = tend - 10;
+		get_backoff_delta_msec(t0, tend, &delta);
+		assert_return(delta == 2, EXIT_FAILURE);
+		tend = now_msec() + 1;
+		t0 = tend - 9;
+		get_backoff_delta_msec(t0, tend, &delta);
+		assert_return(delta == 1, EXIT_FAILURE);
+		tend = now_msec();
+		t0 = tend - 10;
+		get_backoff_delta_msec(t0, tend, &delta);
+		assert_return(delta == 0, EXIT_FAILURE);
+	}
+
+	return EXIT_SUCCESS;
+}
+DEFINE_TEST(test_backoff_time,
+	.description = "check implementation of get_backoff_delta_msec()",
+	.need_spawn = false,
+	);
+
+
 TESTSUITE_MAIN();
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 8/8] modprobe: Add --wait
  2022-06-03 21:50 [PATCH 0/8] Add --wait to modprobe -r Lucas De Marchi
                   ` (6 preceding siblings ...)
  2022-06-03 21:50 ` [PATCH 7/8] testsuite: Add tests for sleep calculation Lucas De Marchi
@ 2022-06-03 21:50 ` Lucas De Marchi
  2022-06-15 16:31 ` [PATCH 0/8] Add --wait to modprobe -r Lucas De Marchi
  8 siblings, 0 replies; 11+ messages in thread
From: Lucas De Marchi @ 2022-06-03 21:50 UTC (permalink / raw)
  To: linux-modules; +Cc: Lucas De Marchi

Retry module removal if it fails due to EAGAIN. This allows user to pass
--wait <timeout>, during which `modprobe -r` will keep trying to remove
the module.

Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
---
 man/modprobe.xml | 17 ++++++++++++
 tools/modprobe.c | 70 +++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 78 insertions(+), 9 deletions(-)

diff --git a/man/modprobe.xml b/man/modprobe.xml
index 0372b6b..db39c7a 100644
--- a/man/modprobe.xml
+++ b/man/modprobe.xml
@@ -388,6 +388,23 @@
           </para>
         </listitem>
       </varlistentry>
+      <varlistentry>
+        <term>
+          <option>-w</option>
+        </term>
+        <term>
+        <option>--wait=</option>TIMEOUT_MSEC
+        </term>
+        <listitem>
+          <para>
+            This option causes <command>modprobe -r</command> to continue trying to
+            remove a module if it fails due to the module being busy, i.e. its refcount
+            is not 0 at the time the call is made. Modprobe tries to remove the module
+            with an incremental sleep time between each tentative up until the maximum
+            wait time in milliseconds passed in this option.
+          </para>
+        </listitem>
+      </varlistentry>
       <varlistentry>
         <term>
           <option>-S</option>
diff --git a/tools/modprobe.c b/tools/modprobe.c
index caaf87f..2a2ae21 100644
--- a/tools/modprobe.c
+++ b/tools/modprobe.c
@@ -32,6 +32,7 @@
 #include <sys/wait.h>
 
 #include <shared/array.h>
+#include <shared/util.h>
 #include <shared/macro.h>
 
 #include <libkmod/libkmod.h>
@@ -55,14 +56,18 @@ static int force = 0;
 static int strip_modversion = 0;
 static int strip_vermagic = 0;
 static int remove_holders = 0;
+static unsigned long long wait_msec = 0;
 static int quiet_inuse = 0;
 
-static const char cmdopts_s[] = "arRibfDcnC:d:S:sqvVh";
+static const char cmdopts_s[] = "arw:RibfDcnC:d:S:sqvVh";
 static const struct option cmdopts[] = {
 	{"all", no_argument, 0, 'a'},
+
 	{"remove", no_argument, 0, 'r'},
 	{"remove-dependencies", no_argument, 0, 5},
 	{"remove-holders", no_argument, 0, 5},
+	{"wait", required_argument, 0, 'w'},
+
 	{"resolve-alias", no_argument, 0, 'R'},
 	{"first-time", no_argument, 0, 3},
 	{"ignore-install", no_argument, 0, 'i'},
@@ -110,6 +115,9 @@ static void help(void)
 		"\t-r, --remove                Remove modules instead of inserting\n"
 		"\t    --remove-dependencies   Deprecated: use --remove-holders\n"
 		"\t    --remove-holders        Also remove module holders (use together with -r)\n"
+		"\t-w, --wait <MSEC>           When removing a module, wait up to MSEC for\n"
+		"\t                            module's refcount to become 0 so it can be\n"
+		"\t                            removed (use together with -r)\n"
 		"\t    --first-time            Fail if module already inserted or removed\n"
 		"\t-i, --ignore-install        Ignore install commands\n"
 		"\t-i, --ignore-remove         Ignore remove commands\n"
@@ -322,6 +330,8 @@ end:
 static int rmmod_do_remove_module(struct kmod_module *mod)
 {
 	const char *modname = kmod_module_get_name(mod);
+	unsigned long long interval_msec = 0, t0_msec = 0,
+		      tend_msec = 0;
 	int flags = 0, err;
 
 	SHOW("rmmod %s\n", modname);
@@ -332,13 +342,45 @@ static int rmmod_do_remove_module(struct kmod_module *mod)
 	if (force)
 		flags |= KMOD_REMOVE_FORCE;
 
-	err = kmod_module_remove_module(mod, flags);
-	if (err == -EEXIST) {
-		if (!first_time)
-			err = 0;
-		else
-			LOG("Module %s is not in kernel.\n", modname);
-	}
+	if (wait_msec)
+		flags |= KMOD_REMOVE_NOLOG;
+
+	do {
+		err = kmod_module_remove_module(mod, flags);
+		if (err == -EEXIST) {
+			if (!first_time)
+				err = 0;
+			else
+				LOG("Module %s is not in kernel.\n", modname);
+			break;
+		} else if (err == -EAGAIN && wait_msec) {
+			unsigned long long until_msec;
+
+			if (!t0_msec) {
+				t0_msec = now_msec();
+				tend_msec = t0_msec + wait_msec;
+				interval_msec = 1;
+			}
+
+			until_msec = get_backoff_delta_msec(t0_msec, tend_msec,
+							  &interval_msec);
+			err = sleep_until_msec(until_msec);
+
+			if (!t0_msec)
+				err = -ENOTSUP;
+
+			if (err < 0) {
+				ERR("Failed to sleep: %s\n", strerror(-err));
+				err = -EAGAIN;
+				break;
+			}
+		} else {
+			break;
+		}
+	} while (interval_msec);
+
+	if (err < 0 && wait_msec)
+		ERR("could not remove '%s': %s\n", modname, strerror(-err));
 
 	return err;
 }
@@ -418,7 +460,7 @@ static int rmmod_do_module(struct kmod_module *mod, int flags)
 	}
 
 	/* 3. @mod itself, but check for refcnt first */
-	if (!cmd && !ignore_loaded) {
+	if (!cmd && !ignore_loaded && !wait_msec) {
 		int usage = kmod_module_get_refcnt(mod);
 
 		if (usage > 0) {
@@ -800,6 +842,16 @@ static int do_modprobe(int argc, char **orig_argv)
 		case 5:
 			remove_holders = 1;
 			break;
+		case 'w': {
+			char *endptr = NULL;
+			wait_msec = strtoul(optarg, &endptr, 0);
+			if (!*optarg || *endptr) {
+				ERR("unexpected wait value '%s'.\n", optarg);
+				err = -1;
+				goto done;
+			}
+			break;
+		}
 		case 3:
 			first_time = 1;
 			break;
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/8] Add --wait to modprobe -r
  2022-06-03 21:50 [PATCH 0/8] Add --wait to modprobe -r Lucas De Marchi
                   ` (7 preceding siblings ...)
  2022-06-03 21:50 ` [PATCH 8/8] modprobe: Add --wait Lucas De Marchi
@ 2022-06-15 16:31 ` Lucas De Marchi
  2022-06-27 16:46   ` Lucas De Marchi
  8 siblings, 1 reply; 11+ messages in thread
From: Lucas De Marchi @ 2022-06-15 16:31 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: linux-modules, mcgrof

Luis, are you still interested in the --wait to modprobe? If so, could
you take a look in this series?

If there is still interest for the fs tests, I want to bring this in and
then do release. Otherwise I may just leave it for the next release when
I plan to move stuff from modprobe to the library, particularly related
to the module removal.

thanks
Lucas De Marchi

On Fri, Jun 03, 2022 at 02:50:39PM -0700, Lucas De Marchi wrote:
>Keep trying to remove the module if it's failing with EAGAIN. This is an
>alternative to
>
>v1: https://lore.kernel.org/linux-modules/20210803202417.462197-1-mcgrof@kernel.org/
>v2: https://lore.kernel.org/linux-modules/20210810051602.3067384-1-mcgrof@kernel.org/
>
>The v2 above or variand thereof would be probably a better way, but
>unfortunately we can't poll the refcnt file in sysfs to get notified
>when it reaches 0. The alternative in v1, with sleep(), uses an arbitrary
>interval/maximum. It's not something I'm very  confortable to add to the
>library side. So, add a quick implementation in modprobe to allow it
>to remove the module and wait up until a maximum timeout. Intention is
>to later migrate part of the logic in modprobe to libkmod, including for
>example the holders removal recently fixed.
>
>Lucas De Marchi (8):
>  modprobe: Move -R to "Query options"
>  libkmod: Allow to ignore log message on module removal
>  module-playground: Add debugfs entry in mod-simple
>  util: Add time-related functions from testsuite
>  util: Add msec variants for time-related functions
>  util: Add exponential backoff sleep
>  testsuite: Add tests for sleep calculation
>  modprobe: Add --wait
>
> libkmod/libkmod-module.c                 | 13 ++--
> libkmod/libkmod.h                        |  2 +
> man/modprobe.xml                         | 17 ++++++
> shared/util.c                            | 72 ++++++++++++++++++++++
> shared/util.h                            | 17 ++++++
> testsuite/module-playground/mod-simple.c | 18 +++++-
> testsuite/test-util.c                    | 41 +++++++++++++
> testsuite/testsuite.c                    | 14 +----
> tools/modprobe.c                         | 76 ++++++++++++++++++++----
> 9 files changed, 240 insertions(+), 30 deletions(-)
>
>-- 
>2.36.1
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/8] Add --wait to modprobe -r
  2022-06-15 16:31 ` [PATCH 0/8] Add --wait to modprobe -r Lucas De Marchi
@ 2022-06-27 16:46   ` Lucas De Marchi
  0 siblings, 0 replies; 11+ messages in thread
From: Lucas De Marchi @ 2022-06-27 16:46 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: linux-modules, mcgrof

On Wed, Jun 15, 2022 at 09:31:29AM -0700, Lucas De Marchi wrote:
>Luis, are you still interested in the --wait to modprobe? If so, could
>you take a look in this series?
>
>If there is still interest for the fs tests, I want to bring this in and
>then do release. Otherwise I may just leave it for the next release when
>I plan to move stuff from modprobe to the library, particularly related
>to the module removal.

I did some additional tests and pushed this as I plan to release a new
version of kmod this week. Next version we can improve and move things
to libkmod.

Lucas De Marchi

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-06-27 16:47 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-03 21:50 [PATCH 0/8] Add --wait to modprobe -r Lucas De Marchi
2022-06-03 21:50 ` [PATCH 1/8] modprobe: Move -R to "Query options" Lucas De Marchi
2022-06-03 21:50 ` [PATCH 2/8] libkmod: Allow to ignore log message on module removal Lucas De Marchi
2022-06-03 21:50 ` [PATCH 3/8] module-playground: Add debugfs entry in mod-simple Lucas De Marchi
2022-06-03 21:50 ` [PATCH 4/8] util: Add time-related functions from testsuite Lucas De Marchi
2022-06-03 21:50 ` [PATCH 5/8] util: Add msec variants for time-related functions Lucas De Marchi
2022-06-03 21:50 ` [PATCH 6/8] util: Add exponential backoff sleep Lucas De Marchi
2022-06-03 21:50 ` [PATCH 7/8] testsuite: Add tests for sleep calculation Lucas De Marchi
2022-06-03 21:50 ` [PATCH 8/8] modprobe: Add --wait Lucas De Marchi
2022-06-15 16:31 ` [PATCH 0/8] Add --wait to modprobe -r Lucas De Marchi
2022-06-27 16:46   ` Lucas De Marchi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.