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 _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs