[RFC 0/4] malloc type argument cleanup (part 1)

[RFC 0/4] malloc type argument cleanup (part 1)

[Stephen Hemminger]

This part documents and provides script to replace the unused
type argument in rte_malloc.  The type was intended to be a string
but never implemented, and if it hasn't been implemented in 10 years
it won't be. Too invasive to completely remove it.

Stephen Hemminger (4):
  rte_malloc: document that type is unused
  devtools/cocci: add script to find unnecessary malloc type
  devtools/cocci: add script to find where rte_calloc should be used
  eal/malloc: remove type argument from internal malloc routines

 devtools/cocci/malloc-type.cocci     | 27 +++++++++++++++++++
 devtools/cocci/prefer-calloc.cocci   | 19 ++++++++++++++
 lib/eal/common/eal_common_memzone.c  |  6 ++---
 lib/eal/common/malloc_heap.c         | 39 ++++++++++++----------------
 lib/eal/common/malloc_heap.h         |  7 +++--
 lib/eal/common/rte_malloc.c          | 16 +++++-------
 lib/eal/include/eal_trace_internal.h |  4 +--
 lib/eal/include/rte_malloc.h         | 21 +++++----------
 8 files changed, 82 insertions(+), 57 deletions(-)
 create mode 100644 devtools/cocci/malloc-type.cocci
 create mode 100644 devtools/cocci/prefer-calloc.cocci

-- 
2.43.0

[RFC 1/4] rte_malloc: document that type is unused

[Stephen Hemminger]

