Re: [PATCH 5/7] XFS: Make use of the init-once slab optimisation.

[Dave Chinner <david@fromorbit.com>]

On Thu, Aug 14, 2008 at 03:00:01PM -0400, Christoph Hellwig wrote:
> On Thu, Aug 14, 2008 at 05:14:41PM +1000, Dave Chinner wrote:
> > To avoid having to initialise some fields of the XFS inode
> > on every allocation, we can use the slab init-once feature
> > to initialise them. All we have to guarantee is that when
> > we free the inode, all it's entries are in the initial state.
> > Add asserts where possible to ensure debug kernels check this
> > initial state before freeing and after allocation.
> 
> Looks good, and I think it this can be moved before the series and
> submitted ASAP.

I'll reorder it....

> > diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
> > index cf6754a..4f4c939 100644
> > --- a/fs/xfs/xfs_itable.c
> > +++ b/fs/xfs/xfs_itable.c
> > @@ -594,21 +594,21 @@ xfs_bulkstat(
> >  						/*
> >  						 * Get the inode cluster buffer
> >  						 */
> > -						ASSERT(xfs_inode_zone != NULL);
> > -						ip = kmem_zone_zalloc(xfs_inode_zone,
> > -								      KM_SLEEP);
> > -						ip->i_ino = ino;
> > -						ip->i_mount = mp;
> > -						spin_lock_init(&ip->i_flags_lock);
> >  						if (bp)
> >  							xfs_buf_relse(bp);
> > +						ip = xfs_inode_alloc(mp, ino);
> > +						if (!ip) {
> > +							bp = NULL;
> > +							rval = ENOMEM;
> > +							break;
> > +						}
> >  						error = xfs_itobp(mp, NULL, ip,
> >  								&dip, &bp, bno,
> >  								XFS_IMAP_BULKSTAT,
> >  								XFS_BUF_LOCK);
> 
> Yikes, what a mess - eventually we should convert this to opencoded cals
> to xfs_imap and xfs_imap_to_bp, but before the function needs to be
> split into manageable chunk.  Another item on the todo list..

Yeah, bulkstat is an utter, utter mess. There's a lot of work needed
to clean it up...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com