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 <jferlan@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>

diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index 22897591de47952d0aa5a738e6bcad9950550451..f134719b8254e96d97adeca3c83e019dfc2f37d0 100644 (file)
--- 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. */
     }
 }