[PATCH 0/3] module: minor allocation optimization

[PATCH 0/3] module: minor allocation optimization

[Luis R. Rodriguez]

Long ago I was debugging kernel/module.c and kernel/kmod.c and had
implemented a sanity check to avoid allocating much of the needed struct
module from what user passes on finit_module() too early, ie, before
checking if the module was actually loaded.

I had disregarded this work as I had bundled it up with some aliasing
work I had done, so incorrectly thought this depended on the alias work.
Upon a second look it does not. The module name set on userspace on
info->name is not the alias but the proper module name, as such using
finished_loading() should suffice once to avoid allocating the module
if its already present.

I've tested this with kmod.sh and found no regressions, the few minor
enhancements are documented in patch 3 in detail. I'll let 0-day hammer
on it as we review the patches [0] [1] but no build issues so far.

[0] https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git/log/?h=20171207-avoid-bogus-layout_and_allocate
[1] https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux-next.git/log/?h=20171207-avoid-bogus-layout_and_allocate

Luis R. Rodriguez (3):
  module: add an early early_mod_check()
  module: move finished_loading()
  module: avoid allocation if module is already present and ready

 kernel/module.c | 81 ++++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 49 insertions(+), 32 deletions(-)

-- 
2.15.0

[PATCH 1/3] module: add an early early_mod_check()

[Luis R. Rodriguez]

We technically do a bit of sanity check with a local
struct module with pointers set to passed user data first.
Only after we do these checks do we embark on allocating
a struct module based on the passed information.

There's a small set of early checks which help us bail
out from processing and avoiding memory allocation, we
currently have these small checks tangled with the allocation
routine. Split this out so that we can expand on the early
checks with a local copy first.

This leaves setup_load_info() in an odd place given its
the one that originally sets up the local struct module
with pointing to the user data. Just move this out into the
laod_module() routine to make the early_mod_check() routine
much simpler.

This change just shuffles code around a bit to make the
next change easier to read, it technically has no functional
changes.

Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 kernel/module.c | 38 ++++++++++++++++++++++++++------------
 1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/kernel/module.c b/kernel/module.c
index 8042b8fcbf14..195ef37e242a 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3287,23 +3287,28 @@ static bool blacklisted(const char *module_name)
 }
 core_param(module_blacklist, module_blacklist, charp, 0400);
 
