xfs
[Top] [All Lists]

Re: deadlocked xfs

To: Timothy Shimmin <tes@xxxxxxx>, Eric Sandeen <sandeen@xxxxxxxxxxx>, markgw@xxxxxxx, xfs-oss <xfs@xxxxxxxxxxx>
Subject: Re: deadlocked xfs
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Fri, 11 Jul 2008 00:17:36 -0400
In-reply-to: <20080711040447.GC11558@disturbed>
References: <4876C667.608@sandeen.net> <4876C9EB.7060601@sgi.com> <4876D1C3.4000006@sgi.com> <20080711040447.GC11558@disturbed>
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Mutt/1.5.18 (2008-05-17)
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.


<Prev in Thread] Current Thread [Next in Thread>