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.
|