Re: [dpdk-dev] [PATCH] lib/librte_eal/common/rte_malloc: remove redundant check for size and alignment

[Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>]

W dniu 15.07.2020 o 15:31, Sarosh Arif pisze:
> Since mallock_socket() always calls malloc_heap_alloc() and
> this check is present inside malloc_heap_alloc() so there is
> no need to place it in mallock_socket().
>
> Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com>
> ---
>   lib/librte_eal/common/rte_malloc.c | 4 ----
>   1 file changed, 4 deletions(-)
>
> diff --git a/lib/librte_eal/common/rte_malloc.c b/lib/librte_eal/common/rte_malloc.c
> index 9d39e58c0..51256117b 100644
> --- a/lib/librte_eal/common/rte_malloc.c
> +++ b/lib/librte_eal/common/rte_malloc.c
> @@ -61,10 +61,6 @@ malloc_socket(const char *type, size_t size, unsigned int align,
>   {
>   	void *ptr;
>   
> -	/* return NULL if size is 0 or alignment is not power-of-2 */
> -	if (size == 0 || (align && !rte_is_power_of_2(align)))
> -		return NULL;
> -
>   	/* if there are no hugepages and if we are not allocating from an
>   	 * external heap, use memory from any socket available. checking for
>   	 * socket being external may return -1 in case of invalid socket, but
Yes the check is duplicated.
However it allows to avoid many unnecessary operations that can cost a 
lot, e.g. locking memory inside rte_malloc_heap_socket_is_external().
I would keep it.

Please also note, that the checks are not quite the same as the 
malloc_heap_alloc is called with changed align value (align == 0 ? 1 : 
align):
         ptr = malloc_heap_alloc(type, size, socket_arg, 0, align == 0 ? 
1 : align, 0, false);
Although there should be no change in check behavior as for both align 
==0 and align == 1 the check will fail and won't be the cause of 
returning NULL.

-- 
Lukasz Wojciechowski
Principal Software Engineer

Samsung R&D Institute Poland
Samsung Electronics
Office +48 22 377 88 25
l.wojciechow@partner.samsung.com