The string type is left over from first version of DPDK and
was never implemented.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/eal/include/rte_malloc.h | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/lib/eal/include/rte_malloc.h b/lib/eal/include/rte_malloc.h
index 54a8ac211e..91c9214c57 100644
--- a/lib/eal/include/rte_malloc.h
+++ b/lib/eal/include/rte_malloc.h
@@ -37,8 +37,7 @@ struct rte_malloc_socket_stats {
  * NUMA socket as the core that calls this function.
  *
  * @param type
- *   A string identifying the type of allocated objects (useful for debug
- *   purposes, such as identifying the cause of a memory leak). Can be NULL.
+ *   Legacy argument, never unused should be NULL.
  * @param size
  *   Size (in bytes) to be allocated.
  * @param align
@@ -64,8 +63,7 @@ rte_malloc(const char *type, size_t size, unsigned align)
  * same NUMA socket as the core that calls this function.
  *
  * @param type
- *   A string identifying the type of allocated objects (useful for debug
- *   purposes, such as identifying the cause of a memory leak). Can be NULL.
+ *   Legacy argument, never unused should be NULL.
  * @param size
  *   Size (in bytes) to be allocated.
  * @param align
@@ -89,8 +87,7 @@ rte_zmalloc(const char *type, size_t size, unsigned align)
  * same NUMA socket as the core that calls this function.
  *
  * @param type
- *   A string identifying the type of allocated objects (useful for debug
- *   purposes, such as identifying the cause of a memory leak). Can be NULL.
+ *   Legacy argument, never unused should be NULL.
  * @param num
  *   Number of elements to be allocated.
  * @param size
@@ -165,8 +162,7 @@ rte_realloc_socket(void *ptr, size_t size, unsigned int align, int socket)
  * is not cleared.
  *
  * @param type
- *   A string identifying the type of allocated objects (useful for debug
- *   purposes, such as identifying the cause of a memory leak). Can be NULL.
+ *   Legacy argument, never unused should be NULL.
  * @param size
  *   Size (in bytes) to be allocated.
  * @param align
@@ -194,8 +190,7 @@ rte_malloc_socket(const char *type, size_t size, unsigned align, int socket)
  * initialised with zeros.
  *
  * @param type
- *   A string identifying the type of allocated objects (useful for debug
- *   purposes, such as identifying the cause of a memory leak). Can be NULL.
+ *   Legacy argument, never unused should be NULL.
  * @param size
  *   Size (in bytes) to be allocated.
  * @param align
@@ -221,8 +216,7 @@ rte_zmalloc_socket(const char *type, size_t size, unsigned align, int socket)
  * initialised with zeros.
  *
  * @param type
- *   A string identifying the type of allocated objects (useful for debug
- *   purposes, such as identifying the cause of a memory leak). Can be NULL.
+ *   Legacy argument, never unused should be NULL.
  * @param num
  *   Number of elements to be allocated.
  * @param size
@@ -502,8 +496,7 @@ rte_malloc_heap_socket_is_external(int socket_id);
  * @param f
  *   A pointer to a file for output
  * @param type
- *   A string identifying the type of objects to dump, or NULL
- *   to dump all objects.
+ *   Legacy argument, never unused should be NULL.
  */
 void
 rte_malloc_dump_stats(FILE *f, const char *type);
-- 
2.43.0

Re: [RFC 1/4] rte_malloc: document that type is unused

[Tyler Retzlaff]

On Thu, Apr 25, 2024 at 11:23:58AM -0700, Stephen Hemminger wrote:
> The string type is left over from first version of DPDK and
> was never implemented.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  lib/eal/include/rte_malloc.h | 21 +++++++--------------
>  1 file changed, 7 insertions(+), 14 deletions(-)
> 
> diff --git a/lib/eal/include/rte_malloc.h b/lib/eal/include/rte_malloc.h
> index 54a8ac211e..91c9214c57 100644
> --- a/lib/eal/include/rte_malloc.h
> +++ b/lib/eal/include/rte_malloc.h
> @@ -37,8 +37,7 @@ struct rte_malloc_socket_stats {
>   * NUMA socket as the core that calls this function.
>   *
>   * @param type
> - *   A string identifying the type of allocated objects (useful for debug
> - *   purposes, such as identifying the cause of a memory leak). Can be NULL.
> + *   Legacy argument, never unused should be NULL.

never unused seems unintended double negative.

should it be 'unused' or 'never used' 

>   * @param size
>   *   Size (in bytes) to be allocated.
>   * @param align
> @@ -64,8 +63,7 @@ rte_malloc(const char *type, size_t size, unsigned align)
>   * same NUMA socket as the core that calls this function.
>   *
>   * @param type
> - *   A string identifying the type of allocated objects (useful for debug
> - *   purposes, such as identifying the cause of a memory leak). Can be NULL.
> + *   Legacy argument, never unused should be NULL.
>   * @param size
>   *   Size (in bytes) to be allocated.
>   * @param align
> @@ -89,8 +87,7 @@ rte_zmalloc(const char *type, size_t size, unsigned align)
>   * same NUMA socket as the core that calls this function.
>   *
>   * @param type
> - *   A string identifying the type of allocated objects (useful for debug
> - *   purposes, such as identifying the cause of a memory leak). Can be NULL.
> + *   Legacy argument, never unused should be NULL.
>   * @param num
>   *   Number of elements to be allocated.
>   * @param size
> @@ -165,8 +162,7 @@ rte_realloc_socket(void *ptr, size_t size, unsigned int align, int socket)
>   * is not cleared.
>   *
>   * @param type
> - *   A string identifying the type of allocated objects (useful for debug
> - *   purposes, such as identifying the cause of a memory leak). Can be NULL.
> + *   Legacy argument, never unused should be NULL.
>   * @param size
>   *   Size (in bytes) to be allocated.
>   * @param align
> @@ -194,8 +190,7 @@ rte_malloc_socket(const char *type, size_t size, unsigned align, int socket)
>   * initialised with zeros.
>   *
>   * @param type
> - *   A string identifying the type of allocated objects (useful for debug
> - *   purposes, such as identifying the cause of a memory leak). Can be NULL.
> + *   Legacy argument, never unused should be NULL.
>   * @param size
>   *   Size (in bytes) to be allocated.
>   * @param align
> @@ -221,8 +216,7 @@ rte_zmalloc_socket(const char *type, size_t size, unsigned align, int socket)
>   * initialised with zeros.
>   *
>   * @param type
> - *   A string identifying the type of allocated objects (useful for debug
> - *   purposes, such as identifying the cause of a memory leak). Can be NULL.
> + *   Legacy argument, never unused should be NULL.
>   * @param num
>   *   Number of elements to be allocated.
>   * @param size
> @@ -502,8 +496,7 @@ rte_malloc_heap_socket_is_external(int socket_id);
>   * @param f
>   *   A pointer to a file for output
>   * @param type
> - *   A string identifying the type of objects to dump, or NULL
> - *   to dump all objects.
> + *   Legacy argument, never unused should be NULL.
>   */
>  void
>  rte_malloc_dump_stats(FILE *f, const char *type);
> -- 
> 2.43.0

Re: [RFC 1/4] rte_malloc: document that type is unused

[Stephen Hemminger]

On Thu, 25 Apr 2024 11:31:31 -0700
Tyler Retzlaff <roretzla@linux.microsoft.com> wrote:

> On Thu, Apr 25, 2024 at 11:23:58AM -0700, Stephen Hemminger wrote:
> > The string type is left over from first version of DPDK and
> > was never implemented.
> > 
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---
> >  lib/eal/include/rte_malloc.h | 21 +++++++--------------
> >  1 file changed, 7 insertions(+), 14 deletions(-)
> > 
> > diff --git a/lib/eal/include/rte_malloc.h b/lib/eal/include/rte_malloc.h
> > index 54a8ac211e..91c9214c57 100644
> > --- a/lib/eal/include/rte_malloc.h
> > +++ b/lib/eal/include/rte_malloc.h
> > @@ -37,8 +37,7 @@ struct rte_malloc_socket_stats {
> >   * NUMA socket as the core that calls this function.
> >   *
> >   * @param type
> > - *   A string identifying the type of allocated objects (useful for debug
> > - *   purposes, such as identifying the cause of a memory leak). Can be NULL.
> > + *   Legacy argument, never unused should be NULL.  
> 
> never unused seems unintended double negative.
> 
> should it be 'unused' or 'never used' 

thanks

[RFC 2/4] devtools/cocci: add script to find unnecessary malloc type

[Stephen Hemminger]

The malloc type argument is unused and should be NULL.
This script finds and fixes those places.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 devtools/cocci/malloc-type.cocci | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)
 create mode 100644 devtools/cocci/malloc-type.cocci

diff --git a/devtools/cocci/malloc-type.cocci b/devtools/cocci/malloc-type.cocci
new file mode 100644
index 0000000000..d1101927e0
--- /dev/null
+++ b/devtools/cocci/malloc-type.cocci
@@ -0,0 +1,27 @@
+//
+// The allocation name field in malloc routines was never
+// implemented and should be NULL
+//
+@@
+expression T != NULL;
+expression num, socket, size, align;
+@@
+(
+- rte_malloc(T, size, align)
++ rte_malloc(NULL, size, align)
+|
+- rte_zmalloc(T, size, align)
++ rte_zmalloc(NULL,  size, align)
+|
+- rte_calloc(T, num, size, align)
++ rte_calloc(NULL, num, size, align)
+|
+- rte_malloc_socket(T, size, align, socket)
++ rte_malloc_socket(NULL, size, align, socket)
+|
+- rte_zmalloc_socket(T, size, align, socket)
++ rte_zmalloc_socket(NULL, size, align, socket)
+|
+- rte_calloc_socket(T, num, size, align, socket)
++ rte_calloc_socket(NULL, num, size, align, socket)
+)
-- 
2.43.0

[RFC 3/4] devtools/cocci: add script to find where rte_calloc should be used

[Stephen Hemminger]

Better to use ret_calloc() than directly multiply up the
sizes of objects.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 devtools/cocci/prefer-calloc.cocci | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
 create mode 100644 devtools/cocci/prefer-calloc.cocci

diff --git a/devtools/cocci/prefer-calloc.cocci b/devtools/cocci/prefer-calloc.cocci
new file mode 100644
index 0000000000..77defed9f6
--- /dev/null
+++ b/devtools/cocci/prefer-calloc.cocci
@@ -0,0 +1,19 @@
+//
+// Prefer use of calloc over zmalloc when allocating multiple objects
+//
+@@
+expression name, T, num, socket, align;
+@@
+(
+- rte_zmalloc(name, num * sizeof(T), align)
++ rte_calloc(name, num, sizeof(T), align)
+|
+- rte_zmalloc(name, sizeof(T) * num, align)
++ rte_calloc(name, num, sizeof(T), align)
+|
+- rte_zmalloc_socket(name, num * sizeof(T), align, socket)
++ rte_calloc_socket(name, num, sizeof(T), align, socket)
+|
+- rte_zmalloc_socket(name, sizeof(T) * num, align, socket)
++ rte_calloc_socket(name, num, sizeof(T), align, socket)
+)
-- 
2.43.0

[RFC 4/4] eal/malloc: remove type argument from internal malloc routines

[Stephen Hemminger]

The type argument is carried through malloc heap routines but
never used.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/eal/common/eal_common_memzone.c  |  6 ++---
 lib/eal/common/malloc_heap.c         | 39 ++++++++++++----------------
 lib/eal/common/malloc_heap.h         |  7 +++--
 lib/eal/common/rte_malloc.c          | 16 +++++-------
 lib/eal/include/eal_trace_internal.h |  4 +--
 5 files changed, 29 insertions(+), 43 deletions(-)

diff --git a/lib/eal/common/eal_common_memzone.c b/lib/eal/common/eal_common_memzone.c
index 32e6b78f87..2d9b6aa3e3 100644
--- a/lib/eal/common/eal_common_memzone.c
+++ b/lib/eal/common/eal_common_memzone.c
@@ -191,14 +191,12 @@ memzone_reserve_aligned_thread_unsafe(const char *name, size_t len,
 	if (len == 0 && bound == 0) {
 		/* no size constraints were placed, so use malloc elem len */
 		requested_len = 0;
-		mz_addr = malloc_heap_alloc_biggest(NULL, socket_id, flags,
-				align, contig);
+		mz_addr = malloc_heap_alloc_biggest(socket_id, flags, align, contig);
 	} else {
 		if (len == 0)
 			requested_len = bound;
 		/* allocate memory on heap */
-		mz_addr = malloc_heap_alloc(NULL, requested_len, socket_id,
-				flags, align, bound, contig);
+		mz_addr = malloc_heap_alloc(requested_len, socket_id, flags, align, bound, contig);
 	}
 	if (mz_addr == NULL) {
 		rte_errno = ENOMEM;
diff --git a/lib/eal/common/malloc_heap.c b/lib/eal/common/malloc_heap.c
index 5ff27548ff..058aaf4209 100644
--- a/lib/eal/common/malloc_heap.c
+++ b/lib/eal/common/malloc_heap.c
@@ -229,8 +229,8 @@ find_biggest_element(struct malloc_heap *heap, size_t *size,
  * the new element after releasing the lock.
  */
 static void *
-heap_alloc(struct malloc_heap *heap, const char *type __rte_unused, size_t size,
-		unsigned int flags, size_t align, size_t bound, bool contig)
+heap_alloc(struct malloc_heap *heap, size_t size, unsigned int flags,
+	   size_t align, size_t bound, bool contig)
 {
 	struct malloc_elem *elem;
 	size_t user_size = size;
@@ -255,8 +255,7 @@ heap_alloc(struct malloc_heap *heap, const char *type __rte_unused, size_t size,
 }
 
 static void *
-heap_alloc_biggest(struct malloc_heap *heap, const char *type __rte_unused,
-		unsigned int flags, size_t align, bool contig)
+heap_alloc_biggest(struct malloc_heap *heap, unsigned int flags, size_t align, bool contig)
 {
 	struct malloc_elem *elem;
 	size_t size;
@@ -640,8 +639,7 @@ alloc_more_mem_on_socket(struct malloc_heap *heap, size_t size, int socket,
 
 /* this will try lower page sizes first */
 static void *
-malloc_heap_alloc_on_heap_id(const char *type, size_t size,
-		unsigned int heap_id, unsigned int flags, size_t align,
+malloc_heap_alloc_on_heap_id(size_t size, unsigned int heap_id, unsigned int flags, size_t align,
 		size_t bound, bool contig)
 {
 	struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config;
@@ -658,7 +656,7 @@ malloc_heap_alloc_on_heap_id(const char *type, size_t size,
 
 	/* for legacy mode, try once and with all flags */
 	if (internal_conf->legacy_mem) {
-		ret = heap_alloc(heap, type, size, flags, align, bound, contig);
+		ret = heap_alloc(heap, size, flags, align, bound, contig);
 		goto alloc_unlock;
 	}
 
@@ -679,7 +677,7 @@ malloc_heap_alloc_on_heap_id(const char *type, size_t size,
 	if (socket_id < 0)
 		size_flags |= RTE_MEMZONE_SIZE_HINT_ONLY;
 
-	ret = heap_alloc(heap, type, size, size_flags, align, bound, contig);
+	ret = heap_alloc(heap, size, size_flags, align, bound, contig);
 	if (ret != NULL)
 		goto alloc_unlock;
 
@@ -689,7 +687,7 @@ malloc_heap_alloc_on_heap_id(const char *type, size_t size,
 
 	if (!alloc_more_mem_on_socket(heap, size, socket_id, flags, align,
 			bound, contig)) {
-		ret = heap_alloc(heap, type, size, flags, align, bound, contig);
+		ret = heap_alloc(heap, size, flags, align, bound, contig);
 
 		/* this should have succeeded */
 		if (ret == NULL)
@@ -730,8 +728,8 @@ malloc_get_numa_socket(void)
 }
 
 void *
-malloc_heap_alloc(const char *type, size_t size, int socket_arg,
-		unsigned int flags, size_t align, size_t bound, bool contig)
+malloc_heap_alloc(size_t size, int socket_arg, unsigned int flags,
+		  size_t align, size_t bound, bool contig)
 {
 	int socket, heap_id, i;
 	void *ret;
@@ -754,8 +752,7 @@ malloc_heap_alloc(const char *type, size_t size, int socket_arg,
 	if (heap_id < 0)
 		return NULL;
 
-	ret = malloc_heap_alloc_on_heap_id(type, size, heap_id, flags, align,
-			bound, contig);
+	ret = malloc_heap_alloc_on_heap_id(size, heap_id, flags, align, bound, contig);
 	if (ret != NULL || socket_arg != SOCKET_ID_ANY)
 		return ret;
 
@@ -765,8 +762,7 @@ malloc_heap_alloc(const char *type, size_t size, int socket_arg,
 	for (i = 0; i < (int) rte_socket_count(); i++) {
 		if (i == heap_id)
 			continue;
-		ret = malloc_heap_alloc_on_heap_id(type, size, i, flags, align,
-				bound, contig);
+		ret = malloc_heap_alloc_on_heap_id(size, i, flags, align, bound, contig);
 		if (ret != NULL)
 			return ret;
 	}
@@ -774,7 +770,7 @@ malloc_heap_alloc(const char *type, size_t size, int socket_arg,
 }
 
 static void *
-heap_alloc_biggest_on_heap_id(const char *type, unsigned int heap_id,
+heap_alloc_biggest_on_heap_id(unsigned int heap_id,
 		unsigned int flags, size_t align, bool contig)
 {
 	struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config;
@@ -785,7 +781,7 @@ heap_alloc_biggest_on_heap_id(const char *type, unsigned int heap_id,
 
 	align = align == 0 ? 1 : align;
 
-	ret = heap_alloc_biggest(heap, type, flags, align, contig);
+	ret = heap_alloc_biggest(heap, flags, align, contig);
 
 	rte_spinlock_unlock(&(heap->lock));
 
@@ -793,8 +789,7 @@ heap_alloc_biggest_on_heap_id(const char *type, unsigned int heap_id,
 }
 
 void *
-malloc_heap_alloc_biggest(const char *type, int socket_arg, unsigned int flags,
-		size_t align, bool contig)
+malloc_heap_alloc_biggest(int socket_arg, unsigned int flags, size_t align, bool contig)
 {
 	int socket, i, cur_socket, heap_id;
 	void *ret;
@@ -817,8 +812,7 @@ malloc_heap_alloc_biggest(const char *type, int socket_arg, unsigned int flags,
 	if (heap_id < 0)
 		return NULL;
 
-	ret = heap_alloc_biggest_on_heap_id(type, heap_id, flags, align,
-			contig);
+	ret = heap_alloc_biggest_on_heap_id(heap_id, flags, align, contig);
 	if (ret != NULL || socket_arg != SOCKET_ID_ANY)
 		return ret;
 
@@ -827,8 +821,7 @@ malloc_heap_alloc_biggest(const char *type, int socket_arg, unsigned int flags,
 		cur_socket = rte_socket_id_by_idx(i);
 		if (cur_socket == socket)
 			continue;
-		ret = heap_alloc_biggest_on_heap_id(type, i, flags, align,
-				contig);
+		ret = heap_alloc_biggest_on_heap_id(i, flags, align, contig);
 		if (ret != NULL)
 			return ret;
 	}
diff --git a/lib/eal/common/malloc_heap.h b/lib/eal/common/malloc_heap.h
index 0c49588005..dfc56d4ae3 100644
--- a/lib/eal/common/malloc_heap.h
+++ b/lib/eal/common/malloc_heap.h
@@ -34,12 +34,11 @@ struct __rte_cache_aligned malloc_heap {
 };
 
 void *
-malloc_heap_alloc(const char *type, size_t size, int socket, unsigned int flags,
-		size_t align, size_t bound, bool contig);
+malloc_heap_alloc(size_t size, int socket, unsigned int flags, size_t align,
+		  size_t bound, bool contig);
 
 void *
-malloc_heap_alloc_biggest(const char *type, int socket, unsigned int flags,
-		size_t align, bool contig);
+malloc_heap_alloc_biggest(int socket, unsigned int flags, size_t align, bool contig);
 
 int
 malloc_heap_create(struct malloc_heap *heap, const char *heap_name);
diff --git a/lib/eal/common/rte_malloc.c b/lib/eal/common/rte_malloc.c
index 6d3c301a23..d1fcb2eff6 100644
--- a/lib/eal/common/rte_malloc.c
+++ b/lib/eal/common/rte_malloc.c
@@ -51,8 +51,7 @@ eal_free_no_trace(void *addr)
 }
 
 static void *
-malloc_socket(const char *type, size_t size, unsigned int align,
-		int socket_arg, const bool trace_ena)
+malloc_socket(size_t size, unsigned int align, int socket_arg, const bool trace_ena)
 {
 	void *ptr;
 
@@ -69,11 +68,11 @@ malloc_socket(const char *type, size_t size, unsigned int align,
 				!rte_eal_has_hugepages())
 		socket_arg = SOCKET_ID_ANY;
 
-	ptr = malloc_heap_alloc(type, size, socket_arg, 0,
+	ptr = malloc_heap_alloc(size, socket_arg, 0,
 			align == 0 ? 1 : align, 0, false);
 
 	if (trace_ena)
-		rte_eal_trace_mem_malloc(type, size, align, socket_arg, ptr);
+		rte_eal_trace_mem_malloc(size, align, socket_arg, ptr);
 	return ptr;
 }
 
@@ -81,16 +80,15 @@ malloc_socket(const char *type, size_t size, unsigned int align,
  * Allocate memory on specified heap.
  */
 void *
-rte_malloc_socket(const char *type, size_t size, unsigned int align,
-		int socket_arg)
+rte_malloc_socket(const char *type __rte_unused, size_t size, unsigned int align, int socket_arg)
 {
-	return malloc_socket(type, size, align, socket_arg, true);
+	return malloc_socket(size, align, socket_arg, true);
 }
 
 void *
-eal_malloc_no_trace(const char *type, size_t size, unsigned int align)
+eal_malloc_no_trace(const char *type __rte_unused, size_t size, unsigned int align)
 {
-	return malloc_socket(type, size, align, SOCKET_ID_ANY, false);
+	return malloc_socket(size, align, SOCKET_ID_ANY, false);
 }
 
 /*
diff --git a/lib/eal/include/eal_trace_internal.h b/lib/eal/include/eal_trace_internal.h
index 09c354717f..fa104c975f 100644
--- a/lib/eal/include/eal_trace_internal.h
+++ b/lib/eal/include/eal_trace_internal.h
@@ -103,9 +103,7 @@ RTE_TRACE_POINT(
 
 RTE_TRACE_POINT(
 	rte_eal_trace_mem_malloc,
-	RTE_TRACE_POINT_ARGS(const char *type, size_t size, unsigned int align,
-		int socket, void *ptr),
-	rte_trace_point_emit_string(type);
+	RTE_TRACE_POINT_ARGS(size_t size, unsigned int align, int socket, void *ptr),
 	rte_trace_point_emit_size_t(size);
 	rte_trace_point_emit_u32(align);
 	rte_trace_point_emit_int(socket);
-- 
2.43.0

Re: [RFC 4/4] eal/malloc: remove type argument from internal malloc routines

[Tyler Retzlaff]

On Thu, Apr 25, 2024 at 11:24:01AM -0700, Stephen Hemminger wrote:
> The type argument is carried through malloc heap routines but
> never used.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  lib/eal/common/eal_common_memzone.c  |  6 ++---
>  lib/eal/common/malloc_heap.c         | 39 ++++++++++++----------------
>  lib/eal/common/malloc_heap.h         |  7 +++--
>  lib/eal/common/rte_malloc.c          | 16 +++++-------
>  lib/eal/include/eal_trace_internal.h |  4 +--
>  5 files changed, 29 insertions(+), 43 deletions(-)
> 
> diff --git a/lib/eal/common/eal_common_memzone.c b/lib/eal/common/eal_common_memzone.c
> index 32e6b78f87..2d9b6aa3e3 100644
> --- a/lib/eal/common/eal_common_memzone.c
> +++ b/lib/eal/common/eal_common_memzone.c
> @@ -191,14 +191,12 @@ memzone_reserve_aligned_thread_unsafe(const char *name, size_t len,
>  	if (len == 0 && bound == 0) {
>  		/* no size constraints were placed, so use malloc elem len */
>  		requested_len = 0;
> -		mz_addr = malloc_heap_alloc_biggest(NULL, socket_id, flags,
> -				align, contig);
> +		mz_addr = malloc_heap_alloc_biggest(socket_id, flags, align, contig);

i may have missed if this was discussed already. for the public api i
understand for now we need to keep the unused parameter in the function
signatures but for internal api/functions i would prefer the parameter
be removed entirely.

also somewhat related side-note i don't think msvc has a way of marking
function parameters unused as is done with __rte_unused. currently i
expand the macro empty and suppress the warning globally which is not
great.

Re: [RFC 4/4] eal/malloc: remove type argument from internal malloc routines

[Stephen Hemminger]

On Fri, 26 Apr 2024 09:16:27 -0700
Tyler Retzlaff <roretzla@linux.microsoft.com> wrote:

> > 
> > diff --git a/lib/eal/common/eal_common_memzone.c b/lib/eal/common/eal_common_memzone.c
> > index 32e6b78f87..2d9b6aa3e3 100644
> > --- a/lib/eal/common/eal_common_memzone.c
> > +++ b/lib/eal/common/eal_common_memzone.c
> > @@ -191,14 +191,12 @@ memzone_reserve_aligned_thread_unsafe(const char *name, size_t len,
> >  	if (len == 0 && bound == 0) {
> >  		/* no size constraints were placed, so use malloc elem len */
> >  		requested_len = 0;
> > -		mz_addr = malloc_heap_alloc_biggest(NULL, socket_id, flags,
> > -				align, contig);
> > +		mz_addr = malloc_heap_alloc_biggest(socket_id, flags, align, contig);  
> 
> i may have missed if this was discussed already. for the public api i
> understand for now we need to keep the unused parameter in the function
> signatures but for internal api/functions i would prefer the parameter
> be removed entirely.
> 
> also somewhat related side-note i don't think msvc has a way of marking
> function parameters unused as is done with __rte_unused. currently i
> expand the macro empty and suppress the warning globally which is not
> great.

I dropped the parameter from all the internal routines. Are you suggesting having
an different name/version for use internally?

Re: [RFC 4/4] eal/malloc: remove type argument from internal malloc routines

[Tyler Retzlaff]

On Fri, Apr 26, 2024 at 03:52:29PM -0700, Stephen Hemminger wrote:
> On Fri, 26 Apr 2024 09:16:27 -0700
> Tyler Retzlaff <roretzla@linux.microsoft.com> wrote:
> 
> > > 
> > > diff --git a/lib/eal/common/eal_common_memzone.c b/lib/eal/common/eal_common_memzone.c
> > > index 32e6b78f87..2d9b6aa3e3 100644
> > > --- a/lib/eal/common/eal_common_memzone.c
> > > +++ b/lib/eal/common/eal_common_memzone.c
> > > @@ -191,14 +191,12 @@ memzone_reserve_aligned_thread_unsafe(const char *name, size_t len,
> > >  	if (len == 0 && bound == 0) {
> > >  		/* no size constraints were placed, so use malloc elem len */
> > >  		requested_len = 0;
> > > -		mz_addr = malloc_heap_alloc_biggest(NULL, socket_id, flags,
> > > -				align, contig);
> > > +		mz_addr = malloc_heap_alloc_biggest(socket_id, flags, align, contig);  
> > 
> > i may have missed if this was discussed already. for the public api i
> > understand for now we need to keep the unused parameter in the function
> > signatures but for internal api/functions i would prefer the parameter
> > be removed entirely.
> > 
> > also somewhat related side-note i don't think msvc has a way of marking
> > function parameters unused as is done with __rte_unused. currently i
> > expand the macro empty and suppress the warning globally which is not
> > great.
> 
> I dropped the parameter from all the internal routines. Are you suggesting having
> an different name/version for use internally?

oh, it looks like i can't read a diff.

ignore me! thanks!

RE: [RFC 4/4] eal/malloc: remove type argument from internal malloc routines

[Morten Brørup]

> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Thursday, 25 April 2024 20.24
> 
> The type argument is carried through malloc heap routines but
> never used.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---

For the series,
Acked-by: Morten Brørup <mb@smartsharesystems.com>

Re: [RFC 0/4] malloc type argument cleanup (part 1)

[Patrick Robb]

[-- Attachment #1: Type: text/plain, Size: 296 bytes --]

Recheck-request: iol-compile-amd64-testing

The DPDK Community Lab updated to the latest Alpine image yesterday, which
resulted in all Alpine builds failing. The failure is unrelated to your
patch, and this recheck should remove the fail on Patchwork, as we have
disabled Alpine testing for now.

[-- Attachment #2: Type: text/html, Size: 361 bytes --]

[PATCH v2 0/3] malloc related cleanups

[Stephen Hemminger]

The type parameter for malloc is only used for tracing.
Fix documentation and don't pass through heap routines.

Stephen Hemminger (3):
  rte_malloc: document that type is for tracing
  eal: remove type argument from internal routines
  event/sw: avoid snprintf truncation

v2 - keep usage for tracing
   - combine event malloc type patch

 drivers/event/sw/iq_chunk.h         |  2 --
 drivers/event/sw/sw_evdev.c         |  2 +-
 lib/eal/common/eal_common_memzone.c |  6 ++---
 lib/eal/common/malloc_heap.c        | 39 ++++++++++++-----------------
 lib/eal/common/malloc_heap.h        |  7 +++---
 lib/eal/common/rte_malloc.c         |  2 +-
 lib/eal/include/rte_malloc.h        | 24 +++++++++---------
 7 files changed, 35 insertions(+), 47 deletions(-)

-- 
2.43.0

[PATCH v2 1/3] rte_malloc: document that type is for tracing

[Stephen Hemminger]

The string type is only used for tracing and not used as
documented by dump_stats.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/eal/include/rte_malloc.h | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/lib/eal/include/rte_malloc.h b/lib/eal/include/rte_malloc.h
index 54a8ac211e..67a459c6a3 100644
--- a/lib/eal/include/rte_malloc.h
+++ b/lib/eal/include/rte_malloc.h
@@ -37,8 +37,8 @@ struct rte_malloc_socket_stats {
  * NUMA socket as the core that calls this function.
  *
  * @param type
- *   A string identifying the type of allocated objects (useful for debug
- *   purposes, such as identifying the cause of a memory leak). Can be NULL.
+ *   A string identifying the type of allocated objects (useful for tracing).
+ *   Can be NULL.
  * @param size
  *   Size (in bytes) to be allocated.
  * @param align
@@ -64,8 +64,8 @@ rte_malloc(const char *type, size_t size, unsigned align)
  * same NUMA socket as the core that calls this function.
  *
  * @param type
- *   A string identifying the type of allocated objects (useful for debug
- *   purposes, such as identifying the cause of a memory leak). Can be NULL.
+ *   A string identifying the type of allocated objects (useful for tracing).
+ *   Can be NULL.
  * @param size
  *   Size (in bytes) to be allocated.
  * @param align
@@ -89,8 +89,8 @@ rte_zmalloc(const char *type, size_t size, unsigned align)
  * same NUMA socket as the core that calls this function.
  *
  * @param type
- *   A string identifying the type of allocated objects (useful for debug
- *   purposes, such as identifying the cause of a memory leak). Can be NULL.
+ *   A string identifying the type of allocated objects (useful for tracing).
+ *   Can be NULL.
  * @param num
  *   Number of elements to be allocated.
  * @param size
@@ -165,8 +165,8 @@ rte_realloc_socket(void *ptr, size_t size, unsigned int align, int socket)
  * is not cleared.
  *
  * @param type
- *   A string identifying the type of allocated objects (useful for debug
- *   purposes, such as identifying the cause of a memory leak). Can be NULL.
+ *   A string identifying the type of allocated objects (useful for tracing).
+ *   Can be NULL.
  * @param size
  *   Size (in bytes) to be allocated.
  * @param align
@@ -194,8 +194,8 @@ rte_malloc_socket(const char *type, size_t size, unsigned align, int socket)
  * initialised with zeros.
  *
  * @param type
- *   A string identifying the type of allocated objects (useful for debug
- *   purposes, such as identifying the cause of a memory leak). Can be NULL.
+ *   A string identifying the type of allocated objects (useful for tracing).
+ *   Can be NULL.
  * @param size
  *   Size (in bytes) to be allocated.
  * @param align
@@ -221,8 +221,8 @@ rte_zmalloc_socket(const char *type, size_t size, unsigned align, int socket)
  * initialised with zeros.
  *
  * @param type
- *   A string identifying the type of allocated objects (useful for debug
- *   purposes, such as identifying the cause of a memory leak). Can be NULL.
+ *   A string identifying the type of allocated objects (useful for tracing).
+ *   Can be NULL.
  * @param num
  *   Number of elements to be allocated.
  * @param size
-- 
2.43.0

[PATCH v2 2/3] eal: remove type argument from internal routines

[Stephen Hemminger]

The type argument is carried through malloc heap routines but
never used there. It is only used a rte_malloc for tracing.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/eal/common/eal_common_memzone.c |  6 ++---
 lib/eal/common/malloc_heap.c        | 39 ++++++++++++-----------------
 lib/eal/common/malloc_heap.h        |  7 +++---
 lib/eal/common/rte_malloc.c         |  2 +-
 4 files changed, 22 insertions(+), 32 deletions(-)

diff --git a/lib/eal/common/eal_common_memzone.c b/lib/eal/common/eal_common_memzone.c
index 32e6b78f87..2d9b6aa3e3 100644
--- a/lib/eal/common/eal_common_memzone.c
+++ b/lib/eal/common/eal_common_memzone.c
@@ -191,14 +191,12 @@ memzone_reserve_aligned_thread_unsafe(const char *name, size_t len,
 	if (len == 0 && bound == 0) {
 		/* no size constraints were placed, so use malloc elem len */
 		requested_len = 0;
-		mz_addr = malloc_heap_alloc_biggest(NULL, socket_id, flags,
-				align, contig);
+		mz_addr = malloc_heap_alloc_biggest(socket_id, flags, align, contig);
 	} else {
 		if (len == 0)
 			requested_len = bound;
 		/* allocate memory on heap */
-		mz_addr = malloc_heap_alloc(NULL, requested_len, socket_id,
-				flags, align, bound, contig);
+		mz_addr = malloc_heap_alloc(requested_len, socket_id, flags, align, bound, contig);
 	}
 	if (mz_addr == NULL) {
 		rte_errno = ENOMEM;
diff --git a/lib/eal/common/malloc_heap.c b/lib/eal/common/malloc_heap.c
index 5ff27548ff..058aaf4209 100644
--- a/lib/eal/common/malloc_heap.c
+++ b/lib/eal/common/malloc_heap.c
@@ -229,8 +229,8 @@ find_biggest_element(struct malloc_heap *heap, size_t *size,
  * the new element after releasing the lock.
  */
 static void *
-heap_alloc(struct malloc_heap *heap, const char *type __rte_unused, size_t size,
-		unsigned int flags, size_t align, size_t bound, bool contig)
+heap_alloc(struct malloc_heap *heap, size_t size, unsigned int flags,
+	   size_t align, size_t bound, bool contig)
 {
 	struct malloc_elem *elem;
 	size_t user_size = size;
@@ -255,8 +255,7 @@ heap_alloc(struct malloc_heap *heap, const char *type __rte_unused, size_t size,
 }
 
 static void *
-heap_alloc_biggest(struct malloc_heap *heap, const char *type __rte_unused,
-		unsigned int flags, size_t align, bool contig)
+heap_alloc_biggest(struct malloc_heap *heap, unsigned int flags, size_t align, bool contig)
 {
 	struct malloc_elem *elem;
 	size_t size;
@@ -640,8 +639,7 @@ alloc_more_mem_on_socket(struct malloc_heap *heap, size_t size, int socket,
 
 /* this will try lower page sizes first */
 static void *
-malloc_heap_alloc_on_heap_id(const char *type, size_t size,
-		unsigned int heap_id, unsigned int flags, size_t align,
+malloc_heap_alloc_on_heap_id(size_t size, unsigned int heap_id, unsigned int flags, size_t align,
 		size_t bound, bool contig)
 {
 	struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config;
@@ -658,7 +656,7 @@ malloc_heap_alloc_on_heap_id(const char *type, size_t size,
 
 	/* for legacy mode, try once and with all flags */
 	if (internal_conf->legacy_mem) {
-		ret = heap_alloc(heap, type, size, flags, align, bound, contig);
+		ret = heap_alloc(heap, size, flags, align, bound, contig);
 		goto alloc_unlock;
 	}
 
@@ -679,7 +677,7 @@ malloc_heap_alloc_on_heap_id(const char *type, size_t size,
 	if (socket_id < 0)
 		size_flags |= RTE_MEMZONE_SIZE_HINT_ONLY;
 
-	ret = heap_alloc(heap, type, size, size_flags, align, bound, contig);
+	ret = heap_alloc(heap, size, size_flags, align, bound, contig);
 	if (ret != NULL)
 		goto alloc_unlock;
 
@@ -689,7 +687,7 @@ malloc_heap_alloc_on_heap_id(const char *type, size_t size,
 
 	if (!alloc_more_mem_on_socket(heap, size, socket_id, flags, align,
 			bound, contig)) {
-		ret = heap_alloc(heap, type, size, flags, align, bound, contig);
+		ret = heap_alloc(heap, size, flags, align, bound, contig);
 
 		/* this should have succeeded */
 		if (ret == NULL)
@@ -730,8 +728,8 @@ malloc_get_numa_socket(void)
 }
 
 void *
-malloc_heap_alloc(const char *type, size_t size, int socket_arg,
-		unsigned int flags, size_t align, size_t bound, bool contig)
+malloc_heap_alloc(size_t size, int socket_arg, unsigned int flags,
+		  size_t align, size_t bound, bool contig)
 {
 	int socket, heap_id, i;
 	void *ret;
@@ -754,8 +752,7 @@ malloc_heap_alloc(const char *type, size_t size, int socket_arg,
 	if (heap_id < 0)
 		return NULL;
 
-	ret = malloc_heap_alloc_on_heap_id(type, size, heap_id, flags, align,
-			bound, contig);
+	ret = malloc_heap_alloc_on_heap_id(size, heap_id, flags, align, bound, contig);
 	if (ret != NULL || socket_arg != SOCKET_ID_ANY)
 		return ret;
 
@@ -765,8 +762,7 @@ malloc_heap_alloc(const char *type, size_t size, int socket_arg,
 	for (i = 0; i < (int) rte_socket_count(); i++) {
 		if (i == heap_id)
 			continue;
-		ret = malloc_heap_alloc_on_heap_id(type, size, i, flags, align,
-				bound, contig);
+		ret = malloc_heap_alloc_on_heap_id(size, i, flags, align, bound, contig);
 		if (ret != NULL)
 			return ret;
 	}
@@ -774,7 +770,7 @@ malloc_heap_alloc(const char *type, size_t size, int socket_arg,
 }
 
 static void *
-heap_alloc_biggest_on_heap_id(const char *type, unsigned int heap_id,
+heap_alloc_biggest_on_heap_id(unsigned int heap_id,
 		unsigned int flags, size_t align, bool contig)
 {
 	struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config;
@@ -785,7 +781,7 @@ heap_alloc_biggest_on_heap_id(const char *type, unsigned int heap_id,
 
 	align = align == 0 ? 1 : align;
 
-	ret = heap_alloc_biggest(heap, type, flags, align, contig);
+	ret = heap_alloc_biggest(heap, flags, align, contig);
 
 	rte_spinlock_unlock(&(heap->lock));
 
@@ -793,8 +789,7 @@ heap_alloc_biggest_on_heap_id(const char *type, unsigned int heap_id,
 }
 
 void *
-malloc_heap_alloc_biggest(const char *type, int socket_arg, unsigned int flags,
-		size_t align, bool contig)
+malloc_heap_alloc_biggest(int socket_arg, unsigned int flags, size_t align, bool contig)
 {
 	int socket, i, cur_socket, heap_id;
 	void *ret;
@@ -817,8 +812,7 @@ malloc_heap_alloc_biggest(const char *type, int socket_arg, unsigned int flags,
 	if (heap_id < 0)
 		return NULL;
 
-	ret = heap_alloc_biggest_on_heap_id(type, heap_id, flags, align,
-			contig);
+	ret = heap_alloc_biggest_on_heap_id(heap_id, flags, align, contig);
 	if (ret != NULL || socket_arg != SOCKET_ID_ANY)
 		return ret;
 
@@ -827,8 +821,7 @@ malloc_heap_alloc_biggest(const char *type, int socket_arg, unsigned int flags,
 		cur_socket = rte_socket_id_by_idx(i);
 		if (cur_socket == socket)
 			continue;
-		ret = heap_alloc_biggest_on_heap_id(type, i, flags, align,
-				contig);
+		ret = heap_alloc_biggest_on_heap_id(i, flags, align, contig);
 		if (ret != NULL)
 			return ret;
 	}
diff --git a/lib/eal/common/malloc_heap.h b/lib/eal/common/malloc_heap.h
index 0c49588005..dfc56d4ae3 100644
--- a/lib/eal/common/malloc_heap.h
+++ b/lib/eal/common/malloc_heap.h
@@ -34,12 +34,11 @@ struct __rte_cache_aligned malloc_heap {
 };
 
 void *
-malloc_heap_alloc(const char *type, size_t size, int socket, unsigned int flags,
-		size_t align, size_t bound, bool contig);
+malloc_heap_alloc(size_t size, int socket, unsigned int flags, size_t align,
+		  size_t bound, bool contig);
 
 void *
-malloc_heap_alloc_biggest(const char *type, int socket, unsigned int flags,
-		size_t align, bool contig);
+malloc_heap_alloc_biggest(int socket, unsigned int flags, size_t align, bool contig);
 
 int
 malloc_heap_create(struct malloc_heap *heap, const char *heap_name);
diff --git a/lib/eal/common/rte_malloc.c b/lib/eal/common/rte_malloc.c
index 6d3c301a23..3eed4d4be6 100644
--- a/lib/eal/common/rte_malloc.c
+++ b/lib/eal/common/rte_malloc.c
@@ -69,7 +69,7 @@ malloc_socket(const char *type, size_t size, unsigned int align,
 				!rte_eal_has_hugepages())
 		socket_arg = SOCKET_ID_ANY;
 
-	ptr = malloc_heap_alloc(type, size, socket_arg, 0,
+	ptr = malloc_heap_alloc(size, socket_arg, 0,
 			align == 0 ? 1 : align, 0, false);
 
 	if (trace_ena)
-- 
2.43.0

[PATCH v2 3/3] event/sw: avoid snprintf truncation

[Stephen Hemminger]

The string used for rte_malloc_socket gets truncated.
With Gcc-14, this warning is generated:
../drivers/event/sw/sw_evdev.c:263:3: warning: 'snprintf' will always be truncated;
    specified size is 12, but format string expands to at least 13 [-Wformat-truncation]
  263 |                 snprintf(buf, sizeof(buf), "sw%d_iq_%d_rob", dev_id, i);
      |                 ^

Replace IQ_ROB_NAMESIZE (12) with a bigger buffer and remove
it since no longer used.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/event/sw/iq_chunk.h | 2 --
 drivers/event/sw/sw_evdev.c | 2 +-
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/event/sw/iq_chunk.h b/drivers/event/sw/iq_chunk.h
index 7a7a8782e6..e638142dbc 100644
--- a/drivers/event/sw/iq_chunk.h
+++ b/drivers/event/sw/iq_chunk.h
@@ -9,8 +9,6 @@
 #include <stdbool.h>
 #include <rte_eventdev.h>
 
-#define IQ_ROB_NAMESIZE 12
-
 struct __rte_cache_aligned sw_queue_chunk {
 	struct rte_event events[SW_EVS_PER_Q_CHUNK];
 	struct sw_queue_chunk *next;
diff --git a/drivers/event/sw/sw_evdev.c b/drivers/event/sw/sw_evdev.c
index 1c01b069fe..44698d8aff 100644
--- a/drivers/event/sw/sw_evdev.c
+++ b/drivers/event/sw/sw_evdev.c
@@ -230,7 +230,7 @@ qid_init(struct sw_evdev *sw, unsigned int idx, int type,
 	unsigned int i;
 	int dev_id = sw->data->dev_id;
 	int socket_id = sw->data->socket_id;
-	char buf[IQ_ROB_NAMESIZE];
+	char buf[64];
 	struct sw_qid *qid = &sw->qids[idx];
 
 	/* Initialize the FID structures to no pinning (-1), and zero packets */
-- 
2.43.0

RE: [PATCH v2 0/3] malloc related cleanups

[Morten Brørup]

> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Saturday, 15 June 2024 18.00
> 
> The type parameter for malloc is only used for tracing.
> Fix documentation and don't pass through heap routines.
> 
> Stephen Hemminger (3):
>   rte_malloc: document that type is for tracing
>   eal: remove type argument from internal routines
>   event/sw: avoid snprintf truncation
> 
> v2 - keep usage for tracing
>    - combine event malloc type patch

For the series,
Acked-by: Morten Brørup <mb@smartsharesystems.com>

Re: [PATCH v2 0/3] malloc related cleanups

[Patrick Robb]

For CI Testing: re-applying to main and retesting because main was in
a bad state (now fixed by Maxime), causing a virtio_smoke fail on this
series.

https://git.dpdk.org/dpdk/commit/?id=6bdc14606724bc7fb3834d5ec59b1cccf98adf28

On Sat, Jun 15, 2024 at 12:02 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> The type parameter for malloc is only used for tracing.
> Fix documentation and don't pass through heap routines.
>
> Stephen Hemminger (3):
>   rte_malloc: document that type is for tracing
>   eal: remove type argument from internal routines
>   event/sw: avoid snprintf truncation
>
> v2 - keep usage for tracing
>    - combine event malloc type patch
>
>  drivers/event/sw/iq_chunk.h         |  2 --
>  drivers/event/sw/sw_evdev.c         |  2 +-
>  lib/eal/common/eal_common_memzone.c |  6 ++---
>  lib/eal/common/malloc_heap.c        | 39 ++++++++++++-----------------
>  lib/eal/common/malloc_heap.h        |  7 +++---
>  lib/eal/common/rte_malloc.c         |  2 +-
>  lib/eal/include/rte_malloc.h        | 24 +++++++++---------
>  7 files changed, 35 insertions(+), 47 deletions(-)
>
> --
> 2.43.0
>

[PATCH v3 0/2] malloc type cleanups

[Stephen Hemminger]

The type parameter for malloc is only used for tracing.
Fix documentation and don't pass through heap routines.

Stephen Hemminger (2):
  rte_malloc: document that type is for tracing
  eal: remove type argument from internal routines

v3 - drop event/sw patch (already fixed)

 lib/eal/common/eal_common_memzone.c |  6 ++---
 lib/eal/common/malloc_heap.c        | 39 ++++++++++++-----------------
 lib/eal/common/malloc_heap.h        |  7 +++---
 lib/eal/common/rte_malloc.c         |  2 +-
 lib/eal/include/rte_malloc.h        | 27 ++++++++++----------
 5 files changed, 35 insertions(+), 46 deletions(-)

-- 
2.43.0

[PATCH v3 1/2] rte_malloc: document that type is for tracing

[Stephen Hemminger]

The string type is only used for tracing and not used as
documented by dump_stats.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/eal/include/rte_malloc.h | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/lib/eal/include/rte_malloc.h b/lib/eal/include/rte_malloc.h
index 54a8ac211e..1f91e7bdde 100644
--- a/lib/eal/include/rte_malloc.h
+++ b/lib/eal/include/rte_malloc.h
@@ -37,8 +37,8 @@ struct rte_malloc_socket_stats {
  * NUMA socket as the core that calls this function.
  *
  * @param type
- *   A string identifying the type of allocated objects (useful for debug
- *   purposes, such as identifying the cause of a memory leak). Can be NULL.
+ *   A string identifying the type of allocated objects (useful for tracing).
+ *   Can be NULL.
  * @param size
  *   Size (in bytes) to be allocated.
  * @param align
@@ -64,8 +64,8 @@ rte_malloc(const char *type, size_t size, unsigned align)
  * same NUMA socket as the core that calls this function.
  *
  * @param type
- *   A string identifying the type of allocated objects (useful for debug
- *   purposes, such as identifying the cause of a memory leak). Can be NULL.
+ *   A string identifying the type of allocated objects (useful for tracing).
+ *   Can be NULL.
  * @param size
  *   Size (in bytes) to be allocated.
  * @param align
@@ -89,8 +89,8 @@ rte_zmalloc(const char *type, size_t size, unsigned align)
  * same NUMA socket as the core that calls this function.
  *
  * @param type
- *   A string identifying the type of allocated objects (useful for debug
- *   purposes, such as identifying the cause of a memory leak). Can be NULL.
+ *   A string identifying the type of allocated objects (useful for tracing).
+ *   Can be NULL.
  * @param num
  *   Number of elements to be allocated.
  * @param size
@@ -165,8 +165,8 @@ rte_realloc_socket(void *ptr, size_t size, unsigned int align, int socket)
  * is not cleared.
  *
  * @param type
- *   A string identifying the type of allocated objects (useful for debug
- *   purposes, such as identifying the cause of a memory leak). Can be NULL.
+ *   A string identifying the type of allocated objects (useful for tracing).
+ *   Can be NULL.
  * @param size
  *   Size (in bytes) to be allocated.
  * @param align
@@ -194,8 +194,8 @@ rte_malloc_socket(const char *type, size_t size, unsigned align, int socket)
  * initialised with zeros.
  *
  * @param type
- *   A string identifying the type of allocated objects (useful for debug
- *   purposes, such as identifying the cause of a memory leak). Can be NULL.
+ *   A string identifying the type of allocated objects (useful for tracing).
+ *   Can be NULL.
  * @param size
  *   Size (in bytes) to be allocated.
  * @param align
@@ -221,8 +221,8 @@ rte_zmalloc_socket(const char *type, size_t size, unsigned align, int socket)
  * initialised with zeros.
  *
  * @param type
- *   A string identifying the type of allocated objects (useful for debug
- *   purposes, such as identifying the cause of a memory leak). Can be NULL.
+ *   A string identifying the type of allocated objects (useful for tracing).
+ *   Can be NULL.
  * @param num
  *   Number of elements to be allocated.
  * @param size
@@ -502,8 +502,7 @@ rte_malloc_heap_socket_is_external(int socket_id);
  * @param f
  *   A pointer to a file for output
  * @param type
- *   A string identifying the type of objects to dump, or NULL
- *   to dump all objects.
+ *   Deprecated parameter unused.
  */
 void
 rte_malloc_dump_stats(FILE *f, const char *type);
-- 
2.43.0

[PATCH v3 2/2] eal: remove type argument from internal routines

[Stephen Hemminger]

The type argument is carried through malloc heap routines but
never used there. It is only used a rte_malloc for tracing.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/eal/common/eal_common_memzone.c |  6 ++---
 lib/eal/common/malloc_heap.c        | 39 ++++++++++++-----------------
 lib/eal/common/malloc_heap.h        |  7 +++---
 lib/eal/common/rte_malloc.c         |  2 +-
 4 files changed, 22 insertions(+), 32 deletions(-)

diff --git a/lib/eal/common/eal_common_memzone.c b/lib/eal/common/eal_common_memzone.c
index 32e6b78f87..2d9b6aa3e3 100644
--- a/lib/eal/common/eal_common_memzone.c
+++ b/lib/eal/common/eal_common_memzone.c
@@ -191,14 +191,12 @@ memzone_reserve_aligned_thread_unsafe(const char *name, size_t len,
 	if (len == 0 && bound == 0) {
 		/* no size constraints were placed, so use malloc elem len */
 		requested_len = 0;
-		mz_addr = malloc_heap_alloc_biggest(NULL, socket_id, flags,
-				align, contig);
+		mz_addr = malloc_heap_alloc_biggest(socket_id, flags, align, contig);
 	} else {
 		if (len == 0)
 			requested_len = bound;
 		/* allocate memory on heap */
-		mz_addr = malloc_heap_alloc(NULL, requested_len, socket_id,
-				flags, align, bound, contig);
+		mz_addr = malloc_heap_alloc(requested_len, socket_id, flags, align, bound, contig);
 	}
 	if (mz_addr == NULL) {
 		rte_errno = ENOMEM;
diff --git a/lib/eal/common/malloc_heap.c b/lib/eal/common/malloc_heap.c
index 5ff27548ff..058aaf4209 100644
--- a/lib/eal/common/malloc_heap.c
+++ b/lib/eal/common/malloc_heap.c
@@ -229,8 +229,8 @@ find_biggest_element(struct malloc_heap *heap, size_t *size,
  * the new element after releasing the lock.
  */
 static void *
-heap_alloc(struct malloc_heap *heap, const char *type __rte_unused, size_t size,
-		unsigned int flags, size_t align, size_t bound, bool contig)
+heap_alloc(struct malloc_heap *heap, size_t size, unsigned int flags,
+	   size_t align, size_t bound, bool contig)
 {
 	struct malloc_elem *elem;
 	size_t user_size = size;
@@ -255,8 +255,7 @@ heap_alloc(struct malloc_heap *heap, const char *type __rte_unused, size_t size,
 }
 
 static void *
-heap_alloc_biggest(struct malloc_heap *heap, const char *type __rte_unused,
-		unsigned int flags, size_t align, bool contig)
+heap_alloc_biggest(struct malloc_heap *heap, unsigned int flags, size_t align, bool contig)
 {
 	struct malloc_elem *elem;
 	size_t size;
@@ -640,8 +639,7 @@ alloc_more_mem_on_socket(struct malloc_heap *heap, size_t size, int socket,
 
 /* this will try lower page sizes first */
 static void *
-malloc_heap_alloc_on_heap_id(const char *type, size_t size,
-		unsigned int heap_id, unsigned int flags, size_t align,
+malloc_heap_alloc_on_heap_id(size_t size, unsigned int heap_id, unsigned int flags, size_t align,
 		size_t bound, bool contig)
 {
 	struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config;
@@ -658,7 +656,7 @@ malloc_heap_alloc_on_heap_id(const char *type, size_t size,
 
 	/* for legacy mode, try once and with all flags */
 	if (internal_conf->legacy_mem) {
-		ret = heap_alloc(heap, type, size, flags, align, bound, contig);
+		ret = heap_alloc(heap, size, flags, align, bound, contig);
 		goto alloc_unlock;
 	}
 
@@ -679,7 +677,7 @@ malloc_heap_alloc_on_heap_id(const char *type, size_t size,
 	if (socket_id < 0)
 		size_flags |= RTE_MEMZONE_SIZE_HINT_ONLY;
 
-	ret = heap_alloc(heap, type, size, size_flags, align, bound, contig);
+	ret = heap_alloc(heap, size, size_flags, align, bound, contig);
 	if (ret != NULL)
 		goto alloc_unlock;
 
@@ -689,7 +687,7 @@ malloc_heap_alloc_on_heap_id(const char *type, size_t size,
 
 	if (!alloc_more_mem_on_socket(heap, size, socket_id, flags, align,
 			bound, contig)) {
-		ret = heap_alloc(heap, type, size, flags, align, bound, contig);
+		ret = heap_alloc(heap, size, flags, align, bound, contig);
 
 		/* this should have succeeded */
 		if (ret == NULL)
@@ -730,8 +728,8 @@ malloc_get_numa_socket(void)
 }
 
 void *
-malloc_heap_alloc(const char *type, size_t size, int socket_arg,
-		unsigned int flags, size_t align, size_t bound, bool contig)
+malloc_heap_alloc(size_t size, int socket_arg, unsigned int flags,
+		  size_t align, size_t bound, bool contig)
 {
 	int socket, heap_id, i;
 	void *ret;
@@ -754,8 +752,7 @@ malloc_heap_alloc(const char *type, size_t size, int socket_arg,
 	if (heap_id < 0)
 		return NULL;
 
-	ret = malloc_heap_alloc_on_heap_id(type, size, heap_id, flags, align,
-			bound, contig);
+	ret = malloc_heap_alloc_on_heap_id(size, heap_id, flags, align, bound, contig);
 	if (ret != NULL || socket_arg != SOCKET_ID_ANY)
 		return ret;
 
@@ -765,8 +762,7 @@ malloc_heap_alloc(const char *type, size_t size, int socket_arg,
 	for (i = 0; i < (int) rte_socket_count(); i++) {
 		if (i == heap_id)
 			continue;
-		ret = malloc_heap_alloc_on_heap_id(type, size, i, flags, align,
-				bound, contig);
+		ret = malloc_heap_alloc_on_heap_id(size, i, flags, align, bound, contig);
 		if (ret != NULL)
 			return ret;
 	}
@@ -774,7 +770,7 @@ malloc_heap_alloc(const char *type, size_t size, int socket_arg,
 }
 
 static void *
-heap_alloc_biggest_on_heap_id(const char *type, unsigned int heap_id,
+heap_alloc_biggest_on_heap_id(unsigned int heap_id,
 		unsigned int flags, size_t align, bool contig)
 {
 	struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config;
@@ -785,7 +781,7 @@ heap_alloc_biggest_on_heap_id(const char *type, unsigned int heap_id,
 
 	align = align == 0 ? 1 : align;
 
-	ret = heap_alloc_biggest(heap, type, flags, align, contig);
+	ret = heap_alloc_biggest(heap, flags, align, contig);
 
 	rte_spinlock_unlock(&(heap->lock));
 
@@ -793,8 +789,7 @@ heap_alloc_biggest_on_heap_id(const char *type, unsigned int heap_id,
 }
 
 void *
-malloc_heap_alloc_biggest(const char *type, int socket_arg, unsigned int flags,
-		size_t align, bool contig)
+malloc_heap_alloc_biggest(int socket_arg, unsigned int flags, size_t align, bool contig)
 {
 	int socket, i, cur_socket, heap_id;
 	void *ret;
@@ -817,8 +812,7 @@ malloc_heap_alloc_biggest(const char *type, int socket_arg, unsigned int flags,
 	if (heap_id < 0)
 		return NULL;
 
-	ret = heap_alloc_biggest_on_heap_id(type, heap_id, flags, align,
-			contig);
+	ret = heap_alloc_biggest_on_heap_id(heap_id, flags, align, contig);
 	if (ret != NULL || socket_arg != SOCKET_ID_ANY)
 		return ret;
 
@@ -827,8 +821,7 @@ malloc_heap_alloc_biggest(const char *type, int socket_arg, unsigned int flags,
 		cur_socket = rte_socket_id_by_idx(i);
 		if (cur_socket == socket)
 			continue;
-		ret = heap_alloc_biggest_on_heap_id(type, i, flags, align,
-				contig);
+		ret = heap_alloc_biggest_on_heap_id(i, flags, align, contig);
 		if (ret != NULL)
 			return ret;
 	}
diff --git a/lib/eal/common/malloc_heap.h b/lib/eal/common/malloc_heap.h
index 0c49588005..dfc56d4ae3 100644
--- a/lib/eal/common/malloc_heap.h
+++ b/lib/eal/common/malloc_heap.h
@@ -34,12 +34,11 @@ struct __rte_cache_aligned malloc_heap {
 };
 
 void *
-malloc_heap_alloc(const char *type, size_t size, int socket, unsigned int flags,
-		size_t align, size_t bound, bool contig);
+malloc_heap_alloc(size_t size, int socket, unsigned int flags, size_t align,
+		  size_t bound, bool contig);
 
 void *
-malloc_heap_alloc_biggest(const char *type, int socket, unsigned int flags,
-		size_t align, bool contig);
+malloc_heap_alloc_biggest(int socket, unsigned int flags, size_t align, bool contig);
 
 int
 malloc_heap_create(struct malloc_heap *heap, const char *heap_name);
diff --git a/lib/eal/common/rte_malloc.c b/lib/eal/common/rte_malloc.c
index 6d3c301a23..3eed4d4be6 100644
--- a/lib/eal/common/rte_malloc.c
+++ b/lib/eal/common/rte_malloc.c
@@ -69,7 +69,7 @@ malloc_socket(const char *type, size_t size, unsigned int align,
 				!rte_eal_has_hugepages())
 		socket_arg = SOCKET_ID_ANY;
 
-	ptr = malloc_heap_alloc(type, size, socket_arg, 0,
+	ptr = malloc_heap_alloc(size, socket_arg, 0,
 			align == 0 ? 1 : align, 0, false);
 
 	if (trace_ena)
-- 
2.43.0

Re: [PATCH v3 0/2] malloc type cleanups

[David Marchand]

On Tue, Jun 18, 2024 at 4:45 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> The type parameter for malloc is only used for tracing.
> Fix documentation and don't pass through heap routines.
>
> Stephen Hemminger (2):
>   rte_malloc: document that type is for tracing
>   eal: remove type argument from internal routines
>

Series applied, thanks.


-- 
David Marchand