[Top] [All Lists]

Re: [PATCH 19/27] xfs: consolidate xfs_vnodeops.c into xfs_inode_ops.c

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 19/27] xfs: consolidate xfs_vnodeops.c into xfs_inode_ops.c
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Wed, 19 Jun 2013 06:55:23 +1000
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20130617160228.GB26043@xxxxxxxxxxxxx>
References: <1371032567-21772-1-git-send-email-david@xxxxxxxxxxxxx> <1371032567-21772-20-git-send-email-david@xxxxxxxxxxxxx> <20130612135916.GA28988@xxxxxxxxxxxxx> <20130613013907.GC29338@dastard> <20130617160228.GB26043@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Mon, Jun 17, 2013 at 09:02:28AM -0700, Christoph Hellwig wrote:
> On Thu, Jun 13, 2013 at 11:39:07AM +1000, Dave Chinner wrote:
> > i.e:
> > 
> >     kernel only -> kernel only -> shared with libxfs
> > 
> > Once there's a clear boundary in the kernel code,
> > merging/simplifying stuff on either side of the boundary is easy to
> > do, but it's a secondary step. I'd consider this something for the
> > 3.12 cycle, not the upcoming 3.11 cycle.
> Much better.
> Btw, especially in inode land we should probably share even less code
> with userspace.  Besides my old patches to kill the struct xfs_inode
> abuse in repair phase 7 there isn't too much use of it left.

Yup - we only need to shared the format defintions, the buffer
operations and the inode fork macros. That's explicity all that is
shared after my current patchset.

FWIW, my current patchset does this to the xfsprogs/include

43 files changed, 1541 insertions(+), 3527 deletions(-)

So that will give you an idea of how much __KERNEL__ crap there

> The db code uses inodes as temporary objects in the attrset code which
> looks everything but efficient, and the mkfs and repair code use it to
> create new files/directories.  Not quite sure bringing in all the kernel
> code there makes so much sense given that we have other infrastructure
> for a lot of these in userspace anyway.

There's plenty that can be done there, but I think the direction we
need to take db in is to be closer to libxfs than it's own special
little snowflake. We need to do this for the IO subsystem to add CRC
validation/calculation support, so re-using anything that libxfs
already provides seems sane to me...

> > FWIW, now that I have the shared files in almost perfect sync, it
> > makes it much easier to make the change in the kernel tree, take the
> > patch, apply a couple of sed filters to it and then apply it
> > directly to the xfsprogs tree. I only have to do the work once now,
> > and so it's much faster to restructure the code now.
> I'm very happy to see this as I'd been aiming at it for a while.
> Any chance you can add a script to pull in kernel changes automatically
> while you're at it?

It's not quite there yet, but getting close. I'm not sure we can get
to automating it without having the same libxfs/ structure in the
kernel code - it's not straight forward to identify which hunks of a
patch are relevant to userspace...


Dave Chinner

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