Skip to content

Commit f94e08b

Browse files
author
Darrick J. Wong
committed
xfs: clean up the end of xfs_attri_item_recover
The end of this function could use some cleanup -- the EAGAIN conditionals make it harder to figure out what's going on with the disposal of xattri_leaf_bp, and the dual error/ret variables aren't needed. Turn the EAGAIN case into a separate block documenting all the subtleties of recovering in the middle of an xattr update chain, which makes the rest of the prologue much simpler. Signed-off-by: Darrick J. Wong <[email protected]> Reviewed-by: Dave Chinner <[email protected]>
1 parent b822ea1 commit f94e08b

File tree

1 file changed

+26
-19
lines changed

1 file changed

+26
-19
lines changed

fs/xfs/xfs_attr_item.c

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -582,7 +582,7 @@ xfs_attri_item_recover(
582582
struct xfs_trans_res tres;
583583
struct xfs_attri_log_format *attrp;
584584
struct xfs_attri_log_nameval *nv = attrip->attri_nameval;
585-
int error, ret = 0;
585+
int error;
586586
int total;
587587
int local;
588588
struct xfs_attrd_log_item *done_item = NULL;
@@ -661,13 +661,31 @@ xfs_attri_item_recover(
661661
xfs_ilock(ip, XFS_ILOCK_EXCL);
662662
xfs_trans_ijoin(tp, ip, 0);
663663

664-
ret = xfs_xattri_finish_update(attr, done_item);
665-
if (ret == -EAGAIN) {
666-
/* There's more work to do, so add it to this transaction */
664+
error = xfs_xattri_finish_update(attr, done_item);
665+
if (error == -EAGAIN) {
666+
/*
667+
* There's more work to do, so add the intent item to this
668+
* transaction so that we can continue it later.
669+
*/
667670
xfs_defer_add(tp, XFS_DEFER_OPS_TYPE_ATTR, &attr->xattri_list);
668-
} else
669-
error = ret;
671+
error = xfs_defer_ops_capture_and_commit(tp, capture_list);
672+
if (error)
673+
goto out_unlock;
674+
675+
/*
676+
* The defer capture structure took its own reference to the
677+
* attr leaf buffer and will give that to the continuation
678+
* transaction. The attr intent struct drives the continuation
679+
* work, so release our refcount on the attr leaf buffer but
680+
* retain the pointer in the intent structure.
681+
*/
682+
if (attr->xattri_leaf_bp)
683+
xfs_buf_relse(attr->xattri_leaf_bp);
670684

685+
xfs_iunlock(ip, XFS_ILOCK_EXCL);
686+
xfs_irele(ip);
687+
return 0;
688+
}
671689
if (error) {
672690
xfs_trans_cancel(tp);
673691
goto out_unlock;
@@ -678,24 +696,13 @@ xfs_attri_item_recover(
678696
out_unlock:
679697
if (attr->xattri_leaf_bp) {
680698
xfs_buf_relse(attr->xattri_leaf_bp);
681-
682-
/*
683-
* If there's more work to do to complete the attr intent, the
684-
* defer capture structure will have taken its own reference to
685-
* the attr leaf buffer and will give that to the continuation
686-
* transaction. The attr intent struct drives the continuation
687-
* work, so release our refcount on the attr leaf buffer but
688-
* retain the pointer in the intent structure.
689-
*/
690-
if (ret != -EAGAIN)
691-
attr->xattri_leaf_bp = NULL;
699+
attr->xattri_leaf_bp = NULL;
692700
}
693701

694702
xfs_iunlock(ip, XFS_ILOCK_EXCL);
695703
xfs_irele(ip);
696704
out:
697-
if (ret != -EAGAIN)
698-
xfs_attr_free_item(attr);
705+
xfs_attr_free_item(attr);
699706
return error;
700707
}
701708

0 commit comments

Comments
 (0)