Some cosmetic prefetcher changes requested by mahrens.

Signed-off-by: Alexander Motin <[email protected]>

Author: amotin

include/sys/dmu_zfetch.h

@@ -65,7 +65,11 @@ typedef struct zstream {
 	zfetch_t	*zs_fetch;	/* parent fetch */
 	boolean_t	zs_missed;	/* stream saw cache misses */
 	zfs_refcount_t	zs_callers;	/* number of pending callers */
-	zfs_refcount_t	zs_blocks; /* number of pending blocks in the stream */
+	/*
+	 * Number of stream references: dnode, callers and pending blocks.
+	 * The stream memory is freed when the number returns to zero.
+	 */
+	zfs_refcount_t	zs_refs;
 } zstream_t;
 
 void		zfetch_init(void);

module/zfs/dbuf.c

@@ -1662,7 +1662,8 @@ dbuf_read(dmu_buf_impl_t *db, zio_t *zio, uint32_t flags)
 		 */
 		if (!err && prefetch) {
 			dmu_zfetch(&dn->dn_zfetch, db->db_blkid, 1, B_TRUE,
-			    B_TRUE, flags & DB_RF_HAVESTRUCT);
+			    db->db_state != DB_CACHED,
+			    flags & DB_RF_HAVESTRUCT);
 		}
 
 		DB_DNODE_EXIT(db);

module/zfs/dmu.c

@@ -540,6 +540,11 @@ dmu_buf_hold_array_by_dnode(dnode_t *dn, uint64_t offset, uint64_t length,
 	blkid = dbuf_whichblock(dn, 0, offset);
 	if ((flags & DMU_READ_NO_PREFETCH) == 0 &&
 	    DNODE_META_IS_CACHEABLE(dn) && length <= zfetch_array_rd_sz) {
+		/*
+		 * Prepare the zfetch before initiating the demand reads, so
+		 * that if multiple threads block on same indirect block, we
+		 * base predictions on the original less racy request order.
+		 */
 		zs = dmu_zfetch_prepare(&dn->dn_zfetch, blkid, nblks,
 		    read && DNODE_IS_CACHEABLE(dn), B_TRUE);
 	}
@@ -555,7 +560,14 @@ dmu_buf_hold_array_by_dnode(dnode_t *dn, uint64_t offset, uint64_t length,
 			return (SET_ERROR(EIO));
 		}
 
-		/* initiate async i/o */
+		/*
+		 * Initiate async demand data read.
+		 * We check the db_state after calling dbuf_read() because
+		 * (1) dbuf_read() may change the state to CACHED due to a
+		 * hit in the ARC, and (2) on a cache miss, a child will
+		 * have been added to "zio" but not yet completed, so the
+		 * state will not yet be CACHED.
+		 */
 		if (read) {
 			(void) dbuf_read(db, zio, dbuf_flags);
 			if (db->db_state != DB_CACHED)

module/zfs/dmu_zfetch.c

@@ -138,7 +138,7 @@ dmu_zfetch_stream_remove(zfetch_t *zf, zstream_t *zs)
 	ASSERT(MUTEX_HELD(&zf->zf_lock));
 	list_remove(&zf->zf_stream, zs);
 	zf->zf_numstreams--;
-	if (zfs_refcount_remove(&zs->zs_blocks, NULL) == 0)
+	if (zfs_refcount_remove(&zs->zs_refs, NULL) == 0)
 		dmu_zfetch_stream_fini(zs);
 }
 
@@ -184,7 +184,7 @@ dmu_zfetch_stream_create(zfetch_t *zf, uint64_t blkid)
 		/*
 		 * Skip if still active.  1 -- zf_stream reference.
 		 */
-		if (zfs_refcount_count(&zs->zs_blocks) != 1)
+		if (zfs_refcount_count(&zs->zs_refs) != 1)
 			continue;
 		if (((now - zs->zs_atime) / NANOSEC) >
 		    zfetch_min_sec_reap)
@@ -217,9 +217,9 @@ dmu_zfetch_stream_create(zfetch_t *zf, uint64_t blkid)
 	zs->zs_fetch = zf;
 	zs->zs_missed = B_FALSE;
 	zfs_refcount_create(&zs->zs_callers);
-	zfs_refcount_create(&zs->zs_blocks);
+	zfs_refcount_create(&zs->zs_refs);
 	/* One reference for zf_stream. */
-	zfs_refcount_add(&zs->zs_blocks, NULL);
+	zfs_refcount_add(&zs->zs_refs, NULL);
 	zf->zf_numstreams++;
 	list_insert_head(&zf->zf_stream, zs);
 }
@@ -239,14 +239,18 @@ dmu_zfetch_stream_done(void *arg, boolean_t io_issued)
 			ZFETCHSTAT_SET(zfetchstat_max_completion_us, delta);
 	}
 
