Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1645167 > unrolled thread

[PATCH 0/6] kmod: few simple enhancements

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/6] kmod: few simple enhancements "Luis R. Rodriguez" <mcgrof@kernel.org> - 2017-05-19 05:30 +0200
    [PATCH 1/6] kmod: add dynamic max concurrent thread count "Luis R. Rodriguez" <mcgrof@kernel.org> - 2017-05-19 05:30 +0200
      Re: [PATCH 1/6] kmod: add dynamic max concurrent thread count Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-05-19 22:50 +0200
        Re: [PATCH 1/6] kmod: add dynamic max concurrent thread count Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-05-20 00:00 +0200

#1645167 — [PATCH 0/6] kmod: few simple enhancements

I've been working on making module loading more deterministic, these are some
of the more straight forward changes I've come up with so far. The others
depend on some further sysctl changes so I'll wait to introduce those, and also
on a new kmod stress driver loader, which I'll also hold off on introducing.

I believe these are pretty straight forward, but please let me know if there
are any questions. In this re-spin I've dropped the kref / refcount proposals
given its just overkill for what we need here.

Luis R. Rodriguez (6):
  kmod: add dynamic max concurrent thread count
  module: use list_for_each_entry_rcu() on find_module_all()
  kmod: provide wrappers for kmod_concurrent inc/dec
  kmod: return -EBUSY if modprobe limit is reached
  kmod: preempt on kmod_umh_threads_get()
  kmod: use simplified rate limit printk

 Documentation/admin-guide/kernel-parameters.txt |  7 ++
 include/linux/kmod.h                            |  3 +-
 init/Kconfig                                    | 23 ++++++
 init/main.c                                     |  1 +
 kernel/kmod.c                                   | 98 +++++++++++++++++--------
 kernel/module.c                                 |  2 +-
 6 files changed, 103 insertions(+), 31 deletions(-)

-- 
2.11.0

[toc] | [next] | [standalone]

#1645169 — [PATCH 1/6] kmod: add dynamic max concurrent thread count

We currently statically limit the number of modprobe threads which
we allow to run concurrently to 50. As per Keith Owens, this was a
completely arbitrary value, and it was set in the 2.3.38 days [0]
over 16 years ago in year 2000.

Although we haven't yet hit our lower limits, experimentation [1]
shows that when and if we hit this limit in the worst case, will be
fatal -- consider get_fs_type() failures upon mount on a system which
has many partitions, some of which might even be with the same
filesystem. Its best to be prudent and increase and set this
value to something more sensible which ensures we're far from hitting
the limit and also allows default build/user run time override.

The worst case is fatal given that once a module fails to load there
is a period of time during which subsequent request for the same module
will fail, so in the case of partitions its not just one request that
could fail, but whole series of partitions. This later issue of a
module request failure domino effect can be addressed later, but
increasing the limit to something more meaninful should at least give us
enough cushion to avoid this for a while.

Set this value up with a bit more meaninful modern limits:

Bump this up to 64  max for small systems (CONFIG_BASE_SMALL)
Bump this up to 128 max for larger systems (!CONFIG_BASE_SMALL)

Also allow the default max limit to be further fine tuned at compile
time and at initialization at run time at boot up using the kernel
parameter: max_modprobes.

[0] https://git.kernel.org/cgit/linux/kernel/git/history/history.git/commit/?id=ab1c4ec7410f6ec64e1511d1a7d850fc99c09b44
[1] https://github.com/mcgrof/test_request_module

Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 Documentation/admin-guide/kernel-parameters.txt |  7 ++++
 include/linux/kmod.h                            |  3 +-
 init/Kconfig                                    | 23 +++++++++++++
 init/main.c                                     |  1 +
 kernel/kmod.c                                   | 43 ++++++++++++++++---------
 5 files changed, 61 insertions(+), 16 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 15f79c27748d..1314a85c10c9 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1748,6 +1748,13 @@
 
 	keepinitrd	[HW,ARM]
 
+	kmod.max_modprobes [KNL]
+			This lets you set the max allowed of concurrent
+			modprobes threads possible on a system overriding the
+			default heuristic of:
+
+				min(max_threads/2, 1 << CONFIG_MAX_KMOD_CONCURRENT)
+
 	kernelcore=	[KNL,X86,IA-64,PPC]
 			Format: nn[KMGTPE] | "mirror"
 			This parameter
diff --git a/include/linux/kmod.h b/include/linux/kmod.h
index c4e441e00db5..302c9dfdcda9 100644
--- a/include/linux/kmod.h
+++ b/include/linux/kmod.h
@@ -38,13 +38,14 @@ int __request_module(bool wait, const char *name, ...);
 #define request_module_nowait(mod...) __request_module(false, mod)
 #define try_then_request_module(x, mod...) \
 	((x) ?: (__request_module(true, mod), (x)))
+void init_kmod_umh(void);
 #else
 static inline int request_module(const char *name, ...) { return -ENOSYS; }
 static inline int request_module_nowait(const char *name, ...) { return -ENOSYS; }
