Skip to content

Commit

Permalink
Fix L2ARC reads when compressed ARC disabled
Browse files Browse the repository at this point in the history
When reading compressed blocks from the L2ARC, with
compressed ARC disabled, arc_hdr_size() returns
LSIZE rather than PSIZE, but the actual read is PSIZE.
This causes l2arc_read_done() to compare the checksum
against the wrong size, resulting in checksum failure.

This manifests as an increase in the kstat l2_cksum_bad
and the read being retried from the main pool, making the
L2ARC ineffective.

Add new L2ARC tests with Compressed ARC enabled/disabled

Blocks are handled differently depending on the state of the
zfs_compressed_arc_enabled tunable.

If a block is compressed on-disk, and compressed_arc is enabled:
- the block is read from disk
- It is NOT decompressed
- It is added to the ARC in its compressed form
- l2arc_write_buffers() may write it to the L2ARC (as is)
- l2arc_read_done() compares the checksum to the BP (compressed)

However, if compressed_arc is disabled:
- the block is read from disk
- It is decompressed
- It is added to the ARC (uncompressed)
- l2arc_write_buffers() will use l2arc_apply_transforms() to
  recompress the block, before writing it to the L2ARC
- l2arc_read_done() compares the checksum to the BP (compressed)
- l2arc_read_done() will use l2arc_untransform() to uncompress it

This test writes out a test file to a pool consisting of one disk
and one cache device, then randomly reads from it. Since the arc_max
in the tests is low, this will feed the L2ARC, and result in reads
from the L2ARC.

We compare the value of the kstat l2_cksum_bad before and after
to determine if any blocks failed to survive the trip through the
L2ARC.

Sponsored-by: The FreeBSD Foundation
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Allan Jude <allanjude@freebsd.org>
Closes #10693
  • Loading branch information
allanjude authored Aug 14, 2020
1 parent faa296c commit fc34dfb
Show file tree
Hide file tree
Showing 5 changed files with 215 additions and 2 deletions.
15 changes: 15 additions & 0 deletions module/zfs/arc.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@
* Copyright (c) 2017, Nexenta Systems, Inc. All rights reserved.
* Copyright (c) 2019, loli10K <ezomori.nozomu@gmail.com>. All rights reserved.
* Copyright (c) 2020, George Amanakis. All rights reserved.
* Copyright (c) 2020, The FreeBSD Foundation [1]
*
* [1] Portions of this software were developed by Allan Jude
* under sponsorship from the FreeBSD Foundation.
*/

/*
Expand Down Expand Up @@ -6152,6 +6156,17 @@ arc_read(zio_t *pio, spa_t *spa, const blkptr_t *bp,
cb->l2rcb_zb = *zb;
cb->l2rcb_flags = zio_flags;

/*
* When Compressed ARC is disabled, but the
* L2ARC block is compressed, arc_hdr_size()
* will have returned LSIZE rather than PSIZE.
*/
if (HDR_GET_COMPRESS(hdr) != ZIO_COMPRESS_OFF &&
!HDR_COMPRESSION_ENABLED(hdr) &&
HDR_GET_PSIZE(hdr) != 0) {
size = HDR_GET_PSIZE(hdr);
}

