[dpdk-dev] [PATCH 1/3] mem: fix index for unmapping segments on failure

[dpdk-dev] [PATCH 1/3] mem: fix index for unmapping segments on failure

[Anatoly Burakov]

Segment index was calculated incorrectly, causing free_seg to
attempt to free segments that do not exist.

Fixes: a5ff05d60fc5 ("mem: support unmapping pages at runtime")
Cc: anatoly.burakov@intel.com

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/librte_eal/linuxapp/eal/eal_memalloc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_memalloc.c b/lib/librte_eal/linuxapp/eal/eal_memalloc.c
index 360d8f7..c441c89 100644
--- a/lib/librte_eal/linuxapp/eal/eal_memalloc.c
+++ b/lib/librte_eal/linuxapp/eal/eal_memalloc.c
@@ -735,8 +735,7 @@ alloc_seg_walk(const struct rte_memseg_list *msl, void *arg)
 						&cur_msl->memseg_arr;
 
 				tmp = rte_fbarray_get(arr, j);
-				if (free_seg(tmp, wa->hi, msl_idx,
-						start_idx + j)) {
+				if (free_seg(tmp, wa->hi, msl_idx, j)) {
 					RTE_LOG(ERR, EAL, "Cannot free page\n");
 					continue;
 				}
-- 
2.7.4

[dpdk-dev] [PATCH 2/3] memalloc: fix wrong return value on freeing segment on fail

[Anatoly Burakov]

Return value should be zero for success, but if unlock and unlink
have succeeded, return value was 1, which triggered failure message
in calling code.

Fixes: a5ff05d60fc5 ("mem: support unmapping pages at runtime")
Cc: anatoly.burakov@intel.com

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/librte_eal/linuxapp/eal/eal_memalloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_memalloc.c b/lib/librte_eal/linuxapp/eal/eal_memalloc.c
index c441c89..3282293 100644
--- a/lib/librte_eal/linuxapp/eal/eal_memalloc.c
+++ b/lib/librte_eal/linuxapp/eal/eal_memalloc.c
@@ -647,7 +647,7 @@ free_seg(struct rte_memseg *ms, struct hugepage_info *hi,
 
 	memset(ms, 0, sizeof(*ms));
 
-	return ret;
+	return ret < 0 ? -1 : 0;
 }
 
 struct alloc_walk_param {
-- 
2.7.4

[dpdk-dev] [PATCH 3/3] memalloc: fix unmapping and marking segments as free

[Anatoly Burakov]

Currently, page deallocation might fail if allocator cannot get page
fd, which will leave VA space still mapped, and will also not mark
page as free.

Fix page deallocation function to always unmap space before trying
to get rid of the page itself, and always mark page as free even if
page deallocation failed.

Fixes: a5ff05d60fc5 ("mem: support unmapping pages at runtime")
Fixes: 1a7dc2252f28 ("mem: revert to using flock and add per-segment lockfiles")
Cc: anatoly.burakov@intel.com

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 lib/librte_eal/linuxapp/eal/eal_memalloc.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_memalloc.c b/lib/librte_eal/linuxapp/eal/eal_memalloc.c
index 3282293..c1a0211 100644
--- a/lib/librte_eal/linuxapp/eal/eal_memalloc.c
+++ b/lib/librte_eal/linuxapp/eal/eal_memalloc.c
@@ -610,14 +610,6 @@ free_seg(struct rte_memseg *ms, struct hugepage_info *hi,
 	/* erase page data */
 	memset(ms->addr, 0, ms->len);
 
-	/* if we are not in single file segments mode, we're going to unmap the
-	 * segment and thus drop the lock on original fd, so take out another
-	 * shared lock before we do that.
-	 */
-	fd = get_seg_fd(path, sizeof(path), hi, list_idx, seg_idx);
-	if (fd < 0)
-		return -1;
-
 	if (mmap(ms->addr, ms->len, PROT_READ,
 			MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0) ==
 				MAP_FAILED) {
@@ -625,6 +617,14 @@ free_seg(struct rte_memseg *ms, struct hugepage_info *hi,
 		return -1;
 	}
 
+	/* if we are not in single file segments mode, we're going to unmap the
+	 * segment and thus drop the lock on original fd, but hugepage dir is
+	 * now locked so we can take out another one without races.
+	 */
+	fd = get_seg_fd(path, sizeof(path), hi, list_idx, seg_idx);
+	if (fd < 0)
+		return -1;
+
 	if (internal_config.single_file_segments) {
 		map_offset = seg_idx * ms->len;
 		if (resize_hugefile(fd, path, list_idx, seg_idx, map_offset,
@@ -735,12 +735,13 @@ alloc_seg_walk(const struct rte_memseg_list *msl, void *arg)
 						&cur_msl->memseg_arr;
 
 				tmp = rte_fbarray_get(arr, j);
-				if (free_seg(tmp, wa->hi, msl_idx, j)) {
-					RTE_LOG(ERR, EAL, "Cannot free page\n");
-					continue;
-				}
-
 				rte_fbarray_set_free(arr, j);
+
+				/* free_seg may attempt to create a file, which
+				 * may fail.
+				 */
+				if (free_seg(tmp, wa->hi, msl_idx, j))
+					RTE_LOG(DEBUG, EAL, "Cannot free page\n");
 			}
 			/* clear the list */
 			if (wa->ms)
-- 
2.7.4

Re: [dpdk-dev] [PATCH 1/3] mem: fix index for unmapping segments on failure

[Liu, Yong]

Tested-by: Marvin Liu <yong.liu@intel.com>

> -----Original Message-----
> From: Burakov, Anatoly
> Sent: Thursday, May 03, 2018 6:11 PM
> To: dev@dpdk.org
> Cc: Liu, Yong <yong.liu@intel.com>; Burakov, Anatoly
> <anatoly.burakov@intel.com>
> Subject: [PATCH 1/3] mem: fix index for unmapping segments on failure
> 
> Segment index was calculated incorrectly, causing free_seg to
> attempt to free segments that do not exist.
> 
> Fixes: a5ff05d60fc5 ("mem: support unmapping pages at runtime")
> Cc: anatoly.burakov@intel.com
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>  lib/librte_eal/linuxapp/eal/eal_memalloc.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/lib/librte_eal/linuxapp/eal/eal_memalloc.c
> b/lib/librte_eal/linuxapp/eal/eal_memalloc.c
> index 360d8f7..c441c89 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_memalloc.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_memalloc.c
> @@ -735,8 +735,7 @@ alloc_seg_walk(const struct rte_memseg_list *msl, void
> *arg)
>  						&cur_msl->memseg_arr;
> 
>  				tmp = rte_fbarray_get(arr, j);
> -				if (free_seg(tmp, wa->hi, msl_idx,
> -						start_idx + j)) {
> +				if (free_seg(tmp, wa->hi, msl_idx, j)) {
>  					RTE_LOG(ERR, EAL, "Cannot free page\n");
>  					continue;
>  				}
> --
> 2.7.4

Re: [dpdk-dev] [PATCH 2/3] memalloc: fix wrong return value on freeing segment on fail

[Ananyev, Konstantin]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Anatoly Burakov
> Sent: Thursday, May 3, 2018 11:11 AM
> To: dev@dpdk.org
> Cc: Liu, Yong <yong.liu@intel.com>; Burakov, Anatoly <anatoly.burakov@intel.com>
> Subject: [dpdk-dev] [PATCH 2/3] memalloc: fix wrong return value on freeing segment on fail
> 
> Return value should be zero for success, but if unlock and unlink
> have succeeded, return value was 1, which triggered failure message
> in calling code.
> 
> Fixes: a5ff05d60fc5 ("mem: support unmapping pages at runtime")
> Cc: anatoly.burakov@intel.com
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---

Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

Re: [dpdk-dev] [PATCH 3/3] memalloc: fix unmapping and marking segments as free

[Ananyev, Konstantin]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Anatoly Burakov
> Sent: Thursday, May 3, 2018 11:11 AM
> To: dev@dpdk.org
> Cc: Liu, Yong <yong.liu@intel.com>; Burakov, Anatoly <anatoly.burakov@intel.com>
> Subject: [dpdk-dev] [PATCH 3/3] memalloc: fix unmapping and marking segments as free
> 
> Currently, page deallocation might fail if allocator cannot get page
> fd, which will leave VA space still mapped, and will also not mark
> page as free.
> 
> Fix page deallocation function to always unmap space before trying
> to get rid of the page itself, and always mark page as free even if
> page deallocation failed.
> 
> Fixes: a5ff05d60fc5 ("mem: support unmapping pages at runtime")
> Fixes: 1a7dc2252f28 ("mem: revert to using flock and add per-segment lockfiles")
> Cc: anatoly.burakov@intel.com
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---

Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

Re: [dpdk-dev] [PATCH 1/3] mem: fix index for unmapping segments on failure

[Ananyev, Konstantin]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Anatoly Burakov
> Sent: Thursday, May 3, 2018 11:11 AM
> To: dev@dpdk.org
> Cc: Liu, Yong <yong.liu@intel.com>; Burakov, Anatoly <anatoly.burakov@intel.com>
> Subject: [dpdk-dev] [PATCH 1/3] mem: fix index for unmapping segments on failure
> 
> Segment index was calculated incorrectly, causing free_seg to
> attempt to free segments that do not exist.
> 
> Fixes: a5ff05d60fc5 ("mem: support unmapping pages at runtime")
> Cc: anatoly.burakov@intel.com
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---

Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

Re: [dpdk-dev] [PATCH 1/3] mem: fix index for unmapping segments on failure

[Thomas Monjalon]

> > Segment index was calculated incorrectly, causing free_seg to
> > attempt to free segments that do not exist.
> > 
> > Fixes: a5ff05d60fc5 ("mem: support unmapping pages at runtime")
> > Cc: anatoly.burakov@intel.com
> > 
> > Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> 
> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

Series applied, thanks