From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpbgsg2.qq.com (smtpbgsg2.qq.com [54.254.200.128]) by dpdk.org (Postfix) with ESMTP id 01B415688 for ; Thu, 1 Jun 2017 03:22:24 +0200 (CEST) X-QQ-mid: bizesmtp13t1496280138tr7hlplv Received: from [10.0.30.74] (unknown [106.120.127.10]) by esmtp4.qq.com (ESMTP) with id ; Thu, 01 Jun 2017 09:22:17 +0800 (CST) X-QQ-SSF: 01100000002000F0FG90B00A0000000 X-QQ-FEAT: nSYbxVChtlQnWwiR1KWeMDGX28RNiC14Ad98plulunf6d7i37Pzg7xDP1oufQ viepNjMohyxIxdurTj+De61ebFe7hmKXruSROxwvwBSMxNWnagreRoyXmfGDZV8905I/bGP Sc0jZDkMNfRll3I9SP+9z4EExpa+McOUGHZCUjJbA0NaDezjhihhrfXwouhqHjvdiRvMseU zOP84T+yg2MJyeesJxSZdZ2RJ3NMun9Q8PKWtAHvnEC4dYe2odVZdfR80POkd6FqP4s5lRq B6guy9jNl5wYY+86xjixb+GXrt+mBipdrGtYwQ8W2jyEMp X-QQ-GoodBg: 0 From: nickcooper-zhangtonghao Message-Id: Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\)) Date: Thu, 1 Jun 2017 09:22:19 +0800 In-Reply-To: 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: Thu, 01 Jun 2017 01:22:26 -0000 Did you think this patch is necessary. I submitted v4. v4: http://dpdk.org/dev/patchwork/patch/24212/ = Thanks. Nick > On May 10, 2017, at 10:20 PM, nickcooper-zhangtonghao = wrote: >=20 > Thanks for your review. >=20 >> 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? >=20 > 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. >=20 > This VMs numa node is -1. For example: >=20 > $ cat /sys/devices/pci0000:00/0000:00:18.6/numa_node > -1 >=20 >=20 >>> 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? >=20 > 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 >>=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. >=20 > Yes