152549 by sweettea on 2021/09/23 (pair: corwin, bjohnston)

Move sysfs creation to first resume. As of 5.15, Christoph Hellwig has rearranged DM to not create the disk kobjects for a table until after all the segments of the table have been constructed: https://listman.redhat.com/archives/dm-devel/2021-August/msg00008.html In that thread, agk suggested that perhaps this should be moved even later, to resume time. So we must move our own kobject creation, off the disk kobjects, to resume time instead of create time. .../dedupeIndex.h .../dedupeIndex.c Pull sysfs creation for dedupe index out into its own function. .../vdo.h .../vdo.c Rename stats_added member to sysfs_added. .../vdoInit.c .../vdoLoad.c Move initialize_vdo_kobjects() and data thereof to vdoLoad. Made it also initialize sysfs stats objects and dedupe index sysfs objects. Co-authored-by: sweettea@redhat.com Co-authored-by: corwin@redhat.com Co-authored-by: bjohnsto@redhat.com
dm-vdo · Oct 1, 2021 · 27bf632 · 27bf632
1 parent 35b7291
commit 27bf632
Show file tree

Hide file tree

Showing 6 changed files with 65 additions and 65 deletions.
diff --git a/vdo/dedupeIndex.c b/vdo/dedupeIndex.c
@@ -16,7 +16,7 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
  * 02110-1301, USA. 
  *
- * $Id: //eng/linux-vdo/src/c++/vdo/kernel/dedupeIndex.c#117 $
+ * $Id: //eng/linux-vdo/src/c++/vdo/kernel/dedupeIndex.c#118 $
  */
 
 #include "dedupeIndex.h"
@@ -832,6 +832,13 @@ int message_vdo_dedupe_index(struct dedupe_index *index, const char *name)
 	return -EINVAL;
 }
 
