From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 1DFAA4BE1 for ; Thu, 22 Jun 2017 17:15:23 +0200 (CEST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga105.fm.intel.com with ESMTP; 22 Jun 2017 08:15:22 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.39,373,1493708400"; d="scan'208";a="1163497321" Received: from smonroyx-mobl.ger.corp.intel.com (HELO [10.237.221.32]) ([10.237.221.32]) by fmsmga001.fm.intel.com with ESMTP; 22 Jun 2017 08:15:21 -0700 To: Tonghao Zhang , dev@dpdk.org References: <1494467793-19887-1-git-send-email-nic@opencloud.tech> Cc: Thomas Monjalon From: Sergio Gonzalez Monroy Message-ID: Date: Thu, 22 Jun 2017 16:15:21 +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: <1494467793-19887-1-git-send-email-nic@opencloud.tech> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v4] eal: Set numa node value for system which not support it. 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: Thu, 22 Jun 2017 15:15:24 -0000 Just fyi, the summary line should be lowercase apart from acronyms (DPDK guidelines). On 11/05/2017 02:56, Tonghao Zhang wrote: > The NUMA node information for PCI devices provided through > sysfs is invalid for AMD Opteron(TM) Processor 62xx and 63xx > on Red Hat Enterprise Linux 6, and VMs on some hypervisors. > It is good to see more checking for valid values. > > Signed-off-by: Tonghao Zhang > --- IMHO the message could be slightly improved by adding some of the replies that you made to your v3. ie. Typical wrong numa node in VMs $ cat /sys/devices/pci0000:00/0000:00:18.6/numa_node -1 > lib/librte_eal/linuxapp/eal/eal_pci.c | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c > index 595622b..c817b4c 100644 > --- a/lib/librte_eal/linuxapp/eal/eal_pci.c > +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c > @@ -310,18 +310,18 @@ > dev->max_vfs = (uint16_t)tmp; > } > > - /* get numa node */ > + /* get numa node, default to 0 if not present */ > snprintf(filename, sizeof(filename), "%s/numa_node", > dirname); > - if (access(filename, R_OK) != 0) { > - /* if no NUMA support, set default to 0 */ > - dev->device.numa_node = 0; > - } else { > - if (eal_parse_sysfs_value(filename, &tmp) < 0) { > - free(dev); > - return -1; > - } > + > + if (eal_parse_sysfs_value(filename, &tmp) == 0 && > + tmp < RTE_MAX_NUMA_NODES) > dev->device.numa_node = tmp; > + else { > + RTE_LOG(WARNING, EAL, > + "numa_node is invalid or not present. " > + "Set it 0 as default\n"); > + dev->device.numa_node = 0; > } > > rte_pci_device_name(addr, dev->name, sizeof(dev->name)); The code changes look fine, so I leave it to Thomas regarding the commit message :) Acked-by: Sergio Gonzalez Monroy