Refcounted DSL Crypto Key Mappings

Since native ZFS encryption was merged, we have been fighting
against a series of bugs that come down to the same problem: Key
mappings (which must be present during all I/O operations) are
created and destroyed based on dataset ownership, but I/Os can
have traditionally been allowed to "leak" into the next txg after
the dataset is disowned.

In the past we have attempted to solve this problem by trying to
ensure that datasets are disowned ater all I/O is finished by
calling txg_wait_synced(), but we have repeatedly found edge cases
that need to be squashed and code paths that might incur a high
number of txg syncs. This patch attempts to resolve this issue
differently, by adding a reference to the key mapping for each txg
it is dirtied in. By doing so, we can remove many of the
unnecessary calls to txg_wait_synced() we have added in the past
and ensure we don't need to deal with this problem in the future.

Reviewed-by: Jorgen Lundman <[email protected]>
Reviewed by: Matthew Ahrens <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Tom Caputi <[email protected]>
Closes #7949 

2 files changed, 9 insertions, 4 deletions

diff --git a/include/sys/dsl_crypt.h b/include/sys/dsl_crypt.h
index c6d2b0a16..e01d53527 100644
--- a/include/sys/dsl_crypt.h
+++ b/include/sys/dsl_crypt.h
@@ -111,7 +111,7 @@ typedef struct dsl_crypto_key {
 	/* link on spa_keystore_t:sk_dsl_keys */
 	avl_node_t dck_avl_link;
 
-	/* refcount of dsl_key_mapping_t's holding this key */
+	/* refcount of holders of this key */
 	zfs_refcount_t dck_holds;
 
 	/* master key used to derive encryption keys */
@@ -181,10 +181,11 @@ int spa_keystore_load_wkey(const char *dsname, dsl_crypto_params_t *dcp,
 int spa_keystore_unload_wkey_impl(spa_t *spa, uint64_t ddobj);
 int spa_keystore_unload_wkey(const char *dsname);
 
-int spa_keystore_create_mapping_impl(spa_t *spa, uint64_t dsobj, dsl_dir_t *dd,
-    void *tag);
-int spa_keystore_create_mapping(spa_t *spa, struct dsl_dataset *ds, void *tag);
+int spa_keystore_create_mapping(spa_t *spa, struct dsl_dataset *ds, void *tag,
+    dsl_key_mapping_t **km_out);
 int spa_keystore_remove_mapping(spa_t *spa, uint64_t dsobj, void *tag);
+void key_mapping_add_ref(dsl_key_mapping_t *km, void *tag);
+void key_mapping_rele(spa_t *spa, dsl_key_mapping_t *km, void *tag);
 int spa_keystore_lookup_key(spa_t *spa, uint64_t dsobj, void *tag,
     dsl_crypto_key_t **dck_out);
 
diff --git a/include/sys/dsl_dataset.h b/include/sys/dsl_dataset.h
index 768241483..7eded35a1 100644
--- a/include/sys/dsl_dataset.h
+++ b/include/sys/dsl_dataset.h
@@ -49,6 +49,7 @@ struct dsl_dataset;
 struct dsl_dir;
 struct dsl_pool;
 struct dsl_crypto_params;
+struct dsl_key_mapping;
 
 #define	DS_FLAG_INCONSISTENT	(1ULL<<0)
 #define	DS_IS_INCONSISTENT(ds)	\
@@ -165,6 +166,7 @@ typedef struct dsl_dataset {
 	uint64_t ds_object;
 	uint64_t ds_fsid_guid;
 	boolean_t ds_is_snapshot;
+	struct dsl_key_mapping *ds_key_mapping;
 
 	/* only used in syncing context, only valid for non-snapshots: */
 	struct dsl_dataset *ds_prev;
@@ -305,10 +307,12 @@ int dsl_dataset_hold_flags(struct dsl_pool *dp, const char *name,
     ds_hold_flags_t flags, void *tag, dsl_dataset_t **dsp);
 boolean_t dsl_dataset_try_add_ref(struct dsl_pool *dp, dsl_dataset_t *ds,
     void *tag);
+int dsl_dataset_create_key_mapping(dsl_dataset_t *ds);
 int dsl_dataset_hold_obj(struct dsl_pool *dp, uint64_t dsobj, void *tag,
     dsl_dataset_t **);
 int dsl_dataset_hold_obj_flags(struct dsl_pool *dp, uint64_t dsobj,
     ds_hold_flags_t flags, void *tag, dsl_dataset_t **);
+void dsl_dataset_remove_key_mapping(dsl_dataset_t *ds);
 void dsl_dataset_rele(dsl_dataset_t *ds, void *tag);
 void dsl_dataset_rele_flags(dsl_dataset_t *ds, ds_hold_flags_t flags,
     void *tag);