xfs
[Top] [All Lists]

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

To: Brian Foster <bfoster@xxxxxxxxxx>
Subject: Re: [PATCH v3 06/11] xfs: use and update the finobt on inode allocation
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Thu, 20 Feb 2014 11:38:50 +1100
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <52FE7629.9090208@xxxxxxxxxx>
References: <1391536182-9048-1-git-send-email-bfoster@xxxxxxxxxx> <1391536182-9048-7-git-send-email-bfoster@xxxxxxxxxx> <20140211071703.GG13647@dastard> <52FE7629.9090208@xxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
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@xxxxxxxxxxxxx

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