From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpbguseast2.qq.com (smtpbguseast2.qq.com [54.204.34.130]) by dpdk.org (Postfix) with ESMTP id 575AD1C00 for ; Wed, 10 May 2017 16:21:04 +0200 (CEST) X-QQ-mid: bizesmtp3t1494426059t6n8eim2n Received: from [192.168.0.102] (unknown [124.204.174.180]) by esmtp4.qq.com (ESMTP) with id ; Wed, 10 May 2017 22:20:57 +0800 (CST) X-QQ-SSF: 01100000008000F0FG80000A0000000 X-QQ-FEAT: a2sbnTq7b+WJAaL0VuiH8v9KxhJF5tKwamv/WmfsvKdRp8/DbnlQ6HaZ255+f Jz19P6oY4jxF6KDFFzTWbxGvW4EEyNGDsLOKWm04kjIdLGzQrXzmEnQfYejovKrKK3LJMUG q+UWVVCyj/F1GTcalgYPnKjpn82l3VCT9v2dbRlU1zLTHhx/DzBRyY0avDPVmv0VE+80xwq 3GH+CA7t3e90K6wZNfoLtEZ3nEedHNP5KqYVgwlQTIXBbeH0HxLf9DuUlnyv7pM9z8WcMz4 lwP2Dtmlseg7NCmMoLjBPzvhj/YvPjfVSQgag+tNwUM4TV X-QQ-GoodBg: 0 From: nickcooper-zhangtonghao Message-Id: Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\)) Date: Wed, 10 May 2017 22:20:59 +0800 In-Reply-To: <1716317.nSDOrcW80P@xps> Cc: dev@dpdk.org, stephen@networkplumber.org To: Thomas Monjalon References: <1494315002-41982-1-git-send-email-nic@opencloud.tech> <1716317.nSDOrcW80P@xps> X-Mailer: Apple Mail (2.3273) X-QQ-SENDSIZE: 520 X-QQ-Bgrelay: 1 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH v3] 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: Wed, 10 May 2017 14:21:06 -0000 Thanks for your review. > On May 10, 2017, at 8:45 PM, Thomas Monjalon = wrote: >=20 >> 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. >=20 > Sorry I don't understand the range of affected platforms. > Is it only on Opteron? Opteron with RHEL6? Is it fixed in recent = kernels? > Which hypervisors? with which kernel? I get numa info from web: https://access.redhat.com/solutions/349913 = and VMs which OS is CentOS 7.0 and kernel is 3.10, are running on VMware = fusion. This VMs numa node is -1. For example: $ cat /sys/devices/pci0000:00/0000:00:18.6/numa_node -1 >> It is good to see more checking for valid values. >=20 > If values are wrong, what can we do? > Here you check that value is not too high. > What about other kind of wrong values? >=20 >> Signed-off-by: Tonghao Zhang > > [...] >> - /* 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) !=3D 0) { >> - /* if no NUMA support, set default to 0 */ >> - dev->device.numa_node =3D 0; >=20 > Why removing the access() check? I review the code of eal_parse_sysfs_value(). If the =E2=80=98filename=E2=80= =99 cannot be accessed. the eal_parse_sysfs_value cannot open it, and returen -1. so using = eal_parse_sysfs_value is simple. >=20 >> - } else { >> - if (eal_parse_sysfs_value(filename, &tmp) < 0) { >> - free(dev); >> - return -1; >> - } >> + >> + if (eal_parse_sysfs_value(filename, &tmp) =3D=3D 0 && >> + tmp < RTE_MAX_NUMA_NODES) >> dev->device.numa_node =3D tmp; >> - } >> + else >> + dev->device.numa_node =3D 0; >=20 > It would deserve at least a warning log. Yes