From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id D8ECBA0495 for ; Mon, 10 Jun 2019 09:08:39 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id DC20C1BE60; Mon, 10 Jun 2019 09:08:38 +0200 (CEST) Received: from mta.qwilt.com (mta.qwilt.com [52.9.191.255]) by dpdk.org (Postfix) with ESMTP id 382421BE59 for ; Mon, 10 Jun 2019 09:08:36 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by mta.qwilt.com (Postfix) with ESMTP id 2D86B82546D; Mon, 10 Jun 2019 07:08:35 +0000 (UTC) X-Virus-Scanned: amavisd-new at qwilt.com Received: from mta.qwilt.com ([127.0.0.1]) by localhost (mta.qwilt.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 6Fw6tDUmUzWj; Mon, 10 Jun 2019 07:08:35 +0000 (UTC) Received: from rd01.it.qwilt.com.qwilt.com (backup [80.179.204.39]) by mta.qwilt.com (Postfix) with ESMTPSA id A56DF82546C; Mon, 10 Jun 2019 07:08:33 +0000 (UTC) From: Arnon Warshavsky To: dev@dpdk.org, thomas@monjalon.net, david.marchand@redhat.com, stephen@networkplumber.org Cc: arnonw@qwilt.com, Arnon Warshavsky Date: Mon, 10 Jun 2019 10:08:29 +0300 Message-Id: <1560150509-22146-1-git-send-email-arnon@qwilt.com> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1559661921-8821-1-git-send-email-arnon@qwilt.com> References: <1559661921-8821-1-git-send-email-arnon@qwilt.com> Subject: [dpdk-dev] [PATCH v5] eal: do not panic on shared memory init X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" This patch changes some void functions to return a value, so that the init sequence may tear down orderly instead of calling panic. Signed-off-by: Arnon Warshavsky --- The calls for launching core messaging threads were left in tact in all 3 eal implementations. This should be addressed in a different patchset. There are still cases where the PMDs or message threads may call panic with no context for the user. For these I will submit a patch suggesting a callback registration, allowing the user to register a context to be called in case of a panic that takes place outside the init sequence. -v2 fix freebsd compilation -v3 fix freebsd compilation, haste from the devil -v4 fix comments by David -v5 fix more comments by david. rename patch lib/librte_eal/freebsd/eal/eal.c | 69 ++++++++++++++++++-------- lib/librte_eal/linux/eal/eal.c | 101 +++++++++++++++++++++++++++------------ 2 files changed, 120 insertions(+), 50 deletions(-) diff --git a/lib/librte_eal/freebsd/eal/eal.c b/lib/librte_eal/freebsd/eal/eal.c index c6ac902..f18f08e 100644 --- a/lib/librte_eal/freebsd/eal/eal.c +++ b/lib/librte_eal/freebsd/eal/eal.c @@ -215,7 +215,7 @@ enum rte_iova_mode * We also don't lock the whole file, so that in future we can use read-locks * on other parts, e.g. memzones, to detect if there are running secondary * processes. */ -static void +static int rte_eal_config_create(void) { void *rte_mem_cfg_addr; @@ -224,60 +224,82 @@ enum rte_iova_mode const char *pathname = eal_runtime_config_path(); if (internal_config.no_shconf) - return; + return 0; if (mem_cfg_fd < 0){ mem_cfg_fd = open(pathname, O_RDWR | O_CREAT, 0600); - if (mem_cfg_fd < 0) - rte_panic("Cannot open '%s' for rte_mem_config\n", pathname); + if (mem_cfg_fd < 0) { + RTE_LOG(ERR, EAL, "Cannot open '%s' for rte_mem_config\n", + pathname); + return -1; + } } retval = ftruncate(mem_cfg_fd, sizeof(*rte_config.mem_config)); if (retval < 0){ close(mem_cfg_fd); - rte_panic("Cannot resize '%s' for rte_mem_config\n", pathname); + mem_cfg_fd = -1; + RTE_LOG(ERR, EAL, "Cannot resize '%s' for rte_mem_config\n", + pathname); + return -1; } retval = fcntl(mem_cfg_fd, F_SETLK, &wr_lock); if (retval < 0){ close(mem_cfg_fd); - rte_exit(EXIT_FAILURE, "Cannot create lock on '%s'. Is another primary " - "process running?\n", pathname); + mem_cfg_fd = -1; + RTE_LOG(ERR, EAL, "Cannot create lock on '%s'. Is another primary " + "process running?\n", pathname); + return -1; } rte_mem_cfg_addr = mmap(NULL, sizeof(*rte_config.mem_config), PROT_READ | PROT_WRITE, MAP_SHARED, mem_cfg_fd, 0); if (rte_mem_cfg_addr == MAP_FAILED){ - rte_panic("Cannot mmap memory for rte_config\n"); + RTE_LOG(ERR, EAL, "Cannot mmap memory for rte_config\n"); + close(mem_cfg_fd); + mem_cfg_fd = -1; + return -1; } memcpy(rte_mem_cfg_addr, &early_mem_config, sizeof(early_mem_config)); rte_config.mem_config = rte_mem_cfg_addr; + + return 0; } /* attach to an existing shared memory config */ -static void +static int rte_eal_config_attach(void) { void *rte_mem_cfg_addr; const char *pathname = eal_runtime_config_path(); if (internal_config.no_shconf) - return; + return 0; if (mem_cfg_fd < 0){ mem_cfg_fd = open(pathname, O_RDWR); - if (mem_cfg_fd < 0) - rte_panic("Cannot open '%s' for rte_mem_config\n", pathname); + if (mem_cfg_fd < 0) { + RTE_LOG(ERR, EAL, "Cannot open '%s' for rte_mem_config\n", + pathname); + return -1; + } } rte_mem_cfg_addr = mmap(NULL, sizeof(*rte_config.mem_config), PROT_READ | PROT_WRITE, MAP_SHARED, mem_cfg_fd, 0); close(mem_cfg_fd); - if (rte_mem_cfg_addr == MAP_FAILED) - rte_panic("Cannot mmap memory for rte_config\n"); + mem_cfg_fd = -1; + if (rte_mem_cfg_addr == MAP_FAILED) { + RTE_LOG(ERR, EAL, "Cannot mmap memory for rte_config! error %i (%s)\n", + errno, strerror(errno)); + return -1; + } rte_config.mem_config = rte_mem_cfg_addr; + + return 0; } /* Detect if we are a primary or a secondary process */ @@ -306,23 +328,29 @@ enum rte_proc_type_t } /* Sets up rte_config structure with the pointer to shared memory config.*/ -static void +static int rte_config_init(void) { rte_config.process_type = internal_config.process_type; switch (rte_config.process_type){ case RTE_PROC_PRIMARY: - rte_eal_config_create(); + if (rte_eal_config_create() < 0) + return -1; break; case RTE_PROC_SECONDARY: - rte_eal_config_attach(); + if (rte_eal_config_attach() < 0) + return -1; rte_eal_mcfg_wait_complete(rte_config.mem_config); break; case RTE_PROC_AUTO: case RTE_PROC_INVALID: - rte_panic("Invalid process type\n"); + RTE_LOG(ERR, EAL, "Invalid process type %d\n", + rte_config.process_type); + return -1; } + + return 0; } /* display usage */ @@ -655,7 +683,10 @@ static void rte_eal_init_alert(const char *msg) return -1; } - rte_config_init(); + if (rte_config_init() < 0) { + rte_eal_init_alert("Cannot init config"); + return -1; + } if (rte_eal_intr_init() < 0) { rte_eal_init_alert("Cannot init interrupt-handling thread"); diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c index 1613996..224ba90 100644 --- a/lib/librte_eal/linux/eal/eal.c +++ b/lib/librte_eal/linux/eal/eal.c @@ -300,7 +300,7 @@ enum rte_iova_mode * We also don't lock the whole file, so that in future we can use read-locks * on other parts, e.g. memzones, to detect if there are running secondary * processes. */ -static void +static int rte_eal_config_create(void) { void *rte_mem_cfg_addr; @@ -309,7 +309,7 @@ enum rte_iova_mode const char *pathname = eal_runtime_config_path(); if (internal_config.no_shconf) - return; + return 0; /* map the config before hugepage address so that we don't waste a page */ if (internal_config.base_virtaddr != 0) @@ -321,29 +321,41 @@ enum rte_iova_mode if (mem_cfg_fd < 0){ mem_cfg_fd = open(pathname, O_RDWR | O_CREAT, 0600); - if (mem_cfg_fd < 0) - rte_panic("Cannot open '%s' for rte_mem_config\n", pathname); + if (mem_cfg_fd < 0) { + RTE_LOG(ERR, EAL, "Cannot open '%s' for rte_mem_config\n", + pathname); + return -1; + } } retval = ftruncate(mem_cfg_fd, sizeof(*rte_config.mem_config)); if (retval < 0){ close(mem_cfg_fd); - rte_panic("Cannot resize '%s' for rte_mem_config\n", pathname); + mem_cfg_fd = -1; + RTE_LOG(ERR, EAL, "Cannot resize '%s' for rte_mem_config\n", + pathname); + return -1; } retval = fcntl(mem_cfg_fd, F_SETLK, &wr_lock); if (retval < 0){ close(mem_cfg_fd); - rte_exit(EXIT_FAILURE, "Cannot create lock on '%s'. Is another primary " - "process running?\n", pathname); + mem_cfg_fd = -1; + RTE_LOG(ERR, EAL, "Cannot create lock on '%s'. Is another primary " + "process running?\n", pathname); + return -1; } rte_mem_cfg_addr = mmap(rte_mem_cfg_addr, sizeof(*rte_config.mem_config), PROT_READ | PROT_WRITE, MAP_SHARED, mem_cfg_fd, 0); if (rte_mem_cfg_addr == MAP_FAILED){ - rte_panic("Cannot mmap memory for rte_config\n"); + close(mem_cfg_fd); + mem_cfg_fd = -1; + RTE_LOG(ERR, EAL, "Cannot mmap memory for rte_config\n"); + return -1; } + memcpy(rte_mem_cfg_addr, &early_mem_config, sizeof(early_mem_config)); rte_config.mem_config = rte_mem_cfg_addr; @@ -353,10 +365,11 @@ enum rte_iova_mode rte_config.mem_config->dma_maskbits = 0; + return 0; } /* attach to an existing shared memory config */ -static void +static int rte_eal_config_attach(void) { struct rte_mem_config *mem_config; @@ -364,33 +377,42 @@ enum rte_iova_mode const char *pathname = eal_runtime_config_path(); if (internal_config.no_shconf) - return; + return 0; if (mem_cfg_fd < 0){ mem_cfg_fd = open(pathname, O_RDWR); - if (mem_cfg_fd < 0) - rte_panic("Cannot open '%s' for rte_mem_config\n", pathname); + if (mem_cfg_fd < 0) { + RTE_LOG(ERR, EAL, "Cannot open '%s' for rte_mem_config\n", + pathname); + return -1; + } } /* map it as read-only first */ mem_config = (struct rte_mem_config *) mmap(NULL, sizeof(*mem_config), PROT_READ, MAP_SHARED, mem_cfg_fd, 0); - if (mem_config == MAP_FAILED) - rte_panic("Cannot mmap memory for rte_config! error %i (%s)\n", - errno, strerror(errno)); + if (mem_config == MAP_FAILED) { + close(mem_cfg_fd); + mem_cfg_fd = -1; + RTE_LOG(ERR, EAL, "Cannot mmap memory for rte_config! error %i (%s)\n", + errno, strerror(errno)); + return -1; + } rte_config.mem_config = mem_config; + + return 0; } /* reattach the shared config at exact memory location primary process has it */ -static void +static int rte_eal_config_reattach(void) { struct rte_mem_config *mem_config; void *rte_mem_cfg_addr; if (internal_config.no_shconf) - return; + return 0; /* save the address primary process has mapped shared config to */ rte_mem_cfg_addr = (void *) (uintptr_t) rte_config.mem_config->mem_cfg_addr; @@ -402,19 +424,26 @@ enum rte_iova_mode mem_config = (struct rte_mem_config *) mmap(rte_mem_cfg_addr, sizeof(*mem_config), PROT_READ | PROT_WRITE, MAP_SHARED, mem_cfg_fd, 0); + + close(mem_cfg_fd); + mem_cfg_fd = -1; + if (mem_config == MAP_FAILED || mem_config != rte_mem_cfg_addr) { - if (mem_config != MAP_FAILED) + if (mem_config != MAP_FAILED) { /* errno is stale, don't use */ - rte_panic("Cannot mmap memory for rte_config at [%p], got [%p]" - " - please use '--base-virtaddr' option\n", - rte_mem_cfg_addr, mem_config); - else - rte_panic("Cannot mmap memory for rte_config! error %i (%s)\n", - errno, strerror(errno)); + RTE_LOG(ERR, EAL, "Cannot mmap memory for rte_config at [%p], got [%p]" + " - please use '--base-virtaddr' option\n", + rte_mem_cfg_addr, mem_config); + return -1; + } + RTE_LOG(ERR, EAL, "Cannot mmap memory for rte_config! error %i (%s)\n", + errno, strerror(errno)); + return -1; } - close(mem_cfg_fd); rte_config.mem_config = mem_config; + + return 0; } /* Detect if we are a primary or a secondary process */ @@ -461,26 +490,33 @@ enum rte_proc_type_t } /* Sets up rte_config structure with the pointer to shared memory config.*/ -static void +static int rte_config_init(void) { rte_config.process_type = internal_config.process_type; switch (rte_config.process_type){ case RTE_PROC_PRIMARY: - rte_eal_config_create(); + if (rte_eal_config_create() < 0) + return -1; eal_update_mem_config(); break; case RTE_PROC_SECONDARY: - rte_eal_config_attach(); + if (rte_eal_config_attach() < 0) + return -1; rte_eal_mcfg_wait_complete(rte_config.mem_config); - rte_eal_config_reattach(); + if (rte_eal_config_reattach() < 0) + return -1; eal_update_internal_config(); break; case RTE_PROC_AUTO: case RTE_PROC_INVALID: - rte_panic("Invalid process type\n"); + RTE_LOG(ERR, EAL, "Invalid process type %d\n", + rte_config.process_type); + return -1; } + + return 0; } /* Unlocks hugepage directories that were locked by eal_hugepage_info_init */ @@ -998,7 +1034,10 @@ static void rte_eal_init_alert(const char *msg) return -1; } - rte_config_init(); + if (rte_config_init() < 0) { + rte_eal_init_alert("Cannot init config"); + return -1; + } if (rte_eal_intr_init() < 0) { rte_eal_init_alert("Cannot init interrupt-handling thread"); -- 1.8.3.1