> Fortunately for us, remote attribute values are written to disk with
> xfs_bwrite(), which is to say that they are not logged. Fix the problem
> by removing all places where we could end up creating a buffer log item
> for a remote attribute value and leave a note explaining why.
This is stil missing a comment that you are using a suitable helper
for marking the buffer stale, and why rmeoving the HOLEBLOCK check
is safe (which I now tink it is based on looking at the caller).
> - error = xfs_trans_read_buf(mp, args->trans,
> + error = xfs_trans_read_buf(mp, NULL,
> mp->m_ddev_targp,
> dblkno, dblkcnt, 0, &bp,
> &xfs_attr3_rmt_buf_ops);
> @@ -411,7 +428,7 @@ xfs_attr_rmtval_get(
> error = xfs_attr_rmtval_copyout(mp, bp, args->dp->i_ino,
> &offset, &valuelen,
> &dst);
> - xfs_trans_brelse(args->trans, bp);
> + xfs_buf_relse(bp);
FYI, I don't think mixing xfs_trans_read_buf and xfs_buf_relse is a good
pattern.
> @@ -48,8 +45,8 @@ xfs_attr3_leaf_freextent(
> * Roll through the "value", invalidating the attribute value's
> * blocks.
> */
> - tblkno = blkno;
> - tblkcnt = blkcnt;
> + tblkno = lp->valueblk;
> + tblkcnt = lp->valuelen;
Nit: these could be easily initialized on the declaration lines. Or
even better if you keep the old calling conventions of passing the
blockno and count by value, in which case we don't need the extra local
variables at all.
> @@ -174,9 +155,7 @@ xfs_attr3_leaf_inactive(
> */
> error = 0;
> for (lp = list, i = 0; i < count; i++, lp++) {
> - tmp = xfs_attr3_leaf_freextent(trans, dp,
> - lp->valueblk, lp->valuelen);
> -
> + tmp = xfs_attr3_rmt_inactive(dp, lp);
So given that we don't touch the transaction I don't think we even
need the memory allocation to defer the marking stale of the buffer
until after the xfs_trans_brelse. But that could be a separate
patch, especially if the block/count calling conventions are kept as-is.
