[Top] [All Lists]

Re: [RFC PATCH 03/11] xfs: support the XFS_BTNUM_FINOBT free inode btree

To: Brian Foster <bfoster@xxxxxxxxxx>
Subject: Re: [RFC PATCH 03/11] xfs: support the XFS_BTNUM_FINOBT free inode btree type
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Sat, 7 Sep 2013 07:22:20 +1000
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <5229BBC6.5000808@xxxxxxxxxx>
References: <1378232708-57156-1-git-send-email-bfoster@xxxxxxxxxx> <1378232708-57156-4-git-send-email-bfoster@xxxxxxxxxx> <20130905005428.GQ23571@dastard> <5228AE80.5050908@xxxxxxxxxx> <20130906000754.GO12779@dastard> <5229BBC6.5000808@xxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Fri, Sep 06, 2013 at 07:25:58AM -0400, Brian Foster wrote:
> On 09/05/2013 08:07 PM, Dave Chinner wrote:
> > On Thu, Sep 05, 2013 at 12:17:04PM -0400, Brian Foster wrote:
> >> On 09/04/2013 08:54 PM, Dave Chinner wrote:
> >>> On Tue, Sep 03, 2013 at 02:25:00PM -0400, Brian Foster wrote:
> ...
> >>>
> >>> What we really need here is for xfs_ialloc_log_agi to consider that
> >>> there are two distinct regions for range logging - the first spaces
> >>> from offset 0 to offset of agi_unlinked, and the second is from the
> >>> the offset of agi_free_root to the end of the xfs_agi_t....
> >>>
> >>> It's abit messy, I know, but we couldn't easily add new padding to
> >>> the AGI in the existing range logging area like was done for the AGF
> >>> because of the unlinked list hash table already defining the end of
> >>> the range logging region....
> >>>
> >>
> >> ... but where would that ever happen? The existing invocations of
> >> xfs_ialloc_log_agi() seem to log either the agi inode count values or
> >> the btree root/level values (i.e., never the range across both). I think
> >> I've introduced at least a couple new invocations throughout this set,
> >> but I've not changed that model (i.e., an XFS_AGI_FREECOUNT instance in
> >> the new lookup code and an XFS_AGI_FREE_ROOT|*_LEVEL instance in the new
> >> btree code).
> > 
> > Right, we don't current log across the range because of the way the
> > code is currently written, but there's no rule that says that
> > logging fields must be done this way.
> > 
> > I can see that there may be reason for logging
> > XFS_AGI_FREE_ROOT|*_LEVEL and XFS_AGI_NEW_INODE all in one go -
> > pointing new inode allocation at recently freed inodes is not
> > unreasonable, and if we split the finobt and update agi_newino in
> > the one update, we will log across this gap.
> > 
> For the sake of argument, it seems a little strange to me to set an
> inode level value in the agi in the context of a btree operation, such
> as a split...

Like we do with the AGF to record changes to the longest
extent in the btree? It's not a stretch to think we might update
the "allocation from here" target in the AGI when we make a specific
type of btree record change.... ;)

True, that is still an isolated logging event, but my point is that
specific btree operations may drive other logging events in the
header than just root/level...


Dave Chinner

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