[v1] node_dev_udev: use workerpool and improve nodedev events

[PATCH v1 12/20] node_device_udev: Fix leak of mdevctlLock, udevThreadCond, and mdevCtlMonitors

Posted by Marc Hartmayer 1 year, 9 months ago

Even if `priv->udev_monitor` was never initialized, the mdevctlLock, udevThread
were. Therefore let's match the order of releasing the resources the order of
allocating the resources in `nodeStateInitialize`.

In addition, use `g_steal_pointer` in `g_list_free_full`.

Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
---
 src/node_device/node_device_udev.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index 7f233652b461..1638a7196709 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -88,6 +88,10 @@ udevEventDataDispose(void *obj)
 
     g_clear_pointer(&priv->initThread, g_free);
 
+    VIR_WITH_MUTEX_LOCK_GUARD(&priv->mdevctlLock) {
+        g_list_free_full(g_steal_pointer(&priv->mdevctlMonitors), g_object_unref);
+    }
+
     if (priv->watch != -1)
         virEventRemoveHandle(priv->watch);
 
@@ -96,16 +100,12 @@ udevEventDataDispose(void *obj)
 
     g_clear_pointer(&priv->udevThread, g_free);
 
-    if (!priv->udev_monitor)
-        return;
-
-    udev = udev_monitor_get_udev(priv->udev_monitor);
-    udev_monitor_unref(priv->udev_monitor);
-    udev_unref(udev);
-
-    VIR_WITH_MUTEX_LOCK_GUARD(&priv->mdevctlLock) {
-        g_list_free_full(priv->mdevctlMonitors, g_object_unref);
+    if (priv->udev_monitor) {
+        udev = udev_monitor_get_udev(priv->udev_monitor);
+        udev_monitor_unref(priv->udev_monitor);
+        udev_unref(udev);
     }
+
     virMutexDestroy(&priv->mdevctlLock);
 
     virCondDestroy(&priv->udevThreadCond);
-- 
2.34.1
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org

Re: [PATCH v1 12/20] node_device_udev: Fix leak of mdevctlLock, udevThreadCond, and mdevCtlMonitors

Posted by Boris Fiuczynski 1 year, 9 months ago

Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>

On 4/19/24 16:49, Marc Hartmayer wrote:
> Even if `priv->udev_monitor` was never initialized, the mdevctlLock, udevThread
> were. Therefore let's match the order of releasing the resources the order of
> allocating the resources in `nodeStateInitialize`.
> 
> In addition, use `g_steal_pointer` in `g_list_free_full`.
> 
> Signed-off-by: Marc Hartmayer<mhartmay@linux.ibm.com>
> ---
>   src/node_device/node_device_udev.c | 18 +++++++++---------
>   1 file changed, 9 insertions(+), 9 deletions(-)

-- 
Mit freundlichen Grüßen/Kind regards
    Boris Fiuczynski

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@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org

Re: [PATCH v1 12/20] node_device_udev: Fix leak of mdevctlLock, udevThreadCond, and mdevCtlMonitors

Posted by Jonathon Jongsma 1 year, 9 months ago

On 4/19/24 9:49 AM, Marc Hartmayer wrote:
> Even if `priv->udev_monitor` was never initialized, the mdevctlLock, udevThread
> were. Therefore let's match the order of releasing the resources the order of
> allocating the resources in `nodeStateInitialize`.
> 
> In addition, use `g_steal_pointer` in `g_list_free_full`.
> 
> Signed-off-by: Marc Hartmayer <mhartmay@linux.ibm.com>
> ---
>   src/node_device/node_device_udev.c | 18 +++++++++---------
>   1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
> index 7f233652b461..1638a7196709 100644
> --- a/src/node_device/node_device_udev.c
> +++ b/src/node_device/node_device_udev.c
> @@ -88,6 +88,10 @@ udevEventDataDispose(void *obj)
>   
>       g_clear_pointer(&priv->initThread, g_free);
>   
> +    VIR_WITH_MUTEX_LOCK_GUARD(&priv->mdevctlLock) {
> +        g_list_free_full(g_steal_pointer(&priv->mdevctlMonitors), g_object_unref);
> +    }
> +
>       if (priv->watch != -1)
>           virEventRemoveHandle(priv->watch);
>   
> @@ -96,16 +100,12 @@ udevEventDataDispose(void *obj)
>   
>       g_clear_pointer(&priv->udevThread, g_free);
>   
> -    if (!priv->udev_monitor)
> -        return;
> -
> -    udev = udev_monitor_get_udev(priv->udev_monitor);
> -    udev_monitor_unref(priv->udev_monitor);
> -    udev_unref(udev);
> -
> -    VIR_WITH_MUTEX_LOCK_GUARD(&priv->mdevctlLock) {
> -        g_list_free_full(priv->mdevctlMonitors, g_object_unref);
> +    if (priv->udev_monitor) {
> +        udev = udev_monitor_get_udev(priv->udev_monitor);
> +        udev_monitor_unref(priv->udev_monitor);
> +        udev_unref(udev);
>       }
> +
>       virMutexDestroy(&priv->mdevctlLock);
>   
>       virCondDestroy(&priv->udevThreadCond);


Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org