On Thu, Apr 18, 2024 at 11:18 AM -0500, Jonathon Jongsma <jjongsma@redhat.com> wrote:
On 4/12/24 8:36 AM, Marc Hartmayer wrote:
Commit a99d876a0f58 ("node_device: Use automatic mutex management") replaced the locking mechanism and accidentally removed the comment with the reason why the lock is taken. Restore this comment and add a new comment about the lock.
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com> --- src/node_device/node_device_udev.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 469538a1395d..5d474acdc2e0 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -72,8 +72,10 @@ struct _udevEventData { /* init thread */ virThread *initThread;
- GList *mdevctlMonitors; + /* Protects @mdevctlMonitors and must be taken when `mdevctl` command is + * called to make sure only one thread can query mdevctl at a time. */ virMutex mdevctlLock; + GList *mdevctlMonitors; int mdevctlTimeout; };
@@ -2074,6 +2076,7 @@ static void mdevctlUpdateThreadFunc(void *opaque G_GNUC_UNUSED) { udevEventData *priv = driver->privateData; + /* ensure only a single thread can query mdevctl at a time */ VIR_LOCK_GUARD lock = virLockGuardLock(&priv->mdevctlLock);
if (nodeDeviceUpdateMediatedDevices() < 0)
Serializing mdevctl queries is not strictly necessary, and in fact is removed in a later patch in this series (14), so I think we can just drop this patch, tbh.
I'm not sure that this mdevctlLock is even necessary. The udevEventData struct is already a lockable object, so I think we could simply get rid of this lock and use the object lock to protect the mdevctlMonitors variable if we wanted.
I’ll squash this and patch 14. I’ll leave the comment that the mdevctlLock protects the mdevclMonitors. We can decide later, whether we can remove the `mdevctlLock` or not.
Jonathon
-- Kind regards / Beste Grüße Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Wolfgang Wendt Geschäftsführung: David Faller Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294