zvol: make calls to platform ops static

There's no need to make the platform ops dynamic dispatch.

This change replaces the dynamic dispatch with static calls to the
platform-specific functions.
To avoid name collisions, prefix all platform-specific functions
with `zvol_os_`.
I actually find `zvol_..._os` slightly nicer to read in the calling
code, but having it as a prefix is useful.

Advantage:
- easier jump-to-definition / grepping
- potential benefits to static analysis
- better legibility

Future work: also prefix remaining `static` functions in zvol_os.c.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Christian Schwarz <christian.schwarz@nutanix.com>
Closes #12965

include/sys/zvol_impl.h

@@ -93,17 +93,13 @@ void zvol_wait_close(zvol_state_t *zv);
 /*
  * platform dependent functions exported to platform independent code
  */
-typedef struct zvol_platform_ops {
-	void (*zv_free)(zvol_state_t *);
-	void (*zv_rename_minor)(zvol_state_t *, const char *);
-	int (*zv_create_minor)(const char *);
-	int (*zv_update_volsize)(zvol_state_t *, uint64_t);
-	boolean_t (*zv_is_zvol)(const char *);
-	void (*zv_clear_private)(zvol_state_t *);
-	void (*zv_set_disk_ro)(zvol_state_t *, int flags);
-	void (*zv_set_capacity)(zvol_state_t *, uint64_t capacity);
-} zvol_platform_ops_t;
-
-void zvol_register_ops(const zvol_platform_ops_t *ops);
+void zvol_os_free(zvol_state_t *zv);
+void zvol_os_rename_minor(zvol_state_t *zv, const char *newname);
+int zvol_os_create_minor(const char *name);
+int zvol_os_update_volsize(zvol_state_t *zv, uint64_t volsize);
+boolean_t zvol_os_is_zvol(const char *path);
+void zvol_os_clear_private(zvol_state_t *zv);
+void zvol_os_set_disk_ro(zvol_state_t *zv, int flags);
+void zvol_os_set_capacity(zvol_state_t *zv, uint64_t capacity);
 
 #endif

module/os/freebsd/zfs/zvol_os.c

@@ -1196,7 +1196,7 @@ zvol_ensure_zilog(zvol_state_t *zv)
 			zv->zv_zilog = zil_open(zv->zv_objset,
 			    zvol_get_data);
 			zv->zv_flags |= ZVOL_WRITTEN_TO;
-			/* replay / destroy done in zvol_create_minor_impl() */
+			/* replay / destroy done in zvol_os_create_minor() */
 			VERIFY0(zv->zv_zilog->zl_header->zh_flags &
 			    ZIL_REPLAY_NEEDED);
 		}
@@ -1204,14 +1204,14 @@ zvol_ensure_zilog(zvol_state_t *zv)
 	}
 }
 
-static boolean_t
-zvol_is_zvol_impl(const char *device)
+boolean_t
+zvol_os_is_zvol(const char *device)
 {
 	return (device && strncmp(device, ZVOL_DIR, strlen(ZVOL_DIR)) == 0);
 }
 
-static void
-zvol_rename_minor(zvol_state_t *zv, const char *newname)
+void
+zvol_os_rename_minor(zvol_state_t *zv, const char *newname)
 {
 	ASSERT(RW_LOCK_HELD(&zvol_state_lock));
 	ASSERT(MUTEX_HELD(&zv->zv_state_lock));
@@ -1282,8 +1282,8 @@ zvol_rename_minor(zvol_state_t *zv, const char *newname)
 /*
  * Remove minor node for the specified volume.
  */
-static void
-zvol_free(zvol_state_t *zv)
+void
+zvol_os_free(zvol_state_t *zv)
 {
 	ASSERT(!RW_LOCK_HELD(&zv->zv_suspend_lock));
 	ASSERT(!MUTEX_HELD(&zv->zv_state_lock));
@@ -1324,8 +1324,8 @@ zvol_free(zvol_state_t *zv)
 /*
  * Create a minor node (plus a whole lot more) for the specified volume.
  */
-static int
-zvol_create_minor_impl(const char *name)
+int
+zvol_os_create_minor(const char *name)
 {
 	zvol_state_t *zv;
 	objset_t *os;
@@ -1463,8 +1463,8 @@ out_doi:
 	return (error);
 }
 
-static void
-zvol_clear_private(zvol_state_t *zv)
+void
+zvol_os_clear_private(zvol_state_t *zv)
 {
 	ASSERT(RW_LOCK_HELD(&zvol_state_lock));
 	if (zv->zv_volmode == ZFS_VOLMODE_GEOM) {
@@ -1492,8 +1492,8 @@ zvol_clear_private(zvol_state_t *zv)
 	}
 }
 
-static int
-zvol_update_volsize(zvol_state_t *zv, uint64_t volsize)
+int
+zvol_os_update_volsize(zvol_state_t *zv, uint64_t volsize)
 {
 	zv->zv_volsize = volsize;
 	if (zv->zv_volmode == ZFS_VOLMODE_GEOM) {
@@ -1522,29 +1522,18 @@ zvol_update_volsize(zvol_state_t *zv, uint64_t volsize)
 	return (0);
 }
 
-static void
-zvol_set_disk_ro_impl(zvol_state_t *zv, int flags)
+void
+zvol_os_set_disk_ro(zvol_state_t *zv, int flags)
 {
 	// XXX? set_disk_ro(zv->zv_zso->zvo_disk, flags);
 }
 
-static void
-zvol_set_capacity_impl(zvol_state_t *zv, uint64_t capacity)
+void
+zvol_os_set_capacity(zvol_state_t *zv, uint64_t capacity)
 {
 	// XXX? set_capacity(zv->zv_zso->zvo_disk, capacity);
 }
 
-const static zvol_platform_ops_t zvol_freebsd_ops = {
-	.zv_free = zvol_free,
-	.zv_rename_minor = zvol_rename_minor,
-	.zv_create_minor = zvol_create_minor_impl,
-	.zv_update_volsize = zvol_update_volsize,
-	.zv_clear_private = zvol_clear_private,
-	.zv_is_zvol = zvol_is_zvol_impl,
-	.zv_set_disk_ro = zvol_set_disk_ro_impl,
-	.zv_set_capacity = zvol_set_capacity_impl,
-};
-
 /*
  * Public interfaces
  */
@@ -1559,7 +1548,6 @@ int
 zvol_init(void)
 {
 	zvol_init_impl();
-	zvol_register_ops(&zvol_freebsd_ops);
 	return (0);
 }
 

module/os/linux/zfs/zvol_os.c

@@ -86,8 +86,8 @@ zv_request_task_free(zv_request_task_t *task)
 /*
  * Given a path, return TRUE if path is a ZVOL.
  */
-static boolean_t
-zvol_is_zvol_impl(const char *path)
+boolean_t
+zvol_os_is_zvol(const char *path)
 {
 	dev_t dev = 0;
 
@@ -507,7 +507,7 @@ retry:
 	/*
 	 * Obtain a copy of private_data under the zvol_state_lock to make
 	 * sure that either the result of zvol free code path setting
-	 * bdev->bd_disk->private_data to NULL is observed, or zvol_free()
+	 * bdev->bd_disk->private_data to NULL is observed, or zvol_os_free()
 	 * is not called on this zv because of the positive zv_open_count.
 	 */
 	zv = bdev->bd_disk->private_data;
@@ -747,8 +747,8 @@ zvol_revalidate_disk(struct gendisk *disk)
 	return (0);
 }
 
-static int
-zvol_update_volsize(zvol_state_t *zv, uint64_t volsize)
+int
+zvol_os_update_volsize(zvol_state_t *zv, uint64_t volsize)
 {
 	struct gendisk *disk = zv->zv_zso->zvo_disk;
 
@@ -762,8 +762,8 @@ zvol_update_volsize(zvol_state_t *zv, uint64_t volsize)
 	return (0);
 }
 
-static void
-zvol_clear_private(zvol_state_t *zv)
+void
+zvol_os_clear_private(zvol_state_t *zv)
 {
 	/*
 	 * Cleared while holding zvol_state_lock as a writer
@@ -944,8 +944,8 @@ out_kmem:
  * "del_gendisk". Thus, consumers need to be careful to account for this
  * latency when calling this function.
  */
-static void
-zvol_free(zvol_state_t *zv)
+void
+zvol_os_free(zvol_state_t *zv)
 {
 
 	ASSERT(!RW_LOCK_HELD(&zv->zv_suspend_lock));
@@ -985,7 +985,7 @@ zvol_wait_close(zvol_state_t *zv)
  * and the specified volume.  Once this function returns the block
  * device is live and ready for use.
  */
-static int
+int
 zvol_os_create_minor(const char *name)
 {
 	zvol_state_t *zv;
@@ -1122,8 +1122,8 @@ out_doi:
 	return (error);
 }
 
-static void
-zvol_rename_minor(zvol_state_t *zv, const char *newname)
+void
+zvol_os_rename_minor(zvol_state_t *zv, const char *newname)
 {
 	int readonly = get_disk_ro(zv->zv_zso->zvo_disk);
 
@@ -1149,31 +1149,20 @@ zvol_rename_minor(zvol_state_t *zv, const char *newname)
 	set_disk_ro(zv->zv_zso->zvo_disk, readonly);
 }
 
-static void
-zvol_set_disk_ro_impl(zvol_state_t *zv, int flags)
+void
+zvol_os_set_disk_ro(zvol_state_t *zv, int flags)
 {
 
 	set_disk_ro(zv->zv_zso->zvo_disk, flags);
 }
 
-static void
-zvol_set_capacity_impl(zvol_state_t *zv, uint64_t capacity)
+void
+zvol_os_set_capacity(zvol_state_t *zv, uint64_t capacity)
 {
 
 	set_capacity(zv->zv_zso->zvo_disk, capacity);
 }
 
-const static zvol_platform_ops_t zvol_linux_ops = {
-	.zv_free = zvol_free,
-	.zv_rename_minor = zvol_rename_minor,
-	.zv_create_minor = zvol_os_create_minor,
-	.zv_update_volsize = zvol_update_volsize,
-	.zv_clear_private = zvol_clear_private,
-	.zv_is_zvol = zvol_is_zvol_impl,
-	.zv_set_disk_ro = zvol_set_disk_ro_impl,
-	.zv_set_capacity = zvol_set_capacity_impl,
-};
-
 int
 zvol_init(void)
 {
@@ -1193,7 +1182,6 @@ zvol_init(void)
 	}
 	zvol_init_impl();
 	ida_init(&zvol_ida);
-	zvol_register_ops(&zvol_linux_ops);
 	return (0);
 }
 

module/zfs/zvol.c

@@ -92,7 +92,6 @@ unsigned int zvol_volmode = ZFS_VOLMODE_GEOM;
 struct hlist_head *zvol_htable;
 static list_t zvol_state_list;
 krwlock_t zvol_state_lock;
-static const zvol_platform_ops_t *ops;
 
 typedef enum {
 	ZVOL_ASYNC_REMOVE_MINORS,
@@ -365,7 +364,7 @@ out:
 		mutex_exit(&zv->zv_state_lock);
 
 	if (error == 0 && zv != NULL)
-		ops->zv_update_volsize(zv, volsize);
+		zvol_os_update_volsize(zv, volsize);
 
 	return (SET_ERROR(error));
 }
@@ -747,15 +746,15 @@ zvol_setup_zv(zvol_state_t *zv)
 	if (error)
 		return (SET_ERROR(error));
 
-	ops->zv_set_capacity(zv, volsize >> 9);
+	zvol_os_set_capacity(zv, volsize >> 9);
 	zv->zv_volsize = volsize;
 
 	if (ro || dmu_objset_is_snapshot(os) ||
 	    !spa_writeable(dmu_objset_spa(os))) {
-		ops->zv_set_disk_ro(zv, 1);
+		zvol_os_set_disk_ro(zv, 1);
 		zv->zv_flags |= ZVOL_RDONLY;
 	} else {
-		ops->zv_set_disk_ro(zv, 0);
+		zvol_os_set_disk_ro(zv, 0);
 		zv->zv_flags &= ~ZVOL_RDONLY;
 	}
 	return (0);
@@ -1119,7 +1118,7 @@ zvol_create_minors_recursive(const char *name)
 	 * taskq_dispatch to parallel prefetch zvol dnodes. Note we don't need
 	 * any lock because all list operation is done on the current thread.
 	 *
-	 * We will use this list to do zvol_create_minor_impl after prefetch
+	 * We will use this list to do zvol_os_create_minor after prefetch
 	 * so we don't have to traverse using dmu_objset_find again.
 	 */
 	list_create(&minors_list, sizeof (minors_job_t),
@@ -1133,7 +1132,7 @@ zvol_create_minors_recursive(const char *name)
 		    &snapdev, NULL);
 
 		if (error == 0 && snapdev == ZFS_SNAPDEV_VISIBLE)
-			(void) ops->zv_create_minor(name);
+			(void) zvol_os_create_minor(name);
 	} else {
 		fstrans_cookie_t cookie = spl_fstrans_mark();
 		(void) dmu_objset_find(name, zvol_create_minors_cb,
@@ -1144,13 +1143,13 @@ zvol_create_minors_recursive(const char *name)
 	taskq_wait_outstanding(system_taskq, 0);
 
 	/*
-	 * Prefetch is completed, we can do zvol_create_minor_impl
+	 * Prefetch is completed, we can do zvol_os_create_minor
 	 * sequentially.
 	 */
 	while ((job = list_head(&minors_list)) != NULL) {
 		list_remove(&minors_list, job);
 		if (!job->error)
-			(void) ops->zv_create_minor(job->name);
+			(void) zvol_os_create_minor(job->name);
 		kmem_strfree(job->name);
 		kmem_free(job, sizeof (minors_job_t));
 	}
@@ -1180,9 +1179,9 @@ zvol_create_minor(const char *name)
 		    "snapdev", &snapdev, NULL);
 
 		if (error == 0 && snapdev == ZFS_SNAPDEV_VISIBLE)
-			(void) ops->zv_create_minor(name);
+			(void) zvol_os_create_minor(name);
 	} else {
-		(void) ops->zv_create_minor(name);
+		(void) zvol_os_create_minor(name);
 	}
 }
 
@@ -1193,7 +1192,7 @@ zvol_create_minor(const char *name)
 static void
 zvol_free_task(void *arg)
 {
-	ops->zv_free(arg);
+	zvol_os_free(arg);
 }
 
 void
@@ -1238,7 +1237,7 @@ zvol_remove_minors_impl(const char *name)
 			 * Cleared while holding zvol_state_lock as a writer
 			 * which will prevent zvol_open() from opening it.
 			 */
-			ops->zv_clear_private(zv);
+			zvol_os_clear_private(zv);
 
 			/* Drop zv_state_lock before zvol_free() */
 			mutex_exit(&zv->zv_state_lock);
@@ -1257,7 +1256,7 @@ zvol_remove_minors_impl(const char *name)
 	/* Drop zvol_state_lock before calling zvol_free() */
 	while ((zv = list_head(&free_list)) != NULL) {
 		list_remove(&free_list, zv);
-		ops->zv_free(zv);
+		zvol_os_free(zv);
 	}
 }
 
@@ -1290,7 +1289,7 @@ zvol_remove_minor_impl(const char *name)
 			}
 			zvol_remove(zv);
 
-			ops->zv_clear_private(zv);
+			zvol_os_clear_private(zv);
 			mutex_exit(&zv->zv_state_lock);
 			break;
 		} else {
@@ -1302,7 +1301,7 @@ zvol_remove_minor_impl(const char *name)
 	rw_exit(&zvol_state_lock);
 
 	if (zv != NULL)
-		ops->zv_free(zv);
+		zvol_os_free(zv);
 }
 
 /*
@@ -1327,14 +1326,14 @@ zvol_rename_minors_impl(const char *oldname, const char *newname)
 		mutex_enter(&zv->zv_state_lock);
 
 		if (strcmp(zv->zv_name, oldname) == 0) {
-			ops->zv_rename_minor(zv, newname);
+			zvol_os_rename_minor(zv, newname);
 		} else if (strncmp(zv->zv_name, oldname, oldnamelen) == 0 &&
 		    (zv->zv_name[oldnamelen] == '/' ||
 		    zv->zv_name[oldnamelen] == '@')) {
 			char *name = kmem_asprintf("%s%c%s", newname,
 			    zv->zv_name[oldnamelen],
 			    zv->zv_name + oldnamelen + 1);
-			ops->zv_rename_minor(zv, name);
+			zvol_os_rename_minor(zv, name);
 			kmem_strfree(name);
 		}
 
@@ -1358,7 +1357,7 @@ zvol_set_snapdev_cb(const char *dsname, void *param)
 
 	switch (arg->snapdev) {
 		case ZFS_SNAPDEV_VISIBLE:
-			(void) ops->zv_create_minor(dsname);
+			(void) zvol_os_create_minor(dsname);
 			break;
 		case ZFS_SNAPDEV_HIDDEN:
 			(void) zvol_remove_minor_impl(dsname);
@@ -1415,14 +1414,14 @@ zvol_set_volmode_impl(char *name, uint64_t volmode)
 		case ZFS_VOLMODE_GEOM:
 		case ZFS_VOLMODE_DEV:
 			(void) zvol_remove_minor_impl(name);
-			(void) ops->zv_create_minor(name);
+			(void) zvol_os_create_minor(name);
 			break;
 		case ZFS_VOLMODE_DEFAULT:
 			(void) zvol_remove_minor_impl(name);
 			if (zvol_volmode == ZFS_VOLMODE_NONE)
 				break;
 			else /* if zvol_volmode is invalid defaults to "geom" */
-				(void) ops->zv_create_minor(name);
+				(void) zvol_os_create_minor(name);
 			break;
 	}
 	spl_fstrans_unmark(cookie);
@@ -1699,13 +1698,7 @@ boolean_t
 zvol_is_zvol(const char *name)
 {
 
-	return (ops->zv_is_zvol(name));
-}
-
-void
-zvol_register_ops(const zvol_platform_ops_t *zvol_ops)
-{
-	ops = zvol_ops;
+	return (zvol_os_is_zvol(name));
 }
 
 int