From: Luis Chamberlain <mcgrof@kernel.org>
To: david@redhat.com, patches@lists.linux.dev,
linux-modules@vger.kernel.org, linux-mm@kvack.org,
linux-kernel@vger.kernel.org, pmladek@suse.com,
petr.pavlu@suse.com, prarit@redhat.com,
torvalds@linux-foundation.org, gregkh@linuxfoundation.org,
rafael@kernel.org
Cc: christophe.leroy@csgroup.eu, tglx@linutronix.de,
peterz@infradead.org, song@kernel.org, rppt@kernel.org,
willy@infradead.org, vbabka@suse.cz, mhocko@suse.com,
dave.hansen@linux.intel.com, mcgrof@kernel.org
Subject: [PATCH 5/7] modules/kmod: replace implementation with a sempahore
Date: Tue, 28 Mar 2023 22:31:47 -0700 [thread overview]
Message-ID: <20230329053149.3976378-6-mcgrof@kernel.org> (raw)
In-Reply-To: <20230329053149.3976378-1-mcgrof@kernel.org>
Simplfy the concurrency delimiter we user for kmod with the semaphore.
I had used the kmod strategy to try to implement a similar concurrency
delimiter for the kernel_read*() calls from the finit_module() path
so to reduce vmalloc() memory pressure. That effort didn't provid yet
conclusive results, but one thing that did became clear is we can use
the suggested alternative solution with semaphores which Linus hinted
at instead of using the atomic / wait strategy.
I've stress tested this with kmod test 0008:
time /data/linux-next/tools/testing/selftests/kmod/kmod.sh -t 0008
And I get only a *slight* delay. That delay however is small, a few
seconds for a full test loop run that runs 150 times, for about ~30-40
seconds. The small delay is worth the simplfication IMHO.
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
kernel/module/kmod.c | 26 +++++++-------------------
1 file changed, 7 insertions(+), 19 deletions(-)
diff --git a/kernel/module/kmod.c b/kernel/module/kmod.c
index b717134ebe17..fd7c461387f8 100644
--- a/kernel/module/kmod.c
+++ b/kernel/module/kmod.c
@@ -40,8 +40,7 @@
* effect. Systems like these are very unlikely if modules are enabled.
*/
#define MAX_KMOD_CONCURRENT 50
-static atomic_t kmod_concurrent_max = ATOMIC_INIT(MAX_KMOD_CONCURRENT);
-static DECLARE_WAIT_QUEUE_HEAD(kmod_wq);
+static CONCURRENCY_LIMITER(kmod_concurrent_max, MAX_KMOD_CONCURRENT);
/*
* This is a restriction on having *all* MAX_KMOD_CONCURRENT threads
@@ -148,29 +147,18 @@ int __request_module(bool wait, const char *fmt, ...)
if (ret)
return ret;
- if (atomic_dec_if_positive(&kmod_concurrent_max) < 0) {
- 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);
- ret = wait_event_killable_timeout(kmod_wq,
- atomic_dec_if_positive(&kmod_concurrent_max) >= 0,
- MAX_KMOD_ALL_BUSY_TIMEOUT * HZ);
- if (!ret) {
- pr_warn_ratelimited("request_module: modprobe %s cannot be processed, kmod busy with %d threads for more than %d seconds now",
- module_name, MAX_KMOD_CONCURRENT, MAX_KMOD_ALL_BUSY_TIMEOUT);
- return -ETIME;
- } else if (ret == -ERESTARTSYS) {
- pr_warn_ratelimited("request_module: sigkill sent for modprobe %s, giving up", module_name);
- return ret;
- }
+ ret = down_timeout(&kmod_concurrent_max, MAX_KMOD_ALL_BUSY_TIMEOUT);
+ if (ret) {
+ pr_warn_ratelimited("request_module: modprobe %s cannot be processed, kmod busy with %d threads for more than %d seconds now",
+ module_name, MAX_KMOD_CONCURRENT, MAX_KMOD_ALL_BUSY_TIMEOUT);
+ return ret;
}
trace_module_request(module_name, wait, _RET_IP_);
ret = call_modprobe(module_name, wait ? UMH_WAIT_PROC : UMH_WAIT_EXEC);
- atomic_inc(&kmod_concurrent_max);
- wake_up(&kmod_wq);
+ up(&kmod_concurrent_max);
return ret;
}
--
2.39.2
next prev parent reply other threads:[~2023-03-29 5:32 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-29 5:31 [PATCH 0/7] module: avoid userspace pressure on unwanted allocations Luis Chamberlain
2023-03-29 5:31 ` [PATCH 1/7] module: move finished_loading() Luis Chamberlain
2023-03-29 5:31 ` [PATCH 2/7] module: extract patient module check into helper Luis Chamberlain
2023-03-29 5:31 ` [PATCH 3/7] module: avoid allocation if module is already present and ready Luis Chamberlain
2023-03-29 5:31 ` [PATCH 4/7] sempahore: add a helper for a concurrency limiter Luis Chamberlain
2023-03-29 7:21 ` Peter Zijlstra
2023-03-29 7:51 ` Luis Chamberlain
2023-03-29 9:19 ` Peter Zijlstra
2023-03-29 9:49 ` Luis Chamberlain
2023-03-29 10:14 ` Peter Zijlstra
2023-03-29 16:50 ` Linus Torvalds
2023-03-30 11:56 ` Peter Zijlstra
2023-03-30 16:23 ` Linus Torvalds
2023-03-31 3:42 ` Sergey Senozhatsky
2023-03-31 8:05 ` Petr Mladek
2023-03-31 3:45 ` Luis Chamberlain
2023-03-31 4:06 ` Matthew Wilcox
2023-03-31 4:13 ` Luis Chamberlain
2023-03-31 4:11 ` Sergey Senozhatsky
2023-03-29 5:31 ` Luis Chamberlain [this message]
2023-03-29 5:31 ` [PATCH 6/7] debugfs: add debugfs_create_atomic64_t for atomic64_t Luis Chamberlain
2023-03-29 5:46 ` Greg KH
2023-03-29 5:31 ` [PATCH 7/7] module: add debug stats to help identify memory pressure Luis Chamberlain
2023-03-29 5:46 ` Greg KH
2023-03-29 6:04 ` Luis Chamberlain
Reply instructions:
You may reply publicly 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=20230329053149.3976378-6-mcgrof@kernel.org \
--to=mcgrof@kernel.org \
--cc=christophe.leroy@csgroup.eu \
--cc=dave.hansen@linux.intel.com \
--cc=david@redhat.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-modules@vger.kernel.org \
--cc=mhocko@suse.com \
--cc=patches@lists.linux.dev \
--cc=peterz@infradead.org \
--cc=petr.pavlu@suse.com \
--cc=pmladek@suse.com \
--cc=prarit@redhat.com \
--cc=rafael@kernel.org \
--cc=rppt@kernel.org \
--cc=song@kernel.org \
--cc=tglx@linutronix.de \
--cc=torvalds@linux-foundation.org \
--cc=vbabka@suse.cz \
--cc=willy@infradead.org \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).