-	if (zfs_refcount_remove(&zs->zs_blocks, NULL) == 0)
+	if (zfs_refcount_remove(&zs->zs_refs, NULL) == 0)
 		dmu_zfetch_stream_fini(zs);
 }
 
 /*
- * This is the predictive prefetch entry point.  It associates dnode access
- * specified with blkid and nblks arguments with prefetch stream, predicts
- * further accesses based on that stats and initiates speculative prefetch.
+ * This is the predictive prefetch entry point.  dmu_zfetch_prepare()
+ * associates dnode access specified with blkid and nblks arguments with
+ * prefetch stream, predicts further accesses based on that stats and returns
+ * the stream pointer on success.  That pointer must later be passed to
+ * dmu_zfetch_run() to initiate the speculative prefetch for the stream and
+ * release it.  dmu_zfetch() is a wrapper for simple cases when window between
+ * prediction and prefetch initiation is not needed.
  * fetch_data argument specifies whether actual data blocks should be fetched:
  *   FALSE -- prefetch only indirect blocks for predicted data blocks;
  *   TRUE -- prefetch predicted data blocks plus following indirect blocks.
@@ -257,9 +261,9 @@ dmu_zfetch_prepare(zfetch_t *zf, uint64_t blkid, uint64_t nblks,
 {
 	zstream_t *zs;
 	int64_t pf_start, ipf_start;
-	int64_t pf_ahead_blks, max_blks, maxblkid;
+	int64_t pf_ahead_blks, max_blks;
 	int max_dist_blks, pf_nblks, ipf_nblks;
-	uint64_t end_of_access_blkid;
+	uint64_t end_of_access_blkid, maxblkid;
 	end_of_access_blkid = blkid + nblks;
 	spa_t *spa = zf->zf_dnode->dn_objset->os_spa;
 
@@ -289,7 +293,8 @@ dmu_zfetch_prepare(zfetch_t *zf, uint64_t blkid, uint64_t nblks,
 	 * A fast path for small files for which no prefetch will
 	 * happen.
 	 */
-	if ((maxblkid = zf->zf_dnode->dn_maxblkid) < 2) {
+	maxblkid = zf->zf_dnode->dn_maxblkid;
+	if (maxblkid < 2) {
 		if (!have_lock)
 			rw_exit(&zf->zf_dnode->dn_struct_rwlock);
 		return (NULL);
@@ -319,15 +324,19 @@ dmu_zfetch_prepare(zfetch_t *zf, uint64_t blkid, uint64_t nblks,
 	if (end_of_access_blkid >= maxblkid) {
 		if (zs != NULL)
 			dmu_zfetch_stream_remove(zf, zs);
-done:
 		mutex_exit(&zf->zf_lock);
 		if (!have_lock)
 			rw_exit(&zf->zf_dnode->dn_struct_rwlock);
 		return (NULL);
 	}
 
-	if (nblks == 0)
-		goto done;	/* Already prefetched this before. */
+	/* Exit if we already prefetched this block before. */
+	if (nblks == 0) {
+		mutex_exit(&zf->zf_lock);
+		if (!have_lock)
+			rw_exit(&zf->zf_dnode->dn_struct_rwlock);
+		return (NULL);
+	}
 
 	if (zs == NULL) {
 		/*
@@ -401,7 +410,7 @@ dmu_zfetch_prepare(zfetch_t *zf, uint64_t blkid, uint64_t nblks,
 
 	zs->zs_atime = gethrtime();
 	/* Protect the stream from reclamation.  2 -- zf_stream + us. */
-	if (zfs_refcount_add(&zs->zs_blocks, NULL) == 2)
+	if (zfs_refcount_add(&zs->zs_refs, NULL) == 2)
 		zs->zs_start_time = zs->zs_atime;
 	/* Count concurrent callers. */
 	zfs_refcount_add(&zs->zs_callers, NULL);
@@ -432,7 +441,7 @@ dmu_zfetch_run(zstream_t *zs, boolean_t missed, boolean_t have_lock)
 	 */
 	if (zfs_refcount_remove(&zs->zs_callers, NULL) != 0) {
 		/* Drop reference taken in dmu_zfetch_prepare(). */
-		if (zfs_refcount_remove(&zs->zs_blocks, NULL) == 0)
+		if (zfs_refcount_remove(&zs->zs_refs, NULL) == 0)
 			dmu_zfetch_stream_fini(zs);
 		return;
 	}
@@ -454,10 +463,10 @@ dmu_zfetch_run(zstream_t *zs, boolean_t missed, boolean_t have_lock)
 	issued = pf_end - pf_start + ipf_end - ipf_start;
 	if (issued > 1) {
 		/* More references on top of taken in dmu_zfetch_prepare(). */
-		zfs_refcount_add_many(&zs->zs_blocks, issued - 1, NULL);
+		zfs_refcount_add_many(&zs->zs_refs, issued - 1, NULL);
 	} else if (issued == 0) {
 		/* Some other thread has done our work, so drop the ref. */
-		if (zfs_refcount_remove(&zs->zs_blocks, NULL) == 0)
+		if (zfs_refcount_remove(&zs->zs_refs, NULL) == 0)
 			dmu_zfetch_stream_fini(zs);
 		return;
 	}