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 _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
