[dpdk-stable] [PATCH v3 1/2] memalloc: do not leave unmapped holes in EAL virtual memory area

[dpdk-stable] [PATCH v3 1/2] memalloc: do not leave unmapped holes in EAL virtual memory area

[Dariusz Stojaczyk]

EAL reserves a huge area in virtual address space
to provide virtual address contiguity for e.g.
future memory extensions (memory hotplug). During
memory hotplug, if the hugepage mmap succeeds but
doesn't suffice EAL's requiriments, the EAL would
unmap this mapping straight away, leaving a hole in
its virtual memory area and making it available
to everyone. As EAL still thinks it owns the entire
region, it may try to mmap it later with MAP_FIXED,
possibly overriding a user's mapping that was made
in the meantime.

This patch ensures each hole is mapped back by EAL,
so that it won't be available to anyone else.

Changes from v2:
  * replaced rte_panic() with a CRIT log.
  * added "git fixline" tags

Changes from v1:
 * checkpatch fixes

Fixes: 582bed1e1d1d ("mem: support mapping hugepages at runtime")
Cc: anatoly.burakov@intel.com
Cc: stable@dpdk.org

Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
---
 lib/librte_eal/linuxapp/eal/eal_memalloc.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/lib/librte_eal/linuxapp/eal/eal_memalloc.c b/lib/librte_eal/linuxapp/eal/eal_memalloc.c
index 8c11f98..6be6680 100644
--- a/lib/librte_eal/linuxapp/eal/eal_memalloc.c
+++ b/lib/librte_eal/linuxapp/eal/eal_memalloc.c
@@ -39,6 +39,7 @@
 #include "eal_filesystem.h"
 #include "eal_internal_cfg.h"
 #include "eal_memalloc.h"
+#include "eal_private.h"
 
 /*
  * not all kernel version support fallocate on hugetlbfs, so fall back to
@@ -490,6 +491,8 @@ alloc_seg(struct rte_memseg *ms, void *addr, int socket_id,
 	int ret = 0;
 	int fd;
 	size_t alloc_sz;
+	int flags;
+	void *new_addr;
 
 	/* takes out a read lock on segment or segment list */
 	fd = get_seg_fd(path, sizeof(path), hi, list_idx, seg_idx);
