diff options
author | Peter Kjellerstedt <peter.kjellerstedt@axis.com> | 2018-05-31 09:42:28 +0200 |
---|---|---|
committer | Richard Purdie <richard.purdie@linuxfoundation.org> | 2018-06-04 15:15:00 +0100 |
commit | b7f6638962b0348ae93c1d5a7696c80e2b7933ed (patch) | |
tree | f4acae06d82634aea24d378f2ec3a16d7abb95cc /meta/recipes-devtools/rpm | |
parent | 9a773747c2af183554b35d45a3355418815bde98 (diff) | |
download | poky-b7f6638962b0348ae93c1d5a7696c80e2b7933ed.tar.gz |
rpm: Restore performance in Docker containers
If the maximum number of open file descriptors is much greater than the
usual 1024 (for example inside a Docker container), the performance
drops significantly.
This was reported upstream in:
https://bugzilla.redhat.com/show_bug.cgi?id=1537564
which resulted in:
https://github.com/rpm-software-management/rpm/pull/444
The pull request above has now been integrated and this commit contains
a backport of its three patches, which together change the behavior of
rpm so that its performance is now independent of the maximum number of
open file descriptors.
(From OE-Core rev: 7feed9ccfc4e656c6264f07e13d7e9ef69bdfb06)
Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
Diffstat (limited to 'meta/recipes-devtools/rpm')
4 files changed, 304 insertions, 0 deletions
diff --git a/meta/recipes-devtools/rpm/files/0001-Factor-out-and-unify-setting-CLOEXEC.patch b/meta/recipes-devtools/rpm/files/0001-Factor-out-and-unify-setting-CLOEXEC.patch new file mode 100644 index 0000000000..6f440c6178 --- /dev/null +++ b/meta/recipes-devtools/rpm/files/0001-Factor-out-and-unify-setting-CLOEXEC.patch | |||
@@ -0,0 +1,148 @@ | |||
1 | From 9c3e5de3240554c8ea1b29d52eeadee4840fefac Mon Sep 17 00:00:00 2001 | ||
2 | From: Kir Kolyshkin <kolyshkin@gmail.com> | ||
3 | Date: Tue, 29 May 2018 17:37:05 -0700 | ||
4 | Subject: [PATCH 1/3] Factor out and unify setting CLOEXEC | ||
5 | |||
6 | Commit 7a7c31f5 ("Set FD_CLOEXEC on opened files before exec from | ||
7 | lua script is called") copied the code that sets CLOEXEC flag on all | ||
8 | possible file descriptors from lib/rpmscript.c to luaext/lposix.c, | ||
9 | essentially creating two copies of the same code (modulo comments | ||
10 | and the unused assignment). | ||
11 | |||
12 | This commit moves the functionality into its own function, without | ||
13 | any code modifications, using the version from luaext/lposix.c. | ||
14 | |||
15 | Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> | ||
16 | Upstream-Status: Accepted [https://github.com/rpm-software-management/rpm/pull/444] | ||
17 | Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com> | ||
18 | --- | ||
19 | lib/rpmscript.c | 18 ++---------------- | ||
20 | luaext/lposix.c | 13 ++----------- | ||
21 | rpmio/rpmio.c | 16 ++++++++++++++++ | ||
22 | rpmio/rpmio_internal.h | 6 ++++++ | ||
23 | 4 files changed, 26 insertions(+), 27 deletions(-) | ||
24 | |||
25 | diff --git a/lib/rpmscript.c b/lib/rpmscript.c | ||
26 | index 747385a5b..b4ccd3246 100644 | ||
27 | --- a/lib/rpmscript.c | ||
28 | +++ b/lib/rpmscript.c | ||
29 | @@ -3,7 +3,6 @@ | ||
30 | #include <sys/types.h> | ||
31 | #include <sys/wait.h> | ||
32 | #include <errno.h> | ||
33 | -#include <unistd.h> | ||
34 | |||
35 | #include <rpm/rpmfileutil.h> | ||
36 | #include <rpm/rpmmacro.h> | ||
37 | @@ -14,6 +13,7 @@ | ||
38 | |||
39 | #include "rpmio/rpmlua.h" | ||
40 | #include "lib/rpmscript.h" | ||
41 | +#include "rpmio/rpmio_internal.h" | ||
42 | |||
43 | #include "lib/rpmplugins.h" /* rpm plugins hooks */ | ||
44 | |||
45 | @@ -170,26 +170,12 @@ static const char * const SCRIPT_PATH = "PATH=/sbin:/bin:/usr/sbin:/usr/bin:/usr | ||
46 | static void doScriptExec(ARGV_const_t argv, ARGV_const_t prefixes, | ||
47 | FD_t scriptFd, FD_t out) | ||
48 | { | ||
49 | - int flag; | ||
50 | - int fdno; | ||
51 | int xx; | ||
52 | - int open_max; | ||
53 | |||
54 | /* SIGPIPE is ignored in rpm, reset to default for the scriptlet */ | ||
55 | (void) signal(SIGPIPE, SIG_DFL); | ||
56 | |||
57 | - /* XXX Force FD_CLOEXEC on all inherited fdno's. */ | ||
58 | - open_max = sysconf(_SC_OPEN_MAX); | ||
59 | - if (open_max == -1) { | ||
60 | - open_max = 1024; | ||
61 | - } | ||
62 | - for (fdno = 3; fdno < open_max; fdno++) { | ||
63 | - flag = fcntl(fdno, F_GETFD); | ||
64 | - if (flag == -1 || (flag & FD_CLOEXEC)) | ||
65 | - continue; | ||
66 | - xx = fcntl(fdno, F_SETFD, FD_CLOEXEC); | ||
67 | - /* XXX W2DO? debug msg for inheirited fdno w/o FD_CLOEXEC */ | ||
68 | - } | ||
69 | + rpmSetCloseOnExec(); | ||
70 | |||
71 | if (scriptFd != NULL) { | ||
72 | int sfdno = Fileno(scriptFd); | ||
73 | diff --git a/luaext/lposix.c b/luaext/lposix.c | ||
74 | index 0a7c26c71..5d7ad3c87 100644 | ||
75 | --- a/luaext/lposix.c | ||
76 | +++ b/luaext/lposix.c | ||
77 | @@ -27,6 +27,7 @@ | ||
78 | #include <unistd.h> | ||
79 | #include <utime.h> | ||
80 | #include <rpm/rpmutil.h> | ||
81 | +#include "rpmio/rpmio_internal.h" | ||
82 | |||
83 | #define MYNAME "posix" | ||
84 | #define MYVERSION MYNAME " library for " LUA_VERSION " / Nov 2003" | ||
85 | @@ -335,21 +336,11 @@ static int Pexec(lua_State *L) /** exec(path,[args]) */ | ||
86 | const char *path = luaL_checkstring(L, 1); | ||
87 | int i,n=lua_gettop(L); | ||
88 | char **argv; | ||
89 | - int flag, fdno, open_max; | ||
90 | |||
91 | if (!have_forked) | ||
92 | return luaL_error(L, "exec not permitted in this context"); | ||
93 | |||
94 | - open_max = sysconf(_SC_OPEN_MAX); | ||
95 | - if (open_max == -1) { | ||
96 | - open_max = 1024; | ||
97 | - } | ||
98 | - for (fdno = 3; fdno < open_max; fdno++) { | ||
99 | - flag = fcntl(fdno, F_GETFD); | ||
100 | - if (flag == -1 || (flag & FD_CLOEXEC)) | ||
101 | - continue; | ||
102 | - fcntl(fdno, F_SETFD, FD_CLOEXEC); | ||
103 | - } | ||
104 | + rpmSetCloseOnExec(); | ||
105 | |||
106 | argv = malloc((n+1)*sizeof(char*)); | ||
107 | if (argv==NULL) return luaL_error(L,"not enough memory"); | ||
108 | diff --git a/rpmio/rpmio.c b/rpmio/rpmio.c | ||
109 | index c7cbc32aa..ea111d2ec 100644 | ||
110 | --- a/rpmio/rpmio.c | ||
111 | +++ b/rpmio/rpmio.c | ||
112 | @@ -1759,3 +1759,19 @@ DIGEST_CTX fdDupDigest(FD_t fd, int id) | ||
113 | |||
114 | return ctx; | ||
115 | } | ||
116 | + | ||
117 | +void rpmSetCloseOnExec(void) | ||
118 | +{ | ||
119 | + int flag, fdno, open_max; | ||
120 | + | ||
121 | + open_max = sysconf(_SC_OPEN_MAX); | ||
122 | + if (open_max == -1) { | ||
123 | + open_max = 1024; | ||
124 | + } | ||
125 | + for (fdno = 3; fdno < open_max; fdno++) { | ||
126 | + flag = fcntl(fdno, F_GETFD); | ||
127 | + if (flag == -1 || (flag & FD_CLOEXEC)) | ||
128 | + continue; | ||
129 | + fcntl(fdno, F_SETFD, FD_CLOEXEC); | ||
130 | + } | ||
131 | +} | ||
132 | diff --git a/rpmio/rpmio_internal.h b/rpmio/rpmio_internal.h | ||
133 | index fbed183b0..370cbdc75 100644 | ||
134 | --- a/rpmio/rpmio_internal.h | ||
135 | +++ b/rpmio/rpmio_internal.h | ||
136 | @@ -41,6 +41,12 @@ DIGEST_CTX fdDupDigest(FD_t fd, int id); | ||
137 | int rpmioSlurp(const char * fn, | ||
138 | uint8_t ** bp, ssize_t * blenp); | ||
139 | |||
140 | +/** | ||
141 | + * Set close-on-exec flag for all opened file descriptors, except | ||
142 | + * stdin/stdout/stderr. | ||
143 | + */ | ||
144 | +void rpmSetCloseOnExec(void); | ||
145 | + | ||
146 | #ifdef __cplusplus | ||
147 | } | ||
148 | #endif | ||
diff --git a/meta/recipes-devtools/rpm/files/0002-Optimize-rpmSetCloseOnExec.patch b/meta/recipes-devtools/rpm/files/0002-Optimize-rpmSetCloseOnExec.patch new file mode 100644 index 0000000000..a27f8e6237 --- /dev/null +++ b/meta/recipes-devtools/rpm/files/0002-Optimize-rpmSetCloseOnExec.patch | |||
@@ -0,0 +1,100 @@ | |||
1 | From 5e6f05cd8dad6c1ee6bd1e6e43f176976c9c3416 Mon Sep 17 00:00:00 2001 | ||
2 | From: Kir Kolyshkin <kolyshkin@gmail.com> | ||
3 | Date: Tue, 29 May 2018 17:52:56 -0700 | ||
4 | Subject: [PATCH 2/3] Optimize rpmSetCloseOnExec | ||
5 | |||
6 | In case maximum number of open files limit is set too high, both | ||
7 | luaext/Pexec() and lib/doScriptExec() spend way too much time | ||
8 | trying to set FD_CLOEXEC flag for all those file descriptors, | ||
9 | resulting in severe increase of time it takes to execute say | ||
10 | rpm or dnf. | ||
11 | |||
12 | This becomes increasingly noticeable when running with e.g. under | ||
13 | Docker, the reason being: | ||
14 | |||
15 | > $ docker run fedora ulimit -n | ||
16 | > 1048576 | ||
17 | |||
18 | One obvious fix is to use procfs to get the actual list of opened fds | ||
19 | and iterate over it. My quick-n-dirty benchmark shows the /proc approach | ||
20 | is about 10x faster than iterating through a list of just 1024 fds, | ||
21 | so it's an improvement even for default ulimit values. | ||
22 | |||
23 | Note that the old method is still used in case /proc is not available. | ||
24 | |||
25 | While at it, | ||
26 | |||
27 | 1. fix the function by making sure we modify (rather than set) | ||
28 | the existing flags. As the only known flag is FD_CLOEXEC, | ||
29 | this change is currently purely aesthetical, but in case | ||
30 | other flags will appear it will become a real bug fix. | ||
31 | |||
32 | 2. get rid of magic number 3; use STDERR_FILENO | ||
33 | |||
34 | Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> | ||
35 | |||
36 | Fixes #444 | ||
37 | |||
38 | Upstream-Status: Accepted [https://github.com/rpm-software-management/rpm/pull/444] | ||
39 | Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com> | ||
40 | --- | ||
41 | rpmio/rpmio.c | 43 ++++++++++++++++++++++++++++++++++--------- | ||
42 | 1 file changed, 34 insertions(+), 9 deletions(-) | ||
43 | |||
44 | diff --git a/rpmio/rpmio.c b/rpmio/rpmio.c | ||
45 | index ea111d2ec..55351c221 100644 | ||
46 | --- a/rpmio/rpmio.c | ||
47 | +++ b/rpmio/rpmio.c | ||
48 | @@ -1760,18 +1760,43 @@ DIGEST_CTX fdDupDigest(FD_t fd, int id) | ||
49 | return ctx; | ||
50 | } | ||
51 | |||
52 | +static void set_cloexec(int fd) | ||
53 | +{ | ||
54 | + int flags = fcntl(fd, F_GETFD); | ||
55 | + | ||
56 | + if (flags == -1 || (flags & FD_CLOEXEC)) | ||
57 | + return; | ||
58 | + | ||
59 | + fcntl(fd, F_SETFD, flags | FD_CLOEXEC); | ||
60 | +} | ||
61 | + | ||
62 | void rpmSetCloseOnExec(void) | ||
63 | { | ||
64 | - int flag, fdno, open_max; | ||
65 | + const int min_fd = STDERR_FILENO; /* don't touch stdin/out/err */ | ||
66 | + int fd; | ||
67 | + | ||
68 | + DIR *dir = opendir("/proc/self/fd"); | ||
69 | + if (dir == NULL) { /* /proc not available */ | ||
70 | + /* iterate over all possible fds, might be slow */ | ||
71 | + int open_max = sysconf(_SC_OPEN_MAX); | ||
72 | + if (open_max == -1) | ||
73 | + open_max = 1024; | ||
74 | |||
75 | - open_max = sysconf(_SC_OPEN_MAX); | ||
76 | - if (open_max == -1) { | ||
77 | - open_max = 1024; | ||
78 | + for (fd = min_fd + 1; fd < open_max; fd++) | ||
79 | + set_cloexec(fd); | ||
80 | + | ||
81 | + return; | ||
82 | } | ||
83 | - for (fdno = 3; fdno < open_max; fdno++) { | ||
84 | - flag = fcntl(fdno, F_GETFD); | ||
85 | - if (flag == -1 || (flag & FD_CLOEXEC)) | ||
86 | - continue; | ||
87 | - fcntl(fdno, F_SETFD, FD_CLOEXEC); | ||
88 | + | ||
89 | + /* iterate over fds obtained from /proc */ | ||
90 | + struct dirent *entry; | ||
91 | + while ((entry = readdir(dir)) != NULL) { | ||
92 | + fd = atoi(entry->d_name); | ||
93 | + if (fd > min_fd) | ||
94 | + set_cloexec(fd); | ||
95 | } | ||
96 | + | ||
97 | + closedir(dir); | ||
98 | + | ||
99 | + return; | ||
100 | } | ||
diff --git a/meta/recipes-devtools/rpm/files/0003-rpmSetCloseOnExec-use-getrlimit.patch b/meta/recipes-devtools/rpm/files/0003-rpmSetCloseOnExec-use-getrlimit.patch new file mode 100644 index 0000000000..389b41b42c --- /dev/null +++ b/meta/recipes-devtools/rpm/files/0003-rpmSetCloseOnExec-use-getrlimit.patch | |||
@@ -0,0 +1,53 @@ | |||
1 | From 307e28b4cb08b05bc044482058eeebc9f59bb9a9 Mon Sep 17 00:00:00 2001 | ||
2 | From: Kir Kolyshkin <kolyshkin@gmail.com> | ||
3 | Date: Tue, 29 May 2018 18:09:27 -0700 | ||
4 | Subject: [PATCH 3/3] rpmSetCloseOnExec: use getrlimit() | ||
5 | |||
6 | In case /proc is not available to get the actual list of opened fds, | ||
7 | we fall back to iterating through the list of all possible fds. | ||
8 | |||
9 | It is possible that during the course of the program execution the limit | ||
10 | on number of open file descriptors might be lowered, so using the | ||
11 | current limit, as returned by sysconf(_SC_OPEN_MAX), might omit some | ||
12 | fds. Therefore, it is better to use rlim_max from the structure | ||
13 | filled in by gertlimit(RLIMIT_NOFILE) to make sure we're checking | ||
14 | all fds. | ||
15 | |||
16 | This slows down the function, but only in the case /proc is not | ||
17 | available, which should be rare in practice. | ||
18 | |||
19 | Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> | ||
20 | Upstream-Status: Accepted [https://github.com/rpm-software-management/rpm/pull/444] | ||
21 | Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com> | ||
22 | --- | ||
23 | rpmio/rpmio.c | 10 +++++++++- | ||
24 | 1 file changed, 9 insertions(+), 1 deletion(-) | ||
25 | |||
26 | diff --git a/rpmio/rpmio.c b/rpmio/rpmio.c | ||
27 | index 55351c221..e051c9863 100644 | ||
28 | --- a/rpmio/rpmio.c | ||
29 | +++ b/rpmio/rpmio.c | ||
30 | @@ -10,6 +10,7 @@ | ||
31 | #include <sys/personality.h> | ||
32 | #endif | ||
33 | #include <sys/utsname.h> | ||
34 | +#include <sys/resource.h> | ||
35 | |||
36 | #include <rpm/rpmlog.h> | ||
37 | #include <rpm/rpmmacro.h> | ||
38 | @@ -1778,7 +1779,14 @@ void rpmSetCloseOnExec(void) | ||
39 | DIR *dir = opendir("/proc/self/fd"); | ||
40 | if (dir == NULL) { /* /proc not available */ | ||
41 | /* iterate over all possible fds, might be slow */ | ||
42 | - int open_max = sysconf(_SC_OPEN_MAX); | ||
43 | + struct rlimit rl; | ||
44 | + int open_max; | ||
45 | + | ||
46 | + if (getrlimit(RLIMIT_NOFILE, &rl) == 0 && rl.rlim_max != RLIM_INFINITY) | ||
47 | + open_max = rl.rlim_max; | ||
48 | + else | ||
49 | + open_max = sysconf(_SC_OPEN_MAX); | ||
50 | + | ||
51 | if (open_max == -1) | ||
52 | open_max = 1024; | ||
53 | |||
diff --git a/meta/recipes-devtools/rpm/rpm_4.14.1.bb b/meta/recipes-devtools/rpm/rpm_4.14.1.bb index ef4b737e9b..e5e87d3903 100644 --- a/meta/recipes-devtools/rpm/rpm_4.14.1.bb +++ b/meta/recipes-devtools/rpm/rpm_4.14.1.bb | |||
@@ -40,6 +40,9 @@ SRC_URI = "git://github.com/rpm-software-management/rpm;branch=rpm-4.14.x \ | |||
40 | file://0004-build-pack.c-remove-static-local-variables-from-buil.patch \ | 40 | file://0004-build-pack.c-remove-static-local-variables-from-buil.patch \ |
41 | file://0001-perl-disable-auto-reqs.patch \ | 41 | file://0001-perl-disable-auto-reqs.patch \ |
42 | file://0001-configure.ac-add-option-for-dbus.patch \ | 42 | file://0001-configure.ac-add-option-for-dbus.patch \ |
43 | file://0001-Factor-out-and-unify-setting-CLOEXEC.patch \ | ||
44 | file://0002-Optimize-rpmSetCloseOnExec.patch \ | ||
45 | file://0003-rpmSetCloseOnExec-use-getrlimit.patch \ | ||
43 | " | 46 | " |
44 | 47 | ||
45 | PE = "1" | 48 | PE = "1" |