Make L2ARC tests more robust

Instead of relying on arbitrary timers after pool export/import or cache device off/online rely on arcstats. This makes the L2ARC tests more robust. Also cleanup some functions related to persistent L2ARC. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Adam Moss <c@yotes.com> Signed-off-by: George Amanakis <gamanakis@gmail.com> Closes openzfs#10983
PrivatePuffin · Oct 5, 2020 · a76e4e6 · a76e4e6
1 parent 4e84f67
commit a76e4e6
Show file tree

Hide file tree

Showing 13 changed files with 149 additions and 77 deletions.
diff --git a/module/zfs/arc.c b/module/zfs/arc.c
@@ -969,7 +969,7 @@ static void l2arc_log_blk_fetch_abort(zio_t *zio);
 
 /* L2ARC persistence block restoration routines. */
 static void l2arc_log_blk_restore(l2arc_dev_t *dev,
-    const l2arc_log_blk_phys_t *lb, uint64_t lb_asize, uint64_t lb_daddr);
+    const l2arc_log_blk_phys_t *lb, uint64_t lb_asize);
 static void l2arc_hdr_restore(const l2arc_log_ent_phys_t *le,
     l2arc_dev_t *dev);
 
@@ -9505,8 +9505,6 @@ l2arc_rebuild_vdev(vdev_t *vd, boolean_t reopen)
 	l2arc_dev_hdr_phys_t	*l2dhdr;
 	uint64_t		l2dhdr_asize;
 	spa_t			*spa;
-	int			err;
-	boolean_t		l2dhdr_valid = B_TRUE;
 
 	dev = l2arc_vdev_get(vd);
 	ASSERT3P(dev, !=, NULL);
@@ -9535,10 +9533,7 @@ l2arc_rebuild_vdev(vdev_t *vd, boolean_t reopen)
 	/*
 	 * Read the device header, if an error is returned do not rebuild L2ARC.
 	 */
-	if ((err = l2arc_dev_hdr_read(dev)) != 0)
-		l2dhdr_valid = B_FALSE;
-
-	if (l2dhdr_valid && dev->l2ad_log_entries > 0) {
+	if (l2arc_dev_hdr_read(dev) == 0 && dev->l2ad_log_entries > 0) {
 		/*
 		 * If we are onlining a cache device (vdev_reopen) that was
 		 * still present (l2arc_vdev_present()) and rebuild is enabled,
@@ -9838,7 +9833,7 @@ l2arc_rebuild(l2arc_dev_t *dev)
 		 * L2BLK_GET_PSIZE returns aligned size for log blocks.
 		 */
 		uint64_t asize = L2BLK_GET_PSIZE((&lbps[0])->lbp_prop);
-		l2arc_log_blk_restore(dev, this_lb, asize, lbps[0].lbp_daddr);
+		l2arc_log_blk_restore(dev, this_lb, asize);
 
 		/*
 		 * log block restored, include its pointer in the list of
@@ -9918,7 +9913,7 @@ l2arc_rebuild(l2arc_dev_t *dev)
 		PTR_SWAP(this_lb, next_lb);
 		this_io = next_io;
 		next_io = NULL;
-		}
+	}
 
 	if (this_io != NULL)
 		l2arc_log_blk_fetch_abort(this_io);
@@ -9985,7 +9980,7 @@ l2arc_dev_hdr_read(l2arc_dev_t *dev)
 
 	err = zio_wait(zio_read_phys(NULL, dev->l2ad_vdev,
 	    VDEV_LABEL_START_SIZE, l2dhdr_asize, abd,
-	    ZIO_CHECKSUM_LABEL, NULL, NULL, ZIO_PRIORITY_ASYNC_READ,
+	    ZIO_CHECKSUM_LABEL, NULL, NULL, ZIO_PRIORITY_SYNC_READ,
 	    ZIO_FLAG_DONT_CACHE | ZIO_FLAG_CANFAIL |
 	    ZIO_FLAG_DONT_PROPAGATE | ZIO_FLAG_DONT_RETRY |
 	    ZIO_FLAG_SPECULATIVE, B_FALSE));
@@ -10156,7 +10151,7 @@ l2arc_log_blk_read(l2arc_dev_t *dev,
  */
 static void
 l2arc_log_blk_restore(l2arc_dev_t *dev, const l2arc_log_blk_phys_t *lb,
-    uint64_t lb_asize, uint64_t lb_daddr)
+    uint64_t lb_asize)
 {
 	uint64_t	size = 0, asize = 0;
 	uint64_t	log_entries = dev->l2ad_log_entries;

diff --git a/tests/zfs-tests/include/libtest.shlib b/tests/zfs-tests/include/libtest.shlib
@@ -4173,6 +4173,45 @@ function get_arcstat # stat
 	esac
 }
 
+#
+# Wait for the specified arcstat to reach non-zero quiescence.
+# If echo is 1 echo the value after reaching quiescence, otherwise
+# if echo is 0 print the arcstat we are waiting on.
+#
+function arcstat_quiescence # stat echo
+{
+	typeset stat=$1
+	typeset echo=$2
+	typeset do_once=true
+
+	if [[ $echo -eq 0 ]]; then
+		echo "Waiting for arcstat $1 quiescence."
+	fi
+
+	while $do_once || [ $stat1 -ne $stat2 ] || [ $stat2 -eq 0 ]; do
+		typeset stat1=$(get_arcstat $stat)
+		sleep 2
+		typeset stat2=$(get_arcstat $stat)
+		do_once=false
+	done
+
+	if [[ $echo -eq 1 ]]; then
+		echo $stat2
+	fi
+}
+
+function arcstat_quiescence_noecho # stat
+{
+	typeset stat=$1
+	arcstat_quiescence $stat 0
+}
+
+function arcstat_quiescence_echo # stat
+{
+	typeset stat=$1
+	arcstat_quiescence $stat 1
+}
+
 #
 # Given an array of pids, wait until all processes
 # have completed and check their return status.

diff --git a/tests/zfs-tests/include/tunables.cfg b/tests/zfs-tests/include/tunables.cfg
@@ -59,6 +59,7 @@ MULTIHOST_HISTORY		multihost.history		zfs_multihost_history
 MULTIHOST_IMPORT_INTERVALS	multihost.import_intervals	zfs_multihost_import_intervals
 MULTIHOST_INTERVAL		multihost.interval		zfs_multihost_interval
 OVERRIDE_ESTIMATE_RECORDSIZE	send.override_estimate_recordsize	zfs_override_estimate_recordsize
+PREFETCH_DISABLE		prefetch.disable		zfs_prefetch_disable
 REMOVAL_SUSPEND_PROGRESS	removal_suspend_progress	zfs_removal_suspend_progress
 REMOVE_MAX_SEGMENT		remove_max_segment		zfs_remove_max_segment
 RESILVER_MIN_TIME_MS		resilver_min_time_ms		zfs_resilver_min_time_ms

diff --git a/tests/zfs-tests/tests/functional/l2arc/l2arc_arcstats_pos.ksh b/tests/zfs-tests/tests/functional/l2arc/l2arc_arcstats_pos.ksh
@@ -62,27 +62,35 @@ export FILE_SIZE=$(( floor($fill_mb / $NUMJOBS) ))M
 
 log_must truncate -s ${cache_sz}M $VDEV_CACHE
 
-typeset log_blk_start=$(get_arcstat l2_log_blk_writes)
-
 log_must zpool create -f $TESTPOOL $VDEV cache $VDEV_CACHE
 
 log_must fio $FIO_SCRIPTS/mkfiles.fio
 log_must fio $FIO_SCRIPTS/random_reads.fio
 
+arcstat_quiescence_noecho l2_size
 log_must zpool offline $TESTPOOL $VDEV_CACHE
+arcstat_quiescence_noecho l2_size
+
 typeset l2_mfu_init=$(get_arcstat l2_mfu_asize)
 typeset l2_mru_init=$(get_arcstat l2_mru_asize)
 typeset l2_prefetch_init=$(get_arcstat l2_prefetch_asize)
 typeset l2_asize_init=$(get_arcstat l2_asize)
-log_must zpool online $TESTPOOL $VDEV_CACHE
 
+log_must zpool online $TESTPOOL $VDEV_CACHE
+arcstat_quiescence_noecho l2_size
 log_must zpool export $TESTPOOL
+arcstat_quiescence_noecho l2_feeds
+
 log_must test $(get_arcstat l2_mfu_asize) -eq 0
 log_must test $(get_arcstat l2_mru_asize) -eq 0
 log_must zpool import -d $VDIR $TESTPOOL
+arcstat_quiescence_noecho l2_size
 
 log_must fio $FIO_SCRIPTS/random_reads.fio
+arcstat_quiescence_noecho l2_size
 log_must zpool offline $TESTPOOL $VDEV_CACHE
+arcstat_quiescence_noecho l2_size
+
 typeset l2_mfu_end=$(get_arcstat l2_mfu_asize)
 typeset l2_mru_end=$(get_arcstat l2_mru_asize)
 typeset l2_prefetch_end=$(get_arcstat l2_prefetch_asize)

diff --git a/tests/zfs-tests/tests/functional/l2arc/l2arc_mfuonly_pos.ksh b/tests/zfs-tests/tests/functional/l2arc/l2arc_mfuonly_pos.ksh
@@ -49,6 +49,7 @@ function cleanup
 
 	log_must set_tunable32 L2ARC_NOPREFETCH $noprefetch
 	log_must set_tunable32 L2ARC_MFUONLY $mfuonly
+	log_must set_tunable32 PREFETCH_DISABLE $zfsprefetch
 }
 log_onexit cleanup
 
@@ -60,6 +61,9 @@ log_must set_tunable32 L2ARC_NOPREFETCH 1
 typeset mfuonly=$(get_tunable L2ARC_MFUONLY)
 log_must set_tunable32 L2ARC_MFUONLY 1
 
+typeset zfsprefetch=$(get_tunable PREFETCH_DISABLE)
+log_must set_tunable32 PREFETCH_DISABLE 1
+
 typeset fill_mb=800
 typeset cache_sz=$(( 1.4 * $fill_mb ))
 export FILE_SIZE=$(( floor($fill_mb / $NUMJOBS) ))M
@@ -75,6 +79,14 @@ log_must fio $FIO_SCRIPTS/random_reads.fio
 
 log_must zpool export $TESTPOOL
 log_must zpool import -d $VDIR $TESTPOOL
+
+# Regardless of l2arc_noprefetch, some MFU buffers might be evicted
+# from ARC, accessed later on as prefetches and transition to MRU as
+# prefetches.
+# If accessed again they are counted as MRU and the l2arc_mru_asize arcstat
+# will not be 0 (mentioned also in zfs-module-parameters.5)
+# For the purposes of this test we mitigate this by disabling (predictive)
+# ZFS prefetches with zfs_prefetch_disable=1.
 log_must test $(get_arcstat l2_mru_asize) -eq 0
 
 log_must zpool destroy -f $TESTPOOL

diff --git a/tests/zfs-tests/tests/functional/l2arc/persist_l2arc_001_pos.ksh b/tests/zfs-tests/tests/functional/l2arc/persist_l2arc_001_pos.ksh
@@ -34,7 +34,7 @@
 #		L2ARC device.
 #	6. Import pool.
 #	7. Read the amount of log blocks rebuilt in arcstats and compare to
-#		(4).
+#		(5).
 #	8. Check if the labels of the L2ARC device are intact.
 #
 #	* We can predict the minimum bytes of L2ARC restored if we subtract
@@ -83,23 +83,28 @@ log_must zpool import -d $VDIR $TESTPOOL
 log_must fio $FIO_SCRIPTS/mkfiles.fio
 log_must fio $FIO_SCRIPTS/random_reads.fio
 
+arcstat_quiescence_noecho l2_size
 log_must zpool export $TESTPOOL
+arcstat_quiescence_noecho l2_feeds
 
 typeset l2_dh_log_blk=$(zdb -l $VDEV_CACHE | grep log_blk_count | \
 	awk '{print $2}')
 
 typeset l2_rebuild_log_blk_start=$(get_arcstat l2_rebuild_log_blks)
 
 log_must zpool import -d $VDIR $TESTPOOL
