From 29183778af488870ed09bb2239740f5d6cdba68b Mon Sep 17 00:00:00 2001 From: John Ferlan Date: Thu, 18 Oct 2018 10:22:18 -0400 Subject: [PATCH] nodedev: Document the udevEventHandleThread Commit cdbe1332 neglected to document the API. So let's add some details about the algorithm and why it was used to help future readers understand the issues encountered. NB: Management of the processing udev device notification is a delicate balance between the udev process, the scheduler, and when exactly the data from/for the socket is received. The balance is particularly important for environments when multiple devices are added into the system more or less simultaneously such as is done for mdev or SRIOV. In these cases old libudev blocking on the udev recv() occurs more frequently. It's expected that future devices will follow similar algorithms. Even though the algorithm does present some challenges for older OS's (such as Centos 6), trying to rewrite the algorithm to fit both models would be more complex and involve pulling the monitor object out of the private data lockable object and would need to be guarded by a separate lock. Devising such an algorithm to work around issues with older OS's at the expense of more modern OS algorithms in newer event processing code may result in unexpected issues, so the choice is to encourage use of newer OS's with newer udev event processing code. Signed-off-by: John Ferlan Reviewed-by: Erik Skultety --- src/node_device/node_device_udev.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 22897591de..f134719b82 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1591,6 +1591,26 @@ udevEventMonitorSanityCheck(udevEventDataPtr priv, } +/** + * udevEventHandleThread + * @opaque: unused + * + * Thread to handle the udevEventHandleCallback processing when udev + * tells us there's a device change for us (add, modify, delete, etc). + * + * Once notified there is data to be processed, the actual @device + * data retrieval by libudev may be delayed due to how threads are + * scheduled. In fact, the event loop could be scheduled earlier than + * the handler thread, thus potentially emitting the very same event + * the handler thread is currently trying to process, simply because + * the data hadn't been retrieved from the socket. + * + * NB: Some older distros, such as CentOS 6, libudev opens sockets + * without the NONBLOCK flag which might cause issues with event + * based algorithm. Although the issue can be mitigated by resetting + * priv->dataReady for each event found; however, the scheduler issues + * would still come into play. + */ static void udevEventHandleThread(void *opaque ATTRIBUTE_UNUSED) { @@ -1637,6 +1657,9 @@ udevEventHandleThread(void *opaque ATTRIBUTE_UNUSED) return; } + /* Trying to move the reset of the @priv->dataReady flag to + * after the udev_monitor_receive_device wouldn't help much + * due to event mgmt and scheduler timing. */ virObjectLock(priv); priv->dataReady = false; virObjectUnlock(priv); @@ -1646,6 +1669,11 @@ udevEventHandleThread(void *opaque ATTRIBUTE_UNUSED) udevHandleOneDevice(device); udev_device_unref(device); + + /* Instead of waiting for the next event after processing @device + * data, let's keep reading from the udev monitor and only wait + * for the next event once either a EAGAIN or a EWOULDBLOCK error + * is encountered. */ } }