> -----Original Message-----
> From: lng-odp-boun...@lists.linaro.org [mailto:lng-odp-
> boun...@lists.linaro.org] On Behalf Of ext Maxim Uvarov
> Sent: Monday, January 26, 2015 2:13 PM
> To: lng-odp@lists.linaro.org
> Subject: [lng-odp] [PATCHv5] hugepages: align mmap size for hugepages
>
> In case of hugepages munmap requires size aligned to page.
>
> Signed-off-by: Maxim Uvarov <maxim.uva...@linaro.org>
> ---
> v5: - fix alloc_hp_size define (somehow arm gcc finds that it can be
> uninitialized;
> - remove goto and make if cases more readable;
>
>
> platform/linux-generic/odp_shared_memory.c | 73 +++++++++++++++++++------
> -----
> test/validation/odp_shm.c | 4 ++
> 2 files changed, 50 insertions(+), 27 deletions(-)
>
> diff --git a/platform/linux-generic/odp_shared_memory.c b/platform/linux-
> generic/odp_shared_memory.c
> index 23a9ceb..4689c5d 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 page_sz, huge_sz;
> + int ret;
> +#ifdef MAP_HUGETLB
> + int need_huge_page = 0;
> + uint64_t alloc_hp_size;
> +#endif
>
> - 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);
> + /* 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;
> }
> @@ -230,32 +235,47 @@ odp_shm_t odp_shm_reserve(const char *name, uint64_t
> size, uint64_t align,
> block = &odp_shm_tbl->block[i];
>
> block->hdl = to_handle(i);
> - block->huge = 0;
> addr = MAP_FAILED;
>
> #ifdef MAP_HUGETLB
> /* Try first huge pages */
> - if (huge_sz && alloc_size > page_sz) {
> - addr = mmap(NULL, alloc_size, PROT_READ | PROT_WRITE,
> - map_flag | MAP_HUGETLB, fd, 0);
> + if (need_huge_page) {
> + ret = 0;
> + if (flags & ODP_SHM_PROC)
> + ret = ftruncate(fd, alloc_hp_size);
It's an error if ftruncate fails, so just return SHM_INVALID.
if (ftruncate(fd, alloc_hp_size) == -1) {
ODP_DBG("odp_shm_reserve: ftruncate failed\n");
return ODP_SHM_INVALID;
}
> +
> + if (ret == 0) {
This if-clause can be then removed...
> + addr = mmap(NULL, alloc_hp_size, PROT_READ | PROT_WRITE,
> + map_flag | MAP_HUGETLB, fd, 0);
> + if (addr == MAP_FAILED) {
> + ODP_DBG("odp_shm_reserve: mmap HP failed\n");
Depending on system / config there may be only e.g. 4 huge pages available. So
it may be pretty common that you run out of huge pages and use normal pages
instead. I'd leave the warning out from here.
> + } else {
> + block->alloc_size = alloc_hp_size;
> + block->huge = 1;
> + block->page_sz = huge_sz;
> + }
> + }
> }
> #endif
>
> /* Use normal pages for small or failed huge page allocations */
> if (addr == MAP_FAILED) {
> - addr = mmap(NULL, alloc_size, PROT_READ | PROT_WRITE,
> - map_flag, fd, 0);
> - block->page_sz = page_sz;
> - } else {
> - block->huge = 1;
> - block->page_sz = huge_sz;
> - }
> -
> - if (addr == MAP_FAILED) {
> - /* Alloc failed */
> - odp_spinlock_unlock(&odp_shm_tbl->lock);
> - ODP_DBG("odp_shm_reserve: mmap failed\n");
> - return ODP_SHM_INVALID;
> + ret = 0;
> + if (flags & ODP_SHM_PROC)
> + ret = ftruncate(fd, alloc_size);
Same thing, return SHM_INVALID on fruncate failure.
-Petri
> + if (ret == 0) {
> + addr = mmap(NULL, alloc_size, PROT_READ | PROT_WRITE,
> + map_flag, fd, 0);
> + if (addr == MAP_FAILED) {
> + odp_spinlock_unlock(&odp_shm_tbl->lock);
> + ODP_DBG("odp_shm_reserve: mmap failed\n");
> + return ODP_SHM_INVALID;
> + } else {
> + block->alloc_size = alloc_size;
> + block->huge = 0;
> + block->page_sz = page_sz;
> + }
> + }
> }
>
> block->addr_orig = addr;
> @@ -267,7 +287,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);
> --
> 1.8.5.1.163.gd7aced9
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/lng-odp
