Skip to content

Commit 85703f6

Browse files
author
Ryan Moeller
authored
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 openzfs#11191
1 parent 0ca45cb commit 85703f6

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
@@ -523,7 +523,8 @@ zfs_write(znode_t *zp, uio_t *uio, int ioflag, cred_t *cr)
523523
* XXX - should we really limit each write to z_max_blksz?
524524
* Perhaps we should use SPA_MAXBLOCKSIZE chunks?
525525
*/
526-
ssize_t nbytes = MIN(n, max_blksz - P2PHASE(woff, max_blksz));
526+
const ssize_t nbytes =
527+
MIN(n, max_blksz - P2PHASE(woff, max_blksz));
527528

528529
ssize_t tx_bytes;
529530
if (abuf == NULL) {
@@ -556,28 +557,33 @@ zfs_write(znode_t *zp, uio_t *uio, int ioflag, cred_t *cr)
556557
}
557558
tx_bytes -= uio->uio_resid;
558559
} else {
560+
/* Implied by abuf != NULL: */
561+
ASSERT3S(n, >=, max_blksz);
562+
ASSERT3S(woff, >=, zp->z_size);
563+
ASSERT0(P2PHASE(woff, max_blksz));
559564
/*
560-
* Is this block ever reached?
565+
* We can simplify nbytes to MIN(n, max_blksz) since
566+
* P2PHASE(woff, max_blksz) is 0, and knowing
567+
* n >= max_blksz lets us simplify further:
561568
*/
562-
tx_bytes = nbytes;
569+
ASSERT3S(nbytes, ==, max_blksz);
563570
/*
564-
* If this is not a full block write, but we are
565-
* extending the file past EOF and this data starts
566-
* block-aligned, use assign_arcbuf(). Otherwise,
567-
* write via dmu_write().
571+
* Thus, we're writing a full block at a block-aligned
572+
* offset and extending the file past EOF.
573+
*
574+
* dmu_assign_arcbuf_by_dbuf() will directly assign the
575+
* arc buffer to a dbuf.
568576
*/
569-
570-
if (tx_bytes == max_blksz) {
571-
error = dmu_assign_arcbuf_by_dbuf(
572-
sa_get_db(zp->z_sa_hdl), woff, abuf, tx);
573-
if (error != 0) {
574-
dmu_return_arcbuf(abuf);
575-
dmu_tx_commit(tx);
576-
break;
577-
}
577+
error = dmu_assign_arcbuf_by_dbuf(
578+
sa_get_db(zp->z_sa_hdl), woff, abuf, tx);
579+
if (error != 0) {
580+
dmu_return_arcbuf(abuf);
581+
dmu_tx_commit(tx);
582+
break;
578583
}
579-
ASSERT(tx_bytes <= uio->uio_resid);
580-
uioskip(uio, tx_bytes);
584+
ASSERT3S(nbytes, <=, uio->uio_resid);
585+
uioskip(uio, nbytes);
586+
tx_bytes = nbytes;
581587
}
582588
if (tx_bytes && zn_has_cached_data(zp) &&
583589
!(ioflag & O_DIRECT)) {

0 commit comments

Comments
 (0)