DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] lib/librte_eal/common/rte_malloc: remove redundant check for size and alignment
@ 2020-07-15 13:31 ` Sarosh Arif
  2020-07-16 22:23   ` Lukasz Wojciechowski
  2020-07-17 14:40   ` Burakov, Anatoly
  0 siblings, 2 replies; 4+ messages in thread
From: Sarosh Arif @ 2020-07-15 13:31 UTC (permalink / raw)
  To: anatoly.burakov; +Cc: dev, Sarosh Arif

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-dev] [PATCH] lib/librte_eal/common/rte_malloc: remove redundant check for size and alignment
  2020-07-15 13:31 ` [dpdk-dev] [PATCH] lib/librte_eal/common/rte_malloc: remove redundant check for size and alignment Sarosh Arif
@ 2020-07-16 22:23   ` Lukasz Wojciechowski
  2020-07-17 14:40   ` Burakov, Anatoly
  1 sibling, 0 replies; 4+ messages in thread
From: Lukasz Wojciechowski @ 2020-07-16 22:23 UTC (permalink / raw)
  To: Sarosh Arif, anatoly.burakov; +Cc: dev

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-dev] [PATCH] lib/librte_eal/common/rte_malloc: remove redundant check for size and alignment
  2020-07-15 13:31 ` [dpdk-dev] [PATCH] lib/librte_eal/common/rte_malloc: remove redundant check for size and alignment Sarosh Arif
  2020-07-16 22:23   ` Lukasz Wojciechowski
@ 2020-07-17 14:40   ` Burakov, Anatoly
  2020-07-17 14:49     ` Burakov, Anatoly
  1 sibling, 1 reply; 4+ messages in thread
From: Burakov, Anatoly @ 2020-07-17 14:40 UTC (permalink / raw)
  To: Sarosh Arif; +Cc: dev

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>

-- 
Thanks,
Anatoly

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-dev] [PATCH] lib/librte_eal/common/rte_malloc: remove redundant check for size and alignment
  2020-07-17 14:40   ` Burakov, Anatoly
@ 2020-07-17 14:49     ` Burakov, Anatoly
  0 siblings, 0 replies; 4+ messages in thread
From: Burakov, Anatoly @ 2020-07-17 14:49 UTC (permalink / raw)
  To: Sarosh Arif; +Cc: dev

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.

-- 
Thanks,
Anatoly

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-07-17 14:49 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20200715133236eucas1p177311544d71d3806b436cf69734ffe08@eucas1p1.samsung.com>
2020-07-15 13:31 ` [dpdk-dev] [PATCH] lib/librte_eal/common/rte_malloc: remove redundant check for size and alignment Sarosh Arif
2020-07-16 22:23   ` Lukasz Wojciechowski
2020-07-17 14:40   ` Burakov, Anatoly
2020-07-17 14:49     ` Burakov, Anatoly

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git