diff mbox

[PATCHv2,2/2] linux-generic: reword messages for odp_shm_reserve

Message ID 1424979778-20444-2-git-send-email-maxim.uvarov@linaro.org
State Accepted
Commit 3fef9787be16aee871161bada3d5d66b9fed37bf
Headers show

Commit Message

Maxim Uvarov Feb. 26, 2015, 7:42 p.m. UTC 
"mmap HP failed" is confusing message. It's not error it's
just debug hint that memory will be allocated with normal pages,
not huge pages. Also all ODP_DBG and etc macro already print
function name, no need to have it in message.

Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
---
 v2: add name to debug message as Mike asked.

 platform/linux-generic/odp_shared_memory.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

Comments

Mike Holmes Feb. 26, 2015, 7:49 p.m. UTC | #1 
On 26 February 2015 at 14:42, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:

> "mmap HP failed" is confusing message. It's not error it's
> just debug hint that memory will be allocated with normal pages,
> not huge pages. Also all ODP_DBG and etc macro already print
> function name, no need to have it in message.
>
> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
>

Reviewed-by: Mike Holmes <mike.holmes@linaro.org>


> ---
>  v2: add name to debug message as Mike asked.
>
>  platform/linux-generic/odp_shared_memory.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/platform/linux-generic/odp_shared_memory.c
> b/platform/linux-generic/odp_shared_memory.c
> index 9f6ce1e..f995168 100644
> --- a/platform/linux-generic/odp_shared_memory.c
> +++ b/platform/linux-generic/odp_shared_memory.c
> @@ -212,7 +212,7 @@ odp_shm_t odp_shm_reserve(const char *name, uint64_t
> size, uint64_t align,
>                 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");
> +                       ODP_DBG("%s: shm_open failed.\n", name);
>                         return ODP_SHM_INVALID;
>                 }
>         } else {
> @@ -224,7 +224,7 @@ odp_shm_t odp_shm_reserve(const char *name, uint64_t
> size, uint64_t align,
>         if (find_block(name, NULL)) {
>                 /* Found a block with the same name */
>                 odp_spinlock_unlock(&odp_shm_tbl->lock);
> -               ODP_DBG("odp_shm_reserve: name already used\n");
> +               ODP_DBG("name \"%s\" already used.\n", name);
>                 return ODP_SHM_INVALID;
>         }
>
> @@ -238,7 +238,7 @@ odp_shm_t odp_shm_reserve(const char *name, uint64_t
> size, uint64_t align,
>         if (i > ODP_SHM_NUM_BLOCKS - 1) {
>                 /* Table full */
>                 odp_spinlock_unlock(&odp_shm_tbl->lock);
> -               ODP_DBG("odp_shm_reserve: no more blocks\n");
> +               ODP_DBG("%s: no more blocks.\n", name);
>                 return ODP_SHM_INVALID;
>         }
>
> @@ -253,14 +253,16 @@ odp_shm_t odp_shm_reserve(const char *name, uint64_t
> size, uint64_t align,
>                 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");
> +                       ODP_DBG("%s: ftruncate huge pages failed.\n",
> name);
>                         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");
> +                       ODP_DBG(" %s:\n"
> +                               "\tNo huge pages, fall back to normal
> pages,\n"
> +                               "\tcheck: /proc/sys/vm/nr_hugepages.\n",
> name);
>                 } else {
>                         block->alloc_size = alloc_hp_size;
>                         block->huge = 1;
> @@ -274,7 +276,7 @@ odp_shm_t odp_shm_reserve(const char *name, uint64_t
> size, uint64_t align,
>                 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");
> +                       ODP_ERR("%s: ftruncate failed.\n", name);
>                         return ODP_SHM_INVALID;
>                 }
>
> @@ -282,7 +284,7 @@ odp_shm_t odp_shm_reserve(const char *name, uint64_t
> size, uint64_t align,
>                                 map_flag, fd, 0);
>                 if (addr == MAP_FAILED) {
>                         odp_spinlock_unlock(&odp_shm_tbl->lock);
> -                       ODP_DBG("odp_shm_reserve: mmap failed\n");
> +                       ODP_DBG("%s mmap failed.\n", name);
>                         return ODP_SHM_INVALID;
>                 } else {
>                         block->alloc_size = alloc_size;
> --
> 1.8.5.1.163.gd7aced9
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
>
Maxim Uvarov Feb. 26, 2015, 9:05 p.m. UTC | #2 
Merged.

Maxim.

On 02/26/2015 10:49 PM, Mike Holmes wrote:
>
>
> On 26 February 2015 at 14:42, Maxim Uvarov <maxim.uvarov@linaro.org 
> <mailto:maxim.uvarov@linaro.org>> wrote:
>
>     "mmap HP failed" is confusing message. It's not error it's
>     just debug hint that memory will be allocated with normal pages,
>     not huge pages. Also all ODP_DBG and etc macro already print
>     function name, no need to have it in message.
>
>     Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org
>     <mailto:maxim.uvarov@linaro.org>>
>
>
> Reviewed-by: Mike Holmes <mike.holmes@linaro.org 
> <mailto:mike.holmes@linaro.org>>
>
>     ---
>      v2: add name to debug message as Mike asked.
>
>      platform/linux-generic/odp_shared_memory.c | 16 +++++++++-------
>      1 file changed, 9 insertions(+), 7 deletions(-)
>
>     diff --git a/platform/linux-generic/odp_shared_memory.c
>     b/platform/linux-generic/odp_shared_memory.c
>     index 9f6ce1e..f995168 100644
>     --- a/platform/linux-generic/odp_shared_memory.c
>     +++ b/platform/linux-generic/odp_shared_memory.c
>     @@ -212,7 +212,7 @@ odp_shm_t odp_shm_reserve(const char *name,
>     uint64_t size, uint64_t align,
>                     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");
>     +                       ODP_DBG("%s: shm_open failed.\n", name);
>                             return ODP_SHM_INVALID;
>                     }
>             } else {
>     @@ -224,7 +224,7 @@ odp_shm_t odp_shm_reserve(const char *name,
>     uint64_t size, uint64_t align,
>             if (find_block(name, NULL)) {
>                     /* Found a block with the same name */
>     odp_spinlock_unlock(&odp_shm_tbl->lock);
>     -               ODP_DBG("odp_shm_reserve: name already used\n");
>     +               ODP_DBG("name \"%s\" already used.\n", name);
>                     return ODP_SHM_INVALID;
>             }
>
>     @@ -238,7 +238,7 @@ odp_shm_t odp_shm_reserve(const char *name,
>     uint64_t size, uint64_t align,
>             if (i > ODP_SHM_NUM_BLOCKS - 1) {
>                     /* Table full */
>     odp_spinlock_unlock(&odp_shm_tbl->lock);
>     -               ODP_DBG("odp_shm_reserve: no more blocks\n");
>     +               ODP_DBG("%s: no more blocks.\n", name);
>                     return ODP_SHM_INVALID;
>             }
>
>     @@ -253,14 +253,16 @@ odp_shm_t odp_shm_reserve(const char *name,
>     uint64_t size, uint64_t align,
>                     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");
>     +                       ODP_DBG("%s: ftruncate huge pages
>     failed.\n", name);
>                             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");
>     +                       ODP_DBG(" %s:\n"
>     +                               "\tNo huge pages, fall back to
>     normal pages,\n"
>     +                               "\tcheck:
>     /proc/sys/vm/nr_hugepages.\n", name);
>                     } else {
>                             block->alloc_size = alloc_hp_size;
>                             block->huge = 1;
>     @@ -274,7 +276,7 @@ odp_shm_t odp_shm_reserve(const char *name,
>     uint64_t size, uint64_t align,
>                     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");
>     +                       ODP_ERR("%s: ftruncate failed.\n", name);
>                             return ODP_SHM_INVALID;
>                     }
>
>     @@ -282,7 +284,7 @@ odp_shm_t odp_shm_reserve(const char *name,
>     uint64_t size, uint64_t align,
>                                     map_flag, fd, 0);
>                     if (addr == MAP_FAILED) {
>     odp_spinlock_unlock(&odp_shm_tbl->lock);
>     -                       ODP_DBG("odp_shm_reserve: mmap failed\n");
>     +                       ODP_DBG("%s mmap failed.\n", name);
>                             return ODP_SHM_INVALID;
>                     } else {
>                             block->alloc_size = alloc_size;
>     --
>     1.8.5.1.163.gd7aced9
>
>
>     _______________________________________________
>     lng-odp mailing list
>     lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org>
>     http://lists.linaro.org/mailman/listinfo/lng-odp
>
>
>
>
> -- 
> *Mike Holmes*
> Linaro  Sr Technical Manager
> LNG - ODP
diff mbox

Patch

diff --git a/platform/linux-generic/odp_shared_memory.c b/platform/linux-generic/odp_shared_memory.c
index 9f6ce1e..f995168 100644
--- a/platform/linux-generic/odp_shared_memory.c
+++ b/platform/linux-generic/odp_shared_memory.c
@@ -212,7 +212,7 @@  odp_shm_t odp_shm_reserve(const char *name, uint64_t size, uint64_t align,
 		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");
+			ODP_DBG("%s: shm_open failed.\n", name);
 			return ODP_SHM_INVALID;
 		}
 	} else {
@@ -224,7 +224,7 @@  odp_shm_t odp_shm_reserve(const char *name, uint64_t size, uint64_t align,
 	if (find_block(name, NULL)) {
 		/* Found a block with the same name */
 		odp_spinlock_unlock(&odp_shm_tbl->lock);
-		ODP_DBG("odp_shm_reserve: name already used\n");
+		ODP_DBG("name \"%s\" already used.\n", name);
 		return ODP_SHM_INVALID;
 	}
 
@@ -238,7 +238,7 @@  odp_shm_t odp_shm_reserve(const char *name, uint64_t size, uint64_t align,
 	if (i > ODP_SHM_NUM_BLOCKS - 1) {
 		/* Table full */
 		odp_spinlock_unlock(&odp_shm_tbl->lock);
-		ODP_DBG("odp_shm_reserve: no more blocks\n");
+		ODP_DBG("%s: no more blocks.\n", name);
 		return ODP_SHM_INVALID;
 	}
 
@@ -253,14 +253,16 @@  odp_shm_t odp_shm_reserve(const char *name, uint64_t size, uint64_t align,
 		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");
+			ODP_DBG("%s: ftruncate huge pages failed.\n", name);
 			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");
+			ODP_DBG(" %s:\n"
+				"\tNo huge pages, fall back to normal pages,\n"
+				"\tcheck: /proc/sys/vm/nr_hugepages.\n", name);
 		} else {
 			block->alloc_size = alloc_hp_size;
 			block->huge = 1;
@@ -274,7 +276,7 @@  odp_shm_t odp_shm_reserve(const char *name, uint64_t size, uint64_t align,
 		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");
+			ODP_ERR("%s: ftruncate failed.\n", name);
 			return ODP_SHM_INVALID;
 		}
 
@@ -282,7 +284,7 @@  odp_shm_t odp_shm_reserve(const char *name, uint64_t size, uint64_t align,
 				map_flag, fd, 0);
 		if (addr == MAP_FAILED) {
 			odp_spinlock_unlock(&odp_shm_tbl->lock);
-			ODP_DBG("odp_shm_reserve: mmap failed\n");
+			ODP_DBG("%s mmap failed.\n", name);
 			return ODP_SHM_INVALID;
 		} else {
 			block->alloc_size = alloc_size;