On Thu, Apr 18, 2024 at 11:18 AM -0500, Jonathon Jongsma <jjongsma@xxxxxxxxxx> 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@xxxxxxxxxxxxx>
>> Signed-off-by: Marc Hartmayer <mhartmay@xxxxxxxxxxxxx>
>> ---
>> 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
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
