[Top] [All Lists]

Re: [RFC PATCH 04/11] xfs: update inode allocation transaction reservati

To: Brian Foster <bfoster@xxxxxxxxxx>
Subject: Re: [RFC PATCH 04/11] xfs: update inode allocation transaction reservations for finobt
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Fri, 6 Sep 2013 10:11:22 +1000
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <5228AE93.2050001@xxxxxxxxxx>
References: <1378232708-57156-1-git-send-email-bfoster@xxxxxxxxxx> <1378232708-57156-5-git-send-email-bfoster@xxxxxxxxxx> <20130905005946.GR23571@dastard> <5228AE93.2050001@xxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
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"...


Dave Chinner

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