base-manager: never create kernel device objects for remove events

There is no point in creating a new kernel device object just to
process a remove event; instead, do any matching with existing
kernel device objects by subsystem and name, which is what the generic
backend already did anyway.

This avoids unnecessary lookup of information in sysfs during removal
events, because the port is anyway already gone when we try to look
those up.

src/mm-base-manager.c

@@ -136,6 +136,24 @@ find_device_by_port (MMBaseManager  *manager,
     return NULL;
 }
 
+static MMDevice *
+find_device_by_port_name (MMBaseManager *manager,
+                          const gchar   *subsystem,
+                          const gchar   *name)
+{
+    GHashTableIter iter;
+    gpointer key, value;
+
+    g_hash_table_iter_init (&iter, manager->priv->devices);
+    while (g_hash_table_iter_next (&iter, &key, &value)) {
+        MMDevice *candidate = MM_DEVICE (value);
+
+        if (mm_device_owns_port_name (candidate, subsystem, name))
+            return candidate;
+    }
+    return NULL;
+}
+
 static MMDevice *
 find_device_by_physdev_uid (MMBaseManager *self,
                             const gchar   *physdev_uid)
@@ -208,24 +226,24 @@ static void     device_inhibited_track_port   (MMBaseManager  *self,
                                                MMKernelDevice *port,
                                                gboolean        manual_scan);
 static void     device_inhibited_untrack_port (MMBaseManager  *self,
-                                               MMKernelDevice *port);
+                                               const gchar    *subsystem,
+                                               const gchar    *name);
 
 static void
