On Wed, Nov 29, 2017 at 09:34:45AM +1100, Dave Chinner wrote: > On Tue, Nov 28, 2017 at 10:49:22AM -0500, Brian Foster wrote: > > On Tue, Nov 28, 2017 at 10:27:19AM +1100, Dave Chinner wrote: > > > On Mon, Nov 27, 2017 at 03:24:34PM -0500, Brian Foster wrote: ... > > > Code looks fine. Comments below are for another follow-on patch. > > > > > > > Actually, what do you think of the following variant (compile tested > > only)? This facilites another cleanup patch I just wrote to kill off > > about half of the (now effectively duplicate) xfs_calc_create_*() > > helpers because the finobt and inode chunk res. helpers only include the > > associated reservation based on the associated feature bits. > > Yup, makes a lot of sense to do that. > > FWIW, because we've got so many alloc vs free type reservations, > would it make more sense to do something like: > > #define _ALLOC true > #define _FREE false > > And use those in the callers rather than true/false? > ... > > @@ -511,7 +542,7 @@ xfs_calc_ifree_reservation( > > xfs_calc_buf_res(3, mp->m_sb.sb_sectsize) + > > xfs_calc_iunlink_remove_reservation(mp) + > > xfs_calc_buf_res(1, 0) + > > - xfs_calc_buf_res(mp->m_ialloc_blks, 0) + > > + xfs_calc_inode_chunk_res(mp, false) + > > xfs_calc_inode_chunk_res(mp, _FREE) + > ..... > > That would make it very clear what type of reservation we are > acutally expecting to take out in the calculation. i.e. the code > is now clearly self documenting :) > Sure, that looks like a nice enhancement. I'll factor that in.. Brian > Cheers, > > Dave. > > -- > Dave Chinner > david@xxxxxxxxxxxxx > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
