diff mbox series

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

Message ID 20240425182738.4771-5-stephen@networkplumber.org (mailing list archive)
State New
Delegated to: Thomas Monjalon
Headers
Series malloc type argument cleanup (part 1) |

Checks

Context Check Description
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/intel-Functional success Functional PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-sample-apps-testing success Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/checkpatch success coding style OK

Commit Message

Stephen Hemminger April 25, 2024, 6:24 p.m. UTC 
  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(-)

Comments

Tyler Retzlaff April 26, 2024, 4:16 p.m. UTC | #1 
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.
Stephen Hemminger April 26, 2024, 10:52 p.m. UTC | #2 
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?
Tyler Retzlaff April 26, 2024, 11:06 p.m. UTC | #3 
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!
diff mbox series

Patch

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);