> Filed https://bugzilla.gnome.org/show_bug.cgi?id=796227
This one is bad, as ulimit might have been changed during the runtime of a
process, and so using rlim_max is more correct and safe. For the same reason I
have added the second commit
> One other thing is, they call getrlimit(RLIMIT_NOFILE, ) and use
> rl.rlim_max which seems to be a mistake -- rlim_cur should be used.
Filed https://bugzilla.gnome.org/show_bug.cgi?id=796227
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or
@cgwalters I took a look at glib implementation, they make use of `fdwalk()` if
available (it's a function unique to SunOS/Solaris AFAICS), and reimplement it
using /proc/self/fd if not (which is the case for Linux). This seems to be an
unnecessary complication (unless we care much about
OK, patch updated to take all the review comments into account (thanks
@Saur2000 for your suggestions) so it's now also fixing the `doScriptExec()`
from `lib/rpmscript.c`.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
For the reference, here's my quick-n-dirty benchmarking code:
```c
#include
#include
#include
#include
#include
#include
static void set_cloexec(int fd)
{
int flags = fcntl(fd, F_GETFD);
if (flags == -1 || (flags & FD_CLOEXEC))
return;
fcntl(fd,
```
kir@kd:~/git/rpm$ time ./a 64 100 p
real0m3.413s
user0m0.416s
sys 0m2.994s
kir@kd:~/git/rpm$ time ./a 64 100 i
real0m3.661s
user0m1.421s
sys 0m2.233s
```
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or
I did some more benchmarks comparing the new /proc way with the old one. It
seems that the old one is faster when `open_max < 60` which should almost never
be the case. Based on that and the suggestion from @Saur2000 I'm dropping the
conditional switch to old method completely.
--
You are
kolyshkin commented on this pull request.
> + fcntl(fd, F_SETFD, FD_CLOEXEC);
+}
+
+static void set_cloexec_all(void)
+{
+ const int min_fd = 3; // don't touch stdin/out/err
+ const int default_open_max = 1024;
+ int fd, open_max;
+
+ open_max =
kolyshkin commented on this pull request.
> @@ -330,26 +330,64 @@ static int Pmkfifo(lua_State *L)/**
> mkfifo(path) */
}
+static void set_cloexec(int fd)
+{
+ int flag = fcntl(fd, F_GETFD);
+
+ if (flag == -1 || (flag & FD_CLOEXEC))
+ return;
+
+
kolyshkin commented on this pull request.
> + struct dirent *entry;
+ while ((entry = readdir(dir)) != NULL) {
+ fd = atoi(entry->d_name);
+ if (fd >= min_fd)
+ set_cloexec(fd);
+ }
+ closedir(dir);
+
+ return;
+
The CI failure on rawhide is unrelated, it's a dnf error complaining about repo
metadata:
> raise ValueError("The supplied metadata version isn't supported")
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
Saur2000 requested changes on this pull request.
> + struct dirent *entry;
+ while ((entry = readdir(dir)) != NULL) {
+ fd = atoi(entry->d_name);
+ if (fd >= min_fd)
+ set_cloexec(fd);
+ }
+ closedir(dir);
+
+ return;
At a quick glance anyways, your code looks fine to me.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
In case maximum number of open files limit is set too high, Pexec() spends way
too much time trying to set FD_CLOEXEC for all those file descriptors,
resulting in severe increase of time it takes to execute rpm or dnf. For
example, this happens while running under Docker because:
> $ docker
Hi all,
[I posted a similar proposal under the thread "How to generate GNU make
dependency file for a SPEC file automatically?" but I'm reposting here with
a better subject]
Proposal: add a new option to rpmbuild, something like
--generate-md5sum= that writes the md5sum (or whatever other hash,
15 matches
Mail list logo
