Skip to content

Commit 04bfe13

Browse files
djwongsmb49
authored andcommitted
xfs: clean up xfs_bui_item_recover iget/trans_alloc/ilock ordering
BugLink: https://bugs.launchpad.net/bugs/2011625 commit 64a3f33 upstream. 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> Signed-off-by: Chandan Babu R <chandan.babu@oracle.com> Acked-by: Darrick J. Wong <djwong@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Luke Nowakowski-Krijger <luke.nowakowskikrijger@canonical.com> Acked-by: Kamal Mostafa <kamal@canonical.com> Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
1 parent c6d6036 commit 04bfe13

File tree

1 file changed

+24
-14
lines changed

1 file changed

+24
-14
lines changed

fs/xfs/xfs_bmap_item.c

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "xfs_bmap_btree.h"
2323
#include "xfs_trans_space.h"
2424
#include "xfs_error.h"
25+
#include "xfs_quota.h"
2526

2627
kmem_zone_t *xfs_bui_zone;
2728
kmem_zone_t *xfs_bud_zone;
@@ -488,29 +489,34 @@ xfs_bui_recover(
488489
return -EFSCORRUPTED;
489490
}
490491

491-
error = xfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate,
492-
XFS_EXTENTADD_SPACE_RES(mp, XFS_DATA_FORK), 0, 0, &tp);
492+
/* Grab the inode. */
493+
error = xfs_iget(mp, NULL, bmap->me_owner, 0, 0, &ip);
493494
if (error)
494495
return error;
495496

496-
budp = xfs_trans_get_bud(tp, buip);
497-
498-
/* Grab the inode. */
499-
error = xfs_iget(mp, tp, bmap->me_owner, 0, XFS_ILOCK_EXCL, &ip);
497+
error = xfs_qm_dqattach(ip);
500498
if (error)
501-
goto err_inode;
499+
goto err_rele;
502500

503501
if (VFS_I(ip)->i_nlink == 0)
504502
xfs_iflags_set(ip, XFS_IRECOVERY);
505503

504+
/* Allocate transaction and do the work. */
505+
error = xfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate,
506+
XFS_EXTENTADD_SPACE_RES(mp, XFS_DATA_FORK), 0, 0, &tp);
507+
if (error)
508+
goto err_rele;
509+
510+
budp = xfs_trans_get_bud(tp, buip);
511+
xfs_ilock(ip, XFS_ILOCK_EXCL);
506512
xfs_trans_ijoin(tp, ip, 0);
507513

508514
count = bmap->me_len;
509515
error = xfs_trans_log_finish_bmap_update(tp, budp, bui_type, ip,
510516
whichfork, bmap->me_startoff, bmap->me_startblock,
511517
&count, state);
512518
if (error)
513-
goto err_inode;
519+
goto err_cancel;
514520

515521
if (count > 0) {
516522
ASSERT(bui_type == XFS_BMAP_UNMAP);
@@ -522,16 +528,20 @@ xfs_bui_recover(
522528
}
523529

524530
set_bit(XFS_BUI_RECOVERED, &buip->bui_flags);
531+
/* Commit transaction, which frees the transaction. */
525532
error = xfs_defer_ops_capture_and_commit(tp, capture_list);
533+
if (error)
534+
goto err_unlock;
535+
526536
xfs_iunlock(ip, XFS_ILOCK_EXCL);
527537
xfs_irele(ip);
528-
return error;
538+
return 0;
529539

530-
err_inode:
540+
err_cancel:
531541
xfs_trans_cancel(tp);
532-
if (ip) {
533-
xfs_iunlock(ip, XFS_ILOCK_EXCL);
534-
xfs_irele(ip);
535-
}
542+
err_unlock:
543+
xfs_iunlock(ip, XFS_ILOCK_EXCL);
544+
err_rele:
545+
xfs_irele(ip);
536546
return error;
537547
}

0 commit comments

Comments
 (0)