From: "Luis R. Rodriguez" <mcgrof@kernel.org>
To: jeyu@kernel.org, rusty@rustcorp.com.au
Cc: keescook@chromium.org, tixxdz@gmail.com, mbenes@suse.cz,
atomlin@redhat.com, pmladek@suse.com, hare@suse.com,
james.l.morris@oracle.com, ebiederm@xmission.com,
davem@davemloft.net, akpm@linux-foundation.org,
torvalds@linux-foundation.org, linux-kernel@vger.kernel.org,
"Luis R. Rodriguez" <mcgrof@kernel.org>
Subject: [PATCH 3/3] module: avoid allocation if module is already present and ready
Date: Thu, 7 Dec 2017 16:15:40 -0800 [thread overview]
Message-ID: <20171208001540.23696-4-mcgrof@kernel.org> (raw)
In-Reply-To: <20171208001540.23696-1-mcgrof@kernel.org>
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
next prev parent reply other threads:[~2017-12-08 0:16 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-08 0:15 [PATCH 0/3] module: minor allocation optimization Luis R. Rodriguez
2017-12-08 0:15 ` [PATCH 1/3] module: add an early early_mod_check() Luis R. Rodriguez
2017-12-19 1:00 ` Jessica Yu
2017-12-08 0:15 ` [PATCH 2/3] module: move finished_loading() Luis R. Rodriguez
2017-12-08 0:15 ` Luis R. Rodriguez [this message]
2017-12-19 1:26 ` module: avoid allocation if module is already present and ready Jessica Yu
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=20171208001540.23696-4-mcgrof@kernel.org \
--to=mcgrof@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=atomlin@redhat.com \
--cc=davem@davemloft.net \
--cc=ebiederm@xmission.com \
--cc=hare@suse.com \
--cc=james.l.morris@oracle.com \
--cc=jeyu@kernel.org \
--cc=keescook@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mbenes@suse.cz \
--cc=pmladek@suse.com \
--cc=rusty@rustcorp.com.au \
--cc=tixxdz@gmail.com \
--cc=torvalds@linux-foundation.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).