+static inline void init_kmod_umh(void) { }
 #define try_then_request_module(x, mod...) (x)
 #endif
 
-
 struct cred;
 struct file;
 
diff --git a/init/Kconfig b/init/Kconfig
index 1d3475fc9496..5974ad7c9d0a 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -2191,6 +2191,29 @@ config TRIM_UNUSED_KSYMS
 
 	  If unsure, or if you need to build out-of-tree modules, say N.
 
+config MAX_KMOD_CONCURRENT
+	int "Max allowed concurrent request_module() calls (6=>64, 10=>1024)"
+	range 6 14
+	default 7 if !BASE_SMALL
+	default 6 if BASE_SMALL
+	help
+	  The kernel restricts the number of possible concurrent calls to
+	  request_module() to help avoid a recursive loop possible with
+	  modules. The default maximum number of concurrent threads allowed
+	  to run request_module() will be:
+
+	    max_modprobes = min(max_threads/2, 1 << CONFIG_MAX_KMOD_CONCURRENT);
+
+	  The value set in CONFIG_MAX_KMOD_CONCURRENT represents then the power
+	  of 2 value used at boot time for the above computation. You can
+	  override the default built value using the kernel parameter:
+
+		kmod.max_modprobes=4096
+
+	  We set this to default to 64 (2^6) concurrent modprobe threads for
+	  small systems, for larger systems this defaults to 128 (2^7)
+	  concurrent modprobe threads.
+
 endif # MODULES
 
 config MODULES_TREE_LOOKUP
diff --git a/init/main.c b/init/main.c
index f866510472d7..25669dfded42 100644
--- a/init/main.c
+++ b/init/main.c
@@ -650,6 +650,7 @@ asmlinkage __visible void __init start_kernel(void)
 	thread_stack_cache_init();
 	cred_init();
 	fork_init();
+	init_kmod_umh();
 	proc_caches_init();
 	buffer_init();
 	key_init();
diff --git a/kernel/kmod.c b/kernel/kmod.c
index 563f97e2be36..6fe6848787b2 100644
--- a/kernel/kmod.c
+++ b/kernel/kmod.c
@@ -46,6 +46,9 @@
 #include <trace/events/module.h>
 
 extern int max_threads;
+unsigned int max_modprobes;
+module_param(max_modprobes, uint, 0644);
+MODULE_PARM_DESC(max_modprobes, "Max number of allowed concurrent modprobes");
 
 #define CAP_BSET	(void *)1
 #define CAP_PI		(void *)2
@@ -127,10 +130,8 @@ int __request_module(bool wait, const char *fmt, ...)
 {
 	va_list args;
 	char module_name[MODULE_NAME_LEN];
-	unsigned int max_modprobes;
 	int ret;
 	static atomic_t kmod_concurrent = ATOMIC_INIT(0);
-#define MAX_KMOD_CONCURRENT 50	/* Completely arbitrary value - KAO */
 	static int kmod_loop_msg;
 
 	/*
@@ -154,19 +155,6 @@ int __request_module(bool wait, const char *fmt, ...)
 	if (ret)
 		return ret;
 
-	/* If modprobe needs a service that is in a module, we get a recursive
-	 * loop.  Limit the number of running kmod threads to max_threads/2 or
-	 * MAX_KMOD_CONCURRENT, whichever is the smaller.  A cleaner method
-	 * would be to run the parents of this process, counting how many times
-	 * kmod was invoked.  That would mean accessing the internals of the
-	 * process tables to get the command line, proc_pid_cmdline is static
-	 * and it is not worth changing the proc code just to handle this case. 
-	 * KAO.
-	 *
-	 * "trace the ppid" is simple, but will fail if someone's
-	 * parent exits.  I think this is as good as it gets. --RR
-	 */
-	max_modprobes = min(max_threads/2, MAX_KMOD_CONCURRENT);
 	atomic_inc(&kmod_concurrent);
 	if (atomic_read(&kmod_concurrent) > max_modprobes) {
 		/* We may be blaming an innocent here, but unlikely */
@@ -188,6 +176,31 @@ int __request_module(bool wait, const char *fmt, ...)
 	return ret;
 }
 EXPORT_SYMBOL(__request_module);
+
+/*
+ * If modprobe needs a service that is in a module, we get a recursive
+ * loop.  Limit the number of running kmod threads to max_threads/2 or
+ * CONFIG_MAX_KMOD_CONCURRENT, whichever is the smaller.  A cleaner method
+ * would be to run the parents of this process, counting how many times
+ * kmod was invoked.  That would mean accessing the internals of the
+ * process tables to get the command line, proc_pid_cmdline is static
+ * and it is not worth changing the proc code just to handle this case.
+ *
+ * "trace the ppid" is simple, but will fail if someone's
+ * parent exits.  I think this is as good as it gets.
+ *
+ * You can override with with a kernel parameter, for instance to allow
+ * 4096 concurrent modprobe instances:
+ *
+ *	kmod.max_modprobes=4096
+ */
+void __init init_kmod_umh(void)
+{
+	if (!max_modprobes)
+		max_modprobes = min(max_threads/2,
+				    1 << CONFIG_MAX_KMOD_CONCURRENT);
+}
+
 #endif /* CONFIG_MODULES */
 
 static void call_usermodehelper_freeinfo(struct subprocess_info *info)
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1645922 — Re: [PATCH 1/6] kmod: add dynamic max concurrent thread count

On Thu, May 18, 2017 at 08:24:39PM -0700, Luis R. Rodriguez wrote:
> We currently statically limit the number of modprobe threads which
> we allow to run concurrently to 50. As per Keith Owens, this was a
> completely arbitrary value, and it was set in the 2.3.38 days [0]
> over 16 years ago in year 2000.
> 
> Although we haven't yet hit our lower limits, experimentation [1]
> shows that when and if we hit this limit in the worst case, will be
> fatal -- consider get_fs_type() failures upon mount on a system which
> has many partitions, some of which might even be with the same
> filesystem. Its best to be prudent and increase and set this
> value to something more sensible which ensures we're far from hitting
> the limit and also allows default build/user run time override.
> 
> The worst case is fatal given that once a module fails to load there
> is a period of time during which subsequent request for the same module
> will fail, so in the case of partitions its not just one request that
> could fail, but whole series of partitions. This later issue of a
> module request failure domino effect can be addressed later, but
> increasing the limit to something more meaninful should at least give us
> enough cushion to avoid this for a while.
> 
> Set this value up with a bit more meaninful modern limits:
> 
> Bump this up to 64  max for small systems (CONFIG_BASE_SMALL)
> Bump this up to 128 max for larger systems (!CONFIG_BASE_SMALL)
> 
> Also allow the default max limit to be further fine tuned at compile
> time and at initialization at run time at boot up using the kernel
> parameter: max_modprobes.
> 
> [0] https://git.kernel.org/cgit/linux/kernel/git/history/history.git/commit/?id=ab1c4ec7410f6ec64e1511d1a7d850fc99c09b44
> [1] https://github.com/mcgrof/test_request_module

If we actually run into this issue, instead of slamming the system with
bazillion concurrent requests, can we wait for the other modprobes to
finish and then continue?

Thanks.

-- 
Dmitry

[toc] | [prev] | [next] | [standalone]

#1645976 — Re: [PATCH 1/6] kmod: add dynamic max concurrent thread count

On Fri, May 19, 2017 at 02:45:29PM -0700, Luis R. Rodriguez wrote:
> On May 19, 2017 1:45 PM, "Dmitry Torokhov" <dmitry.torokhov@gmail.com>
> wrote:
> 
> On Thu, May 18, 2017 at 08:24:39PM -0700, Luis R. Rodriguez wrote:
> > We currently statically limit the number of modprobe threads which
> > we allow to run concurrently to 50. As per Keith Owens, this was a
> > completely arbitrary value, and it was set in the 2.3.38 days [0]
> > over 16 years ago in year 2000.
> >
> > Although we haven't yet hit our lower limits, experimentation [1]
> > shows that when and if we hit this limit in the worst case, will be
> > fatal -- consider get_fs_type() failures upon mount on a system which
> > has many partitions, some of which might even be with the same
> > filesystem. Its best to be prudent and increase and set this
> > value to something more sensible which ensures we're far from hitting
> > the limit and also allows default build/user run time override.
> >
> > The worst case is fatal given that once a module fails to load there
> > is a period of time during which subsequent request for the same module
> > will fail, so in the case of partitions its not just one request that
> > could fail, but whole series of partitions. This later issue of a
> > module request failure domino effect can be addressed later, but
> > increasing the limit to something more meaninful should at least give us
> > enough cushion to avoid this for a while.
> >
> > Set this value up with a bit more meaninful modern limits:
> >
> > Bump this up to 64  max for small systems (CONFIG_BASE_SMALL)
> > Bump this up to 128 max for larger systems (!CONFIG_BASE_SMALL)
> >
> > Also allow the default max limit to be further fine tuned at compile
> > time and at initialization at run time at boot up using the kernel
> > parameter: max_modprobes.
> >
> > [0] https://git.kernel.org/cgit/linux/kernel/git/history/
> history.git/commit/?id=ab1c4ec7410f6ec64e1511d1a7d850fc99c09b44
> > [1] https://github.com/mcgrof/test_request_module
> 
> If we actually run into this issue, instead of slamming the system with
> bazillion concurrent requests, can we wait for the other modprobes to
> finish and then continue?
> 
> 
> Yes ! That I have a patch that does precisely that ! That is actually still
> *not enough* to not fail fatally but this would be subject of another
> series with more debatable approaches.
> 

Then please post it.

> This at least pushes us to closer safer limits for now while also making it
> configurable.

Making it configurable depending on how big/little box is makes no
sense, especially if the above is implemented, as depth of modprobe
invocations depends on configuration and not computing power of the
hardware the system is running on.

Thanks.

-- 
Dmitry

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web