[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

[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 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

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 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 --]

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>