Re: [PATCH 10/55] libxfs: sync dir2 kernel differences

From: Dave Chinner <david@fromorbit.com>
To: Eric Sandeen <sandeen@sandeen.net>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH 10/55] libxfs: sync dir2 kernel differences
Date: Wed, 18 Sep 2013 15:03:10 +1000	[thread overview]
Message-ID: <20130918050310.GQ19103@dastard> (raw)
In-Reply-To: <20130918033645.GN19103@dastard>

On Wed, Sep 18, 2013 at 01:36:45PM +1000, Dave Chinner wrote:
> On Tue, Sep 17, 2013 at 10:14:10AM -0500, Eric Sandeen wrote:
> > On 9/4/13 5:05 PM, Dave Chinner wrote:
> > > From: Dave Chinner <dchinner@redhat.com>
> > > 
> > > Signed-off-by: Dave Chinner <dchinner@redhat.com>
> > > ---
> > >  libxfs/xfs_dir2.c      | 45 +++++++++++++++++++++++++++++++++++++++++++++
> > >  libxfs/xfs_dir2_data.c | 20 ++++++++++----------
> > >  libxfs/xfs_dir2_leaf.c |  4 ++--
> > >  libxfs/xfs_dir2_node.c | 26 ++++++++++++--------------
> > >  4 files changed, 69 insertions(+), 26 deletions(-)
> > > 
> > > diff --git a/libxfs/xfs_dir2.c b/libxfs/xfs_dir2.c
> > > index 6a4027f..830fe3e 100644
> > > --- a/libxfs/xfs_dir2.c
> > > +++ b/libxfs/xfs_dir2.c
> > > @@ -392,6 +392,51 @@ xfs_dir_replace(
> > >  }
> > >  
> > >  /*
> > > + * See if this entry can be added to the directory without allocating space.
> > > + * First checks that the caller couldn't reserve enough space (resblks = 0).
> > > + */
> > > +int
> > > +xfs_dir_canenter(
> > 
> > Retroactive, post-merge question.  :)  This function isn't used in userspace,
> > AFAICT.  What's the intended libxfs philosophy - keep files as identical to
> > kernelspace as possible, used code or not, or remove things which aren't used
> > in userspace?
> 
> A bit of history here. Going back to when the GPL'd version of
> xfscmds was created, the intention was that libxfs would be a
> minimal, cut down version of the kernel code that only had the exact
> functionality that the userspace tools required. That lasted while
> Nathan was the maintainer - most of the changes were being made by
> SGI, and were being made to both Irix and GPL'd code bases and so
> there was significant amounts of code shuffling internally and so a
> minimal implementation made sense.
> 
> With the demise of Irix, there was much less code shuffling going
> on, and all development work moved over to Linux, leading to the
> situation where there was only one kernel code base to maintain, and
> one userspace to maintain.
> 
> When Nathan left, regular updates to the libxfs code pretty much
> stopped - there weren't a lot of people that grokked the libxfs
> codebase and so it was mostly left alone. There was a major update
> for ~2.9.13 with all the xfs repair work that was done back in 2008
> done by Barry Naujok, and since then there's only been sporadic,
> massive kernel code syncs that have involved diffs > +/-10000 lines
> that are impossible to review.
> 
> So, come the CRC work, another major sync was needed, and I decided
> to make such sycnhronisation work easy in a way that Christoph had
> mentioned a few times over the past couple of years. That is, libxfs
> basically becomes a shared copy of the code with minimal differences
> and we separate all the kernel code out into kernel-only and
> shared-libxfs code.
> 
> That's where we are now - with almost all the libxfs code synced in
> user and kernel space.
> 
> Yes, this means that there are some dead functions in userspace, but
> it means that it is trivial to spot differences between the user and
> kernel code, and that is far more important from a maintenance
> perspective that having a bit of dead code lying around.
> 
> Once this started to fall into place, I started finding lots of
> little differences that needed fixing that were hidden by the
> verbosity of previous sync efforts. Indeed, this command points out
> interesting little things like:
> 
> $ for f in `ls libxfs |grep xfs_ |grep -v trace`; do diff -u
> libxfs/$f ../kern/xfsdev/fs/xfs/$f; done |less
> ....
> @@ -612,6 +635,7 @@
>         xfs_trans_log_buf(tp, bp, 0, size - 1);
>  
>         bp->b_ops = blk1->bp->b_ops;
> +       xfs_trans_buf_copy_type(bp, blk1->bp);
>         blk1->bp = bp;
>         blk1->blkno = blkno;
>  
> Which indicate that some bug fixes from teh kernel code haven't been
> brought across.
> 
> There's still more work to do - xfs-rtalloc.c still needs splitting,
> and xfs_btree.c has some readahead differences that need to be
> sorted, but overall the idea is that libxfs and the include/xfs_*h
> files are as close to identical to the kernel code as possible....
> 
> The other advantage of this is that patches made to the kernel apply
> with a minimum of massaging to userspace, which makes keeping the
> userspace code up to data a lot easier.
> 
> I've mentioned it before - I'd like to end up with a fs/xfs/libxfs/
> directory in the kernel that is essentially identical in all ways to
> the userspace libxfs, so all we need to do is copy files to keep
> them in sync. We're not there yet, but it's definitely getting
> closer to being a reality. If we get to this stage, then getting
> userspace support up and running for new features will not require
> any kernel patch ftuzing at all - just copy the files, and start
> working on tool support...

>From the horse's mouth:

$ $ head -44 libxfs/xfs.h |tail -27

/*
 * This header is effectively a "namespace multiplexor" for the
 * user level XFS code.  It provides all of the necessary stuff
 * such that we can build some parts of the XFS kernel code in
 * user space in a controlled fashion, and translates the names
 * used in the kernel into the names which libxfs is going to
 * make available to user tools.
 *
 * It should only ever be #include'd by XFS "kernel" code being
 * compiled in user space.
 *
 * Our goals here are to...
 *      o  "share" large amounts of complex code between user and
 *         kernel space;
 *      o  shield the user tools from changes in the bleeding
 *         edge kernel code, merging source changes when
 *         convenient and not immediately (no symlinks);
 *      o  i.e. be able to merge changes to the kernel source back
 *         into the affected user tools in a controlled fashion;
 *      o  provide a _minimalist_ life-support system for kernel
 *         code in user land, not the "everything + the kitchen
 *         sink" model which libsim had mutated into;
 *      o  allow the kernel code to be completely free of code
 *         specifically there to support the user level build.
 */

$

FYI, the symlink model was what the irix source tree used to link
the XFS kernel code to the xfscmds userspace source tree.
Libsim was the "ifdef SIM" hackery all through the Irix
kernel code used to exclude kernel-only code from the xfscmds
build....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs