Fix do_rootfs eats huge time on docker environment 97/15797/11
authorTadao Tanikawa <tanikawa.tadao@jp.panasonic.com>
Fri, 27 Jul 2018 03:54:43 +0000 (03:54 +0000)
committerStéphane Desneux <stephane.desneux@iot.bzh>
Mon, 10 Sep 2018 21:29:43 +0000 (21:29 +0000)
RPM is causing a serious performance regression on Docker.

Docker can set ulimit -n to 1048576 but it causes huge time
consumption when thousands of packages are installed like
bitbake agl-demo-platform.

This issue is already resolved in upstream of RPM and
yocto follows it at sumo, so backporting it into Flounder.

(From OE-Core rev: 6ecb10e3952af4a77bc79160ecd81117e97d022a)

Bug-AGL: SPEC-1622

Change-Id: Ia8d97daea663f9682928a14ab84199ed6fda6d61
Signed-off-by: Tadao Tanikawa <tanikawa.tadao@jp.panasonic.com>
meta-agl-profile-core/recipes-devtools/rpm/files/0001-Factor-out-and-unify-setting-CLOEXEC.patch [new file with mode: 0644]
meta-agl-profile-core/recipes-devtools/rpm/files/0002-Optimize-rpmSetCloseOnExec.patch [new file with mode: 0644]
meta-agl-profile-core/recipes-devtools/rpm/files/0003-rpmSetCloseOnExec-use-getrlimit.patch [new file with mode: 0644]
meta-agl-profile-core/recipes-devtools/rpm/rpm_%.bbappend [new file with mode: 0644]

diff --git a/meta-agl-profile-core/recipes-devtools/rpm/files/0001-Factor-out-and-unify-setting-CLOEXEC.patch b/meta-agl-profile-core/recipes-devtools/rpm/files/0001-Factor-out-and-unify-setting-CLOEXEC.patch
new file mode 100644 (file)
index 0000000..cfddec6
--- /dev/null
@@ -0,0 +1,140 @@
+From 0a66176074560303bf0870957464239e9757d891 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>
+(cherry picked from commit 9c3e5de3240554c8ea1b29d52eeadee4840fefac)
+---
+ lib/rpmscript.c        | 17 ++---------------
+ luaext/lposix.c        | 13 ++-----------
+ rpmio/rpmio.c          | 14 ++++++++++++++
+ rpmio/rpmio_internal.h |  6 ++++++
+ 4 files changed, 24 insertions(+), 26 deletions(-)
+
+diff --git a/lib/rpmscript.c b/lib/rpmscript.c
+index 98d3f42..61dff83 100644
+--- a/lib/rpmscript.c
++++ b/lib/rpmscript.c
+@@ -18,6 +18,7 @@
+ #include "rpmio/rpmlua.h"
+ #include "lib/rpmscript.h"
++#include "rpmio/rpmio_internal.h"
+ #include "lib/rpmplugins.h"     /* rpm plugins hooks */
+@@ -161,25 +162,11 @@ 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;
+     (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 0a7c26c..5d7ad3c 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 0b90984..747bf3c 100644
+--- a/rpmio/rpmio.c
++++ b/rpmio/rpmio.c
+@@ -1477,4 +1477,18 @@ void fdFiniDigest(FD_t fd, int hashalgo,
+     }
+ }
++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 8c9f1a8..da39250 100644
+--- a/rpmio/rpmio_internal.h
++++ b/rpmio/rpmio_internal.h
+@@ -37,6 +37,12 @@ void fdFiniDigest(FD_t fd, int hashalgo,
+ 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
+-- 
+2.7.4
+
diff --git a/meta-agl-profile-core/recipes-devtools/rpm/files/0002-Optimize-rpmSetCloseOnExec.patch b/meta-agl-profile-core/recipes-devtools/rpm/files/0002-Optimize-rpmSetCloseOnExec.patch
new file mode 100644 (file)
index 0000000..9354dd6
--- /dev/null
@@ -0,0 +1,102 @@
+From 6f56aa01a357bbcf9401d79a378ae380f5f939d4 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
+
+(cherry picked from commit 5e6f05cd8dad6c1ee6bd1e6e43f176976c9c3416)
+---
+ rpmio/rpmio.c | 43 ++++++++++++++++++++++++++++++++++---------
+ 1 file changed, 34 insertions(+), 9 deletions(-)
+
+diff --git a/rpmio/rpmio.c b/rpmio/rpmio.c
+index 747bf3c..8148aa2 100644
+--- a/rpmio/rpmio.c
++++ b/rpmio/rpmio.c
+@@ -1477,18 +1477,43 @@ void fdFiniDigest(FD_t fd, int hashalgo,
+     }
+ }
++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;
+ }
+-- 
+2.7.4
+
diff --git a/meta-agl-profile-core/recipes-devtools/rpm/files/0003-rpmSetCloseOnExec-use-getrlimit.patch b/meta-agl-profile-core/recipes-devtools/rpm/files/0003-rpmSetCloseOnExec-use-getrlimit.patch
new file mode 100644 (file)
index 0000000..d9b813e
--- /dev/null
@@ -0,0 +1,55 @@
+From 1f5438d677dca330642158ec0b1c0366c6a65725 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>
+(cherry picked from commit 307e28b4cb08b05bc044482058eeebc9f59bb9a9)
+---
+ rpmio/rpmio.c | 10 +++++++++-
+ 1 file changed, 9 insertions(+), 1 deletion(-)
+
+diff --git a/rpmio/rpmio.c b/rpmio/rpmio.c
+index 8148aa2..0698e53 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>
+@@ -1495,7 +1496,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;
+-- 
+2.7.4
+
diff --git a/meta-agl-profile-core/recipes-devtools/rpm/rpm_%.bbappend b/meta-agl-profile-core/recipes-devtools/rpm/rpm_%.bbappend
new file mode 100644 (file)
index 0000000..1333a97
--- /dev/null
@@ -0,0 +1,7 @@
+FILESEXTRAPATHS_append := ":${THISDIR}/files"
+
+SRC_URI_append = "\
+    file://0001-Factor-out-and-unify-setting-CLOEXEC.patch \
+    file://0002-Optimize-rpmSetCloseOnExec.patch \
+    file://0003-rpmSetCloseOnExec-use-getrlimit.patch \
+"