asize = vdev_psize_to_asize(vd, size);
if (asize != size) {
abd = abd_alloc_for_io(asize,
Expand Down
3 changes: 2 additions & 1 deletion tests/runfiles/common.run
Original file line number Diff line number Diff line change
Expand Up @@ -536,7 +536,8 @@ user =
tags = ['functional', 'cli_user', 'zpool_status']

[tests/functional/compression]
tests = ['compress_001_pos', 'compress_002_pos', 'compress_003_pos']
tests = ['compress_001_pos', 'compress_002_pos', 'compress_003_pos',
'l2arc_compressed_arc', 'l2arc_compressed_arc_disabled']
tags = ['functional', 'compression']

[tests/functional/cp_files]
Expand Down
4 changes: 3 additions & 1 deletion tests/zfs-tests/tests/functional/compression/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ dist_pkgdata_SCRIPTS = \
compress_001_pos.ksh \
compress_002_pos.ksh \
compress_003_pos.ksh \
compress_004_pos.ksh
compress_004_pos.ksh \
l2arc_compressed_arc.ksh \
l2arc_compressed_arc_disabled.ksh

dist_pkgdata_DATA = \
compress.cfg
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
#!/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 The FreeBSD Foundation [1]
#
# [1] Portions of this software were developed by Allan Jude
# under sponsorship from the FreeBSD Foundation.

. $STF_SUITE/include/libtest.shlib

export SIZE=1G
export VDIR=$TESTDIR/disk.persist_l2arc
export VDEV="$VDIR/a"
export VDEV_CACHE="$VDIR/b"

# fio options
export DIRECTORY=/$TESTPOOL-l2arc
export NUMJOBS=4
export RUNTIME=30
export PERF_RANDSEED=1234
export PERF_COMPPERCENT=66
export PERF_COMPCHUNK=0
export BLOCKSIZE=128K
export SYNC_TYPE=0
export DIRECT=1

#
# DESCRIPTION:
# System with compressed_arc disabled succeeds at reading from L2ARC
#
# STRATEGY:
# 1. Enable compressed_arc.
# 2. Create pool with a cache device and compression enabled.
# 3. Read the number of L2ARC checksum failures.
# 4. Create a random file in that pool and random read for 30 sec.
# 5. Read the number of L2ARC checksum failures.
#

verify_runnable "global"

log_assert "L2ARC with compressed_arc enabled succeeds."

origin_carc_setting=$(get_tunable COMPRESSED_ARC_ENABLED)

function cleanup
{
if poolexists $TESTPOOL-l2arc ; then
destroy_pool $TESTPOOL-l2arc
fi

log_must set_tunable64 COMPRESSED_ARC_ENABLED $origin_carc_setting
}
log_onexit cleanup

# Enable Compressed ARC so that in-ARC and on-disk will match
log_must set_tunable64 COMPRESSED_ARC_ENABLED 1

log_must rm -rf $VDIR
log_must mkdir -p $VDIR
log_must mkfile $SIZE $VDEV

typeset fill_mb=800
typeset cache_sz=$(( floor($fill_mb / 2) ))
export FILE_SIZE=$(( floor($fill_mb / $NUMJOBS) ))M

log_must truncate -s ${cache_sz}M $VDEV_CACHE

log_must zpool create -O compression=lz4 -f $TESTPOOL-l2arc $VDEV cache $VDEV_CACHE

l2_cksum_bad_start=$(get_arcstat l2_cksum_bad)

log_must fio $FIO_SCRIPTS/mkfiles.fio
log_must fio $FIO_SCRIPTS/random_reads.fio

l2_cksum_bad_end=$(get_arcstat l2_cksum_bad)

log_note "L2ARC Failed Checksums before: $l2_cksum_bad_start After:"\
"$l2_cksum_bad_end"
log_must test $(( $l2_cksum_bad_end - $l2_cksum_bad_start )) -eq 0

log_must zpool destroy -f $TESTPOOL-l2arc

log_pass "L2ARC with compressed_arc enabled does not result in checksum errors."
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
#!/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 The FreeBSD Foundation [1]
#
# [1] Portions of this software were developed by Allan Jude
# under sponsorship from the FreeBSD Foundation.

. $STF_SUITE/include/libtest.shlib

export SIZE=1G
export VDIR=$TESTDIR/disk.persist_l2arc
export VDEV="$VDIR/a"
export VDEV_CACHE="$VDIR/b"

# fio options
export DIRECTORY=/$TESTPOOL-l2arc
export NUMJOBS=4
export RUNTIME=30
export PERF_RANDSEED=1234
export PERF_COMPPERCENT=66
export PERF_COMPCHUNK=0
export BLOCKSIZE=128K
export SYNC_TYPE=0
export DIRECT=1

#
# DESCRIPTION:
# System with compressed_arc disabled succeeds at reading from L2ARC
#
# STRATEGY:
# 1. Disable compressed_arc.
# 2. Create pool with a cache device and compression enabled.
# 3. Read the number of L2ARC checksum failures.
# 4. Create a random file in that pool and random read for 30 sec.
# 5. Read the number of L2ARC checksum failures.
#

verify_runnable "global"

log_assert "L2ARC with compressed_arc disabled succeeds."

origin_carc_setting=$(get_tunable COMPRESSED_ARC_ENABLED)

function cleanup
{
if poolexists $TESTPOOL-l2arc ; then
destroy_pool $TESTPOOL-l2arc
fi

log_must set_tunable64 COMPRESSED_ARC_ENABLED $origin_carc_setting
}
log_onexit cleanup

log_must rm -rf $VDIR
log_must mkdir -p $VDIR
log_must mkfile $SIZE $VDEV

# Disable Compressed ARC so that in-ARC and on-disk will not match
log_must set_tunable64 COMPRESSED_ARC_ENABLED 0

typeset fill_mb=800
typeset cache_sz=$(( floor($fill_mb / 2) ))
export FILE_SIZE=$(( floor($fill_mb / $NUMJOBS) ))M

log_must truncate -s ${cache_sz}M $VDEV_CACHE

log_must zpool create -O compression=lz4 -f $TESTPOOL-l2arc $VDEV cache $VDEV_CACHE

l2_cksum_bad_start=$(get_arcstat l2_cksum_bad)

log_must fio $FIO_SCRIPTS/mkfiles.fio
log_must fio $FIO_SCRIPTS/random_reads.fio

l2_cksum_bad_end=$(get_arcstat l2_cksum_bad)

log_note "L2ARC Failed Checksums before: $l2_cksum_bad_start After:"\
"$l2_cksum_bad_end"
log_must test $(( $l2_cksum_bad_end - $l2_cksum_bad_start )) -eq 0

log_must zpool destroy -f $TESTPOOL-l2arc

log_pass "L2ARC with compressed_arc disabled does not result in checksum"\
"errors."

0 comments on commit fc34dfb

Please sign in to comment.