-device_removed (MMBaseManager  *self,
+device_removed (MMBaseManager *self,
-                MMKernelDevice *kernel_device)
+                const gchar   *subsystem,
+                const gchar   *name)
 {
-    g_autoptr(MMDevice)  device = NULL;
+    g_autoptr(MMDevice) device = NULL;
-    const gchar         *name;
 
-    g_return_if_fail (kernel_device != NULL);
+    g_assert (subsystem && name);
-
+    device = find_device_by_port_name (self, subsystem, name);
-    device = find_device_by_port (self, kernel_device);
     if (!device) {
         /* If the device was inhibited and the port is gone, untrack it.
          * This is only needed for ports that were tracked out of device objects.
          * In this case we don't rely on the physdev uid, as API-reported
          * remove kernel events may not include uid. */
-        device_inhibited_untrack_port (self, kernel_device);
+        device_inhibited_untrack_port (self, subsystem, name);
         return;
     }
 
@@ -236,9 +254,8 @@ device_removed (MMBaseManager  *self,
      * ourselves. */
     g_object_ref (device);
 
-    name = mm_kernel_device_get_name (kernel_device);
     mm_obj_info (self, "port %s released by device '%s'", name, mm_device_get_uid (device));
-    mm_device_release_port (device, kernel_device);
+    mm_device_release_port_name (device, subsystem, name);
 
     /* If port probe list gets empty, remove the device object iself */
     if (!mm_device_peek_port_probe_list (device)) {
@@ -283,7 +300,7 @@ device_added (MMBaseManager  *self,
         /* This could mean that device changed, losing its candidate
          * flags (such as Bluetooth RFCOMM devices upon disconnect.
          * Try to forget it. */
-        device_removed (self, port);
+        device_removed (self, mm_kernel_device_get_subsystem (port), name);
         mm_obj_dbg (self, "port %s not candidate", name);
         return;
     }
@@ -346,11 +363,10 @@ handle_kernel_event (MMBaseManager            *self,
                      MMKernelEventProperties  *properties,
                      GError                  **error)
 {
-    MMKernelDevice *kernel_device;
+    const gchar *action;
-    const gchar    *action;
+    const gchar *subsystem;
-    const gchar    *subsystem;
+    const gchar *name;
-    const gchar    *name;
+    const gchar *uid;
-    const gchar    *uid;
 
     action = mm_kernel_event_properties_get_action (properties);
     if (!action) {
@@ -387,25 +403,27 @@ handle_kernel_event (MMBaseManager            *self,
     mm_obj_dbg (self, "  name:      %s", name);
     mm_obj_dbg (self, "  uid:       %s", uid ? uid : "n/a");
 
+    if (g_strcmp0 (action, "add") == 0) {
+        g_autoptr(MMKernelDevice) kernel_device = NULL;
 #if defined WITH_UDEV
-    if (!mm_context_get_test_no_udev ())
+        if (!mm_context_get_test_no_udev ())
-        kernel_device = mm_kernel_device_udev_new_from_properties (properties, error);
+            kernel_device = mm_kernel_device_udev_new_from_properties (properties, error);
-    else
+        else
 #endif
-        kernel_device = mm_kernel_device_generic_new (properties, error);
+            kernel_device = mm_kernel_device_generic_new (properties, error);
+        if (!kernel_device)
+            return FALSE;
 
-    if (!kernel_device)
-        return FALSE;
-
-    if (g_strcmp0 (action, "add") == 0)
         device_added (self, kernel_device, TRUE, TRUE);
-    else if (g_strcmp0 (action, "remove") == 0)
+        return TRUE;
-        device_removed (self, kernel_device);
+    }
-    else
-        g_assert_not_reached ();
-    g_object_unref (kernel_device);
 
-    return TRUE;
+    if (g_strcmp0 (action, "remove") == 0) {
+        device_removed (self, subsystem, name);
+        return TRUE;
+    }
+
+    g_assert_not_reached ();
 }
 
 #if defined WITH_UDEV
@@ -415,13 +433,18 @@ handle_uevent (MMBaseManager *self,
                const gchar   *action,
                GUdevDevice   *device)
 {
-    g_autoptr(MMKernelDevice) kernel_device = NULL;
+    if (g_str_equal (action, "add") || g_str_equal (action, "move") || g_str_equal (action, "change")) {
+        g_autoptr(MMKernelDevice) kernel_device = NULL;
 
-    kernel_device = mm_kernel_device_udev_new (device);
+        kernel_device = mm_kernel_device_udev_new (device);
-    if (g_str_equal (action, "add") || g_str_equal (action, "move") || g_str_equal (action, "change"))
         device_added (self, kernel_device, TRUE, FALSE);
-    else if (g_str_equal (action, "remove"))
+        return;
-        device_removed (self, kernel_device);
+    }
+
+    if (g_str_equal (action, "remove")) {
+        device_removed (self, g_udev_device_get_subsystem (device), g_udev_device_get_name (device));
+        return;
+    }
 }
 
 typedef struct {
@@ -886,7 +909,8 @@ is_device_inhibited (MMBaseManager *self,
 
 static void
 device_inhibited_untrack_port (MMBaseManager  *self,
-                               MMKernelDevice *kernel_port)
+                               const gchar    *subsystem,
+                               const gchar    *name)
 {
     GHashTableIter       iter;
     gchar               *uid;
@@ -900,8 +924,10 @@ device_inhibited_untrack_port (MMBaseManager  *self,
             InhibitedDevicePortInfo *port_info;
 
             port_info = (InhibitedDevicePortInfo *)(l->data);
-            if (mm_kernel_device_cmp (port_info->kernel_port, kernel_port)) {
+
-                mm_obj_dbg (self, "released port %s while inhibited", mm_kernel_device_get_name (kernel_port));
+            if ((g_strcmp0 (subsystem, mm_kernel_device_get_subsystem (port_info->kernel_port)) == 0) &&
+                (g_strcmp0 (name, mm_kernel_device_get_name (port_info->kernel_port)) == 0)) {
+                mm_obj_dbg (self, "released port %s while inhibited", name);
                 inhibited_device_port_info_free (port_info);
                 info->port_infos = g_list_delete_link (info->port_infos, l);
                 return;

src/mm-device.c

@@ -96,31 +96,55 @@ struct _MMDevicePrivate {
 
 /*****************************************************************************/
 
+static MMPortProbe *
+probe_list_lookup_by_device (GList          *port_probes,
+                             MMKernelDevice *kernel_port)
+{
+    GList *l;
+
+    for (l = port_probes; l; l = g_list_next (l)) {
+        MMPortProbe *probe = MM_PORT_PROBE (l->data);
+
+        if (mm_kernel_device_cmp (mm_port_probe_peek_port (probe), kernel_port))
+            return probe;
+    }
+    return NULL;
+}
+
+static MMPortProbe *
+probe_list_lookup_by_name (GList       *port_probes,
+                           const gchar *subsystem,
+                           const gchar *name)
+{
+    GList *l;
+
+    for (l = port_probes; l; l = g_list_next (l)) {
+        MMPortProbe    *probe = MM_PORT_PROBE (l->data);
+        MMKernelDevice *probe_device;
+
+        probe_device = mm_port_probe_peek_port (probe);
+        if ((g_strcmp0 (subsystem, mm_kernel_device_get_subsystem (probe_device)) == 0) &&
+            (g_strcmp0 (name,      mm_kernel_device_get_name      (probe_device)) == 0))
+            return probe;
+    }
+    return NULL;
+}
+
 static MMPortProbe *
 device_find_probe_with_device (MMDevice       *self,
                                MMKernelDevice *kernel_port,
                                gboolean        lookup_ignored)
 {
-    GList *l;
+    MMPortProbe *probe;
 
-    for (l = self->priv->port_probes; l; l = g_list_next (l)) {
+    probe = probe_list_lookup_by_device (self->priv->port_probes, kernel_port);
-        MMPortProbe *probe = MM_PORT_PROBE (l->data);
+    if (probe)
-
+        return probe;
-        if (mm_kernel_device_cmp (mm_port_probe_peek_port (probe), kernel_port))
-            return probe;
-    }
 
     if (!lookup_ignored)
         return NULL;
 
-    for (l = self->priv->ignored_port_probes; l; l = g_list_next (l)) {
+    return probe_list_lookup_by_device (self->priv->ignored_port_probes, kernel_port);
-        MMPortProbe *probe = MM_PORT_PROBE (l->data);
-
-        if (mm_kernel_device_cmp (mm_port_probe_peek_port (probe), kernel_port))
-            return probe;
-    }
-
-    return NULL;
 }
 
 gboolean
@@ -130,6 +154,28 @@ mm_device_owns_port (MMDevice       *self,
     return !!device_find_probe_with_device (self, kernel_port, TRUE);
 }
 
+static MMPortProbe *
+device_find_probe_with_name (MMDevice    *self,
+                             const gchar *subsystem,
+                             const gchar *name)
+{
+    MMPortProbe *probe;
+
+    probe = probe_list_lookup_by_name (self->priv->port_probes, subsystem, name);
+    if (probe)
+        return probe;
+
+    return probe_list_lookup_by_name (self->priv->ignored_port_probes, subsystem, name);
+}
+
+gboolean
+mm_device_owns_port_name (MMDevice    *self,
+                          const gchar *subsystem,
+                          const gchar *name)
+{
+    return !!device_find_probe_with_name (self, subsystem, name);
+}
+
 static void
 add_port_driver (MMDevice       *self,
                  MMKernelDevice *kernel_port)
@@ -190,12 +236,13 @@ mm_device_grab_port (MMDevice       *self,
 }
 
 void
-mm_device_release_port (MMDevice       *self,
+mm_device_release_port_name (MMDevice    *self,
-                        MMKernelDevice *kernel_port)
+                             const gchar *subsystem,
+                             const gchar *name)
 {
     MMPortProbe *probe;
 
-    probe = device_find_probe_with_device (self, kernel_port, TRUE);
+    probe = device_find_probe_with_name (self, subsystem, name);
     if (probe) {
         /* Found, remove from lists and destroy probe */
         if (g_list_find (self->priv->port_probes, probe))

src/mm-device.h

@@ -67,14 +67,19 @@ MMDevice *mm_device_new (const gchar              *uid,
                          gboolean                  virtual,
                          GDBusObjectManagerServer *object_manager);
 
-void     mm_device_grab_port    (MMDevice       *self,
+void     mm_device_grab_port   (MMDevice       *self,
-                                 MMKernelDevice *kernel_port);
+                                MMKernelDevice *kernel_port);
-void     mm_device_release_port (MMDevice       *self,
+gboolean mm_device_owns_port   (MMDevice       *self,
-                                 MMKernelDevice *kernel_port);
+                                MMKernelDevice *kernel_port);
-gboolean mm_device_owns_port    (MMDevice       *self,
+void     mm_device_ignore_port (MMDevice       *self,
-                                 MMKernelDevice *kernel_port);
+                                MMKernelDevice *kernel_port);
-void     mm_device_ignore_port  (MMDevice       *self,
+
-                                 MMKernelDevice *kernel_port);
+gboolean mm_device_owns_port_name    (MMDevice       *self,
+                                      const gchar    *subsystem,
+                                      const gchar    *name);
+void     mm_device_release_port_name (MMDevice       *self,
+                                      const gchar    *subsystem,
+                                      const gchar    *name);
 
 gboolean mm_device_create_modem (MMDevice  *self,
                                  GError   **error);