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


Groups > linux.kernel > #1650985 > unrolled thread

[PATCH v2 4/5] kmod: add helpers for getting kmod limit

Started by"Luis R. Rodriguez" <mcgrof@kernel.org>
First post2017-05-26 02:20 +0200
Last post2017-05-27 03:30 +0200
Articles 3 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [PATCH v2 4/5] kmod: add helpers for getting kmod limit "Luis R. Rodriguez" <mcgrof@kernel.org> - 2017-05-26 02:20 +0200
    Re: [PATCH v2 4/5] kmod: add helpers for getting kmod limit Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-05-26 03:00 +0200
      Re: [PATCH v2 4/5] kmod: add helpers for getting kmod limit "Luis R. Rodriguez" <mcgrof@kernel.org> - 2017-05-27 03:30 +0200

#1650985 — [PATCH v2 4/5] kmod: add helpers for getting kmod limit

From"Luis R. Rodriguez" <mcgrof@kernel.org>
Date2017-05-26 02:20 +0200
Subject[PATCH v2 4/5] kmod: add helpers for getting kmod limit
Message-ID<tLb8Z-6VX-11@gated-at.bofh.it>
This adds helpers for getting access to the kmod limit from
userspace. This knob should help userspace more gracefully and
deterministically handle module loading.

Cc: Tom Gundersen <teg@jklm.no>
Cc: Petr Mladek <pmladek@suse.com>
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 Documentation/sysctl/kernel.txt | 20 ++++++++++++++++++++
 include/linux/kmod.h            |  5 +++++
 kernel/kmod.c                   | 33 +++++++++++++++++++++++++++++++++
 kernel/sysctl.c                 |  7 +++++++
 4 files changed, 65 insertions(+)

diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
index bac23c198360..080ccdca1f10 100644
--- a/Documentation/sysctl/kernel.txt
+++ b/Documentation/sysctl/kernel.txt
@@ -370,6 +370,26 @@ with the "modules_disabled" sysctl.
 
 ==============================================================
 
+kmod-limit:
+
+Get the max amount of concurrent requests (kmod_concurrent) the kernel can
+make out to userspace to call 'modprobe'. This limit is known internally to the
+kernel as max_modprobes. This interface is designed to enable userspace to
+query the kernel for the max_modprobes limit so userspace can more
+deterministically handle module loading by only enabling max_modprobes
+'modprobe' calls at a time.
+
+Dependencies are resolved in userspace through depmod, so one modprobe
+call only bumps the number of concurrent threads (kmod_concurrent) by one.
+Dependencies for a module then are loaded directly in userspace using
+init_module() / finit_module() skipping bumping kmod_concurrent or being
+affected by max_modprobes.
+
+The max_modprobes value is set at build time with CONFIG_MAX_KMOD_CONCURRENT.
+You can override at initialization with the module parameter max_modprobes.
+
+==============================================================
+
 kptr_restrict:
 
 This toggle indicates whether restrictions are placed on
diff --git a/include/linux/kmod.h b/include/linux/kmod.h
index 8e2f302b214a..bbc6d59190aa 100644
--- a/include/linux/kmod.h
+++ b/include/linux/kmod.h
@@ -39,13 +39,18 @@ int __request_module(bool wait, const char *name, ...);
 #define try_then_request_module(x, mod...) \
 	((x) ?: (__request_module(true, mod), (x)))
 void init_kmod_umh(void);
+unsigned int get_kmod_umh_limit(void);
+int sysctl_kmod_limit(struct ctl_table *table, int write,
+		      void __user *buffer, size_t *lenp, loff_t *ppos);
 #else
 static inline int request_module(const char *name, ...) { return -ENOSYS; }
 static inline int request_module_nowait(const char *name, ...) { return -ENOSYS; }
 #define try_then_request_module(x, mod...) (x)
 static inline void init_kmod_umh(void) { }
+static inline unsigned int get_kmod_umh_limit(void) { return 0; }
 #endif
 
+#define get_kmod_umh_limit get_kmod_umh_limit
 
 struct cred;
 struct file;
diff --git a/kernel/kmod.c b/kernel/kmod.c
index cafd27b92d19..17de776cf368 100644
--- a/kernel/kmod.c
+++ b/kernel/kmod.c
@@ -111,6 +111,17 @@ static int call_modprobe(char *module_name, int wait)
 }
 
 /**
+ * get_kmod_umh_limit - get concurrent modprobe thread limit
+ *
+ * Returns the number of allowed concurrent modprobe calls.
+ */
+unsigned int get_kmod_umh_limit(void)
+{
+	return max_modprobes;
+}
+EXPORT_SYMBOL_GPL(get_kmod_umh_limit);
+
+/**
  * __request_module - try to load a kernel module
  * @wait: wait (or not) for the operation to complete
  * @fmt: printf style format string for the name of the module
@@ -194,6 +205,28 @@ void __init init_kmod_umh(void)
 	atomic_set(&kmod_concurrent_max, max_modprobes);
 }
 
+int sysctl_kmod_limit(struct ctl_table *table, int write,
+		      void __user *buffer, size_t *lenp, loff_t *ppos)
+{
+	struct ctl_table t;
+	int ret;
+	unsigned int local_max_modprobes = max_modprobes;
+	unsigned int min = 0;
+	unsigned int max = max_threads/2;
+
+	t = *table;
+	t.data = &local_max_modprobes;
+	t.extra1 = &min;
+	t.extra2 = &max;
+
+	if (write)
+		return -EPERM;
+
+	ret = proc_douintvec_minmax(&t, write, buffer, lenp, ppos);
+
+	return ret;
+}
+
 #endif /* CONFIG_MODULES */
 
 static void call_usermodehelper_freeinfo(struct subprocess_info *info)
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index df9f2a367882..d1c1d1999bb1 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -682,6 +682,13 @@ static struct ctl_table kern_table[] = {
 		.extra1		= &one,
 		.extra2		= &one,
 	},
