Skip to content

Commit

Permalink
Fixing issue for failing ZTS test trim_l2arc
Browse files Browse the repository at this point in the history
PR openzfs#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 <batkinson@lanl.gov>
  • Loading branch information
bwatkinson committed Jul 26, 2024
1 parent 919f8a1 commit 04cd5f7
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 11 deletions.
2 changes: 1 addition & 1 deletion include/sys/dbuf.h
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
19 changes: 13 additions & 6 deletions module/zfs/dbuf.c
Original file line number Diff line number Diff line change
Expand Up @@ -640,18 +640,25 @@ 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 ==
ZFS_CACHE_METADATA && dbuf_is_metadata(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;

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
Expand Down
7 changes: 4 additions & 3 deletions module/zfs/dmu.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion tests/zfs-tests/tests/functional/trim/trim_l2arc.ksh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 04cd5f7

Please sign in to comment.