Fix cloning into already dirty dbufs.

Undirty the dbuf and destroy its buffer when cloning into it.

Coverity ID: CID-1535375
Reported-by: Richard Yao
Reported-by: Benjamin Coddington
Reviewed-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <mav@FreeBSD.org>
Signed-off-by: Pawel Jakub Dawidek <pawel@dawidek.net>
Closes #14655
This commit is contained in:
Pawel Jakub Dawidek 2023-03-24 18:18:35 +01:00 committed by GitHub
parent 5b5f518687
commit ce0e1cc402
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 26 additions and 13 deletions

View File

@ -382,6 +382,7 @@ void dbuf_assign_arcbuf(dmu_buf_impl_t *db, arc_buf_t *buf, dmu_tx_t *tx);
dbuf_dirty_record_t *dbuf_dirty(dmu_buf_impl_t *db, dmu_tx_t *tx); dbuf_dirty_record_t *dbuf_dirty(dmu_buf_impl_t *db, dmu_tx_t *tx);
dbuf_dirty_record_t *dbuf_dirty_lightweight(dnode_t *dn, uint64_t blkid, dbuf_dirty_record_t *dbuf_dirty_lightweight(dnode_t *dn, uint64_t blkid,
dmu_tx_t *tx); dmu_tx_t *tx);
boolean_t dbuf_undirty(dmu_buf_impl_t *db, dmu_tx_t *tx);
arc_buf_t *dbuf_loan_arcbuf(dmu_buf_impl_t *db); arc_buf_t *dbuf_loan_arcbuf(dmu_buf_impl_t *db);
void dmu_buf_write_embedded(dmu_buf_t *dbuf, void *data, void dmu_buf_write_embedded(dmu_buf_t *dbuf, void *data,
bp_embedded_type_t etype, enum zio_compress comp, bp_embedded_type_t etype, enum zio_compress comp,

View File

@ -175,7 +175,6 @@ struct {
continue; \ continue; \
} }
static boolean_t dbuf_undirty(dmu_buf_impl_t *db, dmu_tx_t *tx);
static void dbuf_write(dbuf_dirty_record_t *dr, arc_buf_t *data, dmu_tx_t *tx); static void dbuf_write(dbuf_dirty_record_t *dr, arc_buf_t *data, dmu_tx_t *tx);
static void dbuf_sync_leaf_verify_bonus_dnode(dbuf_dirty_record_t *dr); static void dbuf_sync_leaf_verify_bonus_dnode(dbuf_dirty_record_t *dr);
static int dbuf_read_verify_dnode_crypt(dmu_buf_impl_t *db, uint32_t flags); static int dbuf_read_verify_dnode_crypt(dmu_buf_impl_t *db, uint32_t flags);
@ -2518,7 +2517,7 @@ dbuf_undirty_bonus(dbuf_dirty_record_t *dr)
* Undirty a buffer in the transaction group referenced by the given * Undirty a buffer in the transaction group referenced by the given
* transaction. Return whether this evicted the dbuf. * transaction. Return whether this evicted the dbuf.
*/ */
static boolean_t boolean_t
dbuf_undirty(dmu_buf_impl_t *db, dmu_tx_t *tx) dbuf_undirty(dmu_buf_impl_t *db, dmu_tx_t *tx)
{ {
uint64_t txg = tx->tx_txg; uint64_t txg = tx->tx_txg;

View File

@ -2190,7 +2190,8 @@ dmu_read_l0_bps(objset_t *os, uint64_t object, uint64_t offset, uint64_t length,
for (int i = 0; i < numbufs; i++) { for (int i = 0; i < numbufs; i++) {
dbuf = dbp[i]; dbuf = dbp[i];
db = (dmu_buf_impl_t *)dbuf; db = (dmu_buf_impl_t *)dbuf;
bp = db->db_blkptr;
mutex_enter(&db->db_mtx);
/* /*
* If the block is not on the disk yet, it has no BP assigned. * If the block is not on the disk yet, it has no BP assigned.
@ -2212,10 +2213,16 @@ dmu_read_l0_bps(objset_t *os, uint64_t object, uint64_t offset, uint64_t length,
* The block was modified in the same * The block was modified in the same
* transaction group. * transaction group.
*/ */
mutex_exit(&db->db_mtx);
error = SET_ERROR(EAGAIN); error = SET_ERROR(EAGAIN);
goto out; goto out;
} }
} else {
bp = db->db_blkptr;
} }
mutex_exit(&db->db_mtx);
if (bp == NULL) { if (bp == NULL) {
/* /*
* The block was created in this transaction group, * The block was created in this transaction group,
@ -2273,19 +2280,23 @@ dmu_brt_clone(objset_t *os, uint64_t object, uint64_t offset, uint64_t length,
ASSERT(db->db_blkid != DMU_BONUS_BLKID); ASSERT(db->db_blkid != DMU_BONUS_BLKID);
ASSERT(BP_IS_HOLE(bp) || dbuf->db_size == BP_GET_LSIZE(bp)); ASSERT(BP_IS_HOLE(bp) || dbuf->db_size == BP_GET_LSIZE(bp));
if (db->db_state == DB_UNCACHED) { mutex_enter(&db->db_mtx);
/*
* XXX-PJD: If the dbuf is already cached, calling VERIFY(!dbuf_undirty(db, tx));
* dmu_buf_will_not_fill() will panic on assertion ASSERT(list_head(&db->db_dirty_records) == NULL);
* (db->db_buf == NULL) in dbuf_clear_data(), if (db->db_buf != NULL) {
* which is called from dbuf_noread() in DB_NOFILL arc_buf_destroy(db->db_buf, db);
* case. I'm not 100% sure this is the right thing db->db_buf = NULL;
* to do, but it seems to work.
*/
dmu_buf_will_not_fill(dbuf, tx);
} }
mutex_exit(&db->db_mtx);
dmu_buf_will_not_fill(dbuf, tx);
mutex_enter(&db->db_mtx);
dr = list_head(&db->db_dirty_records); dr = list_head(&db->db_dirty_records);
VERIFY(dr != NULL);
ASSERT3U(dr->dr_txg, ==, tx->tx_txg); ASSERT3U(dr->dr_txg, ==, tx->tx_txg);
dl = &dr->dt.dl; dl = &dr->dt.dl;
dl->dr_overridden_by = *bp; dl->dr_overridden_by = *bp;
@ -2301,6 +2312,8 @@ dmu_brt_clone(objset_t *os, uint64_t object, uint64_t offset, uint64_t length,
BP_PHYSICAL_BIRTH(bp); BP_PHYSICAL_BIRTH(bp);
} }
mutex_exit(&db->db_mtx);
/* /*
* When data in embedded into BP there is no need to create * When data in embedded into BP there is no need to create
* BRT entry as there is no data block. Just copy the BP as * BRT entry as there is no data block. Just copy the BP as