+	{
+		.procname	= "kmod-limit",
+		.data		= NULL, /* filled in by handler */
+		.maxlen		= sizeof(unsigned int),
+		.mode		= 0444,
+		.proc_handler	= sysctl_kmod_limit,
+	},
 #endif
 #ifdef CONFIG_UEVENT_HELPER
 	{
-- 
2.11.0

[toc] | [next] | [standalone]


#1651013

FromDmitry Torokhov <dmitry.torokhov@gmail.com>
Date2017-05-26 03:00 +0200
Message-ID<tLbLH-7dl-3@gated-at.bofh.it>
In reply to#1650985
On Thu, May 25, 2017 at 05:16:29PM -0700, Luis R. Rodriguez wrote:
> This adds helpers for getting access to the kmod limit from
> userspace. This knob should help userspace more gracefully and
> deterministically handle module loading.

I think more details is needed before we add a new ABI to the kernel.
Why can't userspace submit as much as it wants and the kernel decide how
much it will service at once?

> 
> Cc: Tom Gundersen <teg@jklm.no>
> Cc: Petr Mladek <pmladek@suse.com>
> Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
> ---
>  Documentation/sysctl/kernel.txt | 20 ++++++++++++++++++++
>  include/linux/kmod.h            |  5 +++++
>  kernel/kmod.c                   | 33 +++++++++++++++++++++++++++++++++
>  kernel/sysctl.c                 |  7 +++++++
>  4 files changed, 65 insertions(+)
> 
> diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
> index bac23c198360..080ccdca1f10 100644
> --- a/Documentation/sysctl/kernel.txt
> +++ b/Documentation/sysctl/kernel.txt
> @@ -370,6 +370,26 @@ with the "modules_disabled" sysctl.
>  
>  ==============================================================
>  
> +kmod-limit:
> +
> +Get the max amount of concurrent requests (kmod_concurrent) the kernel can
> +make out to userspace to call 'modprobe'. This limit is known internally to the
> +kernel as max_modprobes. This interface is designed to enable userspace to
> +query the kernel for the max_modprobes limit so userspace can more
> +deterministically handle module loading by only enabling max_modprobes
> +'modprobe' calls at a time.
> +
> +Dependencies are resolved in userspace through depmod, so one modprobe
> +call only bumps the number of concurrent threads (kmod_concurrent) by one.
> +Dependencies for a module then are loaded directly in userspace using
> +init_module() / finit_module() skipping bumping kmod_concurrent or being
> +affected by max_modprobes.
> +
> +The max_modprobes value is set at build time with CONFIG_MAX_KMOD_CONCURRENT.
> +You can override at initialization with the module parameter max_modprobes.
> +
> +==============================================================
> +
>  kptr_restrict:
>  
>  This toggle indicates whether restrictions are placed on
> diff --git a/include/linux/kmod.h b/include/linux/kmod.h
> index 8e2f302b214a..bbc6d59190aa 100644
> --- a/include/linux/kmod.h
> +++ b/include/linux/kmod.h
> @@ -39,13 +39,18 @@ int __request_module(bool wait, const char *name, ...);
>  #define try_then_request_module(x, mod...) \
>  	((x) ?: (__request_module(true, mod), (x)))
>  void init_kmod_umh(void);
> +unsigned int get_kmod_umh_limit(void);
> +int sysctl_kmod_limit(struct ctl_table *table, int write,
> +		      void __user *buffer, size_t *lenp, loff_t *ppos);
>  #else
>  static inline int request_module(const char *name, ...) { return -ENOSYS; }
>  static inline int request_module_nowait(const char *name, ...) { return -ENOSYS; }
>  #define try_then_request_module(x, mod...) (x)
>  static inline void init_kmod_umh(void) { }
> +static inline unsigned int get_kmod_umh_limit(void) { return 0; }
>  #endif
>  
> +#define get_kmod_umh_limit get_kmod_umh_limit
>  
>  struct cred;
>  struct file;
> diff --git a/kernel/kmod.c b/kernel/kmod.c
> index cafd27b92d19..17de776cf368 100644
> --- a/kernel/kmod.c
> +++ b/kernel/kmod.c
> @@ -111,6 +111,17 @@ static int call_modprobe(char *module_name, int wait)
>  }
>  
>  /**
> + * get_kmod_umh_limit - get concurrent modprobe thread limit
> + *
> + * Returns the number of allowed concurrent modprobe calls.
> + */
> +unsigned int get_kmod_umh_limit(void)
> +{
> +	return max_modprobes;
> +}
> +EXPORT_SYMBOL_GPL(get_kmod_umh_limit);
> +
> +/**
>   * __request_module - try to load a kernel module
>   * @wait: wait (or not) for the operation to complete
>   * @fmt: printf style format string for the name of the module
> @@ -194,6 +205,28 @@ void __init init_kmod_umh(void)
>  	atomic_set(&kmod_concurrent_max, max_modprobes);
>  }
>  
> +int sysctl_kmod_limit(struct ctl_table *table, int write,
> +		      void __user *buffer, size_t *lenp, loff_t *ppos)
> +{
> +	struct ctl_table t;
> +	int ret;
> +	unsigned int local_max_modprobes = max_modprobes;
> +	unsigned int min = 0;
> +	unsigned int max = max_threads/2;
> +
> +	t = *table;
> +	t.data = &local_max_modprobes;
> +	t.extra1 = &min;
> +	t.extra2 = &max;
> +
> +	if (write)
> +		return -EPERM;
> +
> +	ret = proc_douintvec_minmax(&t, write, buffer, lenp, ppos);
> +
> +	return ret;
> +}
> +
>  #endif /* CONFIG_MODULES */
>  
>  static void call_usermodehelper_freeinfo(struct subprocess_info *info)
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index df9f2a367882..d1c1d1999bb1 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -682,6 +682,13 @@ static struct ctl_table kern_table[] = {
>  		.extra1		= &one,
>  		.extra2		= &one,
>  	},
> +	{
> +		.procname	= "kmod-limit",
> +		.data		= NULL, /* filled in by handler */
> +		.maxlen		= sizeof(unsigned int),
> +		.mode		= 0444,
> +		.proc_handler	= sysctl_kmod_limit,
> +	},
>  #endif
>  #ifdef CONFIG_UEVENT_HELPER
>  	{
> -- 
> 2.11.0
> 

-- 
Dmitry

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


#1651677

From"Luis R. Rodriguez" <mcgrof@kernel.org>
Date2017-05-27 03:30 +0200
Message-ID<tLyIn-4Vw-153@gated-at.bofh.it>
In reply to#1651013
On Thu, May 25, 2017 at 05:56:51PM -0700, Dmitry Torokhov wrote:
> On Thu, May 25, 2017 at 05:16:29PM -0700, Luis R. Rodriguez wrote:
> > This adds helpers for getting access to the kmod limit from
> > userspace. This knob should help userspace more gracefully and
> > deterministically handle module loading.
> 
> I think more details is needed before we add a new ABI to the kernel.
> Why can't userspace submit as much as it wants and the kernel decide how
> much it will service at once?

I suppose I should clarify on the commit log then, that without this heuristic
of #ifdef get_kmod_umh_limit -- *iff* userspace today is allowing more than 50
threads it can mean userspace is allowing a modprobe to fail. Check the results
of test 0008 and 0009 on dmesg, you can end up with really unexpected results.

This knob enables userspace to gracefully send in as many requests as allowed.
I guess this could just be a kernel revision check...  given that once
throttling go in it can be as crazy as it wants.

Also I suppose one could argue that *since* this has not been a real issue
*yet* (we assume), the old userspace of sending as many requests as it needs
is fine.

Will drop this!

  Luis

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web