Illumos 6171 dsl_prop_unregister() slows down dataset eviction.

Reviewed by: Matthew Ahrens <[email protected]> Reviewed by: Prakash Surya <[email protected]> Approved by: Dan McDonald <[email protected]> References: https://www.illumos.org/issues/6171 dsl_prop_unregister() slows down dataset eviction. illumos/illumos-gate@03bad06#diff-832418f8bef771ab321cc8bca7235e4dL297 Porting notes: Notable changes from upstream are caused by the following commits: openzfs@3558fd7 Prototype/structure update for Linux openzfs@2cf7f52 Linux compat 2.6.39: mount_nodev() openzfs@13fe019 Illumos openzfs#3464 Fix compilation error: dsl_prop.c:1211:15: error: ‘dsl_prop_unregister’ undeclared here (not in a function) EXPORT_SYMBOL(dsl_prop_unregister); Replacing EXPORT_SYMBOL(dsl_prop_unregister); with EXPORT_SYMBOL(dsl_prop_unregister_all); Fixing 'space or tab at end of line' in include/sys/dsl_dataset.h Ported-by: kernelOfTruth [email protected]
kernelOfTruth · Nov 7, 2015 · 70559ae · 70559ae
1 parent b23d543
commit 70559ae
Show file tree

Hide file tree

Showing 8 changed files with 172 additions and 216 deletions.
diff --git a/include/sys/dsl_dataset.h b/include/sys/dsl_dataset.h
@@ -185,6 +185,9 @@ typedef struct dsl_dataset {
 	kmutex_t ds_sendstream_lock;
 	list_t ds_sendstreams;
 
+	/* Protected by our dsl_dir's dd_lock */
+	list_t ds_prop_cbs;
+
 	/* Protected by ds_lock; keep at end of struct for better locality */
 	char ds_snapname[MAXNAMELEN];
 } dsl_dataset_t;