+/**********************************************************************/
+int add_vdo_dedupe_index_sysfs(struct dedupe_index *index,
+			       struct kobject *parent)
+{
+	return kobject_add(&index->dedupe_directory, parent, "dedupe");
+}
+
 /**********************************************************************/
 void start_vdo_dedupe_index(struct dedupe_index *index, bool create_flag)
 {
@@ -995,20 +1002,6 @@ int make_vdo_dedupe_index(struct dedupe_index **index_ptr,
 	index->uds_queue
 		= vdo->threads[vdo->thread_config->dedupe_thread].queue;
 	kobject_init(&index->dedupe_directory, &dedupe_directory_type);
-	result = kobject_add(&index->dedupe_directory,
-			     &vdo->vdo_directory,
-			     "dedupe");
-	if (result != VDO_SUCCESS) {
-		// The queue will actually be freed with the VDO, so just give
-		// up our reference to it.
-		UDS_FORGET(index->uds_queue);
-		uds_destroy_index_session(index->index_session);
-		uds_free_configuration(index->configuration);
-		UDS_FREE(index->index_name);
-		UDS_FREE(index);
-		return result;
-	}
-
 	INIT_LIST_HEAD(&index->pending_head);
 	spin_lock_init(&index->pending_lock);
 	spin_lock_init(&index->state_lock);

diff --git a/vdo/dedupeIndex.h b/vdo/dedupeIndex.h
@@ -16,7 +16,7 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
  * 02110-1301, USA. 
  *
- * $Id: //eng/linux-vdo/src/c++/vdo/kernel/dedupeIndex.h#35 $
+ * $Id: //eng/linux-vdo/src/c++/vdo/kernel/dedupeIndex.h#36 $
  */
 
 #ifndef DEDUPE_INDEX_H
@@ -148,6 +148,17 @@ static inline void update_vdo_dedupe_advice(struct data_vio *data_vio)
 	enqueue_vdo_index_operation(data_vio, UDS_UPDATE);
 }
 
+/**
+ * Add the sysfs nodes for the dedupe index.
+ *
+ * @param index        The dedupe index
+ * @param parent  The kobject to attach the sysfs nodes to
+ *
+ * @return 0 or an error code
+ **/
+int add_vdo_dedupe_index_sysfs(struct dedupe_index *index,
+			       struct kobject *parent);
+
 /**
  * Start the dedupe index.
  *

diff --git a/vdo/vdo.c b/vdo/vdo.c
@@ -16,7 +16,7 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
  * 02110-1301, USA. 
  *
- * $Id: //eng/linux-vdo/src/c++/vdo/base/vdo.c#196 $
+ * $Id: //eng/linux-vdo/src/c++/vdo/base/vdo.c#197 $
  */
 
 /*
@@ -387,8 +387,7 @@ void destroy_vdo(struct vdo *vdo)
 
 	// Stop services that need to gather VDO statistics from the worker
 	// threads.
-	if (vdo->stats_added) {
-		vdo->stats_added = false;
+	if (vdo->sysfs_added) {
 		init_completion(&vdo->stats_shutdown);
 		kobject_put(&vdo->stats_directory);
 		wait_for_completion(&vdo->stats_shutdown);
@@ -456,8 +455,7 @@ void destroy_vdo(struct vdo *vdo)
 	 * reference count; when the count goes to zero the VDO object will be
 	 * freed as a side effect.
 	 */
-	if (get_vdo_admin_state_code(&vdo->admin_state)
-	    == VDO_ADMIN_STATE_NEW) {
+	if (!vdo->sysfs_added) {
 		UDS_FREE(vdo);
 	} else {
 		kobject_put(&vdo->vdo_directory);
@@ -494,7 +492,6 @@ int add_vdo_sysfs_stats_dir(struct vdo *vdo)
 		return VDO_CANT_ADD_SYSFS_NODE;
 	}
 
-	vdo->stats_added = true;
 	return VDO_SUCCESS;
 }
 

diff --git a/vdo/vdo.h b/vdo/vdo.h
@@ -16,7 +16,7 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
  * 02110-1301, USA. 
  *
- * $Id: //eng/linux-vdo/src/c++/vdo/base/vdo.h#66 $
+ * $Id: //eng/linux-vdo/src/c++/vdo/base/vdo.h#67 $
  */
 
 #ifndef VDO_H
@@ -144,8 +144,8 @@ struct vdo {
 	struct vdo_statistics stats_buffer;
 	/* Protects the stats_buffer */
 	struct mutex stats_mutex;
-	/* true if sysfs statistics directory is set up */
-	bool stats_added;
+	/* true if sysfs directory is set up */
+	bool sysfs_added;
 	/* Used when shutting down the sysfs statistics */
 	struct completion stats_shutdown;
 

diff --git a/vdo/vdoInit.c b/vdo/vdoInit.c
@@ -16,7 +16,7 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
  * 02110-1301, USA. 
  *
- * $Id: //eng/linux-vdo/src/c++/vdo/base/vdoInit.c#29 $
+ * $Id: //eng/linux-vdo/src/c++/vdo/base/vdoInit.c#30 $
  */
 
 #include "vdoInit.h"
@@ -40,48 +40,12 @@
 #include "vdo.h"
 #include "volumeGeometry.h"
 
-
 /**********************************************************************/
 const char *get_vdo_device_name(const struct dm_target *target)
 {
 	return dm_device_name(dm_table_get_md(target->table));
 }
 
-/**
- * Initialize the vdo and work queue sysfs directories.
- *
- * @param vdo     The vdo being initialized
- * @param target  The device-mapper target this vdo is
- * @param reason  A pointer to hold an error message on failure
- *
- * @return VDO_SUCCESS or an error code
- **/
-static int initialize_vdo_kobjects(struct vdo *vdo,
-				   struct dm_target *target,
-				   char **reason)
-{
-	int result;
-	struct mapped_device *md = dm_table_get_md(target->table);
-
-	kobject_init(&vdo->vdo_directory, &vdo_directory_type);
-	result = kobject_add(&vdo->vdo_directory,
-			     &disk_to_dev(dm_disk(md))->kobj,
-			     "vdo");
-	if (result != 0) {
-		destroy_vdo(vdo);
-		kobject_put(&vdo->vdo_directory);
-		*reason = "Cannot add sysfs node";
-		return result;
-	}
-
-	// Indicate that kobject_put() should now be called on the vdo
-	// directory in order to free the vdo rather than doing so directly.
-	set_vdo_admin_state_code(&vdo->admin_state,
-				 VDO_ADMIN_STATE_INITIALIZED);
-
-	return VDO_SUCCESS;
-}
-
 /**
  * Allocate a vdos threads, queues, and other structures which scale with the
  * thread config.
@@ -173,5 +137,7 @@ int initialize_vdo(struct vdo *vdo,
 		return result;
 	}
 
-	return initialize_vdo_kobjects(vdo, config->owning_target, reason);
+	set_vdo_admin_state_code(&vdo->admin_state,
+				 VDO_ADMIN_STATE_INITIALIZED);
+	return result;
 }
diff --git a/vdo/vdoLoad.c b/vdo/vdoLoad.c
@@ -16,7 +16,7 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
  * 02110-1301, USA. 
  *
- * $Id: //eng/linux-vdo/src/c++/vdo/base/vdoLoad.c#116 $
+ * $Id: //eng/linux-vdo/src/c++/vdo/base/vdoLoad.c#117 $
  */
 
 #include "vdoLoad.h"
@@ -35,6 +35,7 @@
 #include "kernelTypes.h"
 #include "logicalZone.h"
 #include "physicalZone.h"
+#include "poolSysfs.h"
 #include "readOnlyRebuild.h"
 #include "recoveryJournal.h"
 #include "releaseVersions.h"
@@ -74,6 +75,7 @@ static const char *LOAD_PHASE_NAMES[] = {
 	"LOAD_PHASE_WAIT_FOR_READ_ONLY",
 };
 
+
 /**
  * Implements vdo_thread_id_getter_for_phase.
  **/
@@ -185,6 +187,37 @@ static enum slab_depot_load_type get_load_type(struct vdo *vdo)
 	return VDO_SLAB_DEPOT_NORMAL_LOAD;
 }
 
+/**
+ * Initialize the vdo sysfs directory.
+ *
+ * @param vdo     The vdo being initialized
+ *
+ * @return VDO_SUCCESS or an error code
+ **/
+static int initialize_vdo_kobjects(struct vdo *vdo)
+{
+	int result;
+	struct dm_target *target = vdo->device_config->owning_target;
+	struct mapped_device *md = dm_table_get_md(target->table);
+
+	kobject_init(&vdo->vdo_directory, &vdo_directory_type);
+	vdo->sysfs_added = true;
+	result = kobject_add(&vdo->vdo_directory,
+			     &disk_to_dev(dm_disk(md))->kobj,
+			     "vdo");
+	if (result != 0) {
+		return VDO_CANT_ADD_SYSFS_NODE;
+	}
+
+	result = add_vdo_dedupe_index_sysfs(vdo->dedupe_index,
+					    &vdo->vdo_directory);
+	if (result != 0) {
+		return VDO_CANT_ADD_SYSFS_NODE;
+	}
+
+	return add_vdo_sysfs_stats_dir(vdo);
+}
+
 /**
  * Callback to do the destructive parts of loading a VDO.
  *
@@ -272,7 +305,7 @@ static void load_callback(struct vdo_completion *completion)
 
 	case LOAD_PHASE_STATS:
 		finish_vdo_completion(reset_vdo_admin_sub_task(completion),
-				      add_vdo_sysfs_stats_dir(vdo));
+				      initialize_vdo_kobjects(vdo));
 		return;
 
 	case LOAD_PHASE_INDEX: