patches for DPDK stable branches
 help / color / mirror / Atom feed
From: Anatoly Burakov <anatoly.burakov@intel.com>
To: dev@dpdk.org
Cc: Bruce Richardson <bruce.richardson@intel.com>,
	stephen@networkplumber.org, stable@dpdk.org
Subject: [dpdk-stable] [PATCH v4] eal: fix proc type auto detection
Date: Wed, 24 Jul 2019 17:07:41 +0100	[thread overview]
Message-ID: <67a795e77bc9f5ac79ab78a878ae19abbead9f50.1563984454.git.anatoly.burakov@intel.com> (raw)
In-Reply-To: <e76dcac7fe4ecd9f588253950b6496990581440a.1563984251.git.anatoly.burakov@intel.com>

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: stable@dpdk.org

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---

Notes:
    v4:
    - Fixed FreeBSD log message to match Linux version
    
    v3:
    - Added similar changes to FreeBSD version
    
    v2:
    - Adjusted indentation

 lib/librte_eal/freebsd/eal/eal.c | 36 +++++++++++++++++++++++++++----
 lib/librte_eal/linux/eal/eal.c   | 37 +++++++++++++++++++++++++++-----
 2 files changed, 64 insertions(+), 9 deletions(-)

diff --git a/lib/librte_eal/freebsd/eal/eal.c b/lib/librte_eal/freebsd/eal/eal.c
index d53f0fe69..69995bf8f 100644
--- a/lib/librte_eal/freebsd/eal/eal.c
+++ b/lib/librte_eal/freebsd/eal/eal.c
@@ -72,6 +72,13 @@ static struct flock wr_lock = {
 		.l_len = sizeof(early_mem_config.memsegs),
 };
 
+static struct flock rd_lock = {
+	.l_type = F_RDLCK,
+	.l_whence = SEEK_SET,
+	.l_start = offsetof(struct rte_mem_config, memsegs),
+	.l_len = sizeof(early_mem_config.memsegs),
+};
+
 /* Address of global and public configuration */
 static struct rte_config rte_config = {
 		.mem_config = &early_mem_config,
@@ -249,8 +256,21 @@ rte_eal_config_create(void)
 	if (retval < 0){
 		close(mem_cfg_fd);
 		mem_cfg_fd = -1;
-		RTE_LOG(ERR, EAL, "Cannot create lock on '%s'. Is another primary "
-			"process running?\n", pathname);
+		RTE_LOG(ERR, EAL, "Cannot create exclusive lock on '%s'. "
+			"Is another process running?\n", pathname);
+		return -1;
+	}
+
+	/* we hold an exclusive lock - now downgrade it to a read lock to allow
+	 * other processes to also hold onto this file while preventing other
+	 * primaries from spinning up.
+	 */
+	retval = fcntl(mem_cfg_fd, F_SETLK, &rd_lock);
+	if (retval < 0) {
+		close(mem_cfg_fd);
+		mem_cfg_fd = -1;
+		RTE_LOG(ERR, EAL, "Cannot downgrade to shared lock on '%s': %s\n",
+			pathname, strerror(errno));
 		return -1;
 	}
 
@@ -292,6 +312,16 @@ rte_eal_config_attach(void)
 			return -1;
 		}
 	}
+	/* lock the file to prevent primary from initializing while this
+	 * process is still running.
+	 */
+	if (fcntl(mem_cfg_fd, F_SETLK, &rd_lock) < 0) {
+		close(mem_cfg_fd);
+		mem_cfg_fd = -1;
+		RTE_LOG(ERR, EAL, "Cannot create shared lock on '%s': %s\n",
+				pathname, strerror(errno));
+		return -1;
+	}
 
 	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",
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,
+	.l_whence = SEEK_SET,
+	.l_start = offsetof(struct rte_mem_config, memsegs),
+	.l_len = sizeof(early_mem_config.memsegs),
+};
+
 /* Address of global and public configuration */
 static struct rte_config rte_config = {
 		.mem_config = &early_mem_config,
@@ -343,8 +350,21 @@ rte_eal_config_create(void)
 	if (retval < 0){
 		close(mem_cfg_fd);
 		mem_cfg_fd = -1;
-		RTE_LOG(ERR, EAL, "Cannot create lock on '%s'. Is another primary "
-			"process running?\n", pathname);
+		RTE_LOG(ERR, EAL, "Cannot create exclusive lock on '%s'. "
+			"Is another process running?\n", pathname);
+		return -1;
+	}
+
+	/* we hold an exclusive lock - now downgrade it to a read lock to allow
+	 * other processes to also hold onto this file while preventing other
+	 * primaries from spinning up.
+	 */
+	retval = fcntl(mem_cfg_fd, F_SETLK, &rd_lock);
+	if (retval < 0) {
+		close(mem_cfg_fd);
+		mem_cfg_fd = -1;
+		RTE_LOG(ERR, EAL, "Cannot downgrade to shared lock on '%s': %s\n",
+			pathname, strerror(errno));
 		return -1;
 	}
 
@@ -389,6 +409,16 @@ rte_eal_config_attach(void)
 			return -1;
 		}
 	}
+	/* lock the file to prevent primary from initializing while this
+	 * process is still running.
+	 */
+	if (fcntl(mem_cfg_fd, F_SETLK, &rd_lock) < 0) {
+		close(mem_cfg_fd);
+		mem_cfg_fd = -1;
+		RTE_LOG(ERR, EAL, "Cannot create shared lock on '%s': %s\n",
+				pathname, strerror(errno));
+		return -1;
+	}
 
 	/* map it as read-only first */
 	mem_config = (struct rte_mem_config *) mmap(NULL, sizeof(*mem_config),
@@ -427,9 +457,6 @@ rte_eal_config_reattach(void)
 			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) {
 			/* errno is stale, don't use */
-- 
2.17.1

  reply	other threads:[~2019-07-24 16:07 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-23 13:19 [dpdk-stable] [PATCH] " Anatoly Burakov
2019-07-23 18:38 ` [dpdk-stable] [dpdk-dev] " Stephen Hemminger
2019-07-24 10:04 ` [dpdk-stable] [PATCH v2] " Anatoly Burakov
2019-07-24 16:04   ` [dpdk-stable] [PATCH v3] " Anatoly Burakov
2019-07-24 16:07     ` Anatoly Burakov [this message]
2019-07-30  8:13       ` [dpdk-stable] [dpdk-dev] [PATCH v4] " Thomas Monjalon
2019-07-30  9:19         ` Burakov, Anatoly
2019-08-12 10:03       ` David Marchand
2019-08-12 10:21         ` Van Haaren, Harry
2019-08-12 13:30           ` Burakov, Anatoly
2019-08-12 13:31         ` Burakov, Anatoly
2019-10-24 16:07         ` Burakov, Anatoly
2019-10-27 19:41           ` David Marchand

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=67a795e77bc9f5ac79ab78a878ae19abbead9f50.1563984454.git.anatoly.burakov@intel.com \
    --to=anatoly.burakov@intel.com \
    --cc=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=stable@dpdk.org \
    --cc=stephen@networkplumber.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).