-static struct module *layout_and_allocate(struct load_info *info, int flags)
+/* Module within temporary copy, this doesn't do any allocation  */
+static int early_mod_check(struct load_info *info, int flags,
+			   struct module *mod)
 {
-	/* Module within temporary copy. */
-	struct module *mod;
-	unsigned int ndx;
 	int err;
 
-	mod = setup_load_info(info, flags);
-	if (IS_ERR(mod))
-		return mod;
-
 	if (blacklisted(info->name))
-		return ERR_PTR(-EPERM);
+		return -EPERM;
 
 	err = check_modinfo(mod, info, flags);
 	if (err)
-		return ERR_PTR(err);
+		return err;
+
+	return 0;
+}
+
+/* This starts to process the module and finally allocates a copy */
+static struct module *layout_and_allocate(struct load_info *info,
+					  struct module *mod)
+{
+	unsigned int ndx;
+	int err;
 
 	/* Allow arches to frob section contents and sizes.  */
 	err = module_frob_arch_sections(info->hdr, info->sechdrs,
@@ -3654,8 +3659,17 @@ static int load_module(struct load_info *info, const char __user *uargs,
 	if (err)
 		goto free_copy;
 
-	/* Figure out module layout, and allocate all the memory. */
-	mod = layout_and_allocate(info, flags);
+	mod = setup_load_info(info, flags);
+	if (IS_ERR(mod))
+		return -EINVAL;
+
+	/* layout and check */
+	err = early_mod_check(info, flags, mod);
+	if (err)
+		goto free_copy;
+
+	/* Layout and allocate all the memory. */
+	mod = layout_and_allocate(info, mod);
 	if (IS_ERR(mod)) {
 		err = PTR_ERR(mod);
 		goto free_copy;
-- 
2.15.0

Re: module: add an early early_mod_check()

[Jessica Yu]

+++ Luis R. Rodriguez [07/12/17 16:15 -0800]:
>We technically do a bit of sanity check with a local
>struct module with pointers set to passed user data first.
>Only after we do these checks do we embark on allocating
>a struct module based on the passed information.
>
>There's a small set of early checks which help us bail
>out from processing and avoiding memory allocation, we
>currently have these small checks tangled with the allocation
>routine. Split this out so that we can expand on the early
>checks with a local copy first.
>
>This leaves setup_load_info() in an odd place given its
>the one that originally sets up the local struct module
>with pointing to the user data. Just move this out into the
>laod_module() routine to make the early_mod_check() routine
>much simpler.
>
>This change just shuffles code around a bit to make the
>next change easier to read, it technically has no functional
>changes.
>
>Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
>---
> kernel/module.c | 38 ++++++++++++++++++++++++++------------
> 1 file changed, 26 insertions(+), 12 deletions(-)
>
>diff --git a/kernel/module.c b/kernel/module.c
>index 8042b8fcbf14..195ef37e242a 100644
>--- a/kernel/module.c
>+++ b/kernel/module.c
>@@ -3287,23 +3287,28 @@ static bool blacklisted(const char *module_name)
> }
> core_param(module_blacklist, module_blacklist, charp, 0400);
>
>-static struct module *layout_and_allocate(struct load_info *info, int flags)
>+/* Module within temporary copy, this doesn't do any allocation  */
>+static int early_mod_check(struct load_info *info, int flags,
>+			   struct module *mod)
> {
>-	/* Module within temporary copy. */
>-	struct module *mod;
>-	unsigned int ndx;
> 	int err;
>
>-	mod = setup_load_info(info, flags);
>-	if (IS_ERR(mod))
>-		return mod;
>-
> 	if (blacklisted(info->name))
>-		return ERR_PTR(-EPERM);
>+		return -EPERM;
>
> 	err = check_modinfo(mod, info, flags);
> 	if (err)
>-		return ERR_PTR(err);
>+		return err;
>+
>+	return 0;
>+}
>+
>+/* This starts to process the module and finally allocates a copy */
>+static struct module *layout_and_allocate(struct load_info *info,
>+					  struct module *mod)
>+{
>+	unsigned int ndx;
>+	int err;
>
> 	/* Allow arches to frob section contents and sizes.  */
> 	err = module_frob_arch_sections(info->hdr, info->sechdrs,
>@@ -3654,8 +3659,17 @@ static int load_module(struct load_info *info, const char __user *uargs,
> 	if (err)
> 		goto free_copy;
>
>-	/* Figure out module layout, and allocate all the memory. */
>-	mod = layout_and_allocate(info, flags);
>+	mod = setup_load_info(info, flags);
>+	if (IS_ERR(mod))
>+		return -EINVAL;

We need to goto free_copy here too, and propagate the error returned
by setup_load_info() i.e. propagate PTR_ERR(mod).

>+
>+	/* layout and check */
>+	err = early_mod_check(info, flags, mod);
>+	if (err)
>+		goto free_copy;
>+
>+	/* Layout and allocate all the memory. */
>+	mod = layout_and_allocate(info, mod);
> 	if (IS_ERR(mod)) {
> 		err = PTR_ERR(mod);
> 		goto free_copy;
>-- 
>2.15.0
>

[PATCH 2/3] module: move finished_loading()

[Luis R. Rodriguez]

This has no functional change, just moves a routine earlier
as we'll make use of it next.

Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 kernel/module.c | 42 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/kernel/module.c b/kernel/module.c
index 195ef37e242a..fb2afcde5d20 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3287,6 +3287,27 @@ static bool blacklisted(const char *module_name)
 }
 core_param(module_blacklist, module_blacklist, charp, 0400);
 
+/* 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;
+}
+
 /* Module within temporary copy, this doesn't do any allocation  */
 static int early_mod_check(struct load_info *info, int flags,
 			   struct module *mod)
@@ -3377,27 +3398,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)
 {
-- 
2.15.0

[PATCH 3/3] module: avoid allocation if module is already present and ready

[Luis R. Rodriguez]

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 multiple
request_module() calls which are issued blindly without first checking
if the module is already present. Fortunately we have test cases on
selftests for kmod to prove this now.

This is more evident on kmod selftest test case 0008 than 0009, given
get_fs_type() has its own checker for if the module is present before
poking userspace for the module. The two kmod selftests evaluated
below are:

0008 - multithreaded - push kmod_concurrent over max_modprobes for request_module()
0009 - multithreaded - push kmod_concurrent over max_modprobes for get_fs_type()

In terms of run time nothing changes much:

Before:
root@piggy:~# time ./kmod.sh -t 0008
real    0m29.377s
user    0m1.291s
sys     0m17.775s

root@piggy:~# time ./kmod.sh -t 0009
real    1m4.234s
user    0m1.172s
sys     0m15.378s

After:
root@piggy:~# time ./kmod.sh -t 0008
real    0m29.191s
user    0m1.372s
sys     0m17.428s

root@piggy:~# time ./kmod.sh -t 0009
real    1m3.972s
user    0m1.137s
sys     0m15.870s

But if we generate two memory-use plots during each test before and
after this commit we can see test case 0008 ends up consuming much
less memory as the test runs now, saving about 14 MiB in the worst
case.

For test case 0008 since both test run for about 35 seconds we can
collect memory-use in KiB for 35 seconds as the test runs, before
and then after this commit:

 # For test case 0008, run for 35 seconds, before this commit:
free -k -s 1 -c 35 | grep Mem | awk '{print $3}' > log-0008-before.txt
 # For test case 0008, run for 35 seconds, after this commit:
free -k -s 1 -c 35 | grep Mem | awk '{print $3}' > log-0008-after.txt

Likewise for test 0009 since both tests run for about 75 seconds we can
collect memory-use in KiB for 75 seconds as the test runs, before
and then after this commit:

 # For test case 0009, run for 75 seconds, before this commit:
free -k -s 1 -c 75 | grep Mem | awk '{print $3}' > log-0009-before.txt
 # For test case 0008, run for 75 seconds, after this commit:
free -k -s 1 -c 75 | grep Mem | awk '{print $3}' > log-0009-after.txt

Assuming we have a kmod.plot for gnuplut as follows:

cat kmod.gnuplot
set term dumb
set output fileout
plot filein with linespoints title "Memory usage (KiB)"

We can now plot each graph:

gnuplot -e filein='log-0008-before.txt' -e fileout='graph-0008-before.txt' kmod.gnuplot
gnuplot -e filein='log-0009-before.txt' -e fileout='graph-0009-before.txt' kmod.gnuplot
gnuplot -e filein='log-0008-after.txt' -e fileout='graph-0008-after.txt' kmod.gnuplot
gnuplot -e filein='log-0009-after.txt' -e fileout='graph-0009-after.txt' kmod.gnuplot

kmod selftest 0008 before:

  124000 +-----------------------------------------------------------------+
         |        +         + *      +         +        +         +        |
         |                   * *                Memory usage (KiB) ***A*** |
  123000 |-+                 * *             A                           +-|
         |                  *  *            * *       A*A                  |
         |                  A  *   A      *A  *      *   *   *A*A* A       |
         |       A          *   *  ** *A*A     **A *A     A*A     A *      |
  122000 |-+    * A*       *    A**  A         A  A                 *    +-|
         |    *A    A*A*A* *      A                                  A*A*A |
         |   A            A                                                |
  121000 |-+ *                                                           +-|
         |   *                                                             |
         |  *                                                              |
  120000 |-+*                                                            +-|
         |  *                                                              |
         |  *                                                              |
         | *                                                               |
  119000 |*A                                                             +-|
         |                                                                 |
         |        +         +        +         +        +         +        |
  118000 +-----------------------------------------------------------------+
         0        5         10       15        20       25        30       35

kmod selftest 0008 after:

  110000 +-----------------------------------------------------------------+
         |        A         +        +         +        +         +        |
  108000 |-+      *                             Memory usage (KiB) ***A***-|
         |        *                                                        |
         |        **                                                       |
  106000 |-+      **                                                     +-|
         |       * *                                                       |
  104000 |-+     * *                                                     +-|
         |       * *           *A        A*        *A*   *A*A*A*A*A        |
         |       *  *        *A  *  *A* *  A*A*A*AA   A*A          A*A*A*A |
  102000 |-+   A*A  **A*A*A*A     AA   A                                 +-|
         |    *     A                                                      |
  100000 |-+  *                                                          +-|
         |   A                                                             |
         |   *                                                             |
   98000 |-+*                                                            +-|
         |  *                                                              |
   96000 |-+*                                                            +-|
         | *                                                               |
         |*A      +         +        +         +        +         +        |
   94000 +-----------------------------------------------------------------+
         0        5         10       15        20       25        30       35

The max memory-use before and after:

$ sort -n -r log-0008-before.txt | head -1
123904
$ sort -n -r log-0008-after.txt | head -1
108964

Although this is just one test case the saving of about 14940 KiB
(~14 MiB) is considerable enough to consider a two line change.

For test case 0009 we don't see much drastic changes:

kmod selftest 0009 before:

  2.2e+06 +----------------------------------------------------------------+
          |       +       +       +     A  +       +       +       +       |
    2e+06 |-+                           *       Memory Asage (KiA) ***A***-|
          |                A   A        *              *        *          |
  1.8e+06 |-+              *   *        *              *        *        +-|
  1.6e+06 |-+              **  *        *              *        *        +-|
          |                *A  *        *              *        *          |
  1.4e+06 |-+           A  ** **        **         A   **       A        +-|
          |             *  ** * *       **         *   *A       *          |
  1.2e+06 |-+      A    *  ** * *      * *       A *   **       *        +-|
          | AA     *    ** ** A *     A* *       * *  * *      **          |
    1e+06 |-**     * A  ** ** * *     ** *       * ** * *      * *       +-|
          | **     * *  *A* * * A     ** A   A  ** ** * A      * *         |
   800000 |-**     * *  ***  ** *     ** * A *  ** ** * *      * *       +-|
          | **    * *A * **  **  *    ** * * * AA *** *  *  A  * *         |
   600000 |-* * A * **** **  **  *    *A * * * *  **A *  *A *  * A       +-|
   400000 |-* * *A* **** **  **  A    *   *** **  *** A  ** ** A *       +-|
          | * * **A * ** **  **   A   *   *** **  *A **  ***** *  *        |
   200000 |-* ** ** * ** A*  **   *   *   *** **  ** *   * **A*   *A     +-|
          |AA AA A+ A AA  A  AA   AAAAA   AAA AA  A+ A   A AA A   A+AAA    |
        0 +----------------------------------------------------------------+
          0       10      20      30       40      50      60      70      80

kmod selftest 0009 after:

  2.2e+06 +----------------------------------------------------------------+
          |       +       +       +        +       +     A +       +       |
    2e+06 |-+                                   Memory us*ge (KiB) ***A***-|
          |                                              *                 |
  1.8e+06 |-+                                A           *               +-|
  1.6e+06 |-+                                *           *               +-|
          |        A                         *           *                 |
  1.4e+06 |-+      *                         *           **              +-|
          |        *                         *           **                |
  1.2e+06 |-+    A *                         *           **              +-|
          |      * *                      A  *       A   **                |
    1e+06 |-+    * *                      *  **      *  * *              +-|
          |      *** A        A          ** * *      *  * A                |
   800000 |-+AA  ** **  A     **  A    AAA ** *      ** * * A            +-|
          | A* * ** **  *     *A  * A  *   ** *      ** * * *              |
   600000 |-** A*** *****     **  * *  *   ** *     * A * * *            +-|
   400000 |-** ***A ***A * A A**  *** A*   A* *   A * * *  ***           +-|
          |**   *** **** * * ** * A* **    ** A   * *  **  * *             |
   200000 |**   **  **A  ** * * * ** **    *A  * A A*  **  * *           +-|
          |AA   AA+ AA   AA A A AA+A AA    A   AA  AA  AA  A AAAAAAAAAA    |
        0 +----------------------------------------------------------------+
          0       10      20      30       40      50      60      70      80

In terms of max memory use we have:

$ sort -n -r log-0009-before.txt | head -1
2083344
$ sort -n -r log-0009-after.txt | head -1
2076236

So there is not much difference for this commit when we hammer on
get_fs_type().

Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 kernel/module.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel/module.c b/kernel/module.c
index fb2afcde5d20..6a0e22293502 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3321,6 +3321,9 @@ static int early_mod_check(struct load_info *info, int flags,
 	if (err)
 		return err;
 
+	if (finished_loading(info->name))
+		return 0;
+
 	return 0;
 }
 
-- 
2.15.0

Re: module: avoid allocation if module is already present and ready

[Jessica Yu]

+++ Luis R. Rodriguez [07/12/17 16:15 -0800]:
>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 multiple
>request_module() calls which are issued blindly without first checking
>if the module is already present. Fortunately we have test cases on
>selftests for kmod to prove this now.
>
>This is more evident on kmod selftest test case 0008 than 0009, given
>get_fs_type() has its own checker for if the module is present before
>poking userspace for the module. The two kmod selftests evaluated
>below are:
>
>0008 - multithreaded - push kmod_concurrent over max_modprobes for request_module()
>0009 - multithreaded - push kmod_concurrent over max_modprobes for get_fs_type()
>
>In terms of run time nothing changes much:
>
>Before:
>root@piggy:~# time ./kmod.sh -t 0008
>real    0m29.377s
>user    0m1.291s
>sys     0m17.775s
>
>root@piggy:~# time ./kmod.sh -t 0009
>real    1m4.234s
>user    0m1.172s
>sys     0m15.378s
>
>After:
>root@piggy:~# time ./kmod.sh -t 0008
>real    0m29.191s
>user    0m1.372s
>sys     0m17.428s
>
>root@piggy:~# time ./kmod.sh -t 0009
>real    1m3.972s
>user    0m1.137s
>sys     0m15.870s
>
>But if we generate two memory-use plots during each test before and
>after this commit we can see test case 0008 ends up consuming much
>less memory as the test runs now, saving about 14 MiB in the worst
>case.
>
>For test case 0008 since both test run for about 35 seconds we can
>collect memory-use in KiB for 35 seconds as the test runs, before
>and then after this commit:
>
> # For test case 0008, run for 35 seconds, before this commit:
>free -k -s 1 -c 35 | grep Mem | awk '{print $3}' > log-0008-before.txt
> # For test case 0008, run for 35 seconds, after this commit:
>free -k -s 1 -c 35 | grep Mem | awk '{print $3}' > log-0008-after.txt
>
>Likewise for test 0009 since both tests run for about 75 seconds we can
>collect memory-use in KiB for 75 seconds as the test runs, before
>and then after this commit:
>
> # For test case 0009, run for 75 seconds, before this commit:
>free -k -s 1 -c 75 | grep Mem | awk '{print $3}' > log-0009-before.txt
> # For test case 0008, run for 75 seconds, after this commit:
>free -k -s 1 -c 75 | grep Mem | awk '{print $3}' > log-0009-after.txt
>
>Assuming we have a kmod.plot for gnuplut as follows:
>
>cat kmod.gnuplot
>set term dumb
>set output fileout
>plot filein with linespoints title "Memory usage (KiB)"
>
>We can now plot each graph:
>
>gnuplot -e filein='log-0008-before.txt' -e fileout='graph-0008-before.txt' kmod.gnuplot
>gnuplot -e filein='log-0009-before.txt' -e fileout='graph-0009-before.txt' kmod.gnuplot
>gnuplot -e filein='log-0008-after.txt' -e fileout='graph-0008-after.txt' kmod.gnuplot
>gnuplot -e filein='log-0009-after.txt' -e fileout='graph-0009-after.txt' kmod.gnuplot
>
>kmod selftest 0008 before:
>
>  124000 +-----------------------------------------------------------------+
>         |        +         + *      +         +        +         +        |
>         |                   * *                Memory usage (KiB) ***A*** |
>  123000 |-+                 * *             A                           +-|
>         |                  *  *            * *       A*A                  |
>         |                  A  *   A      *A  *      *   *   *A*A* A       |
>         |       A          *   *  ** *A*A     **A *A     A*A     A *      |
>  122000 |-+    * A*       *    A**  A         A  A                 *    +-|
>         |    *A    A*A*A* *      A                                  A*A*A |
>         |   A            A                                                |
>  121000 |-+ *                                                           +-|
>         |   *                                                             |
>         |  *                                                              |
>  120000 |-+*                                                            +-|
>         |  *                                                              |
>         |  *                                                              |
>         | *                                                               |
>  119000 |*A                                                             +-|
>         |                                                                 |
>         |        +         +        +         +        +         +        |
>  118000 +-----------------------------------------------------------------+
>         0        5         10       15        20       25        30       35
>
>kmod selftest 0008 after:
>
>  110000 +-----------------------------------------------------------------+
>         |        A         +        +         +        +         +        |
>  108000 |-+      *                             Memory usage (KiB) ***A***-|
>         |        *                                                        |
>         |        **                                                       |
>  106000 |-+      **                                                     +-|
>         |       * *                                                       |
>  104000 |-+     * *                                                     +-|
>         |       * *           *A        A*        *A*   *A*A*A*A*A        |
>         |       *  *        *A  *  *A* *  A*A*A*AA   A*A          A*A*A*A |
>  102000 |-+   A*A  **A*A*A*A     AA   A                                 +-|
>         |    *     A                                                      |
>  100000 |-+  *                                                          +-|
>         |   A                                                             |
>         |   *                                                             |
>   98000 |-+*                                                            +-|
>         |  *                                                              |
>   96000 |-+*                                                            +-|
>         | *                                                               |
>         |*A      +         +        +         +        +         +        |
>   94000 +-----------------------------------------------------------------+
>         0        5         10       15        20       25        30       35
>
>The max memory-use before and after:
>
>$ sort -n -r log-0008-before.txt | head -1
>123904
>$ sort -n -r log-0008-after.txt | head -1
>108964
>
>Although this is just one test case the saving of about 14940 KiB
>(~14 MiB) is considerable enough to consider a two line change.
>
>For test case 0009 we don't see much drastic changes:
>
>kmod selftest 0009 before:
>
>  2.2e+06 +----------------------------------------------------------------+
>          |       +       +       +     A  +       +       +       +       |
>    2e+06 |-+                           *       Memory Asage (KiA) ***A***-|
>          |                A   A        *              *        *          |
>  1.8e+06 |-+              *   *        *              *        *        +-|
>  1.6e+06 |-+              **  *        *              *        *        +-|
>          |                *A  *        *              *        *          |
>  1.4e+06 |-+           A  ** **        **         A   **       A        +-|
>          |             *  ** * *       **         *   *A       *          |
>  1.2e+06 |-+      A    *  ** * *      * *       A *   **       *        +-|
>          | AA     *    ** ** A *     A* *       * *  * *      **          |
>    1e+06 |-**     * A  ** ** * *     ** *       * ** * *      * *       +-|
>          | **     * *  *A* * * A     ** A   A  ** ** * A      * *         |
>   800000 |-**     * *  ***  ** *     ** * A *  ** ** * *      * *       +-|
>          | **    * *A * **  **  *    ** * * * AA *** *  *  A  * *         |
>   600000 |-* * A * **** **  **  *    *A * * * *  **A *  *A *  * A       +-|
>   400000 |-* * *A* **** **  **  A    *   *** **  *** A  ** ** A *       +-|
>          | * * **A * ** **  **   A   *   *** **  *A **  ***** *  *        |
>   200000 |-* ** ** * ** A*  **   *   *   *** **  ** *   * **A*   *A     +-|
>          |AA AA A+ A AA  A  AA   AAAAA   AAA AA  A+ A   A AA A   A+AAA    |
>        0 +----------------------------------------------------------------+
>          0       10      20      30       40      50      60      70      80
>
>kmod selftest 0009 after:
>
>  2.2e+06 +----------------------------------------------------------------+
>          |       +       +       +        +       +     A +       +       |
>    2e+06 |-+                                   Memory us*ge (KiB) ***A***-|
>          |                                              *                 |
>  1.8e+06 |-+                                A           *               +-|
>  1.6e+06 |-+                                *           *               +-|
>          |        A                         *           *                 |
>  1.4e+06 |-+      *                         *           **              +-|
>          |        *                         *           **                |
>  1.2e+06 |-+    A *                         *           **              +-|
>          |      * *                      A  *       A   **                |
>    1e+06 |-+    * *                      *  **      *  * *              +-|
>          |      *** A        A          ** * *      *  * A                |
>   800000 |-+AA  ** **  A     **  A    AAA ** *      ** * * A            +-|
>          | A* * ** **  *     *A  * A  *   ** *      ** * * *              |
>   600000 |-** A*** *****     **  * *  *   ** *     * A * * *            +-|
>   400000 |-** ***A ***A * A A**  *** A*   A* *   A * * *  ***           +-|
>          |**   *** **** * * ** * A* **    ** A   * *  **  * *             |
>   200000 |**   **  **A  ** * * * ** **    *A  * A A*  **  * *           +-|
>          |AA   AA+ AA   AA A A AA+A AA    A   AA  AA  AA  A AAAAAAAAAA    |
>        0 +----------------------------------------------------------------+
>          0       10      20      30       40      50      60      70      80
>
>In terms of max memory use we have:
>
>$ sort -n -r log-0009-before.txt | head -1
>2083344
>$ sort -n -r log-0009-after.txt | head -1
>2076236
>
>So there is not much difference for this commit when we hammer on
>get_fs_type().
>
>Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
>---
> kernel/module.c | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/kernel/module.c b/kernel/module.c
>index fb2afcde5d20..6a0e22293502 100644
>--- a/kernel/module.c
>+++ b/kernel/module.c
>@@ -3321,6 +3321,9 @@ static int early_mod_check(struct load_info *info, int flags,
> 	if (err)
> 		return err;
>
>+	if (finished_loading(info->name))
>+		return 0;

Did you mean to return -EEXIST here? Otherwise this check doesn't do
anything, i.e. if the module is live then early_mod_check() returns 0
and proceeds to layout_and_allocate() all the same.

Also, finished_loading() wouldn't work here in its current form,
because it also returns true if the module doesn't exist, i.e.
find_module_all() returns NULL. This is because we want to wake up in
add_unformed_module() if the same module failed to load and then went
away, then we can try to add it to the modules list again. Perhaps we
can split out the !mod check in the wait condition in add_unformed_module()
so that we can use finished_loading() here as intended.

Thanks,

Jessica