Commit 23420d05 authored by Brian Foster's avatar Brian Foster Committed by Dave Chinner

xfs: clean up xfs_trans_brelse()

xfs_trans_brelse() is a bit of a historical mess, similar to
xfs_buf_item_unlock(). It is unnecessarily verbose, has snippets of
commented out code, inconsistency with regard to stale items, etc.

Clean up xfs_trans_brelse() to use similar logic and flow as
xfs_buf_item_unlock() with regard to bli reference count handling.
This patch makes no functional changes, but facilitates further
refactoring of the common bli reference count handling code.
Signed-off-by: default avatarBrian Foster <bfoster@redhat.com>
Reviewed-by: default avatarDave Chinner <dchinner@redhat.com>
Signed-off-by: default avatarDave Chinner <david@fromorbit.com>
parent d9183105
...@@ -322,49 +322,40 @@ xfs_trans_read_buf_map( ...@@ -322,49 +322,40 @@ xfs_trans_read_buf_map(
} }
/* /*
* Release the buffer bp which was previously acquired with one of the * Release a buffer previously joined to the transaction. If the buffer is
* xfs_trans_... buffer allocation routines if the buffer has not * modified within this transaction, decrement the recursion count but do not
* been modified within this transaction. If the buffer is modified * release the buffer even if the count goes to 0. If the buffer is not modified
* within this transaction, do decrement the recursion count but do * within the transaction, decrement the recursion count and release the buffer
* not release the buffer even if the count goes to 0. If the buffer is not * if the recursion count goes to 0.
* modified within the transaction, decrement the recursion count and
* release the buffer if the recursion count goes to 0.
* *
* If the buffer is to be released and it was not modified before * If the buffer is to be released and it was not already dirty before this
* this transaction began, then free the buf_log_item associated with it. * transaction began, then also free the buf_log_item associated with it.
* *
* If the transaction pointer is NULL, make this just a normal * If the transaction pointer is NULL, this is a normal xfs_buf_relse() call.
* brelse() call.
*/ */
void void
xfs_trans_brelse( xfs_trans_brelse(
xfs_trans_t *tp, struct xfs_trans *tp,
xfs_buf_t *bp) struct xfs_buf *bp)
{ {
struct xfs_buf_log_item *bip; struct xfs_buf_log_item *bip = bp->b_log_item;
int freed; bool freed;
bool dirty;
/* ASSERT(bp->b_transp == tp);
* Default to a normal brelse() call if the tp is NULL.
*/ if (!tp) {
if (tp == NULL) {
ASSERT(bp->b_transp == NULL);
xfs_buf_relse(bp); xfs_buf_relse(bp);
return; return;
} }
ASSERT(bp->b_transp == tp); trace_xfs_trans_brelse(bip);
bip = bp->b_log_item;
ASSERT(bip->bli_item.li_type == XFS_LI_BUF); ASSERT(bip->bli_item.li_type == XFS_LI_BUF);
ASSERT(!(bip->bli_flags & XFS_BLI_STALE));
ASSERT(!(bip->__bli_format.blf_flags & XFS_BLF_CANCEL));
ASSERT(atomic_read(&bip->bli_refcount) > 0); ASSERT(atomic_read(&bip->bli_refcount) > 0);
trace_xfs_trans_brelse(bip);
/* /*
* If the release is just for a recursive lock, * If the release is for a recursive lookup, then decrement the count
* then decrement the count and return. * and return.
*/ */
if (bip->bli_recur > 0) { if (bip->bli_recur > 0) {
bip->bli_recur--; bip->bli_recur--;
...@@ -372,63 +363,40 @@ xfs_trans_brelse( ...@@ -372,63 +363,40 @@ xfs_trans_brelse(
} }
/* /*
* If the buffer is dirty within this transaction, we can't * If the buffer is invalidated or dirty in this transaction, we can't
* release it until we commit. * release it until we commit.
*/ */
if (test_bit(XFS_LI_DIRTY, &bip->bli_item.li_flags)) if (test_bit(XFS_LI_DIRTY, &bip->bli_item.li_flags))
return; return;
/*
* If the buffer has been invalidated, then we can't release
* it until the transaction commits to disk unless it is re-dirtied
* as part of this transaction. This prevents us from pulling
* the item from the AIL before we should.
*/
if (bip->bli_flags & XFS_BLI_STALE) if (bip->bli_flags & XFS_BLI_STALE)
return; return;
ASSERT(!(bip->bli_flags & XFS_BLI_LOGGED));
/* /*
* Free up the log item descriptor tracking the released item. * Unlink the log item from the transaction and clear the hold flag, if
* set. We wouldn't want the next user of the buffer to get confused.
*/ */
ASSERT(!(bip->bli_flags & XFS_BLI_LOGGED));
xfs_trans_del_item(&bip->bli_item); xfs_trans_del_item(&bip->bli_item);
bip->bli_flags &= ~XFS_BLI_HOLD;
/* /*
* Clear the hold flag in the buf log item if it is set. * Drop the reference to the bli. At this point, the bli must be either
* We wouldn't want the next user of the buffer to * freed or dirty (or both). If freed, there are a couple cases where we
* get confused. * are responsible to free the item. If the bli is clean, we're the last
*/ * user of it. If the fs has shut down, the bli may be dirty and AIL
if (bip->bli_flags & XFS_BLI_HOLD) { * resident, but won't ever be written back. We therefore may also need
bip->bli_flags &= ~XFS_BLI_HOLD; * to remove it from the AIL before freeing it.
}
/*
* Drop our reference to the buf log item.
*/ */
freed = atomic_dec_and_test(&bip->bli_refcount); freed = atomic_dec_and_test(&bip->bli_refcount);
dirty = bip->bli_flags & XFS_BLI_DIRTY;
/* ASSERT(freed || dirty);
* If the buf item is not tracking data in the log, then we must free it if (freed) {
* before releasing the buffer back to the free pool. bool abort = XFS_FORCED_SHUTDOWN(tp->t_mountp);
* ASSERT(abort || !test_bit(XFS_LI_IN_AIL, &bip->bli_item.li_flags));
* If the fs has shutdown and we dropped the last reference, it may fall if (abort)
* on us to release a (possibly dirty) bli if it never made it to the xfs_trans_ail_remove(&bip->bli_item, SHUTDOWN_LOG_IO_ERROR);
* AIL (e.g., the aborted unpin already happened and didn't release it if (!dirty || abort)
* due to our reference). Since we're already shutdown and need xfs_buf_item_relse(bp);
* ail_lock, just force remove from the AIL and release the bli here.
*/
if (XFS_FORCED_SHUTDOWN(tp->t_mountp) && freed) {
xfs_trans_ail_remove(&bip->bli_item, SHUTDOWN_LOG_IO_ERROR);
xfs_buf_item_relse(bp);
} else if (!(bip->bli_flags & XFS_BLI_DIRTY)) {
/***
ASSERT(bp->b_pincount == 0);
***/
ASSERT(atomic_read(&bip->bli_refcount) == 0);
ASSERT(!test_bit(XFS_LI_IN_AIL, &bip->bli_item.li_flags));
ASSERT(!(bip->bli_flags & XFS_BLI_INODE_ALLOC_BUF));
xfs_buf_item_relse(bp);
} }
bp->b_transp = NULL; bp->b_transp = NULL;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment