Re: [dpdk-dev] [PATCH] acl: use rte_calloc for temporary memory allocation

[Vladyslav Buslov <Vladyslav.Buslov@harmonicinc.com>]

Hi Konstantin,

Thanks for your feedback.

Would you accept this change as config file compile-time parameter with libc calloc as default?
It is one line change only so it is easy to ifdef.

Regards,
Vlad

-----Original Message-----
From: Ananyev, Konstantin [mailto:konstantin.ananyev@intel.com] 
Sent: Wednesday, August 31, 2016 4:28 AM
To: Vladyslav Buslov
Cc: dev@dpdk.org
Subject: RE: [PATCH] acl: use rte_calloc for temporary memory allocation

Hi Vladyslav,

> -----Original Message-----
> From: Vladyslav Buslov [mailto:vladyslav.buslov@harmonicinc.com]
> Sent: Tuesday, August 16, 2016 3:01 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Cc: dev@dpdk.org
> Subject: [PATCH] acl: use rte_calloc for temporary memory allocation
> 
> Acl build process uses significant amount of memory which degrades 
> performance by causing page walks when memory is allocated on regular heap using libc calloc.
> 
> This commit changes tb_mem to allocate temporary memory on huge pages with rte_calloc.

We deliberately used standard system memory allocation routines (calloc/free) here.
With current design build phase was never considered to be an 'RT' phase operation.
It is pretty cpu and memory expensive.
So if we'll use RTE memory for build phase it could easily consume all (or most) of it, and might cause DPDK process failure or degradation.
If you really feel that you (and other users) would benefit from using rte_calloc here (I personally still in doubt), then at least it should be a new field inside rte_acl_config, that would allow user to control that behavior.
Though, as I said above, librte_acl was never designed to ' to allocate tens of thousands of ACLs at runtime'.
To add ability to add/delete rules at runtime while keeping lookup time reasonably low some new approach need to be introduced.  
Konstantin

> 
> Signed-off-by: Vladyslav Buslov <vladyslav.buslov@harmonicinc.com>
> ---
>  lib/librte_acl/tb_mem.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/librte_acl/tb_mem.c b/lib/librte_acl/tb_mem.c index 
> 157e608..c373673 100644
> --- a/lib/librte_acl/tb_mem.c
> +++ b/lib/librte_acl/tb_mem.c
> @@ -52,7 +52,7 @@ tb_pool(struct tb_mem_pool *pool, size_t sz)
>  	size_t size;
> 
>  	size = sz + pool->alignment - 1;
> -	block = calloc(1, size + sizeof(*pool->block));
> +	block = rte_calloc("ACL_TBMEM_BLOCK", 1, size + 
> +sizeof(*pool->block), 0);
>  	if (block == NULL) {
>  		RTE_LOG(ERR, MALLOC, "%s(%zu)\n failed, currently allocated "
>  			"by pool: %zu bytes\n", __func__, sz, pool->alloc);
> --
> 2.8.3