Fix dnode_hold_impl() soft lockup

Soft lockups could happen when multiple threads trying to get zrl on the same dnode handle in order to allocate and initialize the dnode marked as DN_SLOT_ALLOCATED. Don't loop from beginning when we can't get zrl, otherwise we would increase the zrl refcount and nobody can actually lock it. Signed-off-by: Li Dongyang <[email protected]> Closes #8426
openzfs · Feb 19, 2019 · ec7f4bc · ec7f4bc
1 parent f545b6a
commit ec7f4bc
Showing 1 changed file with 46 additions and 56 deletions.
diff --git a/module/zfs/dnode.c b/module/zfs/dnode.c
@@ -1401,35 +1401,29 @@ dnode_hold_impl(objset_t *os, uint64_t object, int flag, int slots,
 	}
 
 	ASSERT(dnc->dnc_count == epb);
-	dn = DN_SLOT_UNINIT;
 
 	if (flag & DNODE_MUST_BE_ALLOCATED) {
 		slots = 1;
 
-		while (dn == DN_SLOT_UNINIT) {
-			dnode_slots_hold(dnc, idx, slots);
-			dnh = &dnc->dnc_children[idx];
-
-			if (DN_SLOT_IS_PTR(dnh->dnh_dnode)) {
-				dn = dnh->dnh_dnode;
-				break;
-			} else if (dnh->dnh_dnode == DN_SLOT_INTERIOR) {
-				DNODE_STAT_BUMP(dnode_hold_alloc_interior);
-				dnode_slots_rele(dnc, idx, slots);
-				dbuf_rele(db, FTAG);
-				return (SET_ERROR(EEXIST));
-			} else if (dnh->dnh_dnode != DN_SLOT_ALLOCATED) {
-				DNODE_STAT_BUMP(dnode_hold_alloc_misses);
-				dnode_slots_rele(dnc, idx, slots);
-				dbuf_rele(db, FTAG);
-				return (SET_ERROR(ENOENT));
-			}
+		dnode_slots_hold(dnc, idx, slots);
+		dnh = &dnc->dnc_children[idx];
 
+		if (DN_SLOT_IS_PTR(dnh->dnh_dnode)) {
+			dn = dnh->dnh_dnode;
+		} else if (dnh->dnh_dnode == DN_SLOT_INTERIOR) {
+			DNODE_STAT_BUMP(dnode_hold_alloc_interior);
 			dnode_slots_rele(dnc, idx, slots);
-			if (!dnode_slots_tryenter(dnc, idx, slots)) {
+			dbuf_rele(db, FTAG);
+			return (SET_ERROR(EEXIST));
+		} else if (dnh->dnh_dnode != DN_SLOT_ALLOCATED) {
+			DNODE_STAT_BUMP(dnode_hold_alloc_misses);
+			dnode_slots_rele(dnc, idx, slots);
+			dbuf_rele(db, FTAG);
+			return (SET_ERROR(ENOENT));
+		} else {
+			dnode_slots_rele(dnc, idx, slots);
+			while (!dnode_slots_tryenter(dnc, idx, slots))
 				DNODE_STAT_BUMP(dnode_hold_alloc_lock_retry);
-				continue;
-			}
 
 			/*
 			 * Someone else won the race and called dnode_create()
@@ -1463,45 +1457,41 @@ dnode_hold_impl(objset_t *os, uint64_t object, int flag, int slots,
 			return (SET_ERROR(ENOSPC));
 		}
 
-		while (dn == DN_SLOT_UNINIT) {
-			dnode_slots_hold(dnc, idx, slots);
-
-			if (!dnode_check_slots_free(dnc, idx, slots)) {
-				DNODE_STAT_BUMP(dnode_hold_free_misses);
-				dnode_slots_rele(dnc, idx, slots);
-				dbuf_rele(db, FTAG);
-				return (SET_ERROR(ENOSPC));
-			}
+		dnode_slots_hold(dnc, idx, slots);
 
+		if (!dnode_check_slots_free(dnc, idx, slots)) {
+			DNODE_STAT_BUMP(dnode_hold_free_misses);
 			dnode_slots_rele(dnc, idx, slots);
-			if (!dnode_slots_tryenter(dnc, idx, slots)) {
-				DNODE_STAT_BUMP(dnode_hold_free_lock_retry);
-				continue;
-			}
+			dbuf_rele(db, FTAG);
+			return (SET_ERROR(ENOSPC));
+		}
 
-			if (!dnode_check_slots_free(dnc, idx, slots)) {
-				DNODE_STAT_BUMP(dnode_hold_free_lock_misses);
-				dnode_slots_rele(dnc, idx, slots);
-				dbuf_rele(db, FTAG);
-				return (SET_ERROR(ENOSPC));
-			}
+		dnode_slots_rele(dnc, idx, slots);
+		while (!dnode_slots_tryenter(dnc, idx, slots))
+			DNODE_STAT_BUMP(dnode_hold_free_lock_retry);
 
-			/*
-			 * Allocated but otherwise free dnodes which would
-			 * be in the interior of a multi-slot dnodes need
-			 * to be freed.  Single slot dnodes can be safely
-			 * re-purposed as a performance optimization.
-			 */
-			if (slots > 1)
-				dnode_reclaim_slots(dnc, idx + 1, slots - 1);
+		if (!dnode_check_slots_free(dnc, idx, slots)) {
+			DNODE_STAT_BUMP(dnode_hold_free_lock_misses);
+			dnode_slots_rele(dnc, idx, slots);
+			dbuf_rele(db, FTAG);
+			return (SET_ERROR(ENOSPC));
+		}
 
-			dnh = &dnc->dnc_children[idx];
-			if (DN_SLOT_IS_PTR(dnh->dnh_dnode)) {
-				dn = dnh->dnh_dnode;
-			} else {
-				dn = dnode_create(os, dn_block + idx, db,
-				    object, dnh);
-			}
+		/*
+		 * Allocated but otherwise free dnodes which would
+		 * be in the interior of a multi-slot dnodes need
+		 * to be freed.  Single slot dnodes can be safely
+		 * re-purposed as a performance optimization.
+		 */
+		if (slots > 1)
+			dnode_reclaim_slots(dnc, idx + 1, slots - 1);
+
+		dnh = &dnc->dnc_children[idx];
+		if (DN_SLOT_IS_PTR(dnh->dnh_dnode)) {
+			dn = dnh->dnh_dnode;
+		} else {
+			dn = dnode_create(os, dn_block + idx, db,
+			    object, dnh);
 		}
 
 		mutex_enter(&dn->dn_mtx);