Skip to content

Commit 64a3f33

Browse files
committed
xfs: clean up xfs_bui_item_recover iget/trans_alloc/ilock ordering
In most places in XFS, we have a specific order in which we gather resources: grab the inode, allocate a transaction, then lock the inode. xfs_bui_item_recover doesn't do it in that order, so fix it to be more consistent. This also makes the error bailout code a bit less weird. Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by: Dave Chinner <dchinner@redhat.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Brian Foster <bfoster@redhat.com>
1 parent 919522e commit 64a3f33

File tree

1 file changed

+23
-18
lines changed

1 file changed

+23
-18
lines changed

fs/xfs/xfs_bmap_item.c

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -475,33 +475,34 @@ xfs_bui_item_recover(
475475
(bmap->me_flags & ~XFS_BMAP_EXTENT_FLAGS))
476476
return -EFSCORRUPTED;
477477

478-
error = xfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate,
479-
XFS_EXTENTADD_SPACE_RES(mp, XFS_DATA_FORK), 0, 0, &tp);
480-
if (error)
481-
return error;
482-
483-
budp = xfs_trans_get_bud(tp, buip);
484-
485478
/* Grab the inode. */
486-
error = xfs_iget(mp, tp, bmap->me_owner, 0, XFS_ILOCK_EXCL, &ip);
479+
error = xfs_iget(mp, NULL, bmap->me_owner, 0, 0, &ip);
487480
if (error)
488-
goto err_inode;
481+
return error;
489482

490-
error = xfs_qm_dqattach_locked(ip, false);
483+
error = xfs_qm_dqattach(ip);
491484
if (error)
492-
goto err_inode;
485+
goto err_rele;
493486

494487
if (VFS_I(ip)->i_nlink == 0)
495488
xfs_iflags_set(ip, XFS_IRECOVERY);
496489

490+
/* Allocate transaction and do the work. */
491+
error = xfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate,
492+
XFS_EXTENTADD_SPACE_RES(mp, XFS_DATA_FORK), 0, 0, &tp);
493+
if (error)
494+
goto err_rele;
495+
496+
budp = xfs_trans_get_bud(tp, buip);
497+
xfs_ilock(ip, XFS_ILOCK_EXCL);
497498
xfs_trans_ijoin(tp, ip, 0);
498499

499500
count = bmap->me_len;
500501
error = xfs_trans_log_finish_bmap_update(tp, budp, bui_type, ip,
501502
whichfork, bmap->me_startoff, bmap->me_startblock,
502503
&count, state);
503504
if (error)
504-
goto err_inode;
505+
goto err_cancel;
505506

506507
if (count > 0) {
507508
ASSERT(bui_type == XFS_BMAP_UNMAP);
@@ -512,17 +513,21 @@ xfs_bui_item_recover(
512513
xfs_bmap_unmap_extent(tp, ip, &irec);
513514
}
514515

516+
/* Commit transaction, which frees the transaction. */
515517
error = xfs_defer_ops_capture_and_commit(tp, capture_list);
518+
if (error)
519+
goto err_unlock;
520+
516521
xfs_iunlock(ip, XFS_ILOCK_EXCL);
517522
xfs_irele(ip);
518-
return error;
523+
return 0;
519524

520-
err_inode:
525+
err_cancel:
521526
xfs_trans_cancel(tp);
522-
if (ip) {
523-
xfs_iunlock(ip, XFS_ILOCK_EXCL);
524-
xfs_irele(ip);
525-
}
527+
err_unlock:
528+
xfs_iunlock(ip, XFS_ILOCK_EXCL);
529+
err_rele:
530+
xfs_irele(ip);
526531
return error;
527532
}
528533

0 commit comments

Comments
 (0)