Re: [dpdk-dev] [PATCH] mempool: fix local cache initialization

[Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>]

On 09/06/2016 09:03, Olivier Matz wrote:
> Hi Sergio,
>
> On 06/09/2016 09:57 AM, Sergio Gonzalez Monroy wrote:
>> Hi Olivier,
>>
>> On 08/06/2016 20:14, Olivier Matz wrote:
>>> Hi Sergio,
>>>
>>> Good catch, thanks. The patch looks ok, just few comments
>>> on the commit log:
>>>
>>> On 06/08/2016 05:10 PM, Sergio Gonzalez Monroy wrote:
>>>> The mempool local cache is not being initialize properly leading to
>>> 'initialize' -> 'initialized' ?
>>> and maybe 'is not being' -> 'was not' ?
>>>
>>>> undefined behavior in cases where the allocated memory was used and left
>>>> with data.
>>>>
>>>> Fixes: af75078fece3 ("first public release")
>>> I think it fixes this one instead:
>>>
>>> 213af31e0960 ("mempool: reduce structure size if no cache needed")
>> Fair enough, I thought the issue was there as we never
>> initialized/zeroed the local cache
>> on mempool creation. Usually we would have allocated all mempools on
>> init (or close)
>> and that would be it (initially all memory would be zeroed), but I think
>> you could still
>> manage to reproduce the problem if somehow you where to do something like:
>> rte_malloc(), rte_free(), rte_mempool_create() and the memory was the
>> one we got
>> with malloc and never gets zeroed again.
> Before Keith's commit (213af31e0960), the local cache was initialized
> when doing the memset() because it was included in the mempool
> structure. So I think the problem did not exist before this patch.
> Or did I miss something in your explanation?
>
> Regards,
> Olivier

You are spot on!

I did look at a wrong commit when checking for the old mempool struct.

Cheers,
Sergio