Skip to content

Commit

Permalink
xfs: bypass final dfops roll in trans commit path
Browse files Browse the repository at this point in the history
Once xfs_defer_finish() has completed all deferred operations, it
checks the dirty state of the transaction and rolls it once more to
return a clean transaction for the caller. This primarily to cover
the case where repeated xfs_defer_finish() calls are made in a loop
and we need to make sure that the caller starts the next iteration
with a clean transaction. Otherwise we risk transaction reservation
overrun.

This final transaction roll is not required in the transaction
commit path, however, because the transaction is immediately
committed and freed after dfops completion. Refactor the final roll
into a separate helper such that we can avoid it in the transaction
commit path.  Lift the dfops reset as well so dfops remains valid
until after the last call to xfs_defer_trans_roll(). The reset is
also unnecessary in the transaction commit path because the
transaction is about to complete.

This eliminates unnecessary regrants of transactions where the
associated transaction roll can be replaced by a transaction commit.

Signed-off-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Bill O'Donnell <billodo@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
  • Loading branch information
Brian Foster authored and Darrick J. Wong committed Jul 26, 2018
1 parent 9e28a24 commit b277c37
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 14 deletions.
38 changes: 25 additions & 13 deletions fs/xfs/libxfs/xfs_defer.c
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ xfs_defer_reset(
* If an inode is provided, relog it to the new transaction.
*/
int
xfs_defer_finish(
xfs_defer_finish_noroll(
struct xfs_trans **tp)
{
struct xfs_defer_ops *dop = (*tp)->t_dfops;
Expand Down Expand Up @@ -430,25 +430,37 @@ xfs_defer_finish(
cleanup_fn(*tp, state, error);
}

/*
* Roll the transaction once more to avoid returning to the caller
* with a dirty transaction.
*/
if ((*tp)->t_flags & XFS_TRANS_DIRTY) {
error = xfs_defer_trans_roll(tp);
dop = (*tp)->t_dfops;
}
out:
if (error) {
if (error)
trace_xfs_defer_finish_error((*tp)->t_mountp, dop, error);
} else {
else
trace_xfs_defer_finish_done((*tp)->t_mountp, dop, _RET_IP_);
xfs_defer_reset(dop);
}

return error;
}

int
xfs_defer_finish(
struct xfs_trans **tp)
{
int error;

/*
* Finish and roll the transaction once more to avoid returning to the
* caller with a dirty transaction.
*/
error = xfs_defer_finish_noroll(tp);
if (error)
return error;
if ((*tp)->t_flags & XFS_TRANS_DIRTY) {
error = xfs_defer_trans_roll(tp);
if (error)
return error;
}
xfs_defer_reset((*tp)->t_dfops);
return 0;
}

/*
* Free up any items left in the list.
*/
Expand Down
1 change: 1 addition & 0 deletions fs/xfs/libxfs/xfs_defer.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ enum xfs_defer_ops_type {

void xfs_defer_add(struct xfs_defer_ops *dop, enum xfs_defer_ops_type type,
struct list_head *h);
int xfs_defer_finish_noroll(struct xfs_trans **tp);
int xfs_defer_finish(struct xfs_trans **tp);
void __xfs_defer_cancel(struct xfs_defer_ops *dop);
void xfs_defer_init(struct xfs_trans *tp, struct xfs_defer_ops *dop);
Expand Down
2 changes: 1 addition & 1 deletion fs/xfs/xfs_trans.c
Original file line number Diff line number Diff line change
Expand Up @@ -933,7 +933,7 @@ __xfs_trans_commit(

/* finish deferred items on final commit */
if (!regrant && tp->t_dfops) {
error = xfs_defer_finish(&tp);
error = xfs_defer_finish_noroll(&tp);
if (error) {
xfs_defer_cancel(tp);
goto out_unreserve;
Expand Down

0 comments on commit b277c37

Please sign in to comment.