xfs
[Top] [All Lists]

Re: [RFC PATCH 10/11] xfs: update the finobt on inode free

To: Brian Foster <bfoster@xxxxxxxxxx>
Subject: Re: [RFC PATCH 10/11] xfs: update the finobt on inode free
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Sat, 7 Sep 2013 07:24:41 +1000
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <5229BEE6.3060407@xxxxxxxxxx>
References: <1378232708-57156-1-git-send-email-bfoster@xxxxxxxxxx> <1378232708-57156-11-git-send-email-bfoster@xxxxxxxxxx> <20130905025421.GX23571@dastard> <5228AF00.7080700@xxxxxxxxxx> <20130906002847.GR12779@dastard> <5229BEE6.3060407@xxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Fri, Sep 06, 2013 at 07:39:18AM -0400, Brian Foster wrote:
> On 09/05/2013 08:28 PM, Dave Chinner wrote:
> > On Thu, Sep 05, 2013 at 12:19:12PM -0400, Brian Foster wrote:
> >> On 09/04/2013 10:54 PM, Dave Chinner wrote:
> >>> On Tue, Sep 03, 2013 at 02:25:07PM -0400, Brian Foster wrote:
> ...
> >>
> >> I think I'm parsing you after having another look at the code.
> >> xfs_inobt_lookup() remains as is and is potentially used from
> >> xfs_inobt_insert(). xfs_inobt_insert_rec() is introduced to set the
> >> cursor fields and do the insert and is used here and from
> >> xfs_inobt_insert().
> > 
> > Effectively. xfs_inobt_insert() becomes:
> > 
> >     for (each inode chunk) {
> >             xfs_inobt_lookup(cur, startino)
> >             xfs_inobt_insert_rec(cur, startino, free, free_count)
> >     }
> > 
> > And this code becomes:
> > 
> >     xfs_inobt_lookup(cur, startino);
> >     if (!found) {
> >             if (free_count == 1)
> >                     xfs_inobt_insert_rec(cur, startino, free, free_count)
> >             else
> >                     CORRUPTION
> >             goto out;
> >     }
> > 
> >> At that point, this looks close to xfs_inobt_insert(), but I think using
> >> that here would introduce a duplicate lookup.
> > 
> > Yes, it would. I think just using helpers like this is sufficient
> > for the two different cases, especially as xfs_inobt_insert() needs
> > to be able to handle multiple chunk insertion and we don't have that
> > here...
> > 
> 
> Ok, that was my thinking as well.
> 
> >> Regardless, we'll see what
> >> the whole thing looks like at that point. Thanks for the reviews. :)
> > 
> > No worries. BTW, can you post your rudimentary userspace support so
> > we can run tests that use this code, too?
> > 
> 
> Sure. My xfsprogs branch currently is the application of a slightly
> older version of this set (pre-cleanups I made to make this post-worthy)
> with some hacks to make it apply/compile and a few other patches on top
> of that for mkfs, xfs_db and xfs_repair to work through some basic
> things I ran into when running xfstests.
> 
> Would you prefer I drop the whole thing on the list?

Drop it on the list, maybe just a as tarball rather than a patchset
if it's not ready for review yet.

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

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