| Message ID | 1573590662-31370-1-git-send-email-david.marchand@redhat.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Marchand |
| Headers | show |
| Series |
|
| Related | show |
| Context | Check | Description |
|---|---|---|
| ci/Intel-compilation | success | Compilation OK |
| ci/iol-mellanox-Performance | fail | Performance Testing issues |
| ci/iol-compilation | success | Compile Testing PASS |
| ci/iol-intel-Performance | success | Performance Testing PASS |
| ci/checkpatch | success | coding style OK |
On 12/11/2019 20:31, David Marchand wrote: > From: Krzysztof Kanas <kkanas@marvell.com> > > Caught while investigating timeouts on a ARM64 server. > > Stracing a test process running the eal_flags_autotest, we can see that > the fork helper is checking all possible file descriptors from > getdtablesize() to 2, and close the existing ones. > We can do better by inspecting this forked process /proc/self/fd > directory. > > Besides, checking file descriptors via /proc/self/fd only makes sense for > Linux. This code was a noop on FreeBSD. > > Fixes: af75078fece3 ("first public release") > Cc: stable@dpdk.org > > Signed-off-by: Krzysztof Kanas <kkanas@marvell.com> > Signed-off-by: David Marchand <david.marchand@redhat.com> Seems to be an improvement on what is existing. Acked-by: Kevin Traynor <ktraynor@redhat.com>
On Thu, Nov 14, 2019 at 8:32 PM Kevin Traynor <ktraynor@redhat.com> wrote: > > On 12/11/2019 20:31, David Marchand wrote: > > From: Krzysztof Kanas <kkanas@marvell.com> > > > > Caught while investigating timeouts on a ARM64 server. > > > > Stracing a test process running the eal_flags_autotest, we can see that > > the fork helper is checking all possible file descriptors from > > getdtablesize() to 2, and close the existing ones. > > We can do better by inspecting this forked process /proc/self/fd > > directory. > > > > Besides, checking file descriptors via /proc/self/fd only makes sense for > > Linux. This code was a noop on FreeBSD. > > > > Fixes: af75078fece3 ("first public release") > > Cc: stable@dpdk.org > > > > Signed-off-by: Krzysztof Kanas <kkanas@marvell.com> > > Signed-off-by: David Marchand <david.marchand@redhat.com> Tested-by: Krzysztof Kanas <kkanas@marvell.com> > Seems to be an improvement on what is existing. > > Acked-by: Kevin Traynor <ktraynor@redhat.com> > Applied, thanks. -- David Marchand
diff --git a/app/test/process.h b/app/test/process.h index 128ce41..191d279 100644 --- a/app/test/process.h +++ b/app/test/process.h @@ -11,6 +11,7 @@ #include <stdlib.h> /* NULL */ #include <string.h> /* strerror */ #include <unistd.h> /* readlink */ +#include <dirent.h> #include <sys/wait.h> #include <rte_string_fns.h> /* strlcpy */ @@ -40,7 +41,7 @@ process_dup(const char *const argv[], int numargs, const char *env_value) { int num; char *argv_cpy[numargs + 1]; - int i, fd, status; + int i, status; char path[32]; #ifdef RTE_LIBRTE_PDUMP pthread_t thread; @@ -56,13 +57,50 @@ process_dup(const char *const argv[], int numargs, const char *env_value) argv_cpy[i] = NULL; num = numargs; - /* close all open file descriptors, check /proc/self/fd to only - * call close on open fds. Exclude fds 0, 1 and 2*/ - for (fd = getdtablesize(); fd > 2; fd-- ) { - snprintf(path, sizeof(path), "/proc/" exe "/fd/%d", fd); - if (access(path, F_OK) == 0) +#ifdef RTE_EXEC_ENV_LINUX + { + const char *procdir = "/proc/" self "/fd/"; + struct dirent *dirent; + char *endptr; + int fd, fdir; + DIR *dir; + + /* close all open file descriptors, check /proc/self/fd + * to only call close on open fds. Exclude fds 0, 1 and + * 2 + */ + dir = opendir(procdir); + if (dir == NULL) { + rte_panic("Error opening %s: %s\n", procdir, + strerror(errno)); + } + + fdir = dirfd(dir); + if (fdir < 0) { + status = errno; + closedir(dir); + rte_panic("Error %d obtaining fd for dir %s: %s\n", + fdir, procdir, + strerror(status)); + } + + while ((dirent = readdir(dir)) != NULL) { + errno = 0; + fd = strtol(dirent->d_name, &endptr, 10); + if (errno != 0 || endptr[0] != '\0') { + printf("Error converting name fd %d %s:\n", + fd, dirent->d_name); + continue; + } + + if (fd == fdir || fd <= 2) + continue; + close(fd); + } + closedir(dir); } +#endif printf("Running binary with argv[]:"); for (i = 0; i < num; i++) printf("'%s' ", argv_cpy[i]);