+arcstat_quiescence_noecho l2_size
 
-sleep 2
+typeset l2_rebuild_log_blk_end=$(arcstat_quiescence_echo l2_rebuild_log_blks)
 
-typeset l2_rebuild_log_blk_end=$(get_arcstat l2_rebuild_log_blks)
-
-log_must test $l2_dh_log_blk -eq $(( $l2_rebuild_log_blk_end - $l2_rebuild_log_blk_start ))
+log_must test $l2_dh_log_blk -eq $(( $l2_rebuild_log_blk_end -
+	$l2_rebuild_log_blk_start ))
 log_must test $l2_dh_log_blk -gt 0
 
-log_must zdb -lll $VDEV_CACHE
+log_must zpool offline $TESTPOOL $VDEV_CACHE
+arcstat_quiescence_noecho l2_size
+
+log_must zdb -lllq $VDEV_CACHE
 
 log_must zpool destroy -f $TESTPOOL
 

diff --git a/tests/zfs-tests/tests/functional/l2arc/persist_l2arc_002_pos.ksh b/tests/zfs-tests/tests/functional/l2arc/persist_l2arc_002_pos.ksh
@@ -86,9 +86,9 @@ log_must eval "echo $PASSPHRASE | zfs create -o encryption=on" \
 log_must fio $FIO_SCRIPTS/mkfiles.fio
 log_must fio $FIO_SCRIPTS/random_reads.fio
 
+arcstat_quiescence_noecho l2_size
 log_must zpool export $TESTPOOL
-
-sleep 2
+arcstat_quiescence_noecho l2_feeds
 
 typeset l2_dh_log_blk=$(zdb -l $VDEV_CACHE | grep log_blk_count | \
 	awk '{print $2}')
@@ -97,14 +97,17 @@ typeset l2_rebuild_log_blk_start=$(get_arcstat l2_rebuild_log_blks)
 
 log_must zpool import -d $VDIR $TESTPOOL
 log_must eval "echo $PASSPHRASE | zfs mount -l $TESTPOOL/$TESTFS1"
+arcstat_quiescence_noecho l2_size
 
-sleep 2
-
-typeset l2_rebuild_log_blk_end=$(get_arcstat l2_rebuild_log_blks)
+typeset l2_rebuild_log_blk_end=$(arcstat_quiescence_echo l2_rebuild_log_blks)
 
-log_must test $l2_dh_log_blk -eq $(( $l2_rebuild_log_blk_end - $l2_rebuild_log_blk_start ))
+log_must test $l2_dh_log_blk -eq $(( $l2_rebuild_log_blk_end - \
+	$l2_rebuild_log_blk_start ))
 log_must test $l2_dh_log_blk -gt 0
 
+log_must zpool offline $TESTPOOL $VDEV_CACHE
+arcstat_quiescence_noecho l2_size
+
 log_must zdb -lq $VDEV_CACHE
 
 log_must zpool destroy -f $TESTPOOL

diff --git a/tests/zfs-tests/tests/functional/l2arc/persist_l2arc_003_neg.ksh b/tests/zfs-tests/tests/functional/l2arc/persist_l2arc_003_neg.ksh
@@ -32,7 +32,7 @@
 #	4. Export pool.
 #	5. Import pool.
 #	6. Check in zpool iostat if the cache device has space allocated.
-#	7. Read the file written in (2) and check if l2_hits in
+#	7. Read the file written in (3) and check if l2_hits in
 #		/proc/spl/kstat/zfs/arcstats increased.
 #
 

diff --git a/tests/zfs-tests/tests/functional/l2arc/persist_l2arc_004_pos.ksh b/tests/zfs-tests/tests/functional/l2arc/persist_l2arc_004_pos.ksh
@@ -33,7 +33,7 @@
 #	4. Read amount of log blocks written.
 #	5. Import pool.
 #	6. Read amount of log blocks built.
-#	7. Compare the two amounts
+#	7. Compare the two amounts.
 #	8. Read the file written in (2) and check if l2_hits in
 #		/proc/spl/kstat/zfs/arcstats increased.
 #	9. Check if the labels of the L2ARC device are intact.
