LKML Archive on lore.kernel.org
 help / color / Atom feed
From: "Luis R. Rodriguez" <mcgrof@kernel.org>
To: akpm@linux-foundation.org
Cc: jeyu@redhat.com, shuah@kernel.org, rusty@rustcorp.com.au,
	ebiederm@xmission.com, dmitry.torokhov@gmail.com,
	acme@redhat.com, josh@joshtriplett.org, martin.wilck@suse.com,
	mmarek@suse.com, pmladek@suse.com, hare@suse.com,
	rwright@hpe.com, jeffm@suse.com, DSterba@suse.com,
	fdmanana@suse.com, neilb@suse.com, linux@roeck-us.net,
	rgoldwyn@suse.com, subashab@codeaurora.org, xypron.glpk@gmx.de,
	keescook@chromium.org, atomlin@redhat.com, mbenes@suse.cz,
	paulmck@linux.vnet.ibm.com, dan.j.williams@intel.com,
	jpoimboe@redhat.com, davem@davemloft.net, mingo@redhat.com,
	alan@linux.intel.com, tytso@mit.edu, gregkh@linuxfoundation.org,
	torvalds@linux-foundation.org, dave@stgolabs.net,
	linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org,
	"Luis R. Rodriguez" <mcgrof@kernel.org>
Subject: [PATCH v4 3/3] kmod: throttle kmod thread limit
Date: Wed, 28 Jun 2017 15:31:55 -0700
Message-ID: <20170628223155.26472-4-mcgrof@kernel.org> (raw)
In-Reply-To: <20170628223155.26472-1-mcgrof@kernel.org>

If we reach the limit of modprobe_limit threads running the next
request_module() call will fail. The original reason for adding
a kill was to do away with possible issues with in old circumstances
which would create a recursive series of request_module() calls.

We can do better than just be super aggressive and reject calls
once we've reached the limit by simply making pending callers wait
until the threshold has been reduced, and then throttling them in,
one by one.

This throttling enables requests over the kmod concurrent limit to
be processed once a pending request completes. Only the first item
queued up to wait is woken up. The assumption here is once a task
is woken it will have no other option to also kick the queue to check
if there are more pending tasks -- regardless of whether or not it
was successful.

By throttling and processing only max kmod concurrent tasks we ensure
we avoid unexpected fatal request_module() calls, and we keep memory
consumption on module loading to a minimum.

With x86_64 qemu, with 4 cores, 4 GiB of RAM it takes the following run
time to run both tests:

time ./kmod.sh -t 0008
real    0m16.366s
user    0m0.883s
sys     0m8.916s

time ./kmod.sh -t 0009
real    0m50.803s
user    0m0.791s
sys     0m9.852s

Reviewed-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 kernel/kmod.c                        | 16 +++++++---------
 tools/testing/selftests/kmod/kmod.sh | 24 ++----------------------
 2 files changed, 9 insertions(+), 31 deletions(-)

diff --git a/kernel/kmod.c b/kernel/kmod.c
index ff68198fe83b..6d016c5d97c8 100644
--- a/kernel/kmod.c
+++ b/kernel/kmod.c
@@ -68,6 +68,7 @@ static DECLARE_RWSEM(umhelper_sem);
  */
 #define MAX_KMOD_CONCURRENT 50
 static atomic_t kmod_concurrent_max = ATOMIC_INIT(MAX_KMOD_CONCURRENT);
+static DECLARE_WAIT_QUEUE_HEAD(kmod_wq);
 
 /*
 	modprobe_path is set via /proc/sys.
@@ -140,7 +141,6 @@ int __request_module(bool wait, const char *fmt, ...)
 	va_list args;
 	char module_name[MODULE_NAME_LEN];
 	int ret;
-	static int kmod_loop_msg;
 
 	/*
 	 * We don't allow synchronous module loading from async.  Module
@@ -164,14 +164,11 @@ int __request_module(bool wait, const char *fmt, ...)
 		return ret;
 
 	if (atomic_dec_if_positive(&kmod_concurrent_max) < 0) {
-		/* We may be blaming an innocent here, but unlikely */
-		if (kmod_loop_msg < 5) {
-			printk(KERN_ERR
-			       "request_module: runaway loop modprobe %s\n",
-			       module_name);
-			kmod_loop_msg++;
-		}
-		return -ENOMEM;
+		pr_warn_ratelimited("request_module: kmod_concurrent_max (%u) close to 0 (max_modprobes: %u), for module %s, throttling...",
+				    atomic_read(&kmod_concurrent_max),
+				    MAX_KMOD_CONCURRENT, module_name);
+		wait_event_interruptible(kmod_wq,
+					 atomic_dec_if_positive(&kmod_concurrent_max) >= 0);
 	}
 
 	trace_module_request(module_name, wait, _RET_IP_);
@@ -179,6 +176,7 @@ int __request_module(bool wait, const char *fmt, ...)
 	ret = call_modprobe(module_name, wait ? UMH_WAIT_PROC : UMH_WAIT_EXEC);
 
 	atomic_inc(&kmod_concurrent_max);
