Re: [PATCH v3 06/11] xfs: use and update the finobt on inode allocation

[Dave Chinner <david@fromorbit.com>]

On Fri, Feb 14, 2014 at 03:01:45PM -0500, Brian Foster wrote:
> On 02/11/2014 02:17 AM, Dave Chinner wrote:
> > On Tue, Feb 04, 2014 at 12:49:37PM -0500, Brian Foster wrote:
> ...
> >> +	/*
> >> +	 * Lookup and modify the equivalent record in the inobt.
> >> +	 */
> >> +	tcur = xfs_inobt_init_cursor(mp, tp, agbp, agno, XFS_BTNUM_INO);
> > 
> > In case ou hadn't guessed, I don't like the "tcur/trec" variables
> > because they make me thing "temporary" which they aren't. In this
> > case it is the inobt cursor and record....
> > 
> > In fact, this whole segment could be factored into a function like
> > xfs_dialloc_ag_inobt_update(), hence removing the second cursor from
> > xfs_dialloc_ag() function altogether and that would clean a lot of
> > the logic up....
> > 
> ...
> >> +	/*
> >> +	 * Update the perag and superblock.
> >> +	 */
> >> +	be32_add_cpu(&agi->agi_freecount, -1);
> >> +	xfs_ialloc_log_agi(tp, agbp, XFS_AGI_FREECOUNT);
> >> +	pag->pagi_freecount--;
> >> +
> >> +	xfs_trans_mod_sb(tp, XFS_TRANS_SB_IFREE, -1);
> > 
> > This will need to be done before you update the inobt, though, so
> > you can run the xfs_check_agi_freecount() count in it and it will
> > come out correct....
> > 
> 
> After cleaning up some of this code and taking a closer look, I end up
> with something just short of complete removal of the inobt cursor in
> this function. Reason being... the point above with regard to checking
> the btrees against the agi freecount pre and post modification.

I can't really comment all that well without having seen the
factored code you've written....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs