Fix invalid locking order in rename operation

This commit should prevent a deadlock on dp_config_rwlock when running `zfs rename` by ensuring zvol_rename_minors() is not called under this lock. Signed-off-by: Stanislav Seletskiy <[email protected]> Signed-off-by: Richard Yao <[email protected]> Signed-off-by: Tim Chase <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes openzfs#2652. Closes openzfs#2525.
ryao · Sep 5, 2014 · 9db3139 · 9db3139
1 parent 12d7df6
commit 9db3139
Showing 1 changed file with 20 additions and 17 deletions.
diff --git a/module/zfs/dsl_dataset.c b/module/zfs/dsl_dataset.c
@@ -1612,9 +1612,6 @@ static int
 dsl_dataset_rename_snapshot_sync_impl(dsl_pool_t *dp,
     dsl_dataset_t *hds, void *arg)
 {
-#ifdef _KERNEL
-	char *oldname, *newname;
-#endif
 	dsl_dataset_rename_snapshot_arg_t *ddrsa = arg;
 	dsl_dataset_t *ds;
 	uint64_t val;
@@ -1641,18 +1638,6 @@ dsl_dataset_rename_snapshot_sync_impl(dsl_pool_t *dp,
 	VERIFY0(zap_add(dp->dp_meta_objset, hds->ds_phys->ds_snapnames_zapobj,
 	    ds->ds_snapname, 8, 1, &ds->ds_object, tx));
 
-#ifdef _KERNEL
-	oldname = kmem_alloc(MAXPATHLEN, KM_PUSHPAGE);
-	newname = kmem_alloc(MAXPATHLEN, KM_PUSHPAGE);
-	snprintf(oldname, MAXPATHLEN, "%s@%s", ddrsa->ddrsa_fsname,
-	    ddrsa->ddrsa_oldsnapname);
-	snprintf(newname, MAXPATHLEN, "%s@%s", ddrsa->ddrsa_fsname,
-	    ddrsa->ddrsa_newsnapname);
-	zvol_rename_minors(oldname, newname);
-	kmem_free(newname, MAXPATHLEN);
-	kmem_free(oldname, MAXPATHLEN);
-#endif
-
 	dsl_dataset_rele(ds, FTAG);
 	return (0);
 }
@@ -1680,15 +1665,33 @@ int
 dsl_dataset_rename_snapshot(const char *fsname,
     const char *oldsnapname, const char *newsnapname, boolean_t recursive)
 {
+#ifdef _KERNEL
+	char *oldname, *newname;
+#endif
+	int error;
+
 	dsl_dataset_rename_snapshot_arg_t ddrsa;
 
 	ddrsa.ddrsa_fsname = fsname;
 	ddrsa.ddrsa_oldsnapname = oldsnapname;
 	ddrsa.ddrsa_newsnapname = newsnapname;
 	ddrsa.ddrsa_recursive = recursive;
 
-	return (dsl_sync_task(fsname, dsl_dataset_rename_snapshot_check,
-	    dsl_dataset_rename_snapshot_sync, &ddrsa, 1));
+	error = dsl_sync_task(fsname, dsl_dataset_rename_snapshot_check,
+	    dsl_dataset_rename_snapshot_sync, &ddrsa, 1);
+
+	if (error)
+	    return (SET_ERROR(error));
+
+#ifdef _KERNEL
+	oldname = kmem_asprintf("%s@%s", fsname, oldsnapname);
+	newname = kmem_asprintf("%s@%s", fsname, newsnapname);
+	zvol_rename_minors(oldname, newname);
+	strfree(newname);
+	strfree(oldname);
+#endif
+
+	return (0);
 }
 
 /*