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....
> >+ 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.
--
Dave Chinner
Principal Engineer
SGI Australian Software Group
|