| Message ID | 1422276871-9233-1-git-send-email-maxim.uvarov@linaro.org |
|---|---|
| State | Accepted |
| Commit | 575fbb48e18c5f5017c44c3b178a96c9ba0301d7 |
| Headers | show |
On 01/26/2015 04:49 PM, Savolainen, Petri (NSN - FI/Espoo) wrote: > Reviewed-by: Petri Savolainen <petri.savolainen@linaro.org> > > > ODP_DBG("odp_shm_reserve: mmap HP failed\n") is still there but we can remove it later if it get annoying on some configurations. > ODP_DBG is debug information. So we can see that allocation for hp failed and we go to normal page. Of course we can check block->page_sz but is debug enable I think message is good. Maxim. > -Petri > > >> -----Original Message----- >> From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp- >> bounces@lists.linaro.org] On Behalf Of ext Maxim Uvarov >> Sent: Monday, January 26, 2015 2:55 PM >> To: lng-odp@lists.linaro.org >> Subject: [lng-odp] [PATCHv6] hugepages: align mmap size for hugepages >> >> In case of hugepages munmap requires size aligned to page. >> >> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> >> --- >> v6: error if ftuncate fails in HP case. >> >> platform/linux-generic/odp_shared_memory.c | 73 +++++++++++++++++++------ >> ----- >> test/validation/odp_shm.c | 4 ++ >> 2 files changed, 51 insertions(+), 26 deletions(-) >> >> diff --git a/platform/linux-generic/odp_shared_memory.c b/platform/linux- >> generic/odp_shared_memory.c >> index 23a9ceb..515a26f 100644 >> --- a/platform/linux-generic/odp_shared_memory.c >> +++ b/platform/linux-generic/odp_shared_memory.c >> @@ -179,27 +179,31 @@ 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; >> +#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 +234,50 @@ 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) { >> + if ((flags & ODP_SHM_PROC) && >> + (ftruncate(fd, alloc_hp_size) == -1)) { >> + odp_spinlock_unlock(&odp_shm_tbl->lock); >> + ODP_DBG("odp_shm_reserve: ftruncate HP failed\n"); >> + return ODP_SHM_INVALID; >> + } >> + >> + 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"); >> + } 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 ((flags & ODP_SHM_PROC) && >> + (ftruncate(fd, alloc_size) == -1)) { >> + odp_spinlock_unlock(&odp_shm_tbl->lock); >> + ODP_ERR("odp_shm_reserve: ftruncate failed\n"); >> + return ODP_SHM_INVALID; >> + } >> >> - 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; >> + 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 +289,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
Merged + previous 2 patches from this patchset. Maxim. On 01/26/2015 04:49 PM, Savolainen, Petri (NSN - FI/Espoo) wrote: > Reviewed-by: Petri Savolainen <petri.savolainen@linaro.org> > > > ODP_DBG("odp_shm_reserve: mmap HP failed\n") is still there but we can remove it later if it get annoying on some configurations. > > > -Petri > > >> -----Original Message----- >> From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp- >> bounces@lists.linaro.org] On Behalf Of ext Maxim Uvarov >> Sent: Monday, January 26, 2015 2:55 PM >> To: lng-odp@lists.linaro.org >> Subject: [lng-odp] [PATCHv6] hugepages: align mmap size for hugepages >> >> In case of hugepages munmap requires size aligned to page. >> >> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> >> --- >> v6: error if ftuncate fails in HP case. >> >> platform/linux-generic/odp_shared_memory.c | 73 +++++++++++++++++++------ >> ----- >> test/validation/odp_shm.c | 4 ++ >> 2 files changed, 51 insertions(+), 26 deletions(-) >> >> diff --git a/platform/linux-generic/odp_shared_memory.c b/platform/linux- >> generic/odp_shared_memory.c >> index 23a9ceb..515a26f 100644 >> --- a/platform/linux-generic/odp_shared_memory.c >> +++ b/platform/linux-generic/odp_shared_memory.c >> @@ -179,27 +179,31 @@ 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; >> +#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 +234,50 @@ 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) { >> + if ((flags & ODP_SHM_PROC) && >> + (ftruncate(fd, alloc_hp_size) == -1)) { >> + odp_spinlock_unlock(&odp_shm_tbl->lock); >> + ODP_DBG("odp_shm_reserve: ftruncate HP failed\n"); >> + return ODP_SHM_INVALID; >> + } >> + >> + 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"); >> + } 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 ((flags & ODP_SHM_PROC) && >> + (ftruncate(fd, alloc_size) == -1)) { >> + odp_spinlock_unlock(&odp_shm_tbl->lock); >> + ODP_ERR("odp_shm_reserve: ftruncate failed\n"); >> + return ODP_SHM_INVALID; >> + } >> >> - 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; >> + 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 +289,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
On 26 January 2015 at 08:49, Savolainen, Petri (NSN - FI/Espoo) <petri. savolainen@nsn.com> wrote: > Reviewed-by: Petri Savolainen <petri.savolainen@linaro.org> > > > ODP_DBG("odp_shm_reserve: mmap HP failed\n") is still there but we can > remove it later if it get annoying on some configurations. > It is annoying already, the validation logs are full of it and it obscures the real results > > > -Petri > > > > -----Original Message----- > > From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp- > > bounces@lists.linaro.org] On Behalf Of ext Maxim Uvarov > > Sent: Monday, January 26, 2015 2:55 PM > > To: lng-odp@lists.linaro.org > > Subject: [lng-odp] [PATCHv6] hugepages: align mmap size for hugepages > > > > In case of hugepages munmap requires size aligned to page. > > > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> > > --- > > v6: error if ftuncate fails in HP case. > > > > platform/linux-generic/odp_shared_memory.c | 73 > +++++++++++++++++++------ > > ----- > > test/validation/odp_shm.c | 4 ++ > > 2 files changed, 51 insertions(+), 26 deletions(-) > > > > diff --git a/platform/linux-generic/odp_shared_memory.c b/platform/linux- > > generic/odp_shared_memory.c > > index 23a9ceb..515a26f 100644 > > --- a/platform/linux-generic/odp_shared_memory.c > > +++ b/platform/linux-generic/odp_shared_memory.c > > @@ -179,27 +179,31 @@ 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; > > +#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 +234,50 @@ 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) { > > + if ((flags & ODP_SHM_PROC) && > > + (ftruncate(fd, alloc_hp_size) == -1)) { > > + odp_spinlock_unlock(&odp_shm_tbl->lock); > > + ODP_DBG("odp_shm_reserve: ftruncate HP failed\n"); > > + return ODP_SHM_INVALID; > > + } > > + > > + 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"); > > + } 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 ((flags & ODP_SHM_PROC) && > > + (ftruncate(fd, alloc_size) == -1)) { > > + odp_spinlock_unlock(&odp_shm_tbl->lock); > > + ODP_ERR("odp_shm_reserve: ftruncate failed\n"); > > + return ODP_SHM_INVALID; > > + } > > > > - 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; > > + 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 +289,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 >
On 27 January 2015 at 20:54, Mike Holmes <mike.holmes@linaro.org> wrote: > > > On 26 January 2015 at 08:49, Savolainen, Petri (NSN - FI/Espoo) > <petri.savolainen@nsn.com> wrote: >> >> Reviewed-by: Petri Savolainen <petri.savolainen@linaro.org> >> >> >> ODP_DBG("odp_shm_reserve: mmap HP failed\n") is still there but we can >> remove it later if it get annoying on some configurations. > > > It is annoying already, the validation logs are full of it and it obscures > the real results Seems like this is a good candidate for a logging system with levels and classes. This would be a message of the warning level of the class odp_shm. Such messages could be quietly discarded using a suitable configuration for a specific (or all) unit test. -- Ola > >> >> >> >> -Petri >> >> >> > -----Original Message----- >> > From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp- >> > bounces@lists.linaro.org] On Behalf Of ext Maxim Uvarov >> > Sent: Monday, January 26, 2015 2:55 PM >> > To: lng-odp@lists.linaro.org >> > Subject: [lng-odp] [PATCHv6] hugepages: align mmap size for hugepages >> > >> > In case of hugepages munmap requires size aligned to page. >> > >> > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> >> > --- >> > v6: error if ftuncate fails in HP case. >> > >> > platform/linux-generic/odp_shared_memory.c | 73 >> > +++++++++++++++++++------ >> > ----- >> > test/validation/odp_shm.c | 4 ++ >> > 2 files changed, 51 insertions(+), 26 deletions(-) >> > >> > diff --git a/platform/linux-generic/odp_shared_memory.c >> > b/platform/linux- >> > generic/odp_shared_memory.c >> > index 23a9ceb..515a26f 100644 >> > --- a/platform/linux-generic/odp_shared_memory.c >> > +++ b/platform/linux-generic/odp_shared_memory.c >> > @@ -179,27 +179,31 @@ 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; >> > +#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 +234,50 @@ 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) { >> > + if ((flags & ODP_SHM_PROC) && >> > + (ftruncate(fd, alloc_hp_size) == -1)) { >> > + odp_spinlock_unlock(&odp_shm_tbl->lock); >> > + ODP_DBG("odp_shm_reserve: ftruncate HP failed\n"); >> > + return ODP_SHM_INVALID; >> > + } >> > + >> > + 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"); >> > + } 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 ((flags & ODP_SHM_PROC) && >> > + (ftruncate(fd, alloc_size) == -1)) { >> > + odp_spinlock_unlock(&odp_shm_tbl->lock); >> > + ODP_ERR("odp_shm_reserve: ftruncate failed\n"); >> > + return ODP_SHM_INVALID; >> > + } >> > >> > - 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; >> > + 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 +289,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 > > > > > -- > Mike Holmes > Linaro Sr Technical Manager > LNG - ODP > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp >
On 27 January 2015 at 15:15, Ola Liljedahl <ola.liljedahl@linaro.org> wrote: > On 27 January 2015 at 20:54, Mike Holmes <mike.holmes@linaro.org> wrote: > > > > > > On 26 January 2015 at 08:49, Savolainen, Petri (NSN - FI/Espoo) > > <petri.savolainen@nsn.com> wrote: > >> > >> Reviewed-by: Petri Savolainen <petri.savolainen@linaro.org> > >> > >> > >> ODP_DBG("odp_shm_reserve: mmap HP failed\n") is still there but we can > >> remove it later if it get annoying on some configurations. > > > > > > It is annoying already, the validation logs are full of it and it > obscures > > the real results > Seems like this is a good candidate for a logging system with levels > and classes. This would be a message of the warning level of the class > odp_shm. Such messages could be quietly discarded using a suitable > configuration for a specific (or all) unit test. > > I have used systems that allow logging per logical area and that could help, it does help in controlling the flow where you can't afford the bandwidth to log everything. For the short term in all validation tests we could replace the default log with our own validation logging and throw the messages on the floor. > -- Ola > > > > >> > >> > >> > >> -Petri > >> > >> > >> > -----Original Message----- > >> > From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp- > >> > bounces@lists.linaro.org] On Behalf Of ext Maxim Uvarov > >> > Sent: Monday, January 26, 2015 2:55 PM > >> > To: lng-odp@lists.linaro.org > >> > Subject: [lng-odp] [PATCHv6] hugepages: align mmap size for hugepages > >> > > >> > In case of hugepages munmap requires size aligned to page. > >> > > >> > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> > >> > --- > >> > v6: error if ftuncate fails in HP case. > >> > > >> > platform/linux-generic/odp_shared_memory.c | 73 > >> > +++++++++++++++++++------ > >> > ----- > >> > test/validation/odp_shm.c | 4 ++ > >> > 2 files changed, 51 insertions(+), 26 deletions(-) > >> > > >> > diff --git a/platform/linux-generic/odp_shared_memory.c > >> > b/platform/linux- > >> > generic/odp_shared_memory.c > >> > index 23a9ceb..515a26f 100644 > >> > --- a/platform/linux-generic/odp_shared_memory.c > >> > +++ b/platform/linux-generic/odp_shared_memory.c > >> > @@ -179,27 +179,31 @@ 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; > >> > +#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 +234,50 @@ 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) { > >> > + if ((flags & ODP_SHM_PROC) && > >> > + (ftruncate(fd, alloc_hp_size) == -1)) { > >> > + odp_spinlock_unlock(&odp_shm_tbl->lock); > >> > + ODP_DBG("odp_shm_reserve: ftruncate HP > failed\n"); > >> > + return ODP_SHM_INVALID; > >> > + } > >> > + > >> > + 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"); > >> > + } 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 ((flags & ODP_SHM_PROC) && > >> > + (ftruncate(fd, alloc_size) == -1)) { > >> > + odp_spinlock_unlock(&odp_shm_tbl->lock); > >> > + ODP_ERR("odp_shm_reserve: ftruncate failed\n"); > >> > + return ODP_SHM_INVALID; > >> > + } > >> > > >> > - 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; > >> > + 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 +289,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 > > > > > > > > > > -- > > Mike Holmes > > Linaro Sr Technical Manager > > LNG - ODP > > > > _______________________________________________ > > lng-odp mailing list > > lng-odp@lists.linaro.org > > http://lists.linaro.org/mailman/listinfo/lng-odp > > >
diff --git a/platform/linux-generic/odp_shared_memory.c b/platform/linux-generic/odp_shared_memory.c index 23a9ceb..515a26f 100644 --- a/platform/linux-generic/odp_shared_memory.c +++ b/platform/linux-generic/odp_shared_memory.c @@ -179,27 +179,31 @@ 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; +#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 +234,50 @@ 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) { + if ((flags & ODP_SHM_PROC) && + (ftruncate(fd, alloc_hp_size) == -1)) { + odp_spinlock_unlock(&odp_shm_tbl->lock); + ODP_DBG("odp_shm_reserve: ftruncate HP failed\n"); + return ODP_SHM_INVALID; + } + + 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"); + } 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 ((flags & ODP_SHM_PROC) && + (ftruncate(fd, alloc_size) == -1)) { + odp_spinlock_unlock(&odp_shm_tbl->lock); + ODP_ERR("odp_shm_reserve: ftruncate failed\n"); + return ODP_SHM_INVALID; + } - 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; + 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 +289,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);
In case of hugepages munmap requires size aligned to page. Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> --- v6: error if ftuncate fails in HP case. platform/linux-generic/odp_shared_memory.c | 73 +++++++++++++++++++----------- test/validation/odp_shm.c | 4 ++ 2 files changed, 51 insertions(+), 26 deletions(-)