+	wake_up(&kmod_wq);
 
 	return ret;
 }
diff --git a/tools/testing/selftests/kmod/kmod.sh b/tools/testing/selftests/kmod/kmod.sh
index 10196a62ed09..8cecae9a8bca 100755
--- a/tools/testing/selftests/kmod/kmod.sh
+++ b/tools/testing/selftests/kmod/kmod.sh
@@ -59,28 +59,8 @@ ALL_TESTS="$ALL_TESTS 0004:1:1"
 ALL_TESTS="$ALL_TESTS 0005:10:1"
 ALL_TESTS="$ALL_TESTS 0006:10:1"
 ALL_TESTS="$ALL_TESTS 0007:5:1"
-
-# Disabled tests:
-#
-# 0008 x 150 -  multithreaded - push kmod_concurrent over max_modprobes for request_module()"
-# Current best-effort failure interpretation:
-# Enough module requests get loaded in place fast enough to reach over the
-# max_modprobes limit and trigger a failure -- before we're even able to
-# start processing pending requests.
-ALL_TESTS="$ALL_TESTS 0008:150:0"
-
-# 0009 x 150 - multithreaded - push kmod_concurrent over max_modprobes for get_fs_type()"
-# Current best-effort failure interpretation:
-#
-# get_fs_type() requests modules using aliases as such the optimization in
-# place today to look for already loaded modules will not take effect and
-# we end up requesting a new module to load, this bumps the kmod_concurrent,
-# and in certain circumstances can lead to pushing the kmod_concurrent over
-# the max_modprobe limit.
-#
-# This test fails much easier than test 0008 since the alias optimizations
-# are not in place.
-ALL_TESTS="$ALL_TESTS 0009:150:0"
+ALL_TESTS="$ALL_TESTS 0008:150:1"
+ALL_TESTS="$ALL_TESTS 0009:150:1"
 
 test_modprobe()
 {
-- 
2.11.0

      parent reply index

Thread overview: 69+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-19  3:24 [PATCH 0/6] kmod: few simple enhancements Luis R. Rodriguez
2017-05-19  3:24 ` [PATCH 1/6] kmod: add dynamic max concurrent thread count Luis R. Rodriguez
2017-05-19 20:44   ` Dmitry Torokhov
     [not found]     ` <CAB=NE6XGL24O+JfTNUG0HO4obhDc-v+HyL0SCrQELiZrj2-qNw@mail.gmail.com>
     [not found]       ` <CAB=NE6Wa4Nemh80yaCCwbjrNRLPD+GJMncg12APg9Vq63AWVng@mail.gmail.com>
     [not found]         ` <CAB=NE6Vc6RDAytn2Pkv2V58HFo8ncR0eOHZ3===kbZ2NF78ubg@mail.gmail.com>
     [not found]           ` <CAB=NE6Vqmx=y6muenpuQKynTP=pGWMF8tzoCA0BXD6d63q9wPg@mail.gmail.com>
2017-05-19 21:58             ` Dmitry Torokhov
2017-05-25 16:22               ` Luis R. Rodriguez
2017-05-25 16:38                 ` Dmitry Torokhov
2017-05-25 16:50                   ` Luis R. Rodriguez
2017-05-25 17:30                     ` Dmitry Torokhov
2017-05-25 17:38                       ` Luis R. Rodriguez
2017-05-25 18:06                         ` Luis R. Rodriguez
2017-05-25 18:26                           ` Dmitry Torokhov
2017-05-25 19:01                             ` Luis R. Rodriguez
2017-05-25 21:38                               ` Luis R. Rodriguez
2017-05-19  3:24 ` [PATCH 2/6] module: use list_for_each_entry_rcu() on find_module_all() Luis R. Rodriguez
2017-05-23 14:46   ` Miroslav Benes
2017-05-19  3:24 ` [PATCH 3/6] kmod: provide wrappers for kmod_concurrent inc/dec Luis R. Rodriguez
2017-05-19  3:24 ` [PATCH 4/6] kmod: return -EBUSY if modprobe limit is reached Luis R. Rodriguez
2017-05-19  3:24 ` [PATCH 5/6] kmod: preempt on kmod_umh_threads_get() Luis R. Rodriguez
2017-05-19 22:27   ` Dmitry Torokhov
2017-05-25  0:14     ` Luis R. Rodriguez
2017-05-25  0:45       ` Dmitry Torokhov
2017-05-25  1:00         ` Luis R. Rodriguez
2017-05-25  2:27           ` Dmitry Torokhov
2017-05-25 11:19             ` Petr Mladek
2017-05-25 15:38               ` Luis R. Rodriguez
2017-05-25 16:42               ` Dmitry Torokhov
2017-05-25 15:18             ` Jessica Yu
2017-05-19  3:24 ` [PATCH 6/6] kmod: use simplified rate limit printk Luis R. Rodriguez
2017-05-19 22:23   ` Dmitry Torokhov
2017-05-23  9:00     ` Petr Mladek
2017-05-26  0:16 ` [PATCH v2 0/5] kmod: help make deterministic Luis R. Rodriguez
2017-05-26  0:16   ` [PATCH v2 1/5] module: use list_for_each_entry_rcu() on find_module_all() Luis R. Rodriguez
2017-05-26  0:16   ` [PATCH v2 2/5] kmod: reduce atomic operations on kmod_concurrent Luis R. Rodriguez
2017-05-26  1:11     ` Dmitry Torokhov
2017-05-26 20:03       ` Luis R. Rodriguez
2017-05-26  0:16   ` [PATCH v2 3/5] kmod: add test driver to stress test the module loader Luis R. Rodriguez
2017-05-26  0:16   ` [PATCH v2 4/5] kmod: add helpers for getting kmod limit Luis R. Rodriguez
2017-05-26  0:56     ` Dmitry Torokhov
2017-05-26 20:27       ` Luis R. Rodriguez
2017-05-26  0:16   ` [PATCH v2 5/5] kmod: throttle kmod thread limit Luis R. Rodriguez
2017-05-26 21:12   ` [PATCH v3 0/4] kmod: help make deterministic Luis R. Rodriguez
2017-05-26 21:12     ` [PATCH v3 1/4] module: use list_for_each_entry_rcu() on find_module_all() Luis R. Rodriguez
2017-05-26 21:12     ` [PATCH v3 2/4] kmod: reduce atomic operations on kmod_concurrent and simplify Luis R. Rodriguez
2017-06-23 19:19       ` [PATCH v4 " Luis R. Rodriguez
2017-06-26 11:36       ` [PATCH v3 " Petr Mladek
2017-05-26 21:12     ` [PATCH v3 3/4] kmod: add test driver to stress test the module loader Luis R. Rodriguez
2017-05-26 21:12     ` [PATCH v3 4/4] kmod: throttle kmod thread limit Luis R. Rodriguez
2017-06-22 15:19       ` Petr Mladek
2017-06-23 16:16         ` Luis R. Rodriguez
2017-06-23 17:56           ` Luis R. Rodriguez
2017-06-23 19:16             ` Luis R. Rodriguez
2017-06-26 10:03               ` Petr Mladek
2017-06-26  9:55           ` Petr Mladek
2017-06-23 19:20       ` [PATCH v4 " Luis R. Rodriguez
2017-06-26 11:38         ` Petr Mladek
2017-06-28 22:11           ` Luis R. Rodriguez
2017-06-20 20:56     ` [PATCH v3 0/4] kmod: help make deterministic Luis R. Rodriguez
2017-06-21  0:23       ` Kees Cook
2017-06-26 21:37         ` Jessica Yu
2017-06-26 22:44           ` Luis R. Rodriguez
2017-06-27  0:27             ` Luis R. Rodriguez
2017-06-27  8:13               ` Petr Mladek
2017-06-27 10:04                 ` Jessica Yu
2017-06-27 15:26             ` Jessica Yu
2017-06-28  0:49               ` Luis R. Rodriguez
2017-06-28 22:31     ` [PATCH v4 0/3] " Luis R. Rodriguez
2017-06-28 22:31       ` [PATCH v4 1/3] MAINTAINERS: give kmod some maintainer love Luis R. Rodriguez
2017-06-28 22:31       ` [PATCH v4 2/3] kmod: add test driver to stress test the module loader Luis R. Rodriguez
2017-06-28 22:31       ` Luis R. Rodriguez [this message]

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170628223155.26472-4-mcgrof@kernel.org \
    --to=mcgrof@kernel.org \
    --cc=DSterba@suse.com \
    --cc=acme@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=alan@linux.intel.com \
    --cc=atomlin@redhat.com \
    --cc=dan.j.williams@intel.com \
    --cc=dave@stgolabs.net \
    --cc=davem@davemloft.net \
    --cc=dmitry.torokhov@gmail.com \
    --cc=ebiederm@xmission.com \
    --cc=fdmanana@suse.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hare@suse.com \
    --cc=jeffm@suse.com \
    --cc=jeyu@redhat.com \
    --cc=josh@joshtriplett.org \
    --cc=jpoimboe@redhat.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=martin.wilck@suse.com \
    --cc=mbenes@suse.cz \
    --cc=mingo@redhat.com \
    --cc=mmarek@suse.com \
    --cc=neilb@suse.com \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=pmladek@suse.com \
    --cc=rgoldwyn@suse.com \
    --cc=rusty@rustcorp.com.au \
    --cc=rwright@hpe.com \
    --cc=shuah@kernel.org \
    --cc=subashab@codeaurora.org \
    --cc=torvalds@linux-foundation.org \
    --cc=tytso@mit.edu \
    --cc=xypron.glpk@gmx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org linux-kernel@archiver.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox