From mboxrd@z Thu Jan  1 00:00:00 1970
From: nickcooper-zhangtonghao <nic@opencloud.tech>
Subject: Re: [PATCH v3] eal: Set numa node value for system which
 not support it.
Date: Wed, 10 May 2017 22:20:59 +0800
Message-ID: <B692A579-004D-448E-B95A-A68D38836DD8@opencloud.tech>
References: <1494315002-41982-1-git-send-email-nic@opencloud.tech>
 <1716317.nSDOrcW80P@xps>
Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\))
Content-Type: text/plain;
	charset=utf-8
Content-Transfer-Encoding: quoted-printable
Cc: dev@dpdk.org,
 stephen@networkplumber.org
To: Thomas Monjalon <thomas@monjalon.net>
Return-path: <dev-bounces@dpdk.org>
Received: from smtpbguseast2.qq.com (smtpbguseast2.qq.com [54.204.34.130])
 by dpdk.org (Postfix) with ESMTP id 575AD1C00
 for <dev@dpdk.org>; Wed, 10 May 2017 16:21:04 +0200 (CEST)
In-Reply-To: <1716317.nSDOrcW80P@xps>
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

Thanks for your review.

> On May 10, 2017, at 8:45 PM, Thomas Monjalon <thomas@monjalon.net> =
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 =
<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 <nic@opencloud.tech =
<mailto:nic@opencloud.tech>>
> [...]
>> -	/* 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