From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <sergio.gonzalez.monroy@intel.com>
Received: from mga06.intel.com (mga06.intel.com [134.134.136.31])
 by dpdk.org (Postfix) with ESMTP id BA6AB231E
 for <dev@dpdk.org>; Fri, 21 Jul 2017 17:03:54 +0200 (CEST)
Received: from orsmga003.jf.intel.com ([10.7.209.27])
 by orsmga104.jf.intel.com with ESMTP; 21 Jul 2017 08:03:53 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.40,390,1496127600"; d="scan'208";a="995602840"
Received: from smonroyx-mobl.ger.corp.intel.com (HELO [10.237.221.5])
 ([10.237.221.5])
 by orsmga003.jf.intel.com with ESMTP; 21 Jul 2017 08:03:52 -0700
To: Thomas Monjalon <thomas@monjalon.net>
References: <20170721091119.15701-1-sergio.gonzalez.monroy@intel.com>
 <1688226.sl7kgNVU6i@xps>
Cc: dev@dpdk.org, nic@opencloud.tech
From: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
Message-ID: <d0b81ffa-e75c-7b51-17da-27b04b949536@intel.com>
Date: Fri, 21 Jul 2017 16:03:51 +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: <1688226.sl7kgNVU6i@xps>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [dpdk-dev] [PATCH] pci: limit default numa node to used devices
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Fri, 21 Jul 2017 15:03:55 -0000

On 21/07/2017 15:53, Thomas Monjalon wrote:
> The title and the text below should explain that you move
> the warning log from scan to probe, thanks to a temporary
> negative value.

I thought that saying that I only check for devices managed by dpdk 
explains the purpose,
and the patch itself shows the change from one file to another.

> 21/07/2017 12:11, Sergio Gonzalez Monroy:
>> Commit 8a04cb612589 ("pci: set default numa node for broken systems")
>> added logic to default to NUMA node 0 when sysfs numa_node information
>> was wrong or not available.
>>
>> Unfortunately there are many devices with wrong NUMA node information
>> that DPDK does not care about but still show warnings for them.
>>
>> Instead, only check for invalid NUMA node information for devices
>> managed by the DPDK.
>>
>> Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> [...]
>> -	if (eal_parse_sysfs_value(filename, &tmp) == 0 &&
>> -		tmp < RTE_MAX_NUMA_NODES)
>> +	if (eal_parse_sysfs_value(filename, &tmp) == 0)
>>   		dev->device.numa_node = tmp;
> Why are you removing the check of the value?
> Are you going to accept invalid high values?
> This check was introduced on purpose by this commit:
> 	http://dpdk.org/commit/8a04cb6125

tmp is unsigned long type, so -1 is going to be a large number.
My understanding was that it was basically checking for -1 as numa_node.

If we have valid numa_node greater than RTE_MAX_NUMA_NODES, defaulting 
to 0 is not a good idea, is it?

What I try to achieve with the patch is:
- if no numa_node avilable then parse is going to fail and we set -1.
- if numa_node is present but wrong, my understanding was that it would 
be -1.

Thanks,
Sergio