On Fri, Feb 08, 2013 at 09:51:06AM -0500, Brian Foster wrote:
> The buffer pinned check and trylock sequence in xfs_buf_item_push()
> can race with an active transaction on marking the buffer pinned.
> This can result in the buffer becoming pinned and stale after the
> initial check and the trylock failure, but before the check in
> xfs_buf_trylock() that issues a log force. If the log force is
> issued from this context, a spinlock recursion occurs on xa_lock.
>
> Prepare xfs_buf_item_push() to handle the race by detecting a
> pinned buffer after the trylock failure so xfsaild issues a log
> force from a safe context. This, along with various previous fixes,
> renders the log force in xfs_buf_trylock() redundant.
>
> Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx>
> ---
> fs/xfs/xfs_buf_item.c | 9 ++++++++-
> 1 files changed, 8 insertions(+), 1 deletions(-)
>
> diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
> index 9c4c050..4e3a059 100644
> --- a/fs/xfs/xfs_buf_item.c
> +++ b/fs/xfs/xfs_buf_item.c
> @@ -469,8 +469,15 @@ xfs_buf_item_push(
>
> if (xfs_buf_ispinned(bp))
> return XFS_ITEM_PINNED;
> - if (!xfs_buf_trylock(bp))
> + if (!xfs_buf_trylock(bp)) {
> + /*
> + * Check whether we've raced with the buffer being pinned so
> + * xfsaild will pend up a log force.
"pend up": never heard that one before. ;P
Perhaps "queue up" would be better?
As it is, the comment describes what the code is doing, not why we
are doing this check. i.e. "if we just raced with a buffer being
pinned and the buffer has been marked stale, we could end up
stalling until someone else issues a log force to unpin the stale
buffer. Hence, do an optimistic check for the race condition to get
this buffer moving along quickly if we hit it..."
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
