On 21/07/2017 16:37, Thomas Monjalon wrote:
21/07/2017 18:03, Sergio Gonzalez Monroy:
On 21/07/2017 15:53, Thomas Monjalon wrote:
The title and the text below should explain that you move
the warning log from scan to probe, thanks to a temporary
negative value.
I thought that saying that I only check for devices managed by dpdk
explains the purpose,
and the patch itself shows the change from one file to another.
It is obvious when you look carefully at the code, yes.
I was just giving my help to better explain :)
Just giving my view of the commit message
If you think it can be improve,by all meansfeel free to change it :)
21/07/2017 12:11, Sergio Gonzalez Monroy:
Commit 8a04cb612589 ("pci: set default numa node for broken systems")
added logic to default to NUMA node 0 when sysfs numa_node information
was wrong or not available.
Unfortunately there are many devices with wrong NUMA node information
that DPDK does not care about but still show warnings for them.
Instead, only check for invalid NUMA node information for devices
managed by the DPDK.
Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.mon...@intel.com>
[...]
- if (eal_parse_sysfs_value(filename, &tmp) == 0 &&
- tmp < RTE_MAX_NUMA_NODES)
+ if (eal_parse_sysfs_value(filename, &tmp) == 0)
dev->device.numa_node = tmp;
Why are you removing the check of the value?
Are you going to accept invalid high values?
This check was introduced on purpose by this commit:
http://dpdk.org/commit/8a04cb6125
tmp is unsigned long type, so -1 is going to be a large number.
Oh yes, I missed it was unsigned!
My understanding was that it was basically checking for -1 as numa_node.
If we have valid numa_node greater than RTE_MAX_NUMA_NODES, defaulting
to 0 is not a good idea, is it?
What I try to achieve with the patch is:
- if no numa_node avilable then parse is going to fail and we set -1.
- if numa_node is present but wrong, my understanding was that it would
be -1.
All your explanations make sense when you realize that it is unsigned.
I have one more question,
Does it work to check for a negative value like this?
if (dev->device.numa_node < 0)
numa_node is signed int type in struct rte_device, so it should work.
Regards,
Sergio
