Unbalanced hold vs rele in dsl_crypto.c

[lundman]

Hopefully this will fix some of the deadlocks we have in 0.6.3

[FransUrbo]

I'm building right now. Hopefully I'll have an answer within the hour.

[FransUrbo]

Signed-off-by: Turbo Fredriksson turbo@bayour.com

As discussed in the ZoL issue, this fixes the problem for me. I still have some problems, but it is now extremely likely that that have other causes.

Thanx @lundman !

Send me your PayPal info (or other means) and I'll give you the bounty.

[FransUrbo]

This is the only really obvious one I could find myself that you missed:

diff --git a/module/zfs/dsl_crypto.c b/module/zfs/dsl_crypto.c
index 79d7c4b..3fc4c55 100644
--- a/module/zfs/dsl_crypto.c
+++ b/module/zfs/dsl_crypto.c
@@ -381,6 +381,7 @@ dsl_crypto_key_unload(const char *dsname)
        }
        if (error != 0) {
                dsl_dataset_rele(ds, FTAG);
+               dsl_pool_rele(dp, FTAG);
                return (error);
        }
 #endif

[FransUrbo]

There's also some dsl_dataset_hold/dsl_dataset_rele that's questionable...

[FransUrbo]

Also?

diff --git a/module/zfs/dmu_send.c b/module/zfs/dmu_send.c
index 5ef7973..163bed1 100644
--- a/module/zfs/dmu_send.c
+++ b/module/zfs/dmu_send.c
@@ -518,6 +518,10 @@ dmu_send_impl(void *tag, dsl_pool_t *dp, dsl_dataset_t *ds,
                             zfs_prop_to_name(ZFS_PROP_ENCRYPTION), 8, 1, &crypt, NULL
                             /*,DSL_PROP_GET_EFFECTIVE*/) != 0) {
             rrw_exit(&ds->ds_dir->dd_pool->dp_config_rwlock, FTAG);
+                       if (fromds != NULL)
+                               dsl_dataset_rele(fromds, tag);
+                       dsl_dataset_rele(ds, tag);
+                       dsl_pool_rele(dp, tag);
             return (EINVAL);
         }
         rrw_exit(&ds->ds_dir->dd_pool->dp_config_rwlock, FTAG);
@@ -574,9 +578,6 @@ dmu_send_impl(void *tag, dsl_pool_t *dp, dsl_dataset_t *ds,
                goto out;
        }

-       dsl_dataset_long_hold(ds, FTAG);
-       dsl_pool_rele(dp, tag);
-
        err = traverse_dataset(ds, fromtxg, TRAVERSE_PRE | TRAVERSE_PREFETCH,
            backup_cb, dsp);

@@ -608,7 +609,6 @@ out:
        kmem_free(drr, sizeof (dmu_replay_record_t));
        kmem_free(dsp, sizeof (dmu_sendarg_t));

-       dsl_dataset_long_rele(ds, FTAG);
        dsl_dataset_rele(ds, tag);

        return (err);
@@ -898,7 +898,7 @@ dmu_recv_begin_check(void *arg, dmu_tx_t *tx)
        error = dsl_dataset_hold(dp, tofs, FTAG, &ds);
        if (error == 0) {
         if (!dmu_recv_verify_features(ds, drrb)) {
-            dsl_dataset_rele(ds, dmu_recv_tag);
+            dsl_dataset_rele(ds, FTAG);
             return (ENOTSUP);
         }
                /* target fs already exists; recv into temp clone */
diff --git a/module/zfs/dsl_dataset.c b/module/zfs/dsl_dataset.c
index fba057a..974e553 100644
--- a/module/zfs/dsl_dataset.c
+++ b/module/zfs/dsl_dataset.c
@@ -2340,6 +2340,7 @@ promote_hold(dsl_dataset_promote_arg_t *ddpa, dsl_pool_t *dp, void *tag)
 out:
        if (error != 0)
                promote_rele(ddpa, tag);
+       dsl_dataset_rele(ddpa->ddpa_clone, tag);
        return (error);
 }

[zfsrogue]

More diff to come da?

[lundman]

Sorry, missed the comments. Couple of Turbo's suggestions are in #if 0, but we might as well balance it now. The last one is released inside promote_rele(). dmu_send() had a few additional reles which was odd.