svn commit: r322236 - vendor-sys/illumos/dist/uts/common/fs/zfs

[Andriy Gapon]

Author: avg
Date: Tue Aug  8 11:13:27 2017
New Revision: 322236
URL: https://svnweb.freebsd.org/changeset/base/322236

Log:
  8126 ztest assertion failed in dbuf_dirty due to dn_nlevels changing
  
  illumos/illumos-gate at dcb6872c565819ac88acbc2ece999ef241c8b982
  https://github.com/illumos/illumos-gate/commit/dcb6872c565819ac88acbc2ece999ef241c8b982
  
  https://www.illumos.org/issues/8126
    The sync thread is concurrently modifying dn_phys->dn_nlevels
    while dbuf_dirty() is trying to assert something about it, without
    holding the necessary lock. We need to move this assertion further down
    in the function, after we have acquired the dn_struct_rwlock.
  
  Reviewed by: Pavel Zakharov <pavel.zakharov at delphix.com>
  Reviewed by: Serapheim Dimitropoulos <serapheim at delphix.com>
  Approved by: Robert Mustacchi <rm at joyent.com>
  Author: Matthew Ahrens <mahrens at delphix.com>

Modified:
  vendor-sys/illumos/dist/uts/common/fs/zfs/dbuf.c

Modified: vendor-sys/illumos/dist/uts/common/fs/zfs/dbuf.c
==============================================================================
--- vendor-sys/illumos/dist/uts/common/fs/zfs/dbuf.c	Tue Aug  8 11:10:37 2017	(r322235)
+++ vendor-sys/illumos/dist/uts/common/fs/zfs/dbuf.c	Tue Aug  8 11:13:27 2017	(r322236)
@@ -1542,11 +1542,6 @@ dbuf_dirty(dmu_buf_impl_t *db, dmu_tx_t *tx)
 	    (dmu_tx_is_syncing(tx) ? DN_DIRTY_SYNC : DN_DIRTY_OPEN));
 
 	ASSERT3U(dn->dn_nlevels, >, db->db_level);
-	ASSERT((dn->dn_phys->dn_nlevels == 0 && db->db_level == 0) ||
-	    dn->dn_phys->dn_nlevels > db->db_level ||
-	    dn->dn_next_nlevels[txgoff] > db->db_level ||
-	    dn->dn_next_nlevels[(tx->tx_txg-1) & TXG_MASK] > db->db_level ||
-	    dn->dn_next_nlevels[(tx->tx_txg-2) & TXG_MASK] > db->db_level);
 
 	/*
 	 * We should only be dirtying in syncing context if it's the
@@ -1662,6 +1657,16 @@ dbuf_dirty(dmu_buf_impl_t *db, dmu_tx_t *tx)
 		rw_enter(&dn->dn_struct_rwlock, RW_READER);
 		drop_struct_lock = TRUE;
 	}
+
+	/*
+	 * We need to hold the dn_struct_rwlock to make this assertion,
+	 * because it protects dn_phys / dn_next_nlevels from changing.
+	 */
+	ASSERT((dn->dn_phys->dn_nlevels == 0 && db->db_level == 0) ||
+	    dn->dn_phys->dn_nlevels > db->db_level ||
+	    dn->dn_next_nlevels[txgoff] > db->db_level ||
+	    dn->dn_next_nlevels[(tx->tx_txg-1) & TXG_MASK] > db->db_level ||
+	    dn->dn_next_nlevels[(tx->tx_txg-2) & TXG_MASK] > db->db_level);
 
 	/*
 	 * If we are overwriting a dedup BP, then unless it is snapshotted,