Skip to content

Commit

Permalink
Reduce confusion in zfs_write
Browse files Browse the repository at this point in the history
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 <behlendorf1@llnl.gov>
Signed-off-by: Ryan Moeller <ryan@iXsystems.com>
Closes #11191
  • Loading branch information
Ryan Moeller authored Nov 18, 2020
1 parent 0ca45cb commit 85703f6
Showing 1 changed file with 24 additions and 18 deletions.
42 changes: 24 additions & 18 deletions module/zfs/zfs_vnops.c
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,8 @@ zfs_write(znode_t *zp, uio_t *uio, int ioflag, cred_t *cr)
* XXX - should we really limit each write to z_max_blksz?
* Perhaps we should use SPA_MAXBLOCKSIZE chunks?
*/
ssize_t nbytes = MIN(n, max_blksz - P2PHASE(woff, max_blksz));
const ssize_t nbytes =
MIN(n, max_blksz - P2PHASE(woff, max_blksz));

ssize_t tx_bytes;
if (abuf == NULL) {
Expand Down Expand Up @@ -556,28 +557,33 @@ zfs_write(znode_t *zp, uio_t *uio, int ioflag, cred_t *cr)
}
tx_bytes -= uio->uio_resid;
} else {
/* Implied by abuf != NULL: */
ASSERT3S(n, >=, max_blksz);
ASSERT3S(woff, >=, zp->z_size);
ASSERT0(P2PHASE(woff, max_blksz));
/*
* Is this block ever reached?
* We can simplify nbytes to MIN(n, max_blksz) since
* P2PHASE(woff, max_blksz) is 0, and knowing
* n >= max_blksz lets us simplify further:
*/
tx_bytes = nbytes;
ASSERT3S(nbytes, ==, max_blksz);
/*
* If this is not a full block write, but we are
* extending the file past EOF and this data starts
* block-aligned, use assign_arcbuf(). Otherwise,
* write via dmu_write().
* Thus, we're writing a full block at a block-aligned
* offset and extending the file past EOF.
*
* dmu_assign_arcbuf_by_dbuf() will directly assign the
* arc buffer to a dbuf.
*/

if (tx_bytes == max_blksz) {
error = dmu_assign_arcbuf_by_dbuf(
sa_get_db(zp->z_sa_hdl), woff, abuf, tx);
if (error != 0) {
dmu_return_arcbuf(abuf);
dmu_tx_commit(tx);
break;
}
error = dmu_assign_arcbuf_by_dbuf(
sa_get_db(zp->z_sa_hdl), woff, abuf, tx);
if (error != 0) {
dmu_return_arcbuf(abuf);
dmu_tx_commit(tx);
break;
}
ASSERT(tx_bytes <= uio->uio_resid);
uioskip(uio, tx_bytes);
ASSERT3S(nbytes, <=, uio->uio_resid);
uioskip(uio, nbytes);
tx_bytes = nbytes;
}
if (tx_bytes && zn_has_cached_data(zp) &&
!(ioflag & O_DIRECT)) {
Expand Down

0 comments on commit 85703f6

Please sign in to comment.