From 04cd5f72c9296a542fe1373d2db4190986e7b807 Mon Sep 17 00:00:00 2001 From: Brian Atkinson Date: Wed, 5 Jan 2022 16:20:09 -0700 Subject: [PATCH] Fixing issue for failing ZTS test trim_l2arc PR #12285 introduced a new module parameter (l2arc_exclude_special) to allow for special or dedup allocation class to specify the L2 ARC should not be used. However, we pass along the BP for the dbuf from dbuf_read() to dbuf_read_impl() to account for Direct IO writes. The previous call for dbuf_is_l2cacheable() directly used the db->db_blkptr so it did not take into account the BP passed from dbuf_read_impl(). I updated this so the BP is now passed into this function. If the BP passed is NULL, then the default behavior of dbuf_is_l2cacheable() remains the same by just using the db->db_blkptr. However, the test failure was being caused by trim_l2arc.ksh setting DIRECT=1 before calling random_reads.fio to fill up the L2 ARC. This caused Direct IO reads to take place only filling up the L2 ARC with indirect blocks instead of data blocks. This ultimately led to a failure for this test due to verify_trim_io getting: Too few trim IOs issued 2/5 So I update the test case to not use Direct IO as we are wanting to fill up the L2 ARC with data buffers using random_reads.fio. This allows for the logic of checking the number of trims to be correct now. Signed-off-by: Brian Atkinson --- include/sys/dbuf.h | 2 +- module/zfs/dbuf.c | 19 +++++++++++++------ module/zfs/dmu.c | 7 ++++--- .../tests/functional/trim/trim_l2arc.ksh | 2 +- 4 files changed, 19 insertions(+), 11 deletions(-) diff --git a/include/sys/dbuf.h b/include/sys/dbuf.h index 120116b5e38c..accc396eaeaf 100644 --- a/include/sys/dbuf.h +++ b/include/sys/dbuf.h @@ -475,7 +475,7 @@ dbuf_get_dirty_direct(dmu_buf_impl_t *db) (dbuf_is_metadata(_db) && \ ((_db)->db_objset->os_primary_cache == ZFS_CACHE_METADATA))) -boolean_t dbuf_is_l2cacheable(dmu_buf_impl_t *db); +boolean_t dbuf_is_l2cacheable(dmu_buf_impl_t *db, blkptr_t *db_bp); #ifdef ZFS_DEBUG diff --git a/module/zfs/dbuf.c b/module/zfs/dbuf.c index 703ed4b1e953..9dc899cd672b 100644 --- a/module/zfs/dbuf.c +++ b/module/zfs/dbuf.c @@ -640,7 +640,7 @@ dbuf_is_metadata(dmu_buf_impl_t *db) * L2ARC. */ boolean_t -dbuf_is_l2cacheable(dmu_buf_impl_t *db) +dbuf_is_l2cacheable(dmu_buf_impl_t *db, blkptr_t *bp) { if (db->db_objset->os_secondary_cache == ZFS_CACHE_ALL || (db->db_objset->os_secondary_cache == @@ -648,10 +648,17 @@ dbuf_is_l2cacheable(dmu_buf_impl_t *db) if (l2arc_exclude_special == 0) return (B_TRUE); - blkptr_t *bp = db->db_blkptr; - if (bp == NULL || BP_IS_HOLE(bp)) + /* + * bp must be checked in the event it was passed from + * dbuf_read_impl() as the result of a the BP being set from + * a Direct I/O write in dbuf_read(). See comments in + * dbuf_read(). + */ + blkptr_t *db_bp = bp == NULL ? db->db_blkptr : bp; + + if (db_bp == NULL || BP_IS_HOLE(db_bp)) return (B_FALSE); - uint64_t vdev = DVA_GET_VDEV(bp->blk_dva); + uint64_t vdev = DVA_GET_VDEV(db_bp->blk_dva); vdev_t *rvd = db->db_objset->os_spa->spa_root_vdev; vdev_t *vd = NULL; @@ -1686,7 +1693,7 @@ dbuf_read_impl(dmu_buf_impl_t *db, dnode_t *dn, zio_t *zio, uint32_t flags, if (!DBUF_IS_CACHEABLE(db)) aflags |= ARC_FLAG_UNCACHED; - else if (dbuf_is_l2cacheable(db)) + else if (dbuf_is_l2cacheable(db, bpp)) aflags |= ARC_FLAG_L2CACHE; dbuf_add_ref(db, NULL); @@ -5429,7 +5436,7 @@ dbuf_write(dbuf_dirty_record_t *dr, arc_buf_t *data, dmu_tx_t *tx) dr->dr_zio = arc_write(pio, os->os_spa, txg, &dr->dr_bp_copy, data, !DBUF_IS_CACHEABLE(db), - dbuf_is_l2cacheable(db), &zp, dbuf_write_ready, + dbuf_is_l2cacheable(db, NULL), &zp, dbuf_write_ready, children_ready_cb, dbuf_write_done, db, ZIO_PRIORITY_ASYNC_WRITE, ZIO_FLAG_MUSTSUCCEED, &zb); } diff --git a/module/zfs/dmu.c b/module/zfs/dmu.c index bcb88587a05c..29444c935d11 100644 --- a/module/zfs/dmu.c +++ b/module/zfs/dmu.c @@ -2191,9 +2191,10 @@ dmu_sync(zio_t *pio, uint64_t txg, dmu_sync_cb_t *done, zgd_t *zgd) dsa->dsa_tx = NULL; zio_nowait(arc_write(pio, os->os_spa, txg, zgd->zgd_bp, - dr->dt.dl.dr_data, !DBUF_IS_CACHEABLE(db), dbuf_is_l2cacheable(db), - &zp, dmu_sync_ready, NULL, dmu_sync_done, dsa, - ZIO_PRIORITY_SYNC_WRITE, ZIO_FLAG_CANFAIL, &zb)); + dr->dt.dl.dr_data, !DBUF_IS_CACHEABLE(db), + dbuf_is_l2cacheable(db, NULL), &zp, dmu_sync_ready, NULL, + dmu_sync_done, dsa, ZIO_PRIORITY_SYNC_WRITE, ZIO_FLAG_CANFAIL, + &zb)); return (0); } diff --git a/tests/zfs-tests/tests/functional/trim/trim_l2arc.ksh b/tests/zfs-tests/tests/functional/trim/trim_l2arc.ksh index a93d0b3cc803..62563e0dd4cb 100755 --- a/tests/zfs-tests/tests/functional/trim/trim_l2arc.ksh +++ b/tests/zfs-tests/tests/functional/trim/trim_l2arc.ksh @@ -77,7 +77,7 @@ export PERF_COMPCHUNK=0 export RUNTIME=30 export BLOCKSIZE=128K export SYNC_TYPE=0 -export DIRECT=1 +export DIRECT=0 # Write to the pool. log_must fio $FIO_SCRIPTS/mkfiles.fio