lib/librte_eal/common/rte_malloc: remove redundant check for size and alignment
Checks
Commit Message
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(-)
Comments
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.
On 15-Jul-20 2:31 PM, Sarosh Arif wrote:
> 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>
> ---
Technically not a bug, so i don't think backporting this is necessary.
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
On 17-Jul-20 3:40 PM, Burakov, Anatoly wrote:
> On 15-Jul-20 2:31 PM, Sarosh Arif wrote:
>> 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>
>> ---
>
> Technically not a bug, so i don't think backporting this is necessary.
>
> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
>
Actually, now that i think of it, i agree with Lukasz - while this check
isn't necessary, it's an early exit to avoid memory locking. So, NACK.
@@ -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