diff mbox

[PATCHv4,3/3] hugepages: align mmap size for hugepages

Message ID 1422016474-8676-3-git-send-email-maxim.uvarov@linaro.org
State New
Headers show

Commit Message

Maxim Uvarov Jan. 23, 2015, 12:34 p.m. UTC 
In case of hugepages munmap requires size aligned to page.

Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
---
 v4: if alloc huge pages failed fall back to normal pages with original size.

 platform/linux-generic/odp_shared_memory.c | 43 +++++++++++++++++++++---------
 test/validation/odp_shm.c                  |  4 +++
 2 files changed, 34 insertions(+), 13 deletions(-)

Comments

Maxim Uvarov Jan. 23, 2015, 3:52 p.m. UTC | #1 
On 01/23/2015 04:54 PM, Savolainen, Petri (NSN - FI/Espoo) wrote:
> More readable code with one if-clause and no goto:
>
> 		if (fd != -1 && ftruncate(fd, alloc_hp_size) != -1) {
> 			addr = mmap(NULL, alloc_hp_size, PROT_READ | PROT_WRITE,
>   				    map_flag | MAP_HUGETLB, fd, 0);
> 		}
I thought that mixing variable check and function return does not look 
nice, so I placed it to 2 ifs.
But I can redo it, no problem.

Maxim.
Maxim Uvarov Jan. 23, 2015, 4:25 p.m. UTC | #2 
On 01/23/2015 04:54 PM, Savolainen, Petri (NSN - FI/Espoo) wrote:
> More readable code with one if-clause and no goto:
>
> 		if (fd != -1 && ftruncate(fd, alloc_hp_size) != -1) {
> 			addr = mmap(NULL, alloc_hp_size, PROT_READ | PROT_WRITE,
>   				    map_flag | MAP_HUGETLB, fd, 0);
> 		}
Petri, no that will not work. fd is only if ODP_SHM_PROC. But mmap 
hugepages should be done not depending ODP_SHM_PROC is provided or not.

Maxim.
diff mbox

Patch

diff --git a/platform/linux-generic/odp_shared_memory.c b/platform/linux-generic/odp_shared_memory.c
index 23a9ceb..2004f30 100644
--- a/platform/linux-generic/odp_shared_memory.c
+++ b/platform/linux-generic/odp_shared_memory.c
@@ -179,27 +179,32 @@  odp_shm_t odp_shm_reserve(const char *name, uint64_t size, uint64_t align,
 	int map_flag = MAP_SHARED;
 	/* If already exists: O_EXCL: error, O_TRUNC: truncate to zero */
 	int oflag = O_RDWR | O_CREAT | O_TRUNC;
-	uint64_t alloc_size = size + align;
+	uint64_t alloc_size;
+	uint64_t alloc_hp_size;
 	uint64_t page_sz, huge_sz;
+	int need_huge_page = 0;
 
-	huge_sz = odp_sys_huge_page_size();
 	page_sz = odp_sys_page_size();
+	alloc_size = size + align;
+
+#ifdef MAP_HUGETLB
+	huge_sz = odp_sys_huge_page_size();
+	need_huge_page =  (huge_sz && alloc_size > page_sz);
+	if (need_huge_page) {
+		/* munmap for huge pages requires sizes round up by page */
+		alloc_hp_size = (size + align + (huge_sz - 1))
+			      & (-huge_sz);
+	}
+#endif
 
 	if (flags & ODP_SHM_PROC) {
 		/* Creates a file to /dev/shm */
 		fd = shm_open(name, oflag,
 			      S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
-
 		if (fd == -1) {
 			ODP_DBG("odp_shm_reserve: shm_open failed\n");
 			return ODP_SHM_INVALID;
 		}
-
-		if (ftruncate(fd, alloc_size) == -1) {
-			ODP_DBG("odp_shm_reserve: ftruncate failed\n");
-			return ODP_SHM_INVALID;
-		}
-
 	} else {
 		map_flag |= MAP_ANONYMOUS;
 	}
@@ -235,18 +240,31 @@  odp_shm_t odp_shm_reserve(const char *name, uint64_t size, uint64_t align,
 
 #ifdef MAP_HUGETLB
 	/* Try first huge pages */
-	if (huge_sz && alloc_size > page_sz) {
-		addr = mmap(NULL, alloc_size, PROT_READ | PROT_WRITE,
+	if (need_huge_page) {
+		if (flags & ODP_SHM_PROC) {
+			if (ftruncate(fd, alloc_hp_size) == -1)
+				goto normal_pages;
+		}
+		addr = mmap(NULL, alloc_hp_size, PROT_READ | PROT_WRITE,
 			    map_flag | MAP_HUGETLB, fd, 0);
 	}
+normal_pages:
 #endif
-
 	/* Use normal pages for small or failed huge page allocations */
 	if (addr == MAP_FAILED) {
+		if (flags & ODP_SHM_PROC) {
+			if (ftruncate(fd, alloc_size) == -1) {
+				odp_spinlock_unlock(&odp_shm_tbl->lock);
+				ODP_DBG("odp_shm_reserve: ftruncate failed\n");
+				return ODP_SHM_INVALID;
+			}
+		}
 		addr = mmap(NULL, alloc_size, PROT_READ | PROT_WRITE,
 			    map_flag, fd, 0);
+		block->alloc_size = alloc_size;
 		block->page_sz = page_sz;
 	} else {
+		block->alloc_size = alloc_hp_size;
 		block->huge    = 1;
 		block->page_sz = huge_sz;
 	}
@@ -267,7 +285,6 @@  odp_shm_t odp_shm_reserve(const char *name, uint64_t size, uint64_t align,
 	block->name[ODP_SHM_NAME_LEN - 1] = 0;
 	block->size       = size;
 	block->align      = align;
-	block->alloc_size = alloc_size;
 	block->flags      = flags;
 	block->fd         = fd;
 	block->addr       = addr;
diff --git a/test/validation/odp_shm.c b/test/validation/odp_shm.c
index c26925b..4b1a38e 100644
--- a/test/validation/odp_shm.c
+++ b/test/validation/odp_shm.c
@@ -32,7 +32,11 @@  static void *run_shm_thread(void *arg)
 	CU_ASSERT(0 == info.flags);
 	CU_ASSERT(test_shared_data == info.addr);
 	CU_ASSERT(sizeof(test_shared_data_t) <= info.size);
+#ifdef MAP_HUGETLB
+	CU_ASSERT(odp_sys_huge_page_size() == info.page_size);
+#else
 	CU_ASSERT(odp_sys_page_size() == info.page_size);
+#endif
 	odp_shm_print_all();
 
 	fflush(stdout);