diff --git a/include/sys/dsl_dir.h b/include/sys/dsl_dir.h
@@ -102,7 +102,7 @@ struct dsl_dir {
 
 	/* Protected by dd_lock */
 	kmutex_t dd_lock;
-	list_t dd_prop_cbs; /* list of dsl_prop_cb_record_t's */
+	list_t dd_props; /* list of dsl_prop_record_t's */
 	timestruc_t dd_snap_cmtime; /* last time snapshot namespace changed */
 	uint64_t dd_origin_txg;
 

diff --git a/include/sys/dsl_prop.h b/include/sys/dsl_prop.h
@@ -41,10 +41,17 @@ struct dsl_dir;
 /* The callback func may not call into the DMU or DSL! */
 typedef void (dsl_prop_changed_cb_t)(void *arg, uint64_t newval);
 
+typedef struct dsl_prop_record {
+	list_node_t pr_node; /* link on dd_props */
+	const char *pr_propname;
+	list_t pr_cbs;
+} dsl_prop_record_t;
+
 typedef struct dsl_prop_cb_record {
-	list_node_t cbr_node; /* link on dd_prop_cbs */
+	list_node_t cbr_pr_node; /* link on pr_cbs */
+	list_node_t cbr_ds_node; /* link on ds_prop_cbs */
+	dsl_prop_record_t *cbr_pr;
 	struct dsl_dataset *cbr_ds;
-	const char *cbr_propname;
 	dsl_prop_changed_cb_t *cbr_func;
 	void *cbr_arg;
 } dsl_prop_cb_record_t;
@@ -54,10 +61,11 @@ typedef struct dsl_props_arg {
 	zprop_source_t pa_source;
 } dsl_props_arg_t;
 
+void dsl_prop_init(dsl_dir_t *dd);
+void dsl_prop_fini(dsl_dir_t *dd);
 int dsl_prop_register(struct dsl_dataset *ds, const char *propname,
     dsl_prop_changed_cb_t *callback, void *cbarg);
-int dsl_prop_unregister(struct dsl_dataset *ds, const char *propname,
-    dsl_prop_changed_cb_t *callback, void *cbarg);
+void dsl_prop_unregister_all(struct dsl_dataset *ds, void *cbarg);
 void dsl_prop_notify_all(struct dsl_dir *dd);
 boolean_t dsl_prop_hascb(struct dsl_dataset *ds);
 

diff --git a/module/zfs/dmu_objset.c b/module/zfs/dmu_objset.c
@@ -688,40 +688,8 @@ dmu_objset_evict(objset_t *os)
 	for (t = 0; t < TXG_SIZE; t++)
 		ASSERT(!dmu_objset_is_dirty(os, t));
 
-	if (ds) {
-		if (!ds->ds_is_snapshot) {
-			VERIFY0(dsl_prop_unregister(ds,
-			    zfs_prop_to_name(ZFS_PROP_CHECKSUM),
-			    checksum_changed_cb, os));
-			VERIFY0(dsl_prop_unregister(ds,
-			    zfs_prop_to_name(ZFS_PROP_COMPRESSION),
-			    compression_changed_cb, os));
-			VERIFY0(dsl_prop_unregister(ds,
-			    zfs_prop_to_name(ZFS_PROP_COPIES),
-			    copies_changed_cb, os));
-			VERIFY0(dsl_prop_unregister(ds,
-			    zfs_prop_to_name(ZFS_PROP_DEDUP),
-			    dedup_changed_cb, os));
-			VERIFY0(dsl_prop_unregister(ds,
-			    zfs_prop_to_name(ZFS_PROP_LOGBIAS),
-			    logbias_changed_cb, os));
-			VERIFY0(dsl_prop_unregister(ds,
-			    zfs_prop_to_name(ZFS_PROP_SYNC),
-			    sync_changed_cb, os));
-			VERIFY0(dsl_prop_unregister(ds,
-			    zfs_prop_to_name(ZFS_PROP_REDUNDANT_METADATA),
-			    redundant_metadata_changed_cb, os));
-			VERIFY0(dsl_prop_unregister(ds,
-			    zfs_prop_to_name(ZFS_PROP_RECORDSIZE),
-			    recordsize_changed_cb, os));
-		}
-		VERIFY0(dsl_prop_unregister(ds,
-		    zfs_prop_to_name(ZFS_PROP_PRIMARYCACHE),
-		    primary_cache_changed_cb, os));
-		VERIFY0(dsl_prop_unregister(ds,
-		    zfs_prop_to_name(ZFS_PROP_SECONDARYCACHE),
-		    secondary_cache_changed_cb, os));
-	}
+	if (ds)
+		dsl_prop_unregister_all(ds, os);
 
 	if (os->os_sa)
 		sa_tear_down(os);

diff --git a/module/zfs/dsl_dataset.c b/module/zfs/dsl_dataset.c
@@ -282,6 +282,7 @@ dsl_dataset_evict(void *dbu)
 
 	ASSERT(!list_link_active(&ds->ds_synced_link));
 
+	list_destroy(&ds->ds_prop_cbs);
 	mutex_destroy(&ds->ds_lock);
 	mutex_destroy(&ds->ds_opening_lock);
 	mutex_destroy(&ds->ds_sendstream_lock);
@@ -431,6 +432,9 @@ dsl_dataset_hold_obj(dsl_pool_t *dp, uint64_t dsobj, void *tag,
 		list_create(&ds->ds_sendstreams, sizeof (dmu_sendarg_t),
 		    offsetof(dmu_sendarg_t, dsa_link));
 
+		list_create(&ds->ds_prop_cbs, sizeof (dsl_prop_cb_record_t),
+		    offsetof(dsl_prop_cb_record_t, cbr_ds_node));
+
 		if (doi.doi_type == DMU_OTN_ZAP_METADATA) {
 			int zaperr = zap_contains(mos, dsobj,
 			    DS_FIELD_LARGE_BLOCKS);

diff --git a/module/zfs/dsl_dir.c b/module/zfs/dsl_dir.c
@@ -147,11 +147,7 @@ dsl_dir_evict(void *dbu)
 
 	spa_async_close(dd->dd_pool->dp_spa, dd);
 
-	/*
-	 * The props callback list should have been cleaned up by
-	 * objset_evict().
-	 */
-	list_destroy(&dd->dd_prop_cbs);
+	dsl_prop_fini(dd);
 	mutex_destroy(&dd->dd_lock);
 	kmem_free(dd, sizeof (dsl_dir_t));
 }
@@ -186,9 +182,7 @@ dsl_dir_hold_obj(dsl_pool_t *dp, uint64_t ddobj,
 		dd->dd_dbuf = dbuf;
 		dd->dd_pool = dp;
 		mutex_init(&dd->dd_lock, NULL, MUTEX_DEFAULT, NULL);
-
-		list_create(&dd->dd_prop_cbs, sizeof (dsl_prop_cb_record_t),
-		    offsetof(dsl_prop_cb_record_t, cbr_node));
+		dsl_prop_init(dd);
 
 		dsl_dir_snap_cmtime_update(dd);
 
@@ -246,6 +240,7 @@ dsl_dir_hold_obj(dsl_pool_t *dp, uint64_t ddobj,
 		if (winner != NULL) {
 			if (dd->dd_parent)
 				dsl_dir_rele(dd->dd_parent, dd);
+			dsl_prop_fini(dd);
 			mutex_destroy(&dd->dd_lock);
 			kmem_free(dd, sizeof (dsl_dir_t));
 			dd = winner;
@@ -273,6 +268,7 @@ dsl_dir_hold_obj(dsl_pool_t *dp, uint64_t ddobj,
 errout:
 	if (dd->dd_parent)
 		dsl_dir_rele(dd->dd_parent, dd);
+	dsl_prop_fini(dd);
 	mutex_destroy(&dd->dd_lock);
 	kmem_free(dd, sizeof (dsl_dir_t));
 	dmu_buf_rele(dbuf, tag);