Skip to content

Commit

Permalink
ZIL: Do not clone blocks from the future
Browse files Browse the repository at this point in the history
ZIL claim can not handle block pointers cloned from the future,
since they are not yet allocated at that point.  It may happen
either if the block was just written when it was cloned, or if
the pool was frozen or somehow else rewound on import.

Handle it from two sides: prevent cloning of blocks with physical
birth time from not yet synced or frozen TXG, and abort ZIL claim
if we still detect such blocks due to rewind or something else.

While there, assert that any cloned blocks we claim are really
allocated by calling metaslab_check_free().

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Alexander Motin <mav@FreeBSD.org>
Sponsored by:	iXsystems, Inc.
Closes openzfs#15617
  • Loading branch information
amotin authored and mmatuska committed Dec 27, 2023
1 parent 46c0bfc commit d2ff592
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 10 deletions.
15 changes: 15 additions & 0 deletions module/zfs/dmu.c
Original file line number Diff line number Diff line change
Expand Up @@ -2255,6 +2255,21 @@ dmu_read_l0_bps(objset_t *os, uint64_t object, uint64_t offset, uint64_t length,
goto out;
}

/*
* If the block was allocated in transaction group that is not
* yet synced, we could clone it, but we couldn't write this
* operation into ZIL, or it may be impossible to replay, since
* the block may appear not yet allocated at that point.
*/
if (BP_PHYSICAL_BIRTH(bp) > spa_freeze_txg(os->os_spa)) {
error = SET_ERROR(EINVAL);
goto out;
}
if (BP_PHYSICAL_BIRTH(bp) > spa_last_synced_txg(os->os_spa)) {
error = SET_ERROR(EAGAIN);
goto out;
}

bps[i] = *bp;
}

Expand Down
38 changes: 28 additions & 10 deletions module/zfs/zil.c
Original file line number Diff line number Diff line change
Expand Up @@ -625,11 +625,12 @@ zil_claim_write(zilog_t *zilog, const lr_t *lrc, void *tx, uint64_t first_txg)
}

static int
zil_claim_clone_range(zilog_t *zilog, const lr_t *lrc, void *tx)
zil_claim_clone_range(zilog_t *zilog, const lr_t *lrc, void *tx,
uint64_t first_txg)
{
const lr_clone_range_t *lr = (const lr_clone_range_t *)lrc;
const blkptr_t *bp;
spa_t *spa;
spa_t *spa = zilog->zl_spa;
uint_t ii;

ASSERT3U(lrc->lrc_reclen, >=, sizeof (*lr));
Expand All @@ -644,19 +645,36 @@ zil_claim_clone_range(zilog_t *zilog, const lr_t *lrc, void *tx)
* XXX: Do we need to byteswap lr?
*/

spa = zilog->zl_spa;

for (ii = 0; ii < lr->lr_nbps; ii++) {
bp = &lr->lr_bps[ii];

/*
* 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
* it contains the data.
* When data is embedded into the BP there is no need to create
* BRT entry as there is no data block. Just copy the BP as it
* contains the data.
*/
if (BP_IS_HOLE(bp) || BP_IS_EMBEDDED(bp))
continue;

/*
* We can not handle block pointers from the future, since they
* are not yet allocated. It should not normally happen, but
* just in case lets be safe and just stop here now instead of
* corrupting the pool.
*/
if (!BP_IS_HOLE(bp) && !BP_IS_EMBEDDED(bp)) {
if (BP_PHYSICAL_BIRTH(bp) >= first_txg)
return (SET_ERROR(ENOENT));

/*
* Assert the block is really allocated before we reference it.
*/
metaslab_check_free(spa, bp);
}

for (ii = 0; ii < lr->lr_nbps; ii++) {
bp = &lr->lr_bps[ii];
if (!BP_IS_HOLE(bp) && !BP_IS_EMBEDDED(bp))
brt_pending_add(spa, bp, tx);
}
}

return (0);
Expand All @@ -671,7 +689,7 @@ zil_claim_log_record(zilog_t *zilog, const lr_t *lrc, void *tx,
case TX_WRITE:
return (zil_claim_write(zilog, lrc, tx, first_txg));
case TX_CLONE_RANGE:
return (zil_claim_clone_range(zilog, lrc, tx));
return (zil_claim_clone_range(zilog, lrc, tx, first_txg));
default:
return (0);
}
Expand Down

0 comments on commit d2ff592

Please sign in to comment.