Re: [PATCH] nodedev: Export NUMA node locality for PCI devices

[Michal Privoznik <mprivozn@xxxxxxxxxx>]

On 06.06.2014 14:02, Daniel P. Berrange wrote:
> On Fri, Jun 06, 2014 at 01:09:58PM +0200, Michal Privoznik wrote:
>> A PCI device can be associated with a specific NUMA node. Later, when
>> a guest is pinned to one NUMA node the PCI device can be assigned on
>> different NUMA node. This makes DMA transfers travel across nodes and
>> thus results in suboptimal performance. We should expose the NUMA node
>> locality for PCI devices so management applications can make better
>> decisions.
>>
>> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
>> ---
> 
>> diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
>> index 9a951d9..8e98ad2 100644
>> --- a/src/node_device/node_device_udev.c
>> +++ b/src/node_device/node_device_udev.c
>> @@ -493,6 +493,13 @@ static int udevProcessPCI(struct udev_device *device,
>>           goto out;
>>       }
>>   
>> +    if (udevGetIntSysfsAttr(device,
>> +                            "numa_node",
>> +                            &data->pci_dev.numa_node,
>> +                            10) == PROPERTY_ERROR) {
>> +        goto out;
> 
> Will this result in an error if the 'numa_node' file does not exist
> in sysfs - I wouldn't be suprised if older kernels lack it.

No, if a file doesn't exist PROPERTY_MISSING is returned. PROPERTY_ERROR is returned if the file content can't be parsed as int. BTW: the 'numa_node' file is around since ages:

kernel.git $ git describe --contains 81bb0e198
v2.6.21-rc1~80^2

But in order to deal with even older kernels (or those which are not NUMA aware) I'm squashing in this:

diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index 8e98ad2..91fc16f 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -493,11 +493,16 @@ static int udevProcessPCI(struct udev_device *device,
         goto out;
     }
 
-    if (udevGetIntSysfsAttr(device,
+    rc = udevGetIntSysfsAttr(device,
                             "numa_node",
                             &data->pci_dev.numa_node,
-                            10) == PROPERTY_ERROR) {
+                            10);
+    if (rc == PROPERTY_ERROR) {
         goto out;
+    } else if (rc == PROPERTY_MISSING) {
+        /* The default value is -1, because it can't be 0
+         * as zero is valid node number. */
+        data->pci_dev.numa_node = -1;
     }
 
     if (!virPCIGetPhysicalFunction(syspath, &data->pci_dev.physical_function))


and pushing. Thanks.


Michal

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list