Skip to content

Commit 783be69

Browse files
Ryan Moellerbehlendorf
authored andcommitted
Reduce confusion in zfs_write
Is this block when abuf != NULL ever reached? Yes, it is. Add asserts and comments to prove that when we get here, we have a full block write at an aligned offset extending past EOF. Simplify by removing the check that tx_bytes == max_blksz, since we can assert that it is always true. Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Ryan Moeller <[email protected]> Closes #11191
1 parent af5626a commit 783be69

File tree

1 file changed

+24
-18
lines changed

1 file changed

+24
-18
lines changed

module/zfs/zfs_vnops.c

Lines changed: 24 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -431,7 +431,8 @@ zfs_write(znode_t *zp, uio_t *uio, int ioflag, cred_t *cr)
431431
* XXX - should we really limit each write to z_max_blksz?
432432
* Perhaps we should use SPA_MAXBLOCKSIZE chunks?
433433
*/
434-
ssize_t nbytes = MIN(n, max_blksz - P2PHASE(woff, max_blksz));
434+
const ssize_t nbytes =
435+
MIN(n, max_blksz - P2PHASE(woff, max_blksz));
435436

436437
ssize_t tx_bytes;
437438
if (abuf == NULL) {
@@ -464,28 +465,33 @@ zfs_write(znode_t *zp, uio_t *uio, int ioflag, cred_t *cr)
464465
}
465466
tx_bytes -= uio->uio_resid;
466467
} else {
468+
/* Implied by abuf != NULL: */
469+
ASSERT3S(n, >=, max_blksz);
470+
ASSERT3S(woff, >=, zp->z_size);
471+
ASSERT0(P2PHASE(woff, max_blksz));
467472
/*
468-
* Is this block ever reached?
473+
* We can simplify nbytes to MIN(n, max_blksz) since
474+
* P2PHASE(woff, max_blksz) is 0, and knowing
475+
* n >= max_blksz lets us simplify further:
469476
*/
470-
tx_bytes = nbytes;
477+
ASSERT3S(nbytes, ==, max_blksz);
471478
/*
472-
* If this is not a full block write, but we are
473-
* extending the file past EOF and this data starts
474-
* block-aligned, use assign_arcbuf(). Otherwise,
475-
* write via dmu_write().
479+
* Thus, we're writing a full block at a block-aligned
480+
* offset and extending the file past EOF.
481+
*
482+
* dmu_assign_arcbuf_by_dbuf() will directly assign the
483+
* arc buffer to a dbuf.
476484
*/
477-
478-
if (tx_bytes == max_blksz) {
479-
error = dmu_assign_arcbuf_by_dbuf(
480-
sa_get_db(zp->z_sa_hdl), woff, abuf, tx);
481-
if (error != 0) {
482-
dmu_return_arcbuf(abuf);
483-
dmu_tx_commit(tx);
484-
break;
485-
}
485+
error = dmu_assign_arcbuf_by_dbuf(
486+
sa_get_db(zp->z_sa_hdl), woff, abuf, tx);
487+
if (error != 0) {
488+
dmu_return_arcbuf(abuf);
489+
dmu_tx_commit(tx);
490+
break;
486491
}
487-
ASSERT(tx_bytes <= uio->uio_resid);
488-
uioskip(uio, tx_bytes);
492+
ASSERT3S(nbytes, <=, uio->uio_resid);
493+
uioskip(uio, nbytes);
494+
tx_bytes = nbytes;
489495
}
490496
if (tx_bytes && zn_has_cached_data(zp) &&
491497
!(ioflag & O_DIRECT)) {

0 commit comments

Comments
 (0)