Re: [Patch] Per iclog callback chain lock

[Lachlan McIlroy <lachlan@sgi.com>]

David Chinner wrote:
> On Wed, Apr 02, 2008 at 03:09:31PM +1000, Lachlan McIlroy wrote:
>> Looks fine to me - just one small comment.
> .....
>>> @@ -1257,6 +1255,8 @@ xlog_alloc_log(xfs_mount_t	*mp,
>>> 		iclog->ic_size = XFS_BUF_SIZE(bp) - log->l_iclog_hsize;
>>> 		iclog->ic_state = XLOG_STATE_ACTIVE;
>>> 		iclog->ic_log = log;
>>> +		atomic_set(&iclog->ic_refcnt, 0);
>> Not related to this change but looks like we need it anyway.
>> Did you mean to include it in this change?
> 
> I added it just to be explicit. It's not actually needed as we
> kmem_zalloc() the log structure....
Ahh, it is zeroed.  I was wondering why it hadn't caused any problems.

> 
>>> +			spin_lock(&iclog->ic_callback_lock);
>>> +			cb = iclog->ic_callback;
> ....
>>> +			spin_lock(&log->l_icloglock);
>>> 			ASSERT(iclog->ic_callback == NULL);
>>> +			spin_unlock(&iclog->ic_callback_lock);
>> Okay so we can acquire the l_icloglock while holding ic_callback_lock.
> 
> Yup - it's a new lock, so we can make whatever rules we like ;)
> 
> Cheers,
> 
> Dave.