Re: deadlocked xfs

[Christoph Hellwig <hch@infradead.org>]

On Fri, Jul 11, 2008 at 02:04:47PM +1000, Dave Chinner wrote:
>  		xlog_state_switch_iclogs(log, iclog, iclog->ic_size);
>  
> -		/* If I'm the only one writing to this iclog, sync it to disk */
> -		if (atomic_read(&iclog->ic_refcnt) == 1) {
> +		/*
> +		 * If I'm the only one writing to this iclog, sync it to disk.
> +		 * We need to do an atomic compare and decrement here to avoid
> +		 * racing with concurrent atomic_dec_and_lock() calls in
> +		 * xlog_state_release_iclog() when there is more than one
> +		 * reference to the iclog.
> +		 */
> +		if (atomic_add_unless(&iclog->ic_refcnt, -1, 1)) {
>  			spin_unlock(&log->l_icloglock);
> -			if ((error = xlog_state_release_iclog(log, iclog)))
> +			error = xlog_state_release_iclog(log, iclog);
> +			if (error)
>  				return error;
>  		} else {
> -			atomic_dec(&iclog->ic_refcnt);
>  			spin_unlock(&log->l_icloglock);
>  		}
>  		goto restart;

Independent ofwether it actually fixes the bug (which I think it will)
this looks good.  Doing anything with the return value from atomic_read
except for printing it is most likely bogus, and this one clearly is.