xfs
[Top] [All Lists]

Re: [PATCH 22/26] move xfs_bmbt_killroot to common code

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 22/26] move xfs_bmbt_killroot to common code
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Tue, 5 Aug 2008 12:08:56 +1000
Cc: Christoph Hellwig <hch@xxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <20080805012630.GA22147@xxxxxxxxxxxxx>
Mail-followup-to: Christoph Hellwig <hch@xxxxxxxxxxxxx>, Christoph Hellwig <hch@xxxxxx>, xfs@xxxxxxxxxxx
References: <20080804013542.GW8819@xxxxxx> <20080805011437.GM6119@disturbed> <20080805012630.GA22147@xxxxxxxxxxxxx>
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Mutt/1.5.18 (2008-05-17)
On Mon, Aug 04, 2008 at 09:26:30PM -0400, Christoph Hellwig wrote:
> On Tue, Aug 05, 2008 at 11:14:37AM +1000, Dave Chinner wrote:
> > On Mon, Aug 04, 2008 at 03:35:42AM +0200, Christoph Hellwig wrote:
> > > xfs_bmbt_killroot is a mostly generic implementation of moving from
> > > a real block based root to an inode based root.  So move it to xfs_btree.c
> > > where it can use all the nice infrastructure there and make it pointer
> > > size agnostic
> > > 
> > > The new name for it is xfs_btree_root_to_iroot which is not very
> > > nice but at least slightly more descriptive than the old name.
> > .....
> > > +
> > > + XFS_BTREE_TRACE_CURSOR(cur, XBT_ENTRY);
> > > + level = cur->bc_nlevels - 1;
> > > + ASSERT(level >= 1);
> > 
> > probably should assert root in inode is set here.
> 
> We have that check just before the call, so this might be a little
> overkill..

Fair enough.

> > > + // XXX(hch): this assert is bmap btree specific
> > > +         ASSERT(cur->bc_ops->get_maxrecs(cur, level) ==
> >    ^
> > > +        XFS_BMAP_BROOT_MAXRECS(ifp->if_broot_bytes));
> > 
> > Stray whitespace. As to the assert - what is it really trying to
> > check? That the btree root space in the inode is large enough to
> > fit the max number of records? If so, does it really need to be
> > checked here (i.e. could the caller do it?)
> 
> I don't think it makes much sense at all.  My preference would
> be to simply kill it.

Agreed. Kill it - I don't think it adds any value.

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx


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