zap: add zap_cursor_init_by_dnode; cursor unit tests; mock dnode refcounts

include/sys/zap.h

@@ -443,31 +443,36 @@ void zap_attribute_free(zap_attribute_t *attrp);
 
 struct zap;
 struct zap_leaf;
+
 typedef struct zap_cursor {
 	/* This structure is opaque! */
-	objset_t *zc_objset;
 	struct zap *zc_zap;
 	struct zap_leaf *zc_leaf;
-	uint64_t zc_zapobj;
-	uint64_t zc_serialized;
 	uint64_t zc_hash;
 	uint32_t zc_cd;
 	boolean_t zc_prefetch;
+	/*
+	 * Legacy fields to main source compat with Lustre, which accesses
+	 * them directly. Not to be used in new code!
+	 */
+	objset_t *zc_objset;
+	uint64_t zc_zapobj;
 } zap_cursor_t;
 
 /*
  * Initialize a zap cursor, pointing to the "first" attribute of the zapobj.
  * The entire zapobj will be prefetched. You must call zap_cursor_fini the
  * cursor when you are done with it.
  */
-void zap_cursor_init(zap_cursor_t *zc, objset_t *os, uint64_t zapobj);
+int zap_cursor_init(zap_cursor_t *zc, objset_t *os, uint64_t zapobj);
+int zap_cursor_init_by_dnode(zap_cursor_t *zc, dnode_t *dn);
 void zap_cursor_fini(zap_cursor_t *zc);
 
 /*
  * Initialize a cursor at the beginning, but request that we not prefetch
  * the entire ZAP object.
  */
-void zap_cursor_init_noprefetch(zap_cursor_t *zc, objset_t *os,
+int zap_cursor_init_noprefetch(zap_cursor_t *zc, objset_t *os,
     uint64_t zapobj);
 
 /*
@@ -477,8 +482,10 @@ void zap_cursor_init_noprefetch(zap_cursor_t *zc, objset_t *os,
  * zapobj (ie.  zap_cursor_init_serialized(..., 0) is equivalent to
  * zap_cursor_init(...).)
  */
-void zap_cursor_init_serialized(zap_cursor_t *zc, objset_t *os,
+int zap_cursor_init_serialized(zap_cursor_t *zc, objset_t *os,
     uint64_t zapobj, uint64_t serialized);
+int zap_cursor_init_serialized_by_dnode(zap_cursor_t *zc, dnode_t *dn,
+    uint64_t serialized);
 
 /*
  * Get the attribute currently pointed to by the cursor.  Returns

module/zfs/dsl_scan.c

@@ -1280,6 +1280,7 @@ dsl_errorscrub_pause_resume_sync(void *arg, dmu_tx_t *tx)
 		spa->spa_scan_pass_errorscrub_pause = gethrestime_sec();
 		scn->errorscrub_phys.dep_paused_flags = B_TRUE;
 		dsl_errorscrub_sync_state(scn, tx);
+		zap_cursor_fini(&scn->errorscrub_cursor);
 		spa_event_notify(spa, NULL, NULL, ESC_ZFS_ERRORSCRUB_PAUSED);
 	} else {
 		ASSERT3U(*cmd, ==, POOL_SCRUB_NORMAL);

module/zfs/zap.c

@@ -1072,84 +1072,116 @@ zap_lookup_int_key(objset_t *os, uint64_t obj, uint64_t key, uint64_t *valuep)
 
 /* zap_cursor */
 
-static void
-zap_cursor_init_impl(zap_cursor_t *zc, objset_t *os, uint64_t zapobj,
+static int
+zap_cursor_init_by_dnode_impl(zap_cursor_t *zc, dnode_t *dn,
     uint64_t serialized, boolean_t prefetch)
 {
-	zc->zc_objset = os;
 	zc->zc_zap = NULL;
 	zc->zc_leaf = NULL;
-	zc->zc_zapobj = zapobj;
-	zc->zc_serialized = serialized;
-	zc->zc_hash = 0;
-	zc->zc_cd = 0;
+
+	int err = zap_lock_by_dnode(dn, NULL, RW_READER, TRUE, FALSE,
+	    zc, &zc->zc_zap);
+	if (err != 0)
+		return (err);
+
 	zc->zc_prefetch = prefetch;
+	zc->zc_objset = dn->dn_objset;
+	zc->zc_zapobj = dn->dn_object;
+
+	int hb = zap_hashbits(zc->zc_zap);
+	zc->zc_hash = serialized << (64 - hb);
+	zc->zc_cd = serialized >> hb;
+	if (zc->zc_cd >= zap_maxcd(zc->zc_zap)) /* corrupt serialized */
+		zc->zc_cd = 0;
+
+	/*
+	 * Drop ZAP read lock, but keep the hold, so the holds on the
+	 * underlying dnode and header dbuf are maintained.
+	 */
+	rw_exit(&zc->zc_zap->zap_rwlock);
+
+	return (0);
 }
 
-void
+static int
+zap_cursor_init_impl(zap_cursor_t *zc, objset_t *os, uint64_t zapobj,
+    uint64_t serialized, uint32_t prefetch)
+{
+	dnode_t *dn = NULL;
+	int err = dnode_hold(os, zapobj, FTAG, &dn);
+	if (err != 0) {
+		zc->zc_zap = NULL;
+		zc->zc_leaf = NULL;
+		return (err);
+	}
+
+	err = zap_cursor_init_by_dnode_impl(zc, dn, serialized, prefetch);
+
+	dnode_rele(dn, FTAG);
+
+	return (err);
+}
+
+int
 zap_cursor_init(zap_cursor_t *zc, objset_t *os, uint64_t zapobj)
 {
-	zap_cursor_init_impl(zc, os, zapobj, 0, B_TRUE);
+	return (zap_cursor_init_impl(zc, os, zapobj, 0, B_TRUE));
 }
 
-void
+int
+zap_cursor_init_by_dnode(zap_cursor_t *zc, dnode_t *dn)
+{
+	return (zap_cursor_init_by_dnode_impl(zc, dn, 0, B_TRUE));
+}
+
+int
 zap_cursor_init_noprefetch(zap_cursor_t *zc, objset_t *os, uint64_t zapobj)
 {
-	zap_cursor_init_impl(zc, os, zapobj, 0, B_FALSE);
+	return (zap_cursor_init_impl(zc, os, zapobj, 0, B_FALSE));
 }
 
-void
+int
 zap_cursor_init_serialized(zap_cursor_t *zc, objset_t *os, uint64_t zapobj,
     uint64_t serialized)
 {
-	zap_cursor_init_impl(zc, os, zapobj, serialized, B_TRUE);
+	return (zap_cursor_init_impl(zc, os, zapobj, serialized, B_TRUE));
+}
+
+int
+zap_cursor_init_serialized_by_dnode(zap_cursor_t *zc, dnode_t *dn,
+    uint64_t serialized)
+{
+	return (zap_cursor_init_by_dnode_impl(zc, dn, serialized, B_TRUE));
 }
 
 void
 zap_cursor_fini(zap_cursor_t *zc)
 {
-	if (zc->zc_zap) {
-		rw_enter(&zc->zc_zap->zap_rwlock, RW_READER);
-		zap_unlock(zc->zc_zap, NULL);
-		zc->zc_zap = NULL;
-	}
 	if (zc->zc_leaf) {
 		rw_enter(&zc->zc_leaf->l_rwlock, RW_READER);
 		zap_put_leaf(zc->zc_leaf);
-		zc->zc_leaf = NULL;
 	}
-	zc->zc_objset = NULL;
+	if (zc->zc_zap) {
+		rw_enter(&zc->zc_zap->zap_rwlock, RW_READER);
+		zap_unlock(zc->zc_zap, zc);
+	}
+	memset(zc, 0, sizeof (zap_cursor_t));
 }
 
 int
 zap_cursor_retrieve(zap_cursor_t *zc, zap_attribute_t *za)
 {
 	int err;
 
+	if (zc->zc_zap == NULL)
+		/* zap_cursor_init failed, cursor is invalid */
+		return (SET_ERROR(EIO));
+
 	if (zc->zc_hash == -1ULL)
 		return (SET_ERROR(ENOENT));
 
-	if (zc->zc_zap == NULL) {
-		int hb;
-		err = zap_lock(zc->zc_objset, zc->zc_zapobj, NULL,
-		    RW_READER, TRUE, FALSE, NULL, &zc->zc_zap);
-		if (err != 0)
-			return (err);
-
-		/*
-		 * To support zap_cursor_init_serialized, advance, retrieve,
-		 * we must add to the existing zc_cd, which may already
-		 * be 1 due to the zap_cursor_advance.
-		 */
-		ASSERT0(zc->zc_hash);
-		hb = zap_hashbits(zc->zc_zap);
-		zc->zc_hash = zc->zc_serialized << (64 - hb);
-		zc->zc_cd += zc->zc_serialized >> hb;
-		if (zc->zc_cd >= zap_maxcd(zc->zc_zap)) /* corrupt serialized */
-			zc->zc_cd = 0;
-	} else {
-		rw_enter(&zc->zc_zap->zap_rwlock, RW_READER);
-	}
+	rw_enter(&zc->zc_zap->zap_rwlock, RW_READER);
+
 	if (!zc->zc_zap->zap_ismicro) {
 		err = fzap_cursor_retrieve(zc->zc_zap, zc, za);
 	} else {
@@ -1184,6 +1216,7 @@ zap_cursor_retrieve(zap_cursor_t *zc, zap_attribute_t *za)
 			err = SET_ERROR(ENOENT);
 		}
 	}
+
 	rw_exit(&zc->zc_zap->zap_rwlock);
 	return (err);
 }
@@ -1199,10 +1232,9 @@ zap_cursor_advance(zap_cursor_t *zc)
 uint64_t
 zap_cursor_serialize(zap_cursor_t *zc)
 {
-	if (zc->zc_hash == -1ULL)
+	if (zc->zc_zap == NULL || zc->zc_hash == -1ULL)
 		return (-1ULL);
-	if (zc->zc_zap == NULL)
-		return (zc->zc_serialized);
+
 	ASSERT0((zc->zc_hash & zap_maxcd(zc->zc_zap)));
 	ASSERT(zc->zc_cd < zap_maxcd(zc->zc_zap));
 

tests/unit/mock_dmu.c

@@ -28,6 +28,7 @@
 #include <sys/zfeature.h>
 
 #include "mock_dmu.h"
+#include "unit.h"
 
 /*
  * A mock dbuf. A real dmu_buf_t (first for casting) plus the attached user
@@ -48,6 +49,7 @@ typedef struct mock_dbuf mock_dbuf_t;
  */
 struct mock_dnode {
 	dnode_t			mdn_dn;
+	uint64_t		mdn_refcount;
 	size_t			mdn_blksize;
 	size_t			mdn_nblocks;
 	mock_dbuf_t		**mdn_blocks;
@@ -110,6 +112,7 @@ mock_dnode_create(size_t blksize, dmu_object_type_t type)
 	ASSERT(IS_P2ALIGNED(blksize, 512));
 
 	mock_dnode_t *mdn = kmem_zalloc(sizeof (mock_dnode_t), KM_SLEEP);
+	mdn->mdn_refcount = 1;
 	mdn->mdn_dn.dn_type = type;
 	mdn->mdn_dn.dn_object = 1;	/* arbitrary non-zero object number */
 	mdn->mdn_blksize = blksize;
@@ -156,6 +159,12 @@ mock_dnode_block_data(mock_dnode_t *mdn, uint64_t blkid)
 	return (mdn->mdn_blocks[blkid]->mdb_db.db_data);
 }
 
+uint64_t
+mock_dnode_refcount(mock_dnode_t *mdn)
+{
+	return (mdn->mdn_refcount);
+}
+
 /* Mock transaction */
 
 mock_dmu_tx_t *
@@ -258,14 +267,21 @@ dmu_object_set_blocksize(objset_t *os, uint64_t object, uint64_t size,
 boolean_t
 dnode_add_ref(dnode_t *dn, const void *tag)
 {
-	(void) dn; (void) tag;
+	(void) tag;
+	mock_dnode_t *mdn = (mock_dnode_t *)dn;
+	if (mdn->mdn_refcount == 0)
+		return (B_FALSE);
+	mdn->mdn_refcount++;
 	return (B_TRUE);
 }
 
 void
 dnode_rele(dnode_t *dn, const void *tag)
 {
-	(void) dn; (void) tag;
+	(void) tag;
+	mock_dnode_t *mdn = (mock_dnode_t *)dn;
+	unit_gt(mdn->mdn_refcount, 0);
+	mdn->mdn_refcount--;
 }
 
 /*

tests/unit/mock_dmu.h

@@ -40,6 +40,9 @@ size_t mock_dnode_block_count(mock_dnode_t *mdn);
 /* Returns a pointer to the data under the given block id. */
 const void *mock_dnode_block_data(mock_dnode_t *mdn, uint64_t blkid);
 
+/* Returns the current dnode ref (hold) count. */
+uint64_t mock_dnode_refcount(mock_dnode_t *mdn);
+
 /* Create/destroy a mock transaction handle. */
 mock_dmu_tx_t *mock_tx_create(void);
 void mock_tx_destroy(mock_dmu_tx_t *tx);