From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sergio Gonzalez Monroy Subject: Re: [PATCH v4] eal: Set numa node value for system which not support it. Date: Thu, 22 Jun 2017 16:15:21 +0100 Message-ID: References: <1494467793-19887-1-git-send-email-nic@opencloud.tech> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Cc: Thomas Monjalon To: Tonghao Zhang , dev@dpdk.org 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) In-Reply-To: <1494467793-19887-1-git-send-email-nic@opencloud.tech> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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