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 <n...@opencloud.tech>
---

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 <sergio.gonzalez.mon...@intel.com>

Reply via email to