Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dmu: Allow buffer fills to fail #15665

Merged
merged 1 commit into from
Dec 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion include/os/freebsd/spl/sys/uio.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ zfs_uio_setoffset(zfs_uio_t *uio, offset_t off)
}

static inline void
zfs_uio_advance(zfs_uio_t *uio, size_t size)
zfs_uio_advance(zfs_uio_t *uio, ssize_t size)
{
zfs_uio_resid(uio) -= size;
zfs_uio_offset(uio) += size;
Expand Down
2 changes: 1 addition & 1 deletion include/os/linux/spl/sys/uio.h
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ zfs_uio_setoffset(zfs_uio_t *uio, offset_t off)
}

static inline void
zfs_uio_advance(zfs_uio_t *uio, size_t size)
zfs_uio_advance(zfs_uio_t *uio, ssize_t size)
{
uio->uio_resid -= size;
uio->uio_loffset += size;
Expand Down
4 changes: 2 additions & 2 deletions include/sys/dbuf.h
Original file line number Diff line number Diff line change
Expand Up @@ -380,8 +380,8 @@ dmu_buf_impl_t *dbuf_find(struct objset *os, uint64_t object, uint8_t level,
int dbuf_read(dmu_buf_impl_t *db, zio_t *zio, uint32_t flags);
void dmu_buf_will_clone(dmu_buf_t *db, dmu_tx_t *tx);
void dmu_buf_will_not_fill(dmu_buf_t *db, dmu_tx_t *tx);
void dmu_buf_will_fill(dmu_buf_t *db, dmu_tx_t *tx);
void dmu_buf_fill_done(dmu_buf_t *db, dmu_tx_t *tx);
void dmu_buf_will_fill(dmu_buf_t *db, dmu_tx_t *tx, boolean_t canfail);
boolean_t dmu_buf_fill_done(dmu_buf_t *db, dmu_tx_t *tx, boolean_t failed);
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_lightweight(dnode_t *dn, uint64_t blkid,
Expand Down
2 changes: 1 addition & 1 deletion lib/libspl/include/sys/uio.h
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ zfs_uio_iov_at_index(zfs_uio_t *uio, uint_t idx, void **base, uint64_t *len)
}

static inline void
zfs_uio_advance(zfs_uio_t *uio, size_t size)
zfs_uio_advance(zfs_uio_t *uio, ssize_t size)
{
uio->uio_resid -= size;
uio->uio_loffset += size;
Expand Down
4 changes: 2 additions & 2 deletions module/os/freebsd/zfs/dmu_os.c
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ dmu_write_pages(objset_t *os, uint64_t object, uint64_t offset, uint64_t size,
ASSERT(i == 0 || i == numbufs-1 || tocpy == db->db_size);

if (tocpy == db->db_size)
dmu_buf_will_fill(db, tx);
dmu_buf_will_fill(db, tx, B_FALSE);
else
dmu_buf_will_dirty(db, tx);

Expand All @@ -123,7 +123,7 @@ dmu_write_pages(objset_t *os, uint64_t object, uint64_t offset, uint64_t size,
}

if (tocpy == db->db_size)
dmu_buf_fill_done(db, tx);
dmu_buf_fill_done(db, tx, B_FALSE);

offset += tocpy;
size -= tocpy;
Expand Down
33 changes: 24 additions & 9 deletions module/zfs/dbuf.c
Original file line number Diff line number Diff line change
Expand Up @@ -2749,7 +2749,7 @@ dmu_buf_will_not_fill(dmu_buf_t *db_fake, dmu_tx_t *tx)
}

void
dmu_buf_will_fill(dmu_buf_t *db_fake, dmu_tx_t *tx)
dmu_buf_will_fill(dmu_buf_t *db_fake, dmu_tx_t *tx, boolean_t canfail)
{
dmu_buf_impl_t *db = (dmu_buf_impl_t *)db_fake;

Expand All @@ -2767,8 +2767,14 @@ dmu_buf_will_fill(dmu_buf_t *db_fake, dmu_tx_t *tx)
* Block cloning: We will be completely overwriting a block
* cloned in this transaction group, so let's undirty the
* pending clone and mark the block as uncached. This will be
* as if the clone was never done.
* as if the clone was never done. But if the fill can fail
* we should have a way to return back to the cloned data.
*/
if (canfail && dbuf_find_dirty_eq(db, tx->tx_txg) != NULL) {
mutex_exit(&db->db_mtx);
dmu_buf_will_dirty(db_fake, tx);
return;
}
VERIFY(!dbuf_undirty(db, tx));
db->db_state = DB_UNCACHED;
}
Expand Down Expand Up @@ -2829,32 +2835,41 @@ dbuf_override_impl(dmu_buf_impl_t *db, const blkptr_t *bp, dmu_tx_t *tx)
dl->dr_overridden_by.blk_birth = dr->dr_txg;
}

void
dmu_buf_fill_done(dmu_buf_t *dbuf, dmu_tx_t *tx)
boolean_t
dmu_buf_fill_done(dmu_buf_t *dbuf, dmu_tx_t *tx, boolean_t failed)
{
(void) tx;
dmu_buf_impl_t *db = (dmu_buf_impl_t *)dbuf;
dbuf_states_t old_state;
mutex_enter(&db->db_mtx);
DBUF_VERIFY(db);

old_state = db->db_state;
db->db_state = DB_CACHED;
if (old_state == DB_FILL) {
if (db->db_state == DB_FILL) {
if (db->db_level == 0 && db->db_freed_in_flight) {
ASSERT(db->db_blkid != DMU_BONUS_BLKID);
/* we were freed while filling */
/* XXX dbuf_undirty? */
memset(db->db.db_data, 0, db->db.db_size);
db->db_freed_in_flight = FALSE;
db->db_state = DB_CACHED;
DTRACE_SET_STATE(db,
"fill done handling freed in flight");
failed = B_FALSE;
} else if (failed) {
VERIFY(!dbuf_undirty(db, tx));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERIFY0

db->db_buf = NULL;
dbuf_clear_data(db);
DTRACE_SET_STATE(db, "fill failed");
} else {
db->db_state = DB_CACHED;
DTRACE_SET_STATE(db, "fill done");
}
cv_broadcast(&db->db_changed);
} else {
db->db_state = DB_CACHED;
failed = B_FALSE;
}
mutex_exit(&db->db_mtx);
return (failed);
}

void
Expand Down Expand Up @@ -2989,7 +3004,7 @@ dbuf_assign_arcbuf(dmu_buf_impl_t *db, arc_buf_t *buf, dmu_tx_t *tx)
DTRACE_SET_STATE(db, "filling assigned arcbuf");
mutex_exit(&db->db_mtx);
(void) dbuf_dirty(db, tx);
dmu_buf_fill_done(&db->db, tx);
dmu_buf_fill_done(&db->db, tx, B_FALSE);
}

void
Expand Down
21 changes: 9 additions & 12 deletions module/zfs/dmu.c
Original file line number Diff line number Diff line change
Expand Up @@ -1134,14 +1134,14 @@ dmu_write_impl(dmu_buf_t **dbp, int numbufs, uint64_t offset, uint64_t size,
ASSERT(i == 0 || i == numbufs-1 || tocpy == db->db_size);

if (tocpy == db->db_size)
dmu_buf_will_fill(db, tx);
dmu_buf_will_fill(db, tx, B_FALSE);
else
dmu_buf_will_dirty(db, tx);

(void) memcpy((char *)db->db_data + bufoff, buf, tocpy);

if (tocpy == db->db_size)
dmu_buf_fill_done(db, tx);
dmu_buf_fill_done(db, tx, B_FALSE);

offset += tocpy;
size -= tocpy;
Expand Down Expand Up @@ -1349,27 +1349,24 @@ dmu_write_uio_dnode(dnode_t *dn, zfs_uio_t *uio, uint64_t size, dmu_tx_t *tx)

ASSERT(size > 0);

bufoff = zfs_uio_offset(uio) - db->db_offset;
offset_t off = zfs_uio_offset(uio);
bufoff = off - db->db_offset;
tocpy = MIN(db->db_size - bufoff, size);

ASSERT(i == 0 || i == numbufs-1 || tocpy == db->db_size);

if (tocpy == db->db_size)
dmu_buf_will_fill(db, tx);
dmu_buf_will_fill(db, tx, B_TRUE);
else
dmu_buf_will_dirty(db, tx);

/*
* XXX zfs_uiomove could block forever (eg.nfs-backed
* pages). There needs to be a uiolockdown() function
* to lock the pages in memory, so that zfs_uiomove won't
* block.
*/
err = zfs_uio_fault_move((char *)db->db_data + bufoff,
tocpy, UIO_WRITE, uio);

if (tocpy == db->db_size)
dmu_buf_fill_done(db, tx);
if (tocpy == db->db_size && dmu_buf_fill_done(db, tx, err)) {
/* The fill was reverted. Undo any uio progress. */
zfs_uio_advance(uio, off - zfs_uio_offset(uio));
}

if (err)
break;
Expand Down
2 changes: 1 addition & 1 deletion module/zfs/dmu_recv.c
Original file line number Diff line number Diff line change
Expand Up @@ -2532,7 +2532,7 @@ receive_spill(struct receive_writer_arg *rwa, struct drr_spill *drrs,
* size of the provided arc_buf_t.
*/
if (db_spill->db_size != drrs->drr_length) {
dmu_buf_will_fill(db_spill, tx);
dmu_buf_will_fill(db_spill, tx, B_FALSE);
VERIFY0(dbuf_spill_set_blksz(db_spill,
drrs->drr_length, tx));
}
Expand Down
2 changes: 1 addition & 1 deletion module/zfs/dsl_bookmark.c
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,7 @@ dsl_bookmark_create_sync_impl_snap(const char *bookmark, const char *snapshot,
dmu_buf_t *db;
VERIFY0(dmu_spill_hold_by_bonus(local_rl->rl_bonus,
DB_RF_MUST_SUCCEED, FTAG, &db));
dmu_buf_will_fill(db, tx);
dmu_buf_will_fill(db, tx, B_FALSE);
VERIFY0(dbuf_spill_set_blksz(db, P2ROUNDUP(bonuslen,
SPA_MINBLOCKSIZE), tx));
local_rl->rl_phys = db->db_data;
Expand Down
Loading