From fc34dfba8e8238683e90e3fa83d16be3343886f6 Mon Sep 17 00:00:00 2001 From: Allan Jude Date: Fri, 14 Aug 2020 02:31:20 -0400 Subject: [PATCH] Fix L2ARC reads when compressed ARC disabled 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 Signed-off-by: Allan Jude Closes #10693 --- module/zfs/arc.c | 15 +++ tests/runfiles/common.run | 3 +- .../tests/functional/compression/Makefile.am | 4 +- .../compression/l2arc_compressed_arc.ksh | 97 ++++++++++++++++++ .../l2arc_compressed_arc_disabled.ksh | 98 +++++++++++++++++++ 5 files changed, 215 insertions(+), 2 deletions(-) create mode 100755 tests/zfs-tests/tests/functional/compression/l2arc_compressed_arc.ksh create mode 100755 tests/zfs-tests/tests/functional/compression/l2arc_compressed_arc_disabled.ksh diff --git a/module/zfs/arc.c b/module/zfs/arc.c index c80fe555a4cc..19ad42d29049 100644 --- a/module/zfs/arc.c +++ b/module/zfs/arc.c @@ -26,6 +26,10 @@ * Copyright (c) 2017, Nexenta Systems, Inc. All rights reserved. * Copyright (c) 2019, loli10K . 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. */ /* @@ -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, diff --git a/tests/runfiles/common.run b/tests/runfiles/common.run index 931eee4fafee..dcfd1cc91a4f 100644 --- a/tests/runfiles/common.run +++ b/tests/runfiles/common.run @@ -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] diff --git a/tests/zfs-tests/tests/functional/compression/Makefile.am b/tests/zfs-tests/tests/functional/compression/Makefile.am index 25a5bca232f1..bf5901e54d7c 100644 --- a/tests/zfs-tests/tests/functional/compression/Makefile.am +++ b/tests/zfs-tests/tests/functional/compression/Makefile.am @@ -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 diff --git a/tests/zfs-tests/tests/functional/compression/l2arc_compressed_arc.ksh b/tests/zfs-tests/tests/functional/compression/l2arc_compressed_arc.ksh new file mode 100755 index 000000000000..5980ce156934 --- /dev/null +++ b/tests/zfs-tests/tests/functional/compression/l2arc_compressed_arc.ksh @@ -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." diff --git a/tests/zfs-tests/tests/functional/compression/l2arc_compressed_arc_disabled.ksh b/tests/zfs-tests/tests/functional/compression/l2arc_compressed_arc_disabled.ksh new file mode 100755 index 000000000000..4c3b6a61c25f --- /dev/null +++ b/tests/zfs-tests/tests/functional/compression/l2arc_compressed_arc_disabled.ksh @@ -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."