From 6e3c109bc007597692054c66675b8f45b76744a4 Mon Sep 17 00:00:00 2001 From: Alexander Motin Date: Mon, 2 Dec 2024 20:08:40 -0500 Subject: [PATCH] Fix regression in dmu_buf_will_fill() Direct I/O implementation added condition to call dbuf_undirty() only in case of block cloning. But the condition is not right if the block is no longer dirty in this TXG, but still in DB_NOFILL state. It resulted in block not reverting to DB_UNCACHED and following NULL de-reference on attempt to access absent db_data. While there, add assertions for db_data to make debugging easier. Reviewed-by: Brian Atkinson Reviewed-by: Brian Behlendorf Signed-off-by: Alexander Motin Sponsored by: iXsystems, Inc. Closes #16829 --- module/os/freebsd/zfs/dmu_os.c | 7 ++++++- module/zfs/dbuf.c | 2 +- module/zfs/dmu.c | 4 ++++ 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/module/os/freebsd/zfs/dmu_os.c b/module/os/freebsd/zfs/dmu_os.c index 0a0af102e..370ce2d80 100644 --- a/module/os/freebsd/zfs/dmu_os.c +++ b/module/os/freebsd/zfs/dmu_os.c @@ -103,6 +103,7 @@ dmu_write_pages(objset_t *os, uint64_t object, uint64_t offset, uint64_t size, db->db_offset + bufoff); thiscpy = MIN(PAGESIZE, tocpy - copied); va = zfs_map_page(*ma, &sf); + ASSERT(db->db_data != NULL); memcpy((char *)db->db_data + bufoff, va, thiscpy); zfs_unmap_page(sf); ma += 1; @@ -172,6 +173,7 @@ dmu_read_pages(objset_t *os, uint64_t object, vm_page_t *ma, int count, ASSERT3U(db->db_size, >, PAGE_SIZE); bufoff = IDX_TO_OFF(m->pindex) % db->db_size; va = zfs_map_page(m, &sf); + ASSERT(db->db_data != NULL); memcpy(va, (char *)db->db_data + bufoff, PAGESIZE); zfs_unmap_page(sf); vm_page_valid(m); @@ -211,8 +213,10 @@ dmu_read_pages(objset_t *os, uint64_t object, vm_page_t *ma, int count, */ tocpy = MIN(db->db_size - bufoff, PAGESIZE - pgoff); ASSERT3S(tocpy, >=, 0); - if (m != bogus_page) + if (m != bogus_page) { + ASSERT(db->db_data != NULL); memcpy(va + pgoff, (char *)db->db_data + bufoff, tocpy); + } pgoff += tocpy; ASSERT3S(pgoff, >=, 0); @@ -290,6 +294,7 @@ dmu_read_pages(objset_t *os, uint64_t object, vm_page_t *ma, int count, bufoff = IDX_TO_OFF(m->pindex) % db->db_size; tocpy = MIN(db->db_size - bufoff, PAGESIZE); va = zfs_map_page(m, &sf); + ASSERT(db->db_data != NULL); memcpy(va, (char *)db->db_data + bufoff, tocpy); if (tocpy < PAGESIZE) { ASSERT3S(i, ==, *rahead - 1); diff --git a/module/zfs/dbuf.c b/module/zfs/dbuf.c index cbd07d19a..190d8ded3 100644 --- a/module/zfs/dbuf.c +++ b/module/zfs/dbuf.c @@ -2921,7 +2921,7 @@ dmu_buf_will_fill(dmu_buf_t *db_fake, dmu_tx_t *tx, boolean_t canfail) * pending clone and mark the block as uncached. This will be * as if the clone was never done. */ - if (dr && dr->dt.dl.dr_brtwrite) { + if (db->db_state == DB_NOFILL) { VERIFY(!dbuf_undirty(db, tx)); db->db_state = DB_UNCACHED; } diff --git a/module/zfs/dmu.c b/module/zfs/dmu.c index 362415a25..4830f4850 100644 --- a/module/zfs/dmu.c +++ b/module/zfs/dmu.c @@ -1221,6 +1221,7 @@ dmu_read_impl(dnode_t *dn, uint64_t offset, uint64_t size, bufoff = offset - db->db_offset; tocpy = MIN(db->db_size - bufoff, size); + ASSERT(db->db_data != NULL); (void) memcpy(buf, (char *)db->db_data + bufoff, tocpy); offset += tocpy; @@ -1278,6 +1279,7 @@ dmu_write_impl(dmu_buf_t **dbp, int numbufs, uint64_t offset, uint64_t size, else dmu_buf_will_dirty(db, tx); + ASSERT(db->db_data != NULL); (void) memcpy((char *)db->db_data + bufoff, buf, tocpy); if (tocpy == db->db_size) @@ -1426,6 +1428,7 @@ dmu_read_uio_dnode(dnode_t *dn, zfs_uio_t *uio, uint64_t size) bufoff = zfs_uio_offset(uio) - db->db_offset; tocpy = MIN(db->db_size - bufoff, size); + ASSERT(db->db_data != NULL); err = zfs_uio_fault_move((char *)db->db_data + bufoff, tocpy, UIO_READ, uio); @@ -1550,6 +1553,7 @@ top: else dmu_buf_will_dirty(db, tx); + ASSERT(db->db_data != NULL); err = zfs_uio_fault_move((char *)db->db_data + bufoff, tocpy, UIO_WRITE, uio); -- 2.11.4.GIT