On 12-Aug-19 11:03 AM, David Marchand wrote:
On Wed, Jul 24, 2019 at 6:08 PM Anatoly Burakov <anatoly.bura...@intel.com> wrote:Currently, primary process holds an exclusive lock on the config file, thereby preventing other primaries from spinning up. However, when the primary dies, the lock is no longer being held, even though there might be other secondary processes still running. The fix is two-fold. First of all, downgrade the primary process's exclusive lock to a shared lock once we have it. Second of all, also take out shared locks on the config from the secondaries. We are using fcntl() locks, which get dropped when the file handle is closed, so also remove the closure of config file handle. Fixes: af75078fece3 ("first public release") Cc: sta...@dpdk.org Signed-off-by: Anatoly Burakov <anatoly.bura...@intel.com> ---
<snip>
rte_mem_cfg_addr = mmap(NULL, sizeof(*rte_config.mem_config), PROT_READ, MAP_SHARED, mem_cfg_fd, 0); @@ -330,8 +360,6 @@ rte_eal_config_reattach(void) 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) { RTE_LOG(ERR, EAL, "Cannot mmap memory for rte_config! error %i (%s)\n",We are missing a mem_cfg_fd cleanup if mmap failed.
Good catch! Will fix.
diff --git a/lib/librte_eal/linux/eal/eal.c b/lib/librte_eal/linux/eal/eal.c index 34db78753..0f0726703 100644 --- a/lib/librte_eal/linux/eal/eal.c +++ b/lib/librte_eal/linux/eal/eal.c @@ -83,6 +83,13 @@ static struct flock wr_lock = { .l_len = sizeof(early_mem_config.memsegs), }; +static struct flock rd_lock = { + .l_type = F_RDLCK,
<snip>
- if (mem_config == MAP_FAILED || mem_config != rte_mem_cfg_addr) { if (mem_config != MAP_FAILED) { /* errno is stale, don't use */Idem. Reviewed-by: David Marchand <david.march...@redhat.com> With https://patchwork.dpdk.org/patch/56501/, the code is now really close between Linux and FreeBSD, could it go to common entirely?
I would prefer to keep them separate due to upcoming Windows port. -- Thanks, Anatoly
