From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <anatoly.burakov@intel.com>
Received: from mga07.intel.com (mga07.intel.com [134.134.136.100])
 by dpdk.org (Postfix) with ESMTP id 8AC4BF04;
 Fri, 21 Sep 2018 10:57:09 +0200 (CEST)
X-Amp-Result: SKIPPED(no attachment in message)
X-Amp-File-Uploaded: False
Received: from fmsmga007.fm.intel.com ([10.253.24.52])
 by orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;
 21 Sep 2018 01:57:08 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.54,284,1534834800"; d="scan'208";a="71782574"
Received: from aburakov-mobl1.ger.corp.intel.com (HELO [10.237.220.55])
 ([10.237.220.55])
 by fmsmga007.fm.intel.com with ESMTP; 21 Sep 2018 01:57:04 -0700
To: Ilya Maximets <i.maximets@samsung.com>, dev@dpdk.org
Cc: solal.pirelli@gmail.com, stable@dpdk.org
References: <2624b855f3454691212cdb244f04926631c391a2.1535544966.git.anatoly.burakov@intel.com>
 <CGME20180920125054epcas2p26c547d287926ed85abad1fe3fbb4ec59@epcas2p2.samsung.com>
 <7e4178219213303b982e505ae4cb4387d9d3814a.1537447684.git.anatoly.burakov@intel.com>
 <20180921064524eucas1p147ff66419605b9b141969e1a672196f9~WV1A0hUgX1031210312eucas1p1k@eucas1p1.samsung.com>
From: "Burakov, Anatoly" <anatoly.burakov@intel.com>
Message-ID: <5e5d7251-8d51-e413-ae49-cbaae562fa19@intel.com>
Date: Fri, 21 Sep 2018 09:57:02 +0100
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101
 Thunderbird/52.9.1
MIME-Version: 1.0
In-Reply-To: <20180921064524eucas1p147ff66419605b9b141969e1a672196f9~WV1A0hUgX1031210312eucas1p1k@eucas1p1.samsung.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Subject: Re: [dpdk-stable] [PATCH v2] mem: fix undefined behavior in NUMA
	code
X-BeenThere: stable@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches for DPDK stable branches <stable.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/stable>,
 <mailto:stable-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/stable/>
List-Post: <mailto:stable@dpdk.org>
List-Help: <mailto:stable-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/stable>,
 <mailto:stable-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Fri, 21 Sep 2018 08:57:10 -0000

On 21-Sep-18 7:47 AM, Ilya Maximets wrote:
> On 20.09.2018 15:50, Anatoly Burakov wrote:
>> When NUMA-aware hugepages config option is set, we rely on
>> libnuma to tell the kernel to allocate hugepages on a specific
>> NUMA node. However, we allocate node mask before we check if
>> NUMA is available in the first place, which, according to
>> the manpage [1], causes undefined behaviour.
>>
>> Fix by only using nodemask when we have NUMA available.
>>
>> [1] https://linux.die.net/man/3/numa_alloc_onnode
>>
>> Bugzilla ID: 20
>>
>> Fixes: 1b72605d2416 ("mem: balanced allocation of hugepages")
>> Cc: i.maximets@samsung.com
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>> ---
>>   lib/librte_eal/linuxapp/eal/eal_memory.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c b/lib/librte_eal/linuxapp/eal/eal_memory.c
>> index dbf19499e..1a2a84a65 100644
>> --- a/lib/librte_eal/linuxapp/eal/eal_memory.c
>> +++ b/lib/librte_eal/linuxapp/eal/eal_memory.c
>> @@ -263,7 +263,7 @@ map_all_hugepages(struct hugepage_file *hugepg_tbl, struct hugepage_info *hpi,
>>   	int node_id = -1;
>>   	int essential_prev = 0;
>>   	int oldpolicy;
>> -	struct bitmask *oldmask = numa_allocate_nodemask();
>> +	struct bitmask *oldmask = NULL;
>>   	bool have_numa = true;
>>   	unsigned long maxnode = 0;
>>   
>> @@ -275,6 +275,7 @@ map_all_hugepages(struct hugepage_file *hugepg_tbl, struct hugepage_info *hpi,
>>   
>>   	if (have_numa) {
>>   		RTE_LOG(DEBUG, EAL, "Trying to obtain current memory policy.\n");
>> +		oldmask = numa_allocate_nodemask();
>>   		if (get_mempolicy(&oldpolicy, oldmask->maskp,
>>   				  oldmask->size + 1, 0, 0) < 0) {
>>   			RTE_LOG(ERR, EAL,
>> @@ -401,8 +402,8 @@ map_all_hugepages(struct hugepage_file *hugepg_tbl, struct hugepage_info *hpi,
>>   				strerror(errno));
>>   			numa_set_localalloc();
>>   		}
>> +		numa_free_cpumask(oldmask);
>>   	}
>> -	numa_free_cpumask(oldmask);
> 
> There will be 'oldmask' leak in case no 'socket-mem' requested.

Oh, right, you're correct!

> 
>>   #endif
>>   	return i;
>>   }
>>
> 


-- 
Thanks,
Anatoly