-
Notifications
You must be signed in to change notification settings - Fork 505
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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: rpm-software-management/rpm#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. Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com> Signed-off-by: Armin Kuster <akuster808@gmail.com>
- Loading branch information
Showing
4 changed files
with
304 additions
and
0 deletions.
There are no files selected for viewing
148 changes: 148 additions & 0 deletions
148
meta/recipes-devtools/rpm/files/0001-Factor-out-and-unify-setting-CLOEXEC.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,148 @@ | ||
From 9c3e5de3240554c8ea1b29d52eeadee4840fefac Mon Sep 17 00:00:00 2001 | ||
From: Kir Kolyshkin <kolyshkin@gmail.com> | ||
Date: Tue, 29 May 2018 17:37:05 -0700 | ||
Subject: [PATCH 1/3] Factor out and unify setting CLOEXEC | ||
|
||
Commit 7a7c31f5 ("Set FD_CLOEXEC on opened files before exec from | ||
lua script is called") copied the code that sets CLOEXEC flag on all | ||
possible file descriptors from lib/rpmscript.c to luaext/lposix.c, | ||
essentially creating two copies of the same code (modulo comments | ||
and the unused assignment). | ||
|
||
This commit moves the functionality into its own function, without | ||
any code modifications, using the version from luaext/lposix.c. | ||
|
||
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> | ||
Upstream-Status: Accepted [https://github.com/rpm-software-management/rpm/pull/444] | ||
Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com> | ||
--- | ||
lib/rpmscript.c | 18 ++---------------- | ||
luaext/lposix.c | 13 ++----------- | ||
rpmio/rpmio.c | 16 ++++++++++++++++ | ||
rpmio/rpmio_internal.h | 6 ++++++ | ||
4 files changed, 26 insertions(+), 27 deletions(-) | ||
|
||
diff --git a/lib/rpmscript.c b/lib/rpmscript.c | ||
index 747385a5b..b4ccd3246 100644 | ||
--- a/lib/rpmscript.c | ||
+++ b/lib/rpmscript.c | ||
@@ -3,7 +3,6 @@ | ||
#include <sys/types.h> | ||
#include <sys/wait.h> | ||
#include <errno.h> | ||
-#include <unistd.h> | ||
|
||
#include <rpm/rpmfileutil.h> | ||
#include <rpm/rpmmacro.h> | ||
@@ -14,6 +13,7 @@ | ||
|
||
#include "rpmio/rpmlua.h" | ||
#include "lib/rpmscript.h" | ||
+#include "rpmio/rpmio_internal.h" | ||
|
||
#include "lib/rpmplugins.h" /* rpm plugins hooks */ | ||
|
||
@@ -170,26 +170,12 @@ static const char * const SCRIPT_PATH = "PATH=/sbin:/bin:/usr/sbin:/usr/bin:/usr | ||
static void doScriptExec(ARGV_const_t argv, ARGV_const_t prefixes, | ||
FD_t scriptFd, FD_t out) | ||
{ | ||
- int flag; | ||
- int fdno; | ||
int xx; | ||
- int open_max; | ||
|
||
/* SIGPIPE is ignored in rpm, reset to default for the scriptlet */ | ||
(void) signal(SIGPIPE, SIG_DFL); | ||
|
||
- /* XXX Force FD_CLOEXEC on all inherited fdno's. */ | ||
- open_max = sysconf(_SC_OPEN_MAX); | ||
- if (open_max == -1) { | ||
- open_max = 1024; | ||
- } | ||
- for (fdno = 3; fdno < open_max; fdno++) { | ||
- flag = fcntl(fdno, F_GETFD); | ||
- if (flag == -1 || (flag & FD_CLOEXEC)) | ||
- continue; | ||
- xx = fcntl(fdno, F_SETFD, FD_CLOEXEC); | ||
- /* XXX W2DO? debug msg for inheirited fdno w/o FD_CLOEXEC */ | ||
- } | ||
+ rpmSetCloseOnExec(); | ||
|
||
if (scriptFd != NULL) { | ||
int sfdno = Fileno(scriptFd); | ||
diff --git a/luaext/lposix.c b/luaext/lposix.c | ||
index 0a7c26c71..5d7ad3c87 100644 | ||
--- a/luaext/lposix.c | ||
+++ b/luaext/lposix.c | ||
@@ -27,6 +27,7 @@ | ||
#include <unistd.h> | ||
#include <utime.h> | ||
#include <rpm/rpmutil.h> | ||
+#include "rpmio/rpmio_internal.h" | ||
|
||
#define MYNAME "posix" | ||
#define MYVERSION MYNAME " library for " LUA_VERSION " / Nov 2003" | ||
@@ -335,21 +336,11 @@ static int Pexec(lua_State *L) /** exec(path,[args]) */ | ||
const char *path = luaL_checkstring(L, 1); | ||
int i,n=lua_gettop(L); | ||
char **argv; | ||
- int flag, fdno, open_max; | ||
|
||
if (!have_forked) | ||
return luaL_error(L, "exec not permitted in this context"); | ||
|
||
- open_max = sysconf(_SC_OPEN_MAX); | ||
- if (open_max == -1) { | ||
- open_max = 1024; | ||
- } | ||
- for (fdno = 3; fdno < open_max; fdno++) { | ||
- flag = fcntl(fdno, F_GETFD); | ||
- if (flag == -1 || (flag & FD_CLOEXEC)) | ||
- continue; | ||
- fcntl(fdno, F_SETFD, FD_CLOEXEC); | ||
- } | ||
+ rpmSetCloseOnExec(); | ||
|
||
argv = malloc((n+1)*sizeof(char*)); | ||
if (argv==NULL) return luaL_error(L,"not enough memory"); | ||
diff --git a/rpmio/rpmio.c b/rpmio/rpmio.c | ||
index c7cbc32aa..ea111d2ec 100644 | ||
--- a/rpmio/rpmio.c | ||
+++ b/rpmio/rpmio.c | ||
@@ -1759,3 +1759,19 @@ DIGEST_CTX fdDupDigest(FD_t fd, int id) | ||
|
||
return ctx; | ||
} | ||
+ | ||
+void rpmSetCloseOnExec(void) | ||
+{ | ||
+ int flag, fdno, open_max; | ||
+ | ||
+ open_max = sysconf(_SC_OPEN_MAX); | ||
+ if (open_max == -1) { | ||
+ open_max = 1024; | ||
+ } | ||
+ for (fdno = 3; fdno < open_max; fdno++) { | ||
+ flag = fcntl(fdno, F_GETFD); | ||
+ if (flag == -1 || (flag & FD_CLOEXEC)) | ||
+ continue; | ||
+ fcntl(fdno, F_SETFD, FD_CLOEXEC); | ||
+ } | ||
+} | ||
diff --git a/rpmio/rpmio_internal.h b/rpmio/rpmio_internal.h | ||
index fbed183b0..370cbdc75 100644 | ||
--- a/rpmio/rpmio_internal.h | ||
+++ b/rpmio/rpmio_internal.h | ||
@@ -41,6 +41,12 @@ DIGEST_CTX fdDupDigest(FD_t fd, int id); | ||
int rpmioSlurp(const char * fn, | ||
uint8_t ** bp, ssize_t * blenp); | ||
|
||
+/** | ||
+ * Set close-on-exec flag for all opened file descriptors, except | ||
+ * stdin/stdout/stderr. | ||
+ */ | ||
+void rpmSetCloseOnExec(void); | ||
+ | ||
#ifdef __cplusplus | ||
} | ||
#endif |
100 changes: 100 additions & 0 deletions
100
meta/recipes-devtools/rpm/files/0002-Optimize-rpmSetCloseOnExec.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,100 @@ | ||
From 5e6f05cd8dad6c1ee6bd1e6e43f176976c9c3416 Mon Sep 17 00:00:00 2001 | ||
From: Kir Kolyshkin <kolyshkin@gmail.com> | ||
Date: Tue, 29 May 2018 17:52:56 -0700 | ||
Subject: [PATCH 2/3] Optimize rpmSetCloseOnExec | ||
|
||
In case maximum number of open files limit is set too high, both | ||
luaext/Pexec() and lib/doScriptExec() spend way too much time | ||
trying to set FD_CLOEXEC flag for all those file descriptors, | ||
resulting in severe increase of time it takes to execute say | ||
rpm or dnf. | ||
|
||
This becomes increasingly noticeable when running with e.g. under | ||
Docker, the reason being: | ||
|
||
> $ docker run fedora ulimit -n | ||
> 1048576 | ||
|
||
One obvious fix is to use procfs to get the actual list of opened fds | ||
and iterate over it. My quick-n-dirty benchmark shows the /proc approach | ||
is about 10x faster than iterating through a list of just 1024 fds, | ||
so it's an improvement even for default ulimit values. | ||
|
||
Note that the old method is still used in case /proc is not available. | ||
|
||
While at it, | ||
|
||
1. fix the function by making sure we modify (rather than set) | ||
the existing flags. As the only known flag is FD_CLOEXEC, | ||
this change is currently purely aesthetical, but in case | ||
other flags will appear it will become a real bug fix. | ||
|
||
2. get rid of magic number 3; use STDERR_FILENO | ||
|
||
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> | ||
|
||
Fixes #444 | ||
|
||
Upstream-Status: Accepted [https://github.com/rpm-software-management/rpm/pull/444] | ||
Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com> | ||
--- | ||
rpmio/rpmio.c | 43 ++++++++++++++++++++++++++++++++++--------- | ||
1 file changed, 34 insertions(+), 9 deletions(-) | ||
|
||
diff --git a/rpmio/rpmio.c b/rpmio/rpmio.c | ||
index ea111d2ec..55351c221 100644 | ||
--- a/rpmio/rpmio.c | ||
+++ b/rpmio/rpmio.c | ||
@@ -1760,18 +1760,43 @@ DIGEST_CTX fdDupDigest(FD_t fd, int id) | ||
return ctx; | ||
} | ||
|
||
+static void set_cloexec(int fd) | ||
+{ | ||
+ int flags = fcntl(fd, F_GETFD); | ||
+ | ||
+ if (flags == -1 || (flags & FD_CLOEXEC)) | ||
+ return; | ||
+ | ||
+ fcntl(fd, F_SETFD, flags | FD_CLOEXEC); | ||
+} | ||
+ | ||
void rpmSetCloseOnExec(void) | ||
{ | ||
- int flag, fdno, open_max; | ||
+ const int min_fd = STDERR_FILENO; /* don't touch stdin/out/err */ | ||
+ int fd; | ||
+ | ||
+ DIR *dir = opendir("/proc/self/fd"); | ||
+ if (dir == NULL) { /* /proc not available */ | ||
+ /* iterate over all possible fds, might be slow */ | ||
+ int open_max = sysconf(_SC_OPEN_MAX); | ||
+ if (open_max == -1) | ||
+ open_max = 1024; | ||
|
||
- open_max = sysconf(_SC_OPEN_MAX); | ||
- if (open_max == -1) { | ||
- open_max = 1024; | ||
+ for (fd = min_fd + 1; fd < open_max; fd++) | ||
+ set_cloexec(fd); | ||
+ | ||
+ return; | ||
} | ||
- for (fdno = 3; fdno < open_max; fdno++) { | ||
- flag = fcntl(fdno, F_GETFD); | ||
- if (flag == -1 || (flag & FD_CLOEXEC)) | ||
- continue; | ||
- fcntl(fdno, F_SETFD, FD_CLOEXEC); | ||
+ | ||
+ /* iterate over fds obtained from /proc */ | ||
+ struct dirent *entry; | ||
+ while ((entry = readdir(dir)) != NULL) { | ||
+ fd = atoi(entry->d_name); | ||
+ if (fd > min_fd) | ||
+ set_cloexec(fd); | ||
} | ||
+ | ||
+ closedir(dir); | ||
+ | ||
+ return; | ||
} |
53 changes: 53 additions & 0 deletions
53
meta/recipes-devtools/rpm/files/0003-rpmSetCloseOnExec-use-getrlimit.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
From 307e28b4cb08b05bc044482058eeebc9f59bb9a9 Mon Sep 17 00:00:00 2001 | ||
From: Kir Kolyshkin <kolyshkin@gmail.com> | ||
Date: Tue, 29 May 2018 18:09:27 -0700 | ||
Subject: [PATCH 3/3] rpmSetCloseOnExec: use getrlimit() | ||
|
||
In case /proc is not available to get the actual list of opened fds, | ||
we fall back to iterating through the list of all possible fds. | ||
|
||
It is possible that during the course of the program execution the limit | ||
on number of open file descriptors might be lowered, so using the | ||
current limit, as returned by sysconf(_SC_OPEN_MAX), might omit some | ||
fds. Therefore, it is better to use rlim_max from the structure | ||
filled in by gertlimit(RLIMIT_NOFILE) to make sure we're checking | ||
all fds. | ||
|
||
This slows down the function, but only in the case /proc is not | ||
available, which should be rare in practice. | ||
|
||
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> | ||
Upstream-Status: Accepted [https://github.com/rpm-software-management/rpm/pull/444] | ||
Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com> | ||
--- | ||
rpmio/rpmio.c | 10 +++++++++- | ||
1 file changed, 9 insertions(+), 1 deletion(-) | ||
|
||
diff --git a/rpmio/rpmio.c b/rpmio/rpmio.c | ||
index 55351c221..e051c9863 100644 | ||
--- a/rpmio/rpmio.c | ||
+++ b/rpmio/rpmio.c | ||
@@ -10,6 +10,7 @@ | ||
#include <sys/personality.h> | ||
#endif | ||
#include <sys/utsname.h> | ||
+#include <sys/resource.h> | ||
|
||
#include <rpm/rpmlog.h> | ||
#include <rpm/rpmmacro.h> | ||
@@ -1778,7 +1779,14 @@ void rpmSetCloseOnExec(void) | ||
DIR *dir = opendir("/proc/self/fd"); | ||
if (dir == NULL) { /* /proc not available */ | ||
/* iterate over all possible fds, might be slow */ | ||
- int open_max = sysconf(_SC_OPEN_MAX); | ||
+ struct rlimit rl; | ||
+ int open_max; | ||
+ | ||
+ if (getrlimit(RLIMIT_NOFILE, &rl) == 0 && rl.rlim_max != RLIM_INFINITY) | ||
+ open_max = rl.rlim_max; | ||
+ else | ||
+ open_max = sysconf(_SC_OPEN_MAX); | ||
+ | ||
if (open_max == -1) | ||
open_max = 1024; | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters