Re: [PATCH 2/7] XFS: replace fixed size busy extent array with an rbtree

[Dave Chinner <david@fromorbit.com>]

On Wed, Oct 08, 2008 at 02:49:28PM -0400, Christoph Hellwig wrote:
> > @@ -131,14 +131,8 @@ STATIC void
> >  xfs_free_perag(
> >  	xfs_mount_t	*mp)
> >  {
> > -	if (mp->m_perag) {
> > -		int	agno;
> > -
> > -		for (agno = 0; agno < mp->m_maxagi; agno++)
> > -			if (mp->m_perag[agno].pagb_list)
> > -				kmem_free(mp->m_perag[agno].pagb_list);
> > +	if (mp->m_perag)
> >  		kmem_free(mp->m_perag);
> > -	}
> 
> kmem_free(NULL) is fine, so no need for the if.  And with that there's
> no need for this one-line wrapper and we can just do the free in the
> caller.

Ok, I'll do that.

> >  typedef struct xfs_log_busy_slot {
> > -	xfs_agnumber_t		lbc_ag;
> > -	ushort			lbc_idx;	/* index in perag.busy[] */
> > +	struct xfs_busy_extent	*lbc_busyp;
> >  } xfs_log_busy_slot_t;
> 
> Just use xfs_busy_extent directly - there's only about a handful places
> using xfs_log_busy_slot anyway.
> 
> >  xfs_log_busy_slot_t *
> > -xfs_trans_add_busy(xfs_trans_t *tp, xfs_agnumber_t ag, xfs_extlen_t idx)
> > +xfs_trans_add_busy(
> > +	xfs_trans_t		*tp,
> > +	struct xfs_busy_extent	*busyp)
> 
> And this one can lose it's return value.  It's always the second
> argmument and ignored by all callers anyway.

I'm not concerned about this as the busy slot stuff in the struct
xfs_trans gets removed in the last patch of the series. I'm
half-tempted to integrate this one with the initial rbtree patch
as all that intermediate stuffing around just goes away.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com