WIP: LVM2 module uevent processing optimization

doc/udisks2-sections.txt.daemon.sections.in

@@ -196,6 +196,8 @@ UDisksLinuxProvider
 udisks_linux_provider_new
 udisks_linux_provider_get_udev_client
 udisks_linux_provider_get_coldplug
+udisks_linux_provider_get_modules_coldplug
+udisks_linux_provider_get_last_uevent
 <SUBSECTION Standard>
 UDISKS_TYPE_LINUX_PROVIDER
 UDISKS_LINUX_PROVIDER

modules/lvm2/udiskslinuxlogicalvolume.c

@@ -148,12 +148,17 @@ udisks_linux_logical_volume_update (UDisksLinuxLogicalVolume     *logical_volume
                                     gboolean                     *needs_polling_ret)
 {
   UDisksLogicalVolume *iface;
+  UDisksLinuxModuleLVM2 *module;
+  UDisksDaemon *daemon;
   const char *type;
   gboolean active;
   const char *pool_objpath;
   const char *origin_objpath;
   guint64 size = 0;
 
+  module = udisks_linux_volume_group_object_get_module (group_object);
+  daemon = udisks_module_get_daemon (UDISKS_MODULE (module));
+
   iface = UDISKS_LOGICAL_VOLUME (logical_volume);
 
   udisks_logical_volume_set_name (iface, lv_info->lv_name);
@@ -225,7 +230,7 @@ udisks_linux_logical_volume_update (UDisksLinuxLogicalVolume     *logical_volume
        *
        * https://www.redhat.com/archives/linux-lvm/2014-January/msg00030.html
        */
-      udisks_daemon_util_lvm2_trigger_udev (dev_file);
+      udisks_daemon_util_trigger_uevent (daemon, dev_file);
       logical_volume->needs_udev_hack = FALSE;
       g_free (dev_file);
     }

modules/lvm2/udiskslinuxmodulelvm2.c

@@ -24,6 +24,7 @@
 #include <blockdev/lvm.h>
 
 #include <src/udisksdaemon.h>
+#include <src/udiskslinuxprovider.h>
 #include <src/udiskslogging.h>
 #include <src/udiskslinuxdevice.h>
 #include <src/udisksmodulemanager.h>
@@ -57,8 +58,10 @@ struct _UDisksLinuxModuleLVM2 {
   /* maps from volume group name to UDisksLinuxVolumeGroupObject instances. */
   GHashTable *name_to_volume_group;
 
-  gint delayed_update_id;
-  gboolean coldplug_done;
+  gint64 last_update_requested;
+  GTask *update_task;
+
+  gboolean scalable_mode;
 };
 
 typedef struct _UDisksLinuxModuleLVM2Class UDisksLinuxModuleLVM2Class;
@@ -85,7 +88,8 @@ udisks_linux_module_lvm2_constructed (GObject *object)
   UDisksLinuxModuleLVM2 *module = UDISKS_LINUX_MODULE_LVM2 (object);
 
   module->name_to_volume_group = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, (GDestroyNotify) g_object_unref);
-  module->coldplug_done = FALSE;
+  module->last_update_requested = 0;
+  module->scalable_mode = FALSE;
 
   if (G_OBJECT_CLASS (udisks_linux_module_lvm2_parent_class)->constructed)
     G_OBJECT_CLASS (udisks_linux_module_lvm2_parent_class)->constructed (object);