@@ -70,29 +70,31 @@ log_must zpool create -f $TESTPOOL $VDEV cache $VDEV_CACHE
 log_must fio $FIO_SCRIPTS/mkfiles.fio
 log_must fio $FIO_SCRIPTS/random_reads.fio
 
+arcstat_quiescence_noecho l2_size
 log_must zpool export $TESTPOOL
-
-sleep 2
+arcstat_quiescence_noecho l2_feeds
 
 typeset log_blk_end=$(get_arcstat l2_log_blk_writes)
-
 typeset log_blk_rebuild_start=$(get_arcstat l2_rebuild_log_blks)
 
 log_must zpool import -d $VDIR $TESTPOOL
 
 typeset l2_hits_start=$(get_arcstat l2_hits)
 
 log_must fio $FIO_SCRIPTS/random_reads.fio
+arcstat_quiescence_noecho l2_size
 
+typeset log_blk_rebuild_end=$(arcstat_quiescence_echo l2_rebuild_log_blks)
 typeset l2_hits_end=$(get_arcstat l2_hits)
 
-typeset log_blk_rebuild_end=$(get_arcstat l2_rebuild_log_blks)
-
 log_must test $(( $log_blk_rebuild_end - $log_blk_rebuild_start )) -eq \
 	$(( $log_blk_end - $log_blk_start ))
 
 log_must test $l2_hits_end -gt $l2_hits_start
 
+log_must zpool offline $TESTPOOL $VDEV_CACHE
+arcstat_quiescence_noecho l2_size
+
 log_must zdb -lq $VDEV_CACHE
 
 log_must zpool destroy -f $TESTPOOL

diff --git a/tests/zfs-tests/tests/functional/l2arc/persist_l2arc_005_pos.ksh b/tests/zfs-tests/tests/functional/l2arc/persist_l2arc_005_pos.ksh
@@ -36,7 +36,7 @@
 #	6. Import pool.
 #	7. Mount the encrypted ZFS file system.
 #	8. Read amount of log blocks built.
-#	9. Compare the two amounts
+#	9. Compare the two amounts.
 #	10. Read the file written in (3) and check if l2_hits in
 #		/proc/spl/kstat/zfs/arcstats increased.
 #	11. Check if the labels of the L2ARC device are intact.
@@ -76,12 +76,11 @@ log_must eval "echo $PASSPHRASE | zfs create -o encryption=on" \
 log_must fio $FIO_SCRIPTS/mkfiles.fio
 log_must fio $FIO_SCRIPTS/random_reads.fio
 
+arcstat_quiescence_noecho l2_size
 log_must zpool export $TESTPOOL
-
-sleep 2
+arcstat_quiescence_noecho l2_feeds
 
 typeset log_blk_end=$(get_arcstat l2_log_blk_writes)
-
 typeset log_blk_rebuild_start=$(get_arcstat l2_rebuild_log_blks)
 
 log_must zpool import -d $VDIR $TESTPOOL
@@ -90,16 +89,19 @@ log_must eval "echo $PASSPHRASE | zfs mount -l $TESTPOOL/$TESTFS1"
 typeset l2_hits_start=$(get_arcstat l2_hits)
 
 log_must fio $FIO_SCRIPTS/random_reads.fio
+arcstat_quiescence_noecho l2_size
 
+typeset log_blk_rebuild_end=$(arcstat_quiescence_echo l2_rebuild_log_blks)
 typeset l2_hits_end=$(get_arcstat l2_hits)
 
-typeset log_blk_rebuild_end=$(get_arcstat l2_rebuild_log_blks)
-
 log_must test $(( $log_blk_rebuild_end - $log_blk_rebuild_start )) -eq \
 	$(( $log_blk_end - $log_blk_start ))
 
 log_must test $l2_hits_end -gt $l2_hits_start
 
+log_must zpool offline $TESTPOOL $VDEV_CACHE
+arcstat_quiescence_noecho l2_size
+
 log_must zdb -lq $VDEV_CACHE
 
 log_must zpool destroy -f $TESTPOOL