[dpdk-dev] fbarray: support no-shconf

Message ID 80aff8bc0255cdab2331dcb0b21219d50f154937.1527006762.git.anatoly.burakov@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Burakov, Anatoly May 22, 2018, 4:35 p.m. UTC
  When using --no-shconf option, the expectation is that no multiprocess
will be supported as no shared files are created. However, fbarray
still creates some shared files that prevent multiple processes with
the same prefix from starting.

Fix this by avoiding creating shared files whenever noshconf option is
specified. Since virtual areas we get from eal_get_virtual_area() are
read-only, remap them as writable.

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

Notes:
    Without this patch, EAL flags autotest will fail when attempting
    to run a test with the same prefix as primary, and --no-shconf
    specified.
    
    Technically, we never spelled out any guarantees about --no-shconf
    mode, and we've been sloppy about it, so even though we don't create
    the shared config, we still create lots of other miscelaneous files.
    This patch only fixes issue with fbarray, as this affects intialization
    of different primaries with the same prefix (fbarrays are shared too),
    but does not address the other instances where we create "shared" files
    such as hugepage info.

 lib/librte_eal/common/eal_common_fbarray.c | 71 ++++++++++++++++++------------
 1 file changed, 42 insertions(+), 29 deletions(-)
  

Comments

Thomas Monjalon May 22, 2018, 8:38 p.m. UTC | #1
22/05/2018 18:35, Anatoly Burakov:
> When using --no-shconf option, the expectation is that no multiprocess
> will be supported as no shared files are created. However, fbarray
> still creates some shared files that prevent multiple processes with
> the same prefix from starting.
> 
> Fix this by avoiding creating shared files whenever noshconf option is
> specified. Since virtual areas we get from eal_get_virtual_area() are
> read-only, remap them as writable.
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
> 
> Notes:
>     Without this patch, EAL flags autotest will fail when attempting
>     to run a test with the same prefix as primary, and --no-shconf
>     specified.
>     
>     Technically, we never spelled out any guarantees about --no-shconf
>     mode, and we've been sloppy about it, so even though we don't create
>     the shared config, we still create lots of other miscelaneous files.
>     This patch only fixes issue with fbarray, as this affects intialization
>     of different primaries with the same prefix (fbarrays are shared too),
>     but does not address the other instances where we create "shared" files
>     such as hugepage info.

Just for confirmation: this patch won't be integrated in 18.05.
  
Burakov, Anatoly May 23, 2018, 8:40 a.m. UTC | #2
On 22-May-18 9:38 PM, Thomas Monjalon wrote:
> 22/05/2018 18:35, Anatoly Burakov:
>> When using --no-shconf option, the expectation is that no multiprocess
>> will be supported as no shared files are created. However, fbarray
>> still creates some shared files that prevent multiple processes with
>> the same prefix from starting.
>>
>> Fix this by avoiding creating shared files whenever noshconf option is
>> specified. Since virtual areas we get from eal_get_virtual_area() are
>> read-only, remap them as writable.
>>
>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>> ---
>>
>> Notes:
>>      Without this patch, EAL flags autotest will fail when attempting
>>      to run a test with the same prefix as primary, and --no-shconf
>>      specified.
>>      
>>      Technically, we never spelled out any guarantees about --no-shconf
>>      mode, and we've been sloppy about it, so even though we don't create
>>      the shared config, we still create lots of other miscelaneous files.
>>      This patch only fixes issue with fbarray, as this affects intialization
>>      of different primaries with the same prefix (fbarrays are shared too),
>>      but does not address the other instances where we create "shared" files
>>      such as hugepage info.
> 
> Just for confirmation: this patch won't be integrated in 18.05.
> 

OK, no objection to that. I'll work on an expanded version for 18.08 
fixing all the inconsistencies then.
  

Patch

diff --git a/lib/librte_eal/common/eal_common_fbarray.c b/lib/librte_eal/common/eal_common_fbarray.c
index 019f84c..2c8b2c2 100644
--- a/lib/librte_eal/common/eal_common_fbarray.c
+++ b/lib/librte_eal/common/eal_common_fbarray.c
@@ -434,39 +434,52 @@  rte_fbarray_init(struct rte_fbarray *arr, const char *name, unsigned int len,
 	if (data == NULL)
 		goto fail;
 
-	eal_get_fbarray_path(path, sizeof(path), name);
+	if (internal_config.no_shconf) {
+		/* remap virtual area as writable */
+		void *new_data = mmap(data, mmap_len, PROT_READ | PROT_WRITE,
+				MAP_FIXED | MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+		if (new_data == MAP_FAILED) {
+			RTE_LOG(DEBUG, EAL, "%s(): couldn't remap anonymous memory: %s\n",
+					__func__, strerror(errno));
+			goto fail;
+		}
+	} else {
+		eal_get_fbarray_path(path, sizeof(path), name);
 
-	/*
-	 * Each fbarray is unique to process namespace, i.e. the filename
-	 * depends on process prefix. Try to take out a lock and see if we
-	 * succeed. If we don't, someone else is using it already.
-	 */
-	fd = open(path, O_CREAT | O_RDWR, 0600);
-	if (fd < 0) {
-		RTE_LOG(DEBUG, EAL, "%s(): couldn't open %s: %s\n", __func__,
-				path, strerror(errno));
-		rte_errno = errno;
-		goto fail;
-	} else if (flock(fd, LOCK_EX | LOCK_NB)) {
-		RTE_LOG(DEBUG, EAL, "%s(): couldn't lock %s: %s\n", __func__,
-				path, strerror(errno));
-		rte_errno = EBUSY;
-		goto fail;
-	}
+		/*
+		 * Each fbarray is unique to process namespace, i.e. the
+		 * filename depends on process prefix. Try to take out a lock
+		 * and see if we succeed. If we don't, someone else is using it
+		 * already.
+		 */
+		fd = open(path, O_CREAT | O_RDWR, 0600);
+		if (fd < 0) {
+			RTE_LOG(DEBUG, EAL, "%s(): couldn't open %s: %s\n",
+					__func__, path, strerror(errno));
+			rte_errno = errno;
+			goto fail;
+		} else if (flock(fd, LOCK_EX | LOCK_NB)) {
+			RTE_LOG(DEBUG, EAL, "%s(): couldn't lock %s: %s\n",
+					__func__, path, strerror(errno));
+			rte_errno = EBUSY;
+			goto fail;
+		}
 
-	/* take out a non-exclusive lock, so that other processes could still
-	 * attach to it, but no other process could reinitialize it.
-	 */
-	if (flock(fd, LOCK_SH | LOCK_NB)) {
-		rte_errno = errno;
-		goto fail;
-	}
+		/* take out a non-exclusive lock, so that other processes could
+		 * still attach to it, but no other process could reinitialize
+		 * it.
+		 */
+		if (flock(fd, LOCK_SH | LOCK_NB)) {
+			rte_errno = errno;
+			goto fail;
+		}
 
-	if (resize_and_map(fd, data, mmap_len))
-		goto fail;
+		if (resize_and_map(fd, data, mmap_len))
+			goto fail;
 
-	/* we've mmap'ed the file, we can now close the fd */
-	close(fd);
+		/* we've mmap'ed the file, we can now close the fd */
+		close(fd);
+	}
 
 	/* initialize the data */
 	memset(data, 0, mmap_len);