xfs
[Top] [All Lists]

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

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 2/7] XFS: replace fixed size busy extent array with an rbtree
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Thu, 9 Oct 2008 11:06:59 +1100
Cc: xfs@xxxxxxxxxxx
In-reply-to: <20081008184928.GB6823@infradead.org>
Mail-followup-to: Christoph Hellwig <hch@xxxxxxxxxxxxx>, xfs@xxxxxxxxxxx
References: <1223417377-8679-1-git-send-email-david@fromorbit.com> <1223417377-8679-3-git-send-email-david@fromorbit.com> <20081008184928.GB6823@infradead.org>
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Mutt/1.5.18 (2008-05-17)
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@xxxxxxxxxxxxx


<Prev in Thread] Current Thread [Next in Thread>