From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 505FFC330 for ; Thu, 9 Jun 2016 10:14:15 +0200 (CEST) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga101.fm.intel.com with ESMTP; 09 Jun 2016 01:14:14 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.26,443,1459839600"; d="scan'208";a="118807510" Received: from smonroyx-mobl.ger.corp.intel.com (HELO [10.237.220.57]) ([10.237.220.57]) by fmsmga004.fm.intel.com with ESMTP; 09 Jun 2016 01:14:14 -0700 To: Olivier Matz , dev@dpdk.org References: <1465398627-35022-1-git-send-email-sergio.gonzalez.monroy@intel.com> <57586EAC.7090304@6wind.com> <1cafd17e-ffe1-83cc-e745-d3dedc5d44cf@intel.com> <575922BF.8040509@6wind.com> From: Sergio Gonzalez Monroy Message-ID: <4901ef09-2877-f95b-a0f6-96699c01416e@intel.com> Date: Thu, 9 Jun 2016 09:14:13 +0100 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 MIME-Version: 1.0 In-Reply-To: <575922BF.8040509@6wind.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH] mempool: fix local cache initialization X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 09 Jun 2016 08:14:17 -0000 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