Re: [dpdk-dev] [PATCH] eal: fix rte_zalloc_socket to zero memory

["Burakov, Anatoly" <anatoly.burakov@intel.com>]

On 10-Dec-18 10:26 AM, Bruce Richardson wrote:
> On Fri, Dec 07, 2018 at 05:24:20PM -0500, David Harton wrote:
>> The zalloc and calloc functions do not actually zero the memory.
>> Added memset to rte_zmalloc_socket() so allocated memory is cleared.
>>
>> Signed-off-by: David Harton <dharton@cisco.com>
>> ---
>>   lib/librte_eal/common/rte_malloc.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/librte_eal/common/rte_malloc.c b/lib/librte_eal/common/rte_malloc.c
>> index 0da5ad5e8..be382e534 100644
>> --- a/lib/librte_eal/common/rte_malloc.c
>> +++ b/lib/librte_eal/common/rte_malloc.c
>> @@ -74,7 +74,9 @@ rte_malloc(const char *type, size_t size, unsigned align)
>>   void *
>>   rte_zmalloc_socket(const char *type, size_t size, unsigned align, int socket)
>>   {
>> -	return rte_malloc_socket(type, size, align, socket);
>> +	void *new_ptr = rte_malloc_socket(type, size, align, socket);
>> +	if (new_ptr) memset(new_ptr, 0, size);
>> +	return new_ptr;
>>   }
> 
> While this is functionally correct, I believe this memset should not
> actually be needed. A few years back we changed the behaviour in DPDK to
> always zero memory on free, rather than zeroing on allocate. This worked
> fine because the kernel always gave us zeroed hugepages and zeroing them a
> second time was a waste of cycles. The percentage of memory that was
> subsequently freed and reallocated was small so zeroing on free saved quite
> a bit of processing time, especially at app startup.
> 
> If, following all the memory rework in recent releases, this scheme of
> zeroing on free no longer works, I'd rather see that fixed than go back to
> the scheme of zeroing on allocation. [Assuming we do fix it, a comment
> explaining the missing memset would also be good to avoid future patches
> here]

Bruce is correct. Memory is zeroed-out on free, and we get zeroed-out 
pages from the kernel, so memory from rte_malloc and rte_zmalloc are (or 
should be) functionally equivalent.

If there are any circumstances where memory is not being freed (and 
there were a few bugs like that), then i'd rather fix those too.

> 
> Regards,
> /Bruce
> 


-- 
Thanks,
Anatoly