[libvirt] [PATCH v2] 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. Based largely off a
description provided by Erik Skultety <eskultet(a)redhat.com>.
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. In these cases scheduler blocking on the udev recv() call
is more noticeable. 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(a)redhat.com>
---
v1: https://www.redhat.com/archives/libvir-list/2018-October/msg01053.html
Changes are from code review with some minor tweaks/editing as I
went along. Mistakes are my own!
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..f2c2299d4d 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: Usage of event based socket algorithm causes some issues with
+ * older platforms such as CentOS 6 in which the data socket is opened
+ * without the NONBLOCK bit set. That can be avoided by moving the reset
+ * priv->dataReady to just after the udev_monitor_receive_device; however,
+ * 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. */
}
}
--
2.17.2