From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 9FDE0A04F0 for ; Thu, 19 Dec 2019 15:41:46 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 986F41BF75; Thu, 19 Dec 2019 15:41:46 +0100 (CET) Received: from mail-wm1-f68.google.com (mail-wm1-f68.google.com [209.85.128.68]) by dpdk.org (Postfix) with ESMTP id 2E5101BF74 for ; Thu, 19 Dec 2019 15:41:45 +0100 (CET) Received: by mail-wm1-f68.google.com with SMTP id d73so5697585wmd.1 for ; Thu, 19 Dec 2019 06:41:45 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=oUmfKWZwh9PjznKxt8gxnaOPO/B7P1NUTmTTFkVo6yU=; b=BlUNgA8YNZCVibwDz76QYbVchWjVppn63GEQJwr6py4s7jPL4Nux3/GDkoiJghBPMT cADGDBeOt8RkTy8SJrkV1P2flfD1lZDrX35mmzzG3JDAsge00XI7zFqePMKDC9u0H5Ni gRYoon+Yh3keICMtf5VfARXpkXHxeKqLBqSEWw49v8qLrSAkxmeK6f+9SSmr6l1qDPB5 vchm76vRiG51HmU58yXSX8+OzVm8s+yng9Mj84QW/mM+jYecCsTgxA38GXsNyhxhWxvl CeN1BuyINiVZgqs1KRnqHvwEUyVgkCBMcrjBMsqkG+5klsiewYTNp4H7W0HPZ3h7uB0z D09A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=oUmfKWZwh9PjznKxt8gxnaOPO/B7P1NUTmTTFkVo6yU=; b=Iohv4D6cXMZJYYsqthdmQ4tyobOzBUduu9GZLPJmywQqeaBLxHyhusGjdVb6Od61lB jjpzfEqgWP+eL0ctuvj409fOSk2bYEamXcXihqNsNXbAukx4oCpyAWjZC9VKMp84TJgt ijrYPhA7ND08PLtE0SXcVz2NESMeJBRk4ZGdWhfd0KpaRG8qUcZJ4+6DKf5yoy1opwe4 gAP6WSJF+3Q8t5pmoEFWxDqUzttGAuy2LteFopcN73aeT02WCawOLILYJrp5BvNuSvTK Hb8dznELhNrYNqu1bURlZo94SV+cgr6HP5sDNcxOZCCRZAqMQ/4HKlPnW16PtWaoHLtt 7vYA== X-Gm-Message-State: APjAAAUOC8STrwxzL5Baxzc45vWDd4OEz5WfAjNyfE/qaOHxDpYuLrMk GnlOoLBliHnI4N+npJsWNI4= X-Google-Smtp-Source: APXvYqxc7BigKlwVcDLLVk/HyqUHK0mOQMPVjdag07tszRTV0wZyhQ2k0rFfROq0vSgwC6QSnDYidw== X-Received: by 2002:a7b:c450:: with SMTP id l16mr9920129wmi.166.1576766504913; Thu, 19 Dec 2019 06:41:44 -0800 (PST) Received: from localhost ([88.98.246.218]) by smtp.gmail.com with ESMTPSA id m10sm6553484wrx.19.2019.12.19.06.41.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 Dec 2019 06:41:44 -0800 (PST) From: luca.boccassi@gmail.com To: Krzysztof Kanas Cc: David Marchand , Kevin Traynor , dpdk stable Date: Thu, 19 Dec 2019 14:34:26 +0000 Message-Id: <20191219143447.21506-119-luca.boccassi@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20191219143447.21506-1-luca.boccassi@gmail.com> References: <20191219143447.21506-1-luca.boccassi@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [dpdk-stable] patch 'test: optimise fd closing in forks' has been queued to LTS release 17.11.10 X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" Hi, FYI, your patch has been queued to LTS release 17.11.10 Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet. It will be pushed if I get no objections before 12/21/19. So please shout if anyone has objections. Also note that after the patch there's a diff of the upstream commit vs the patch applied to the branch. This will indicate if there was any rebasing needed to apply to the stable branch. If there were code changes for rebasing (ie: not only metadata diffs), please double check that the rebase was correctly done. Thanks. Luca Boccassi --- >From b4b273762aacf76a06374c3672a5065a0e693c5b Mon Sep 17 00:00:00 2001 From: Krzysztof Kanas Date: Tue, 12 Nov 2019 21:31:02 +0100 Subject: [PATCH] test: optimise fd closing in forks [ upstream commit 18562261abadfbdf7e19006c381bcb1d6fd6c2fe ] 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") Signed-off-by: Krzysztof Kanas Signed-off-by: David Marchand Tested-by: Krzysztof Kanas Acked-by: Kevin Traynor --- test/test/process.h | 51 +++++++++++++++++++++++++++++++++++++++------ 1 file changed, 45 insertions(+), 6 deletions(-) diff --git a/test/test/process.h b/test/test/process.h index 51ced12237..505d2f9e7a 100644 --- a/test/test/process.h +++ b/test/test/process.h @@ -34,6 +34,8 @@ #ifndef _PROCESS_H_ #define _PROCESS_H_ +#include + #ifdef RTE_EXEC_ENV_BSDAPP #define self "curproc" #define exe "file" @@ -53,7 +55,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]; pid_t pid = fork(); @@ -66,13 +68,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]); -- 2.20.1 --- Diff of the applied patch vs upstream commit (please double-check if non-empty: --- --- - 2019-12-19 14:32:31.059399500 +0000 +++ 0119-test-optimise-fd-closing-in-forks.patch 2019-12-19 14:32:26.297301633 +0000 @@ -1,8 +1,10 @@ -From 18562261abadfbdf7e19006c381bcb1d6fd6c2fe Mon Sep 17 00:00:00 2001 +From b4b273762aacf76a06374c3672a5065a0e693c5b Mon Sep 17 00:00:00 2001 From: Krzysztof Kanas Date: Tue, 12 Nov 2019 21:31:02 +0100 Subject: [PATCH] test: optimise fd closing in forks +[ upstream commit 18562261abadfbdf7e19006c381bcb1d6fd6c2fe ] + Caught while investigating timeouts on a ARM64 server. Stracing a test process running the eal_flags_autotest, we can see that @@ -15,38 +17,38 @@ Linux. This code was a noop on FreeBSD. Fixes: af75078fece3 ("first public release") -Cc: stable@dpdk.org Signed-off-by: Krzysztof Kanas Signed-off-by: David Marchand Tested-by: Krzysztof Kanas Acked-by: Kevin Traynor --- - app/test/process.h | 50 ++++++++++++++++++++++++++++++++++++++++------ - 1 file changed, 44 insertions(+), 6 deletions(-) + test/test/process.h | 51 +++++++++++++++++++++++++++++++++++++++------ + 1 file changed, 45 insertions(+), 6 deletions(-) -diff --git a/app/test/process.h b/app/test/process.h -index 128ce41219..191d2796a9 100644 ---- a/app/test/process.h -+++ b/app/test/process.h -@@ -11,6 +11,7 @@ - #include /* NULL */ - #include /* strerror */ - #include /* readlink */ -+#include - #include +diff --git a/test/test/process.h b/test/test/process.h +index 51ced12237..505d2f9e7a 100644 +--- a/test/test/process.h ++++ b/test/test/process.h +@@ -34,6 +34,8 @@ + #ifndef _PROCESS_H_ + #define _PROCESS_H_ - #include /* strlcpy */ -@@ -40,7 +41,7 @@ process_dup(const char *const argv[], int numargs, const char *env_value) ++#include ++ + #ifdef RTE_EXEC_ENV_BSDAPP + #define self "curproc" + #define exe "file" +@@ -53,7 +55,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) + + pid_t pid = fork(); +@@ -66,13 +68,50 @@ process_dup(const char *const argv[], int numargs, const char *env_value) argv_cpy[i] = NULL; num = numargs;