[PATCH v3 1/2] xfs: recheck buffer pinned status after push trylock failure

[Dave Chinner]

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 at redhat.com>
> ---
>  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 at fromorbit.com