On Thu, Sep 05, 2013 at 12:17:23PM -0400, Brian Foster wrote: > On 09/04/2013 08:59 PM, Dave Chinner wrote: > > On Tue, Sep 03, 2013 at 02:25:01PM -0400, Brian Foster wrote: > >> Update inode allocation transaction reservations for the finobt. A > >> create via record modification requires a log reservation for the > >> additional finobt record. Any such allocation could result in an > >> finobt removal if the inode chunk has become fully allocated, thus > >> we include a reservation for a finobt btree merge as well. > >> Allocation of a new inode chunk must account for splits in the > >> finobt as well as the existing ialloc tree. > > > > These transaction reservation changes are only necessary for > > filesystems with free inode btrees, otherwise they just use more log > > space than is necessary. > > > > Yeah, good point. > > > Can you add helper functions for the free inode btree reservations, > > and have them return 0 when the feature is not enabled? That way the > > code stays pretty clean, is self documenting and doesn't take > > unnecessary space when the feature is not enabled.... > > > > So not new functions that duplicate the entire calculations for the > finobt enabled cases, but new functions that define the additional > requirements for the finobt on top of the existing reservation > calculations for particular operations (i.e., similar to the recent > patch to fix the inode log size, if I recall). Sounds good. That's exactly what I'm thinking - just like the xfs_calc_inode_res() helper. This really helps document the transaction reservation calculations - the comments help, but they are no substitute for being able to say "*that line* is what reserves space for the inodes modified in the transaction"... Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