@@ -585,6 +588,20 @@ alloc_seg(struct rte_memseg *ms, void *addr, int socket_id,
 
 mapped:
 	munmap(addr, alloc_sz);
+	flags = MAP_FIXED;
+#ifdef RTE_ARCH_PPC_64
+	flags |= MAP_HUGETLB;
+#endif
+	new_addr = eal_get_virtual_area(addr, &alloc_sz, alloc_sz, 0, flags);
+	if (new_addr != addr) {
+		if (new_addr != NULL)
+			munmap(new_addr, alloc_sz);
+		/* we're leaving a hole in our virtual address space. if
+		 * somebody else maps this hole now, we could accidentally
+		 * override it in the future.
+		 */
+		RTE_LOG(CRIT, EAL, "Can't mmap holes in our virtual address space\n");
+	}
 resized:
 	if (internal_config.single_file_segments) {
 		resize_hugefile(fd, path, list_idx, seg_idx, map_offset,
-- 
2.7.4

[dpdk-stable] [PATCH v3 2/2] memalloc: keep in mind a failed MAP_FIXED mmap may still perform an unmap

[Dariusz Stojaczyk]

This isn't documented in the manuals, but a failed
mmap(..., MAP_FIXED) may still unmap overlapping
regions. In such case, we need to remap these regions
back into our address space to ensure mem contiguity.
We do it unconditionally now on mmap failure just to
be safe.

Verified on Linux 4.9.0-4-amd64. I was getting
ENOMEM when trying to map hugetlbfs with no space
left, and the previous anonymous mapping was still
being removed.

Changes from v2:
 * added "git fixline" tags

Changes from v1:
 * checkpatch fixes
 * remapping is now done regardless of the mmap errno

Fixes: 582bed1e1d1d ("mem: support mapping hugepages at runtime")
Cc: anatoly.burakov@intel.com
Cc: stable@dpdk.org

Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
---
 lib/librte_eal/linuxapp/eal/eal_memalloc.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_memalloc.c b/lib/librte_eal/linuxapp/eal/eal_memalloc.c
index 6be6680..81c94d5 100644
--- a/lib/librte_eal/linuxapp/eal/eal_memalloc.c
+++ b/lib/librte_eal/linuxapp/eal/eal_memalloc.c
@@ -527,7 +527,10 @@ alloc_seg(struct rte_memseg *ms, void *addr, int socket_id,
 	if (va == MAP_FAILED) {
 		RTE_LOG(DEBUG, EAL, "%s(): mmap() failed: %s\n", __func__,
 			strerror(errno));
-		goto resized;
+		/* mmap failed, but the previous region might have been
+		 * unmapped anyway. try to remap it
+		 */
+		goto unmapped;
 	}
 	if (va != addr) {
 		RTE_LOG(DEBUG, EAL, "%s(): wrong mmap() address\n", __func__);
@@ -588,6 +591,7 @@ alloc_seg(struct rte_memseg *ms, void *addr, int socket_id,
 
 mapped:
 	munmap(addr, alloc_sz);
+unmapped:
 	flags = MAP_FIXED;
 #ifdef RTE_ARCH_PPC_64
 	flags |= MAP_HUGETLB;
-- 
2.7.4

Re: [dpdk-stable] [PATCH v3 2/2] memalloc: keep in mind a failed MAP_FIXED mmap may still perform an unmap

[Burakov, Anatoly]

On 01-Jun-18 1:59 PM, Dariusz Stojaczyk wrote:
> This isn't documented in the manuals, but a failed
> mmap(..., MAP_FIXED) may still unmap overlapping
> regions. In such case, we need to remap these regions
> back into our address space to ensure mem contiguity.
> We do it unconditionally now on mmap failure just to
> be safe.
> 
> Verified on Linux 4.9.0-4-amd64. I was getting
> ENOMEM when trying to map hugetlbfs with no space
> left, and the previous anonymous mapping was still
> being removed.
> 
> Changes from v2:
>   * added "git fixline" tags
> 
> Changes from v1:
>   * checkpatch fixes
>   * remapping is now done regardless of the mmap errno
> 
> Fixes: 582bed1e1d1d ("mem: support mapping hugepages at runtime")
> Cc: anatoly.burakov@intel.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
> ---

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

Version changes should not be part of commit message. Thomas, could you 
please fix this on apply?

-- 
Thanks,
Anatoly

Re: [dpdk-stable] [PATCH v3 1/2] memalloc: do not leave unmapped holes in EAL virtual memory area

[Burakov, Anatoly]

On 01-Jun-18 1:59 PM, Dariusz Stojaczyk wrote:
> EAL reserves a huge area in virtual address space
> to provide virtual address contiguity for e.g.
> future memory extensions (memory hotplug). During
> memory hotplug, if the hugepage mmap succeeds but
> doesn't suffice EAL's requiriments, the EAL would
> unmap this mapping straight away, leaving a hole in
> its virtual memory area and making it available
> to everyone. As EAL still thinks it owns the entire
> region, it may try to mmap it later with MAP_FIXED,
> possibly overriding a user's mapping that was made
> in the meantime.
> 
> This patch ensures each hole is mapped back by EAL,
> so that it won't be available to anyone else.
> 
> Changes from v2:
>    * replaced rte_panic() with a CRIT log.
>    * added "git fixline" tags
> 
> Changes from v1:
>   * checkpatch fixes
> 
> Fixes: 582bed1e1d1d ("mem: support mapping hugepages at runtime")
> Cc: anatoly.burakov@intel.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
> ---

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

Thomas, could you please fix the commit message on apply?

-- 
Thanks,
Anatoly

Re: [dpdk-stable] [PATCH v3 1/2] memalloc: do not leave unmapped holes in EAL virtual memory area

[Thomas Monjalon]

01/06/2018 17:06, Burakov, Anatoly:
> On 01-Jun-18 1:59 PM, Dariusz Stojaczyk wrote:
> > EAL reserves a huge area in virtual address space
> > to provide virtual address contiguity for e.g.
> > future memory extensions (memory hotplug). During
> > memory hotplug, if the hugepage mmap succeeds but
> > doesn't suffice EAL's requiriments, the EAL would
> > unmap this mapping straight away, leaving a hole in
> > its virtual memory area and making it available
> > to everyone. As EAL still thinks it owns the entire
> > region, it may try to mmap it later with MAP_FIXED,
> > possibly overriding a user's mapping that was made
> > in the meantime.
> > 
> > This patch ensures each hole is mapped back by EAL,
> > so that it won't be available to anyone else.
> > 
> > Changes from v2:
> >    * replaced rte_panic() with a CRIT log.
> >    * added "git fixline" tags
> > 
> > Changes from v1:
> >   * checkpatch fixes
> > 
> > Fixes: 582bed1e1d1d ("mem: support mapping hugepages at runtime")
> > Cc: anatoly.burakov@intel.com
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
> > ---
> 
> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
> 
> Thomas, could you please fix the commit message on apply?

Yes

Applied, thanks