Re: [patch] Use atomics for iclog reference counting

[Timothy Shimmin <tes@sgi.com>]

David Chinner wrote:
> On Fri, Feb 15, 2008 at 02:34:53PM +1100, Timothy Shimmin wrote:
>> In the 1st 2 cases of the patch:
>>> @@ -675,7 +675,7 @@ xfs_log_unmount_write(xfs_mount_t *mp)
>>>
>>>  		spin_lock(&log->l_icloglock);
>>>  		iclog = log->l_iclog;
>>> -		iclog->ic_refcnt++;
>>> +		atomic_inc(&iclog->ic_refcnt);
>>>  		spin_unlock(&log->l_icloglock);
>>>  		xlog_state_want_sync(log, iclog);
>>>  		(void) xlog_state_release_iclog(log, iclog);
>>> @@ -713,7 +713,7 @@ xfs_log_unmount_write(xfs_mount_t *mp)
>>>  		 */
>>>  		spin_lock(&log->l_icloglock);
>>>  		iclog = log->l_iclog;
>>> -		iclog->ic_refcnt++;
>>> +		atomic_inc(&iclog->ic_refcnt);
>>>  		spin_unlock(&log->l_icloglock);
>> Do we still really need to take the lock etc?
> 
> log->iclog is protected by the l_icloglock as well, 

Ah, yep :)

> so the lock
> needs to be retained to prevent races when reading and taking a
> reference to it. IOWs, the l_icloglock still synchronises increments
> and the final decrement on an iclog; we only need the atomic counter
> to enable unlocked refcount decrements when the refcount is > 1.
>  
Yep.

--Tim