Non-l2arc pool reads shouldn't be l2arc misses

The current l2_misses accounting behavior treats all reads to pools 
without a configured l2arc as an l2arc miss, IFF there is at least 
one other pool on the system which does have an l2arc configured.

This makes it extremely hard to tune for an improved l2arc hit/miss 
ratio because this ratio will be modulated by reads from pools which 
do not (and should not) have l2arc devices; its upper limit will 
depend on the ratio of reads from l2arc'd pools and non-l2arc'd pools.

This PR prevents ARC reads affecting l2arc stats (n.b. l2_misses is 
the only relevant one) where the target spa doesn't have an l2arc.

Includes new test - l2arc_l2miss_pos.ksh

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: George Amanakis <gamanakis@gmail.com>
Signed-off-by: Adam Moss <c@yotes.com>
Closes #10921
This commit is contained in:
Adam D. Moss 2020-10-20 11:39:52 -07:00 committed by GitHub
parent 241c62bdd7
commit 666aa69f32
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 119 additions and 9 deletions

View File

@ -6305,7 +6305,11 @@ top:
metadata, misses); metadata, misses);
} }
if (vd != NULL && l2arc_ndev != 0 && !(l2arc_norw && devw)) { /* Check if the spa even has l2 configured */
const boolean_t spa_has_l2 = l2arc_ndev != 0 &&
spa->spa_l2cache.sav_count > 0;
if (vd != NULL && spa_has_l2 && !(l2arc_norw && devw)) {
/* /*
* Read from the L2ARC if the following are true: * Read from the L2ARC if the following are true:
* 1. The L2ARC vdev was previously cached. * 1. The L2ARC vdev was previously cached.
@ -6406,15 +6410,24 @@ top:
} else { } else {
if (vd != NULL) if (vd != NULL)
spa_config_exit(spa, SCL_L2ARC, vd); spa_config_exit(spa, SCL_L2ARC, vd);
/* /*
* Skip ARC stat bump for block pointers with * Only a spa with l2 should contribute to l2
* embedded data. The data are read from the blkptr * miss stats. (Including the case of having a
* itself via decode_embedded_bp_compressed(). * faulted cache device - that's also a miss.)
*/ */
if (l2arc_ndev != 0 && !embedded_bp) { if (spa_has_l2) {
DTRACE_PROBE1(l2arc__miss, /*
arc_buf_hdr_t *, hdr); * Skip ARC stat bump for block pointers with
ARCSTAT_BUMP(arcstat_l2_misses); * embedded data. The data are read from the
* blkptr itself via
* decode_embedded_bp_compressed().
*/
if (!embedded_bp) {
DTRACE_PROBE1(l2arc__miss,
arc_buf_hdr_t *, hdr);
ARCSTAT_BUMP(arcstat_l2_misses);
}
} }
} }

View File

@ -899,7 +899,7 @@ post =
tags = ['functional', 'log_spacemap'] tags = ['functional', 'log_spacemap']
[tests/functional/l2arc] [tests/functional/l2arc]
tests = ['l2arc_arcstats_pos', 'l2arc_mfuonly_pos', tests = ['l2arc_arcstats_pos', 'l2arc_mfuonly_pos', 'l2arc_l2miss_pos',
'persist_l2arc_001_pos', 'persist_l2arc_002_pos', 'persist_l2arc_001_pos', 'persist_l2arc_002_pos',
'persist_l2arc_003_neg', 'persist_l2arc_004_pos', 'persist_l2arc_005_pos', 'persist_l2arc_003_neg', 'persist_l2arc_004_pos', 'persist_l2arc_005_pos',
'persist_l2arc_006_pos', 'persist_l2arc_007_pos', 'persist_l2arc_008_pos'] 'persist_l2arc_006_pos', 'persist_l2arc_007_pos', 'persist_l2arc_008_pos']

View File

@ -3,6 +3,7 @@ dist_pkgdata_SCRIPTS = \
cleanup.ksh \ cleanup.ksh \
setup.ksh \ setup.ksh \
l2arc_arcstats_pos.ksh \ l2arc_arcstats_pos.ksh \
l2arc_l2miss_pos.ksh \
l2arc_mfuonly_pos.ksh \ l2arc_mfuonly_pos.ksh \
persist_l2arc_001_pos.ksh \ persist_l2arc_001_pos.ksh \
persist_l2arc_002_pos.ksh \ persist_l2arc_002_pos.ksh \

View File

@ -24,6 +24,7 @@ export SIZE=1G
export VDIR=$TESTDIR/disk.l2arc export VDIR=$TESTDIR/disk.l2arc
export VDEV="$VDIR/a" export VDEV="$VDIR/a"
export VDEV_CACHE="$VDIR/b" export VDEV_CACHE="$VDIR/b"
export VDEV1="$VDIR/c"
# fio options # fio options
export DIRECTORY=/$TESTPOOL export DIRECTORY=/$TESTPOOL

View File

@ -0,0 +1,94 @@
#!/bin/ksh -p
#
# CDDL HEADER START
#
# This file and its contents are supplied under the terms of the
# Common Development and Distribution License ("CDDL"), version 1.0.
# You may only use this file in accordance with the terms of version
# 1.0 of the CDDL.
#
# A full copy of the text of the CDDL should have accompanied this
# source. A copy of the CDDL is also available via the Internet at
# http://www.illumos.org/license/CDDL.
#
# CDDL HEADER END
#
#
# Copyright (c) 2020, Adam Moss. All rights reserved.
#
. $STF_SUITE/include/libtest.shlib
. $STF_SUITE/tests/functional/l2arc/l2arc.cfg
#
# DESCRIPTION:
# l2arc_misses does not increment upon reads from a pool without l2arc
#
# STRATEGY:
# 1. Create pool with a cache device.
# 2. Create pool without a cache device.
# 3. Create a random file in the no-cache-device pool,
# and random read for 10 sec.
# 4. Check that l2arc_misses hasn't risen
# 5. Create a random file in the pool with the cache device,
# and random read for 10 sec.
# 6. Check that l2arc_misses has risen
#
verify_runnable "global"
log_assert "l2arc_misses does not increment upon reads from a pool without l2arc."
function cleanup
{
if poolexists $TESTPOOL ; then
destroy_pool $TESTPOOL
fi
if poolexists $TESTPOOL1 ; then
destroy_pool $TESTPOOL1
fi
}
log_onexit cleanup
typeset fill_mb=800
typeset cache_sz=$(( 1.4 * $fill_mb ))
export FILE_SIZE=$(( floor($fill_mb / $NUMJOBS) ))M
log_must truncate -s ${cache_sz}M $VDEV_CACHE
log_must zpool create -O compression=off -f $TESTPOOL $VDEV cache $VDEV_CACHE
log_must zpool create -O compression=off -f $TESTPOOL1 $VDEV1
# I/O to pool without l2arc - expect that l2_misses stays constant
export DIRECTORY=/$TESTPOOL1
log_must fio $FIO_SCRIPTS/mkfiles.fio
log_must fio $FIO_SCRIPTS/random_reads.fio
# attempt to remove entries for pool from ARC so we would try
# to hit the nonexistent L2ARC for subsequent reads
log_must zpool export $TESTPOOL1
log_must zpool import $TESTPOOL1 -d $VDEV1
typeset starting_miss_count=$(get_arcstat l2_misses)
log_must fio $FIO_SCRIPTS/random_reads.fio
log_must test $(get_arcstat l2_misses) -eq $starting_miss_count
# I/O to pool with l2arc - expect that l2_misses rises
export DIRECTORY=/$TESTPOOL
log_must fio $FIO_SCRIPTS/mkfiles.fio
log_must fio $FIO_SCRIPTS/random_reads.fio
# wait for L2ARC writes to actually happen
arcstat_quiescence_noecho l2_size
# attempt to remove entries for pool from ARC so we would try
# to hit L2ARC for subsequent reads
log_must zpool export $TESTPOOL
log_must zpool import $TESTPOOL -d $VDEV
log_must fio $FIO_SCRIPTS/random_reads.fio
log_must test $(get_arcstat l2_misses) -gt $starting_miss_count
log_must zpool destroy -f $TESTPOOL
log_must zpool destroy -f $TESTPOOL1
log_pass "l2arc_misses does not increment upon reads from a pool without l2arc."

View File

@ -25,5 +25,6 @@ verify_runnable "global"
log_must rm -rf $VDIR log_must rm -rf $VDIR
log_must mkdir -p $VDIR log_must mkdir -p $VDIR
log_must mkfile $SIZE $VDEV log_must mkfile $SIZE $VDEV
log_must mkfile $SIZE $VDEV1
log_pass log_pass