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

[Dave Chinner <david@xxxxxxxxxxxxx>]

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@xxxxxxxxxxxxx