Brad Bishop | 316dfdd | 2018-06-25 12:45:53 -0400 | [diff] [blame] | 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 | } |