From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id F05EC316B for ; Mon, 10 Dec 2018 11:45:35 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 10 Dec 2018 02:45:34 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,338,1539673200"; d="scan'208";a="99484831" Received: from aburakov-mobl1.ger.corp.intel.com (HELO [10.237.220.93]) ([10.237.220.93]) by orsmga006.jf.intel.com with ESMTP; 10 Dec 2018 02:45:33 -0800 To: Bruce Richardson , David Harton Cc: dev@dpdk.org References: <20181207222420.9508-1-dharton@cisco.com> <20181210102622.GA10896@bricha3-MOBL.ger.corp.intel.com> From: "Burakov, Anatoly" Message-ID: <70a5d2ea-b0de-a2a7-1901-415b4b5a192a@intel.com> Date: Mon, 10 Dec 2018 10:45:32 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.3.2 MIME-Version: 1.0 In-Reply-To: <20181210102622.GA10896@bricha3-MOBL.ger.corp.intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH] eal: fix rte_zalloc_socket to zero memory X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 10 Dec 2018 10:45:36 -0000 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 >> --- >> 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