From patchwork Sun Mar 19 21:49:22 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Luis Chamberlain X-Patchwork-Id: 13180612 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2A785C76196 for ; Sun, 19 Mar 2023 21:49:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229913AbjCSVtr (ORCPT ); Sun, 19 Mar 2023 17:49:47 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42132 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229481AbjCSVta (ORCPT ); Sun, 19 Mar 2023 17:49:30 -0400 Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:3::133]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id BEE8912BD3; Sun, 19 Mar 2023 14:49:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=Sender:Content-Transfer-Encoding: MIME-Version:References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From: Reply-To:Content-Type:Content-ID:Content-Description; bh=LipVinnRLoEDEa+vjjA5/3unXwIjvgFv3lLbKmxn/BU=; b=lpYOtGR+pKWQS+CVFCSXb+R381 Nng6PMZnpsAokhXG2OF7PbF8Gt1IWfH80MLQBlBr4DtQ5cVWOcQqDWDomlqsfoy/i+MsUwg5SKVcI TeTE/4PtWACysO4cRjffqu8iEAUzgvNzSdzxofBRFgIQdjXI0aoxhtskOIqWl1y9/W2t8kXqBzZVQ 4t8O5/YvGPKUKqv7bxB2rf4pShpU16ObKP+AcbEii6meGT+ku02fhuxgzPbCVPk+U+1B0v1xQDLXm 1RxLkRSxxfpJbPRg9eC2ejBNAs8JVU7SUG0vcz4eMB1eJW+SjdX4FCUDm1+VyigwDwhmP53pKt6NL W2ZCe26g==; Received: from mcgrof by bombadil.infradead.org with local (Exim 4.96 #2 (Red Hat Linux)) id 1pe0u8-007WjQ-09; Sun, 19 Mar 2023 21:49:28 +0000 From: Luis Chamberlain To: linux-modules@vger.kernel.org, linux-kernel@vger.kernel.org, pmladek@suse.com, david@redhat.com, petr.pavlu@suse.com, prarit@redhat.com Cc: christophe.leroy@csgroup.eu, song@kernel.org, mcgrof@kernel.org Subject: [RFT 1/5] module: move finished_loading() Date: Sun, 19 Mar 2023 14:49:22 -0700 Message-Id: <20230319214926.1794108-2-mcgrof@kernel.org> X-Mailer: git-send-email 2.38.1 In-Reply-To: <20230319214926.1794108-1-mcgrof@kernel.org> References: <20230319214926.1794108-1-mcgrof@kernel.org> MIME-Version: 1.0 Sender: Luis Chamberlain Precedence: bulk List-ID: This has no functional change, just moves a routine earlier as we'll make use of it next. Signed-off-by: Luis Chamberlain --- kernel/module/main.c | 42 +++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/kernel/module/main.c b/kernel/module/main.c index 929644d79d38..560d00e60744 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -2444,27 +2444,6 @@ static int post_relocation(struct module *mod, const struct load_info *info) return module_finalize(info->hdr, info->sechdrs, mod); } -/* Is this module of this name done loading? No locks held. */ -static bool finished_loading(const char *name) -{ - struct module *mod; - bool ret; - - /* - * The module_mutex should not be a heavily contended lock; - * if we get the occasional sleep here, we'll go an extra iteration - * in the wait_event_interruptible(), which is harmless. - */ - sched_annotate_sleep(); - mutex_lock(&module_mutex); - mod = find_module_all(name, strlen(name), true); - ret = !mod || mod->state == MODULE_STATE_LIVE - || mod->state == MODULE_STATE_GOING; - mutex_unlock(&module_mutex); - - return ret; -} - /* Call module constructors. */ static void do_mod_ctors(struct module *mod) { @@ -2628,6 +2607,27 @@ static int may_init_module(void) return 0; } +/* Is this module of this name done loading? No locks held. */ +static bool finished_loading(const char *name) +{ + struct module *mod; + bool ret; + + /* + * The module_mutex should not be a heavily contended lock; + * if we get the occasional sleep here, we'll go an extra iteration + * in the wait_event_interruptible(), which is harmless. + */ + sched_annotate_sleep(); + mutex_lock(&module_mutex); + mod = find_module_all(name, strlen(name), true); + ret = !mod || mod->state == MODULE_STATE_LIVE + || mod->state == MODULE_STATE_GOING; + mutex_unlock(&module_mutex); + + return ret; +} + /* * We try to place it in the list now to make sure it's unique before * we dedicate too many resources. In particular, temporary percpu From patchwork Sun Mar 19 21:49:23 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Luis Chamberlain X-Patchwork-Id: 13180609 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9A189C76195 for ; Sun, 19 Mar 2023 21:49:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229877AbjCSVtr (ORCPT ); Sun, 19 Mar 2023 17:49:47 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42148 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229862AbjCSVtb (ORCPT ); Sun, 19 Mar 2023 17:49:31 -0400 Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:3::133]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id DAE4712BE8; Sun, 19 Mar 2023 14:49:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=Sender:Content-Transfer-Encoding: MIME-Version:References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From: Reply-To:Content-Type:Content-ID:Content-Description; bh=/I7PkqiaJ7cvsv6QfoYBZBie1XYAgt8HAKL9Z+MGT0c=; b=XaW6cprtXgJZd5vp0qZa6RgAQ8 qXGqeTLagGV8UADCUbSDQtkGocZaShmHOpfJuUowrxqMYfPffLDm5h8IUU8ShyDzbHY1c9+00nDWf oUHl88nu2u1+Y+gvlw480489v7vVvr5GfKXUQ5FQeKLc+gh87iSvaAa0EsSKk9E2FsivrdU1lK9M7 wZXk0X5j/JbcXzXUo86TRmfIhRT3SbZm+iACvwEd5uQeLClpE/taaOmj0+N+dpzTKAwnCKBh47MgF PfoRm8Uds7+obrvS5oAjTP2nHb/LNASGqMIgH+xKrQ2IfgZRivo0V3yHS2zRRLhRS9lxdqMQeoMd3 uVxCa8IQ==; Received: from mcgrof by bombadil.infradead.org with local (Exim 4.96 #2 (Red Hat Linux)) id 1pe0u8-007WjS-0N; Sun, 19 Mar 2023 21:49:28 +0000 From: Luis Chamberlain To: linux-modules@vger.kernel.org, linux-kernel@vger.kernel.org, pmladek@suse.com, david@redhat.com, petr.pavlu@suse.com, prarit@redhat.com Cc: christophe.leroy@csgroup.eu, song@kernel.org, mcgrof@kernel.org Subject: [RFT 2/5] module: extract patient module check into helper Date: Sun, 19 Mar 2023 14:49:23 -0700 Message-Id: <20230319214926.1794108-3-mcgrof@kernel.org> X-Mailer: git-send-email 2.38.1 In-Reply-To: <20230319214926.1794108-1-mcgrof@kernel.org> References: <20230319214926.1794108-1-mcgrof@kernel.org> MIME-Version: 1.0 Sender: Luis Chamberlain Precedence: bulk List-ID: The patient module check inside add_unformed_module() is large enough as we need it. It is a bit hard to read too, so just move it to a helper and do the inverse checks first to help shift the code and make it easier to read. The new helper then is module_patient_check_exists(). Signed-off-by: Luis Chamberlain --- kernel/module/main.c | 71 +++++++++++++++++++++++++------------------- 1 file changed, 40 insertions(+), 31 deletions(-) diff --git a/kernel/module/main.c b/kernel/module/main.c index 560d00e60744..3644438ff96e 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -2628,6 +2628,43 @@ static bool finished_loading(const char *name) return ret; } +/* Must be called with module_mutex held */ +static int module_patient_check_exists(const char *name) +{ + struct module *old; + int err = 0; + + old = find_module_all(name, strlen(name), true); + if (old == NULL) + return 0; + + if (old->state == MODULE_STATE_COMING + || old->state == MODULE_STATE_UNFORMED) { + /* Wait in case it fails to load. */ + mutex_unlock(&module_mutex); + err = wait_event_interruptible(module_wq, + finished_loading(name)); + if (err) + return err; + + /* The module might have gone in the meantime. */ + mutex_lock(&module_mutex); + old = find_module_all(name, strlen(name), true); + } + + /* + * We are here only when the same module was being loaded. Do + * not try to load it again right now. It prevents long delays + * caused by serialized module load failures. It might happen + * when more devices of the same type trigger load of + * a particular module. + */ + if (old && old->state == MODULE_STATE_LIVE) + return -EEXIST; + else + return -EBUSY; +} + /* * We try to place it in the list now to make sure it's unique before * we dedicate too many resources. In particular, temporary percpu @@ -2636,41 +2673,14 @@ static bool finished_loading(const char *name) static int add_unformed_module(struct module *mod) { int err; - struct module *old; mod->state = MODULE_STATE_UNFORMED; mutex_lock(&module_mutex); - old = find_module_all(mod->name, strlen(mod->name), true); - if (old != NULL) { - if (old->state == MODULE_STATE_COMING - || old->state == MODULE_STATE_UNFORMED) { - /* Wait in case it fails to load. */ - mutex_unlock(&module_mutex); - err = wait_event_interruptible(module_wq, - finished_loading(mod->name)); - if (err) - goto out_unlocked; - - /* The module might have gone in the meantime. */ - mutex_lock(&module_mutex); - old = find_module_all(mod->name, strlen(mod->name), - true); - } - - /* - * We are here only when the same module was being loaded. Do - * not try to load it again right now. It prevents long delays - * caused by serialized module load failures. It might happen - * when more devices of the same type trigger load of - * a particular module. - */ - if (old && old->state == MODULE_STATE_LIVE) - err = -EEXIST; - else - err = -EBUSY; + err = module_patient_check_exists(mod->name); + if (err) goto out; - } + mod_update_bounds(mod); list_add_rcu(&mod->list, &modules); mod_tree_insert(mod); @@ -2678,7 +2688,6 @@ static int add_unformed_module(struct module *mod) out: mutex_unlock(&module_mutex); -out_unlocked: return err; } From patchwork Sun Mar 19 21:49:24 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Luis Chamberlain X-Patchwork-Id: 13180613 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4BFC8C77B61 for ; Sun, 19 Mar 2023 21:49:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229632AbjCSVtu (ORCPT ); Sun, 19 Mar 2023 17:49:50 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42144 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229806AbjCSVtb (ORCPT ); Sun, 19 Mar 2023 17:49:31 -0400 Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:3::133]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id BECFC12BCE; Sun, 19 Mar 2023 14:49:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=Sender:Content-Transfer-Encoding: MIME-Version:References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From: Reply-To:Content-Type:Content-ID:Content-Description; bh=a2k40cfDvwQf7JPgjDLK8Q4I5Dlky1AGZanNutR4j48=; b=SwGIxlRBNjoIa2X8GFxoM6VC9z k3/PeqV3amwDnKgIsM5Q2hWs8vZ9ZXAMSb8/hOZtiX9esSJz6xJumrJF97D4g9T50izMRI2iogk7A ftSFrd0cOlNehEJItvp18YMvKaM7azk7nQcWXt3s0CoTZbGYSTk5DcZeO00pbGil6cRYu1ZMZju/2 d2gxSLaKkJXh8zGP7PhifCOULU5mupYg/4T44Iu1guOw9QUukMk/YzA3/KqesH44QILx2wOtSq/RI 1sr8kdfqbNvLNTiKXaDc0KcSQGjF5P9dAeEkWstglna/dksEJ9n9WWFXb70eILfSqEPaT7dJoGyfm BDfih+CA==; Received: from mcgrof by bombadil.infradead.org with local (Exim 4.96 #2 (Red Hat Linux)) id 1pe0u8-007WjU-0V; Sun, 19 Mar 2023 21:49:28 +0000 From: Luis Chamberlain To: linux-modules@vger.kernel.org, linux-kernel@vger.kernel.org, pmladek@suse.com, david@redhat.com, petr.pavlu@suse.com, prarit@redhat.com Cc: christophe.leroy@csgroup.eu, song@kernel.org, mcgrof@kernel.org Subject: [RFT 3/5] module: avoid allocation if module is already present and ready Date: Sun, 19 Mar 2023 14:49:24 -0700 Message-Id: <20230319214926.1794108-4-mcgrof@kernel.org> X-Mailer: git-send-email 2.38.1 In-Reply-To: <20230319214926.1794108-1-mcgrof@kernel.org> References: <20230319214926.1794108-1-mcgrof@kernel.org> MIME-Version: 1.0 Sender: Luis Chamberlain Precedence: bulk List-ID: load_module() will allocate a struct module before even checking if the module is already loaded. This can create unecessary memory pressure since we can easily just check if the module is already present early with the copy of the module information from userspace after we've validated it a bit. This can only be an issue if a system is getting hammered with userspace loading modules. Note that there are two ways to load modules, one is auto-loading in-kernel and that pings back to userspace to just call modprobe, and we already have a way to restrict the count and load from there on the kernel usermode helper. However that does not stop a system from issuing tons of system calls to load a module. So userspace itself *is* supposed to check if a module is present before loading it. But we're observing situations where tons of the same module are in effect being loaded. Although some of these are acknolwedged as in-kernel bugs such as the ACPI frequency modules we can also help a bit more in the modules side to avoid those dramatic situations. All that is just memory being allocated to then be thrown away. To avoid memory pressure for such stupid cases put a stop gap for them. We now check for the module being present *before* allocation, and then right after we are going to add it to the system. On a 8vcpu 8 GiB RAM system using kdevops and testing against selftests kmod.sh -t 0008 I see a saving in the *highest* side of memory consumption of up to ~ 84 MiB. This can be obvserved and visualized below. The time it takes to run the test is also not affected. The gnuplot is set to a range from 400000 KiB (390 Mib) - 580000 (566 Mib) given the tests peak around that range. cat kmod-simple.plot set term dumb set output fileout set yrange [400000:580000] plot filein with linespoints title "Memory usage (KiB)" Before: root@kmod ~ # /data/linux-next/tools/testing/selftests/kmod/kmod.sh -t 0008 root@kmod ~ # free -k -s 1 -c 40 | grep Mem | awk '{print $3}' > log-0008-before.txt ^C root@kmod ~ # sort -n -r log-0008-before.txt | head -1 528732 So ~516.33 MiB After: root@kmod ~ # /data/linux-next/tools/testing/selftests/kmod/kmod.sh -t 0008 root@kmod ~ # free -k -s 1 -c 40 | grep Mem | awk '{print $3}' > log-0008-after.txt ^C root@kmod ~ # sort -n -r log-0008-after.txt | head -1 442516 So ~432.14 MiB That's about 84 ~MiB in savings in the worst case. The graphs: root@kmod ~ # gnuplot -e "filein='log-0008-before.txt'; fileout='graph-0008-before.txt'" kmod.plot root@kmod ~ # gnuplot -e "filein='log-0008-after.txt'; fileout='graph-0008-after.txt'" kmod.plot root@kmod ~ # cat graph-0008-before.txt 580000 +-----------------------------------------------------------------+ | + + + + + + + | 560000 |-+ Memory usage (KiB) ***A***-| | | 540000 |-+ +-| | | | *A *AA*AA*A*AA *A*AA A*A*A *AA*A*AA*A A | 520000 |-+A*A*AA *AA*A *A*AA*A*AA *A*A A *A+-| |*A | 500000 |-+ +-| | | 480000 |-+ +-| | | 460000 |-+ +-| | | | | 440000 |-+ +-| | | 420000 |-+ +-| | + + + + + + + | 400000 +-----------------------------------------------------------------+ 0 5 10 15 20 25 30 35 40 root@kmod ~ # cat graph-0008-after.txt 580000 +-----------------------------------------------------------------+ | + + + + + + + | 560000 |-+ Memory usage (KiB) ***A***-| | | 540000 |-+ +-| | | | | 520000 |-+ +-| | | 500000 |-+ +-| | | 480000 |-+ +-| | | 460000 |-+ +-| | | | *A *A*A | 440000 |-+A*A*AA*A A A*A*AA A*A*AA*A*AA*A*AA*A*AA*AA*A*AA*A*AA-| |*A *A*AA*A | 420000 |-+ +-| | + + + + + + + | 400000 +-----------------------------------------------------------------+ 0 5 10 15 20 25 30 35 40 Signed-off-by: Luis Chamberlain --- kernel/module/main.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/kernel/module/main.c b/kernel/module/main.c index 3644438ff96e..0ad26455def2 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -2787,7 +2787,11 @@ static int early_mod_check(struct load_info *info, int flags) if (err) return err; - return 0; + mutex_lock(&module_mutex); + err = module_patient_check_exists(info->mod->name); + mutex_unlock(&module_mutex); + + return err; } /* From patchwork Sun Mar 19 21:49:25 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Luis Chamberlain X-Patchwork-Id: 13180610 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id A73E2C77B60 for ; Sun, 19 Mar 2023 21:49:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230044AbjCSVtt (ORCPT ); Sun, 19 Mar 2023 17:49:49 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42140 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229755AbjCSVtb (ORCPT ); Sun, 19 Mar 2023 17:49:31 -0400 Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:3::133]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B2A6712862; Sun, 19 Mar 2023 14:49:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=Sender:Content-Transfer-Encoding: MIME-Version:References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From: Reply-To:Content-Type:Content-ID:Content-Description; bh=T1LQ0uGx/wzovMrrATWhqTlL4fpNlSzd8BTXmfhLms0=; b=wweFuraI7yI2EoSo2+nvarPd1c vD55fpRAYB9qdho9OwP4d5sjw8lhcYwAjpL3IyoIXYGb/6ZHCB+qsYw8mW6mfVkVnMVUN6kjos0SA RcfXHEFndNBBUa5ISv6tONWsvltQCeiTSEg7/nwggFYHdcb9Vk7qml5tIvP1Sx/R+aW6+r8aW+Wq+ 6fHjR9m7zwpoHqDpUdUqnlQZNUPQJti3itDxZhvXZDyE0Rnu509NHAsRoXGG2jp0aBSObN/MMq5Ia DUnKZ+yFhL6Ix6HfJshwy38DwjHbmYwqP/A1YsXmXviRPXSn4WdvAUK3nZFwpwqTWn4wb8Taw2Ksw n6WKXRpg==; Received: from mcgrof by bombadil.infradead.org with local (Exim 4.96 #2 (Red Hat Linux)) id 1pe0u8-007WjW-0b; Sun, 19 Mar 2023 21:49:28 +0000 From: Luis Chamberlain To: linux-modules@vger.kernel.org, linux-kernel@vger.kernel.org, pmladek@suse.com, david@redhat.com, petr.pavlu@suse.com, prarit@redhat.com Cc: christophe.leroy@csgroup.eu, song@kernel.org, mcgrof@kernel.org Subject: [RFT 4/5] module: use list_add_tail_rcu() when adding module Date: Sun, 19 Mar 2023 14:49:25 -0700 Message-Id: <20230319214926.1794108-5-mcgrof@kernel.org> X-Mailer: git-send-email 2.38.1 In-Reply-To: <20230319214926.1794108-1-mcgrof@kernel.org> References: <20230319214926.1794108-1-mcgrof@kernel.org> MIME-Version: 1.0 Sender: Luis Chamberlain Precedence: bulk List-ID: Put a new module at the end of the list intead of making new modules at the top of the list. find_module_all() start the hunt using the first entry on the list, if we assume that the modules which are first loaded are the most frequently looked for modules this should provide a tiny optimization. With a 8vcpu 8 GiB RAM kvm guest with kdevops with kdevops this saves about 4 MiB of RAM max use during the kmod tests 0008 which loops and runs loading a module in a loop through kthreads while not affecting the time to test. There could be some minor savings for systems that have repeated module requests for subsystems that are not yet optimized (ACPI frequency drivers come to mind) so in theory this could help there but that is just pure speculation. Signed-off-by: Luis Chamberlain --- kernel/module/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/module/main.c b/kernel/module/main.c index 0ad26455def2..32955b7819b3 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -2682,7 +2682,7 @@ static int add_unformed_module(struct module *mod) goto out; mod_update_bounds(mod); - list_add_rcu(&mod->list, &modules); + list_add_tail_rcu(&mod->list, &modules); mod_tree_insert(mod); err = 0; From patchwork Sun Mar 19 21:49:26 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Luis Chamberlain X-Patchwork-Id: 13180608 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C980CC6FD1F for ; Sun, 19 Mar 2023 21:49:47 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230017AbjCSVtq (ORCPT ); Sun, 19 Mar 2023 17:49:46 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42130 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229632AbjCSVta (ORCPT ); Sun, 19 Mar 2023 17:49:30 -0400 Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:3::133]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B56D51287D; Sun, 19 Mar 2023 14:49:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=Sender:Content-Transfer-Encoding: MIME-Version:References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From: Reply-To:Content-Type:Content-ID:Content-Description; bh=SpoMKXqIXTbAoX8SsquPbYaqmeShuOVFsw+coEwE9Sc=; b=o+tz2jWFMhZJyPTK8dgGqD2cfI riYUa2UHNQr0IFbnFP93Rui1gzk9MAB3K+zNkRz9V6qGUdpT5jXhPaMs4OflNi6Az0GkLAIeV/0Af xr4/3DThd6AJtnh8yUO7O0O5GLlCveswIUZO5cRSEaD2kZhibkMJmVr67qH7QyCoV98krEpwOnY3b N5KSbPG79zdLB6zhs22WA1R3zfN+LQHMCC3qtuUj863AKJs2cD4mpFzK9IMsi41WTpSrr6MYO8IGt gOaF/nVbxk/x7MnmJiphaB7IbbReFV6PTiyQs3qxvGDePJELqHeRk6wcw/UV9p7CDVL6ZzjY93knL gL9gmLOw==; Received: from mcgrof by bombadil.infradead.org with local (Exim 4.96 #2 (Red Hat Linux)) id 1pe0u8-007WjY-0i; Sun, 19 Mar 2023 21:49:28 +0000 From: Luis Chamberlain To: linux-modules@vger.kernel.org, linux-kernel@vger.kernel.org, pmladek@suse.com, david@redhat.com, petr.pavlu@suse.com, prarit@redhat.com Cc: christophe.leroy@csgroup.eu, song@kernel.org, mcgrof@kernel.org Subject: [RFT 5/5] module: add a sanity check prior to allowing kernel module auto-loading Date: Sun, 19 Mar 2023 14:49:26 -0700 Message-Id: <20230319214926.1794108-6-mcgrof@kernel.org> X-Mailer: git-send-email 2.38.1 In-Reply-To: <20230319214926.1794108-1-mcgrof@kernel.org> References: <20230319214926.1794108-1-mcgrof@kernel.org> MIME-Version: 1.0 Sender: Luis Chamberlain Precedence: bulk List-ID: request_module() is what we use for kernel-module autoloading. But this today is pretty stupid, it just requests for the module without even checking if its needed. And so we bounce to userspace and hope for the best. We can instead do a simple check to see if the module is already present. This will however contend the module_mutex, but it should never be heavily contended. However, module auto-loading is a special use case in the kernel and kernel/kmod already limits the amount of concurrent requests from the kernel to 50 so we know the kernel can only contend on the module_mutex for querying if a module exists 50 times at any single point in time. This work is only valuable if it proves useful for booting up large systems where a lot of current kernel heuristics use many kernel module auto-loading features and they could benefit from this. There are no metrics yet to show, but at least this doesn't penalize much existing systems. Signed-off-by: Luis Chamberlain --- kernel/module/internal.h | 1 + kernel/module/kmod.c | 7 +++++++ kernel/module/main.c | 18 ++++++++++++++++++ 3 files changed, 26 insertions(+) diff --git a/kernel/module/internal.h b/kernel/module/internal.h index 6ae29bb8836f..cb00555780bd 100644 --- a/kernel/module/internal.h +++ b/kernel/module/internal.h @@ -86,6 +86,7 @@ struct find_symbol_arg { enum mod_license license; }; +bool module_autoload_proceed(const char *name); int mod_verify_sig(const void *mod, struct load_info *info); int try_to_force_load(struct module *mod, const char *reason); bool find_symbol(struct find_symbol_arg *fsa); diff --git a/kernel/module/kmod.c b/kernel/module/kmod.c index b717134ebe17..67efc6de902f 100644 --- a/kernel/module/kmod.c +++ b/kernel/module/kmod.c @@ -28,6 +28,8 @@ #include +#include "internal.h" + /* * Assuming: * @@ -167,8 +169,13 @@ int __request_module(bool wait, const char *fmt, ...) trace_module_request(module_name, wait, _RET_IP_); + if (!module_autoload_proceed(module_name)) { + ret = 0; + goto out; + } ret = call_modprobe(module_name, wait ? UMH_WAIT_PROC : UMH_WAIT_EXEC); +out: atomic_inc(&kmod_concurrent_max); wake_up(&kmod_wq); diff --git a/kernel/module/main.c b/kernel/module/main.c index 32955b7819b3..2a84d747865a 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -2665,6 +2665,24 @@ static int module_patient_check_exists(const char *name) return -EBUSY; } +/* + * We allow contention of the module_mutex here because kernel module + * auto-loading a special feature *and* because we are only allowing + * MAX_KMOD_CONCURRENT possible checks at a time for a module. + */ +bool module_autoload_proceed(const char *name) +{ + int err; + + mutex_lock(&module_mutex); + err = module_patient_check_exists(name); + mutex_unlock(&module_mutex); + + if (err == -EEXIST) + return false; + return true; +} + /* * We try to place it in the list now to make sure it's unique before * we dedicate too many resources. In particular, temporary percpu