From mboxrd@z Thu Jan 1 00:00:00 1970 From: Olivier Matz Subject: Re: [PATCH] mempool: fix local cache initialization Date: Thu, 9 Jun 2016 10:03:11 +0200 Message-ID: <575922BF.8040509@6wind.com> References: <1465398627-35022-1-git-send-email-sergio.gonzalez.monroy@intel.com> <57586EAC.7090304@6wind.com> <1cafd17e-ffe1-83cc-e745-d3dedc5d44cf@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit To: Sergio Gonzalez Monroy , dev@dpdk.org Return-path: Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67]) by dpdk.org (Postfix) with ESMTP id D88A7C1C6 for ; Thu, 9 Jun 2016 10:03:17 +0200 (CEST) In-Reply-To: <1cafd17e-ffe1-83cc-e745-d3dedc5d44cf@intel.com> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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