@@ -96,6 +100,8 @@ udisks_linux_module_lvm2_finalize (GObject *object)
 {
   UDisksLinuxModuleLVM2 *module = UDISKS_LINUX_MODULE_LVM2 (object);
 
+  /* Note: won't be called until module->update_task finishes */
+
   g_hash_table_unref (module->name_to_volume_group);
 
   if (G_OBJECT_CLASS (udisks_linux_module_lvm2_parent_class)->finalize)
@@ -202,6 +208,13 @@ udisks_linux_module_lvm2_new_manager (UDisksModule *module)
 
 /* ---------------------------------------------------------------------------------------------------- */
 
+typedef struct {
+  gint64 task_timestamp;
+  gboolean sync_task;
+} LVMUpdateTaskData;
+
+static gboolean delayed_lvm_update (gpointer user_data);
+
 static void
 lvm_update_vgs (GObject      *source_obj,
                 GAsyncResult *result,
@@ -214,12 +227,22 @@ lvm_update_vgs (GObject      *source_obj,
   GTask *task = G_TASK (result);
   GError *error = NULL;
   VGsPVsData *data = g_task_propagate_pointer (task, &error);
+  LVMUpdateTaskData *task_data = user_data;
   BDLVMVGdata **vgs, **vgs_p;
   BDLVMPVdata **pvs, **pvs_p;
 
   GHashTableIter vg_name_iter;
   gpointer key, value;
   const gchar *vg_name;
+  gint64 task_timestamp;
+  gboolean sync_task;
+  guint lv_count = 0;
+
+  g_warn_if_fail (task_data != NULL);
+
+  task_timestamp = task_data->task_timestamp;
+  sync_task = task_data->sync_task;
+  g_free (task_data);
 
   if (! data)
     {
@@ -233,6 +256,10 @@ lvm_update_vgs (GObject      *source_obj,
           /* this should never happen */
           udisks_warning ("LVM2 plugin: failure but no error when getting VGs!");
         }
+      g_clear_object (&module->update_task);
+      /* queue new task if a new uevent has been received during the task processing time */
+      if (!sync_task && task_timestamp < module->last_update_requested)
+        g_idle_add (delayed_lvm_update, module);
       return;
     }
   vgs = data->vgs;
@@ -283,7 +310,20 @@ lvm_update_vgs (GObject      *source_obj,
         if (g_strcmp0 ((*pvs_p)->vg_name, vg_name) == 0)
             vg_pvs = g_slist_prepend (vg_pvs, bd_lvm_pvdata_copy (*pvs_p));
 
-      udisks_linux_volume_group_object_update (group, *vgs_p, vg_pvs);
+      udisks_linux_volume_group_object_update (group, *vgs_p, vg_pvs, sync_task);
+      lv_count += udisks_linux_volume_group_object_get_lv_count (group);
+    }
+
+  if (lv_count > LVM2_SCALABLE_MODE_THRESHOLD)
+    {
+      if (! module->scalable_mode)
+        udisks_warning ("lvm2: Total of %u logical volumes detected in the system, switching the scalable mode on.", lv_count);
+      module->scalable_mode = TRUE;
+    }
+  else
+    {
+      /* stepping down from scalable mode is currently allowed */
+      module->scalable_mode = FALSE;
     }
 
   /* UDisksLinuxVolumeGroupObject carries copies of BDLVMPVdata that belong to the VG.
@@ -296,55 +336,94 @@ lvm_update_vgs (GObject      *source_obj,
   /* only free the containers, the contents were passed further */
   g_free (vgs);
   g_free (pvs);
+
+  /* we hold a reference to the task */
+  g_clear_object (&module->update_task);
+
+  /* If this update was sync, it was blocking the main (uevent processing) thread and
+   * there was no chance the module->last_update_requested timestamp would change. */
+  if (!sync_task && task_timestamp < module->last_update_requested)
+    {
+      /* Further uevents have been received while the update task was running,
+       * queue a new update. */
+      g_idle_add (delayed_lvm_update, module);
+    }
 }
 
 static void
-lvm_update (UDisksLinuxModuleLVM2 *module)
+lvm_update (UDisksLinuxModuleLVM2 *module, gint64 timestamp, gboolean coldplug, gboolean force_update)
 {
-  GTask *task;
+  UDisksDaemon *daemon;
+  UDisksLinuxProvider *provider;
+  LVMUpdateTaskData *task_data;
+
+  daemon = udisks_module_get_daemon (UDISKS_MODULE (module));
+  provider = udisks_daemon_get_linux_provider (daemon);
+
+  if (!force_update && udisks_linux_provider_get_last_uevent (provider) <= module->last_update_requested)
+    {
+      udisks_debug ("lvm2: no uevent received since last update, skipping");
+      return;
+    }
+
+  /* store timestamp of a last update requested */
+  module->last_update_requested = timestamp;
+  if (module->update_task)
+    {
+      udisks_debug ("lvm2: update already in progress, will queue another one once finished");
+      return;
+    }
+
+  task_data = g_new0 (LVMUpdateTaskData, 1);
+  task_data->task_timestamp = module->last_update_requested;
 
   /* the callback (lvm_update_vgs) is called in the default main loop (context) */
-  task = g_task_new (module,
-                     NULL /* cancellable */,
-                     lvm_update_vgs,
-                     NULL /* callback_data */);
-
-  /* holds a reference to 'task' until it is finished */
-  g_task_run_in_thread (task, (GTaskThreadFunc) vgs_task_func);
-  g_object_unref (task);
+  module->update_task = g_task_new (module,
+                                    NULL /* cancellable */,
+                                    lvm_update_vgs,
+                                    task_data /* callback_data */);
+
+  /* the callback is responsible for releasing the task reference */
+  if (coldplug || !module->scalable_mode)
+    {
+      task_data->sync_task = TRUE;
+      g_task_run_in_thread_sync (module->update_task, (GTaskThreadFunc) vgs_task_func);
+      lvm_update_vgs (G_OBJECT (module), G_ASYNC_RESULT (module->update_task), task_data);
+    }
+  else
+    {
+      task_data->sync_task = FALSE;
+      g_task_run_in_thread (module->update_task, (GTaskThreadFunc) vgs_task_func);
+    }
 }
 
 static gboolean
 delayed_lvm_update (gpointer user_data)
 {
   UDisksLinuxModuleLVM2 *module = UDISKS_LINUX_MODULE_LVM2 (user_data);
 
-  lvm_update (module);
-  module->delayed_update_id = 0;
+  udisks_debug ("lvm2: spawning another update due to incoming uevent during last update");
+
+  /* delayed updates are always async */
+  lvm_update (module, g_get_monotonic_time (), FALSE, TRUE);
 
   return FALSE;
 }
 
 static void
-trigger_delayed_lvm_update (UDisksLinuxModuleLVM2 *module)
+trigger_delayed_lvm_update (UDisksLinuxModuleLVM2 *module, gint64 timestamp)
 {
-  if (module->delayed_update_id > 0)
-    return;
+  UDisksDaemon *daemon;
+  UDisksLinuxProvider *provider;
+  gboolean coldplug;
 
-  if (! module->coldplug_done)
-    {
-      /* Update immediately when doing coldplug, i.e. when lvm2 module has just
-       * been activated. This is not 100% effective as this affects only the
-       * first request but from the plugin nature we don't know whether
-       * coldplugging has been finished or not. Might be subject to change in
-       * the future. */
-      module->coldplug_done = TRUE;
-      lvm_update (module);
-    }
-  else
-    {
-      module->delayed_update_id = g_timeout_add (100, delayed_lvm_update, module);
-    }
+  daemon = udisks_module_get_daemon (UDISKS_MODULE (module));
+  provider = udisks_daemon_get_linux_provider (daemon);
+
+  coldplug = udisks_linux_provider_get_coldplug (provider) ||
+             udisks_linux_provider_get_modules_coldplug (provider);
+
+  lvm_update (module, timestamp, coldplug, FALSE);
 }
 
 static gboolean
@@ -381,6 +460,7 @@ is_recorded_as_physical_volume (UDisksLinuxModuleLVM2 *module,
   return ret;
 }
 
+/* should only be called from the main thread */
 static GDBusObjectSkeleton **
 udisks_linux_module_lvm2_new_object (UDisksModule      *module,
                                      UDisksLinuxDevice *device)
@@ -397,7 +477,7 @@ udisks_linux_module_lvm2_new_object (UDisksModule      *module,
   if (is_logical_volume (device)
       || has_physical_volume_label (device)
       || is_recorded_as_physical_volume (UDISKS_LINUX_MODULE_LVM2 (module), device))
-    trigger_delayed_lvm_update (UDISKS_LINUX_MODULE_LVM2 (module));
+    trigger_delayed_lvm_update (UDISKS_LINUX_MODULE_LVM2 (module), device->timestamp);
 
   return NULL;
 }

modules/lvm2/udiskslinuxvolumegroupobject.c

@@ -702,7 +702,7 @@ update_vg (GObject      *source_obj,
 }
 
 void
-udisks_linux_volume_group_object_update (UDisksLinuxVolumeGroupObject *object, BDLVMVGdata *vg_info, GSList *pvs)
+udisks_linux_volume_group_object_update (UDisksLinuxVolumeGroupObject *object, BDLVMVGdata *vg_info, GSList *pvs, gboolean update_sync)
 {
   VGUpdateData *data = g_new0 (VGUpdateData, 1);
   gchar *vg_name = g_strdup (vg_info->name);
@@ -716,7 +716,15 @@ udisks_linux_volume_group_object_update (UDisksLinuxVolumeGroupObject *object, B
   g_task_set_task_data (task, vg_name, g_free);
 
   /* holds a reference to 'task' until it is finished */
-  g_task_run_in_thread (task, (GTaskThreadFunc) lvs_task_func);
+  if (update_sync)
+    {
+      g_task_run_in_thread_sync (task, (GTaskThreadFunc) lvs_task_func);
+      update_vg (G_OBJECT (object), G_ASYNC_RESULT (task), data);
+    }
+  else
+    {
+      g_task_run_in_thread (task, (GTaskThreadFunc) lvs_task_func);
+    }
 
   g_object_unref (task);
 }
@@ -896,3 +904,20 @@ udisks_linux_volume_group_object_get_name (UDisksLinuxVolumeGroupObject *object)
   g_return_val_if_fail (UDISKS_IS_LINUX_VOLUME_GROUP_OBJECT (object), NULL);
   return object->name;
 }
+
+/**
+ * udisks_linux_volume_group_object_get_lv_count:
+ * @object: A #UDisksLinuxVolumeGroupObject.
+ *
+ * Gets the number of logical volumes the volume group contains.
+ *
+ * Returns: The number of logical volumes for the object.
+ */
+guint
+udisks_linux_volume_group_object_get_lv_count (UDisksLinuxVolumeGroupObject *object)
+{
+  g_return_val_if_fail (UDISKS_IS_LINUX_VOLUME_GROUP_OBJECT (object), 0);
+  g_assert (object->logical_volumes != NULL);
+
+  return g_hash_table_size (object->logical_volumes);
+}

modules/lvm2/udiskslinuxvolumegroupobject.h

@@ -40,7 +40,9 @@ const gchar                    *udisks_linux_volume_group_object_get_name      (
 UDisksLinuxModuleLVM2          *udisks_linux_volume_group_object_get_module    (UDisksLinuxVolumeGroupObject *object);
 void                            udisks_linux_volume_group_object_update        (UDisksLinuxVolumeGroupObject *object,
                                                                                 BDLVMVGdata                  *vginfo,
-                                                                                GSList                       *pvs);
+                                                                                GSList                       *pvs,
+                                                                                gboolean                      update_sync);
+guint                           udisks_linux_volume_group_object_get_lv_count  (UDisksLinuxVolumeGroupObject *object);
 
 void                            udisks_linux_volume_group_object_poll          (UDisksLinuxVolumeGroupObject *object);
 

modules/lvm2/udiskslvm2daemonutil.c

@@ -263,11 +263,3 @@ udisks_daemon_util_lvm2_name_is_reserved (const gchar *name)
 }
 
 /* -------------------------------------------------------------------------------- */
-
-void
-udisks_daemon_util_lvm2_trigger_udev (const gchar *device_file)
-{
-  int fd = open (device_file, O_RDWR);
-  if (fd >= 0)
-    close (fd);
-}

modules/lvm2/udiskslvm2daemonutil.h

@@ -35,8 +35,6 @@ gboolean udisks_daemon_util_lvm2_wipe_block      (UDisksDaemon  *daemon,
 
 gboolean udisks_daemon_util_lvm2_name_is_reserved (const gchar *name);
 
-void udisks_daemon_util_lvm2_trigger_udev (const gchar *device_file);
-
 G_END_DECLS
 
 #endif /* __UDISKS_LVM2_DAEMON_UTIL_H__ */

modules/lvm2/udiskslvm2types.h

@@ -25,6 +25,14 @@
 #define LVM2_MODULE_NAME "lvm2"
 #define LVM2_POLICY_ACTION_ID "org.freedesktop.udisks2.lvm2.manage-lvm"
 
+/**
+ * LVM2_SCALABLE_MODE_THRESHOLD:
+ *
+ * Number of logical volumes detected in the system needed to
+ * switch the scalable mode on.
+ */
+#define LVM2_SCALABLE_MODE_THRESHOLD 100
+
 struct _UDisksLinuxModuleLVM2;
 typedef struct _UDisksLinuxModuleLVM2 UDisksLinuxModuleLVM2;