[dpdk-dev] [PATCH v3] eal: Set numa node value for system which not support it.
nickcooper-zhangtonghao
nic at opencloud.tech
Thu Jun 1 03:22:19 CEST 2017
Did you think this patch is necessary. I submitted v4.
v4:
http://dpdk.org/dev/patchwork/patch/24212/ <http://dpdk.org/dev/patchwork/patch/24212/>
Thanks.
Nick
> On May 10, 2017, at 10:20 PM, nickcooper-zhangtonghao <nic at opencloud.tech> wrote:
>
> Thanks for your review.
>
>> On May 10, 2017, at 8:45 PM, Thomas Monjalon <thomas at monjalon.net <mailto:thomas at monjalon.net>> 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.
>>
>> 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><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.
>>
>> If values are wrong, what can we do?
>> Here you check that value is not too high.
>> What about other kind of wrong values?
>>
>>> Signed-off-by: Tonghao Zhang <nic at opencloud.tech <mailto:nic at opencloud.tech><mailto:nic at opencloud.tech <mailto:nic at 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) != 0) {
>>> - /* if no NUMA support, set default to 0 */
>>> - dev->device.numa_node = 0;
>>
>> Why removing the access() check?
>
> I review the code of eal_parse_sysfs_value(). If the ‘filename’ cannot be accessed.
> the eal_parse_sysfs_value cannot open it, and returen -1. so using eal_parse_sysfs_value is simple.
>
>>
>>> - } 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
>>> + dev->device.numa_node = 0;
>>
>> It would deserve at least a warning log.
>
> Yes
More information about the dev
mailing list