hammer2 - stabilization, fix hammer2_inode_get()
authorMatthew Dillon <dillon@apollo.backplane.com>
Sat, 8 Dec 2018 04:26:15 +0000 (20:26 -0800)
committerMatthew Dillon <dillon@apollo.backplane.com>
Sat, 8 Dec 2018 04:26:15 +0000 (20:26 -0800)
* Fix a lock order reversal in hammer2_inode_get() which can be
  triggered under heavy loads.

sys/vfs/hammer2/hammer2.h
sys/vfs/hammer2/hammer2_chain.c
sys/vfs/hammer2/hammer2_cluster.c
sys/vfs/hammer2/hammer2_inode.c

index fb12a40..25bfebe 100644 (file)
@@ -1590,6 +1590,8 @@ void hammer2_chain_ref(hammer2_chain_t *chain);
 void hammer2_chain_ref_hold(hammer2_chain_t *chain);
 void hammer2_chain_drop(hammer2_chain_t *chain);
 void hammer2_chain_drop_unhold(hammer2_chain_t *chain);
+void hammer2_chain_unhold(hammer2_chain_t *chain);
+void hammer2_chain_rehold(hammer2_chain_t *chain);
 int hammer2_chain_lock(hammer2_chain_t *chain, int how);
 void hammer2_chain_lock_unhold(hammer2_chain_t *chain, int how);
 void hammer2_chain_load_data(hammer2_chain_t *chain);
@@ -1967,6 +1969,8 @@ hammer2_cluster_t *hammer2_cluster_alloc(hammer2_pfs_t *pmp,
                                hammer2_blockref_t *bref);
 void hammer2_cluster_ref(hammer2_cluster_t *cluster);
 void hammer2_cluster_drop(hammer2_cluster_t *cluster);
+void hammer2_cluster_unhold(hammer2_cluster_t *cluster);
+void hammer2_cluster_rehold(hammer2_cluster_t *cluster);
 void hammer2_cluster_lock(hammer2_cluster_t *cluster, int how);
 int hammer2_cluster_check(hammer2_cluster_t *cluster, hammer2_key_t lokey,
                        int flags);
index 4bca74b..b2d1ac6 100644 (file)
@@ -430,7 +430,7 @@ hammer2_chain_drop(hammer2_chain_t *chain)
  * lock and then simply unlocking the chain.
  */
 void
-hammer2_chain_drop_unhold(hammer2_chain_t *chain)
+hammer2_chain_unhold(hammer2_chain_t *chain)
 {
        u_int lockcnt;
        int iter = 0;
@@ -465,9 +465,23 @@ hammer2_chain_drop_unhold(hammer2_chain_t *chain)
                        cpu_pause();
                }
        }
+}
+
+void
+hammer2_chain_drop_unhold(hammer2_chain_t *chain)
+{
+       hammer2_chain_unhold(chain);
        hammer2_chain_drop(chain);
 }
 
+void
+hammer2_chain_rehold(hammer2_chain_t *chain)
+{
+       hammer2_chain_lock(chain, HAMMER2_RESOLVE_SHARED);
+       atomic_add_int(&chain->lockcnt, 1);
+       hammer2_chain_unlock(chain);
+}
+
 /*
  * Handles the (potential) last drop of chain->refs from 1->0.  Called with
  * the mutex exclusively locked, refs == 1, and lockcnt 0.  SMP races are
@@ -1007,9 +1021,8 @@ hammer2_chain_lock(hammer2_chain_t *chain, int how)
                 * even for non-blocking operation, because the unlock code
                 * live-loops on lockcnt == 1 when dropping the last lock.
                 *
-                * If the non-blocking operation fails we have to use a
-                * ref+drop+unhold sequence to undo the mess (or write a
-                * hammer2_chain_unhold() function that doesn't drop).
+                * If the non-blocking operation fails we have to use an
+                * unhold sequence to undo the mess.
                 *
                 * NOTE: LOCKAGAIN must always succeed without blocking,
                 *       even if NONBLOCK is specified.
@@ -1020,15 +1033,13 @@ hammer2_chain_lock(hammer2_chain_t *chain, int how)
                                hammer2_mtx_sh_again(&chain->lock);
                        } else {
                                if (hammer2_mtx_sh_try(&chain->lock) != 0) {
-                                       hammer2_chain_ref(chain);
-                                       hammer2_chain_drop_unhold(chain);
+                                       hammer2_chain_unhold(chain);
                                        return EAGAIN;
                                }
                        }
                } else {
                        if (hammer2_mtx_ex_try(&chain->lock) != 0) {
-                               hammer2_chain_ref(chain);
-                               hammer2_chain_drop_unhold(chain);
+                               hammer2_chain_unhold(chain);
                                return EAGAIN;
                        }
                }
index 5a323a3..3225636 100644 (file)
@@ -260,6 +260,34 @@ hammer2_cluster_lock(hammer2_cluster_t *cluster, int how)
        }
 }
 
+void
+hammer2_cluster_unhold(hammer2_cluster_t *cluster)
+{
+       hammer2_chain_t *chain;
+       int i;
+
+       for (i = 0; i < cluster->nchains; ++i) {
+               chain = cluster->array[i].chain;
+               if (chain == NULL)
+                       continue;
+               hammer2_chain_unhold(chain);
+       }
+}
+
+void
+hammer2_cluster_rehold(hammer2_cluster_t *cluster)
+{
+       hammer2_chain_t *chain;
+       int i;
+
+       for (i = 0; i < cluster->nchains; ++i) {
+               chain = cluster->array[i].chain;
+               if (chain == NULL)
+                       continue;
+               hammer2_chain_rehold(chain);
+       }
+}
+
 /*
  * Calculate the clustering state for the cluster and set its focus.
  * This routine must be called with care.  For example, it should not
index aaaab62..32e19d4 100644 (file)
@@ -799,6 +799,10 @@ hammer2_igetv(hammer2_inode_t *ip, int *errorp)
 }
 
 /*
+ * XXX this API needs a rewrite.  It needs to be split into a
+ * hammer2_inode_alloc() and hammer2_inode_build() to allow us to get
+ * rid of the inode/chain lock reversal fudge.
+ *
  * Returns the inode associated with the passed-in cluster, allocating a new
  * hammer2_inode structure if necessary, then synchronizing it to the passed
  * xop cluster.  When synchronizing, if idx >= 0, only cluster index (idx)
@@ -849,11 +853,24 @@ hammer2_inode_get(hammer2_pfs_t *pmp, hammer2_xop_head_t *xop,
 again:
        nip = hammer2_inode_lookup(pmp, inum);
        if (nip) {
+               /*
+                * We may have to unhold the cluster to avoid a deadlock
+                * against vnlru (and possibly other XOPs).
+                */
+               if (xop) {
+                       if (hammer2_mtx_ex_try(&nip->lock) != 0) {
+                               hammer2_cluster_unhold(&xop->cluster);
+                               hammer2_mtx_ex(&nip->lock);
+                               hammer2_cluster_rehold(&xop->cluster);
+                       }
+               } else {
+                       hammer2_mtx_ex(&nip->lock);
+               }
+
                /*
                 * Handle SMP race (not applicable to the super-root spmp
                 * which can't index inodes due to duplicative inode numbers).
                 */
-               hammer2_mtx_ex(&nip->lock);
                if (pmp->spmp_hmp == NULL &&
                    (nip->flags & HAMMER2_INODE_ONRBTREE) == 0) {
                        hammer2_mtx_unlock(&nip->lock);