[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: Thu, 13 Jun 2013 11:39:07 +1000
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20130612135916.GA28988@xxxxxxxxxxxxx>
References: <1371032567-21772-1-git-send-email-david@xxxxxxxxxxxxx> <1371032567-21772-20-git-send-email-david@xxxxxxxxxxxxx> <20130612135916.GA28988@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Wed, Jun 12, 2013 at 06:59:16AM -0700, Christoph Hellwig wrote:
> I can't say I really like the _ops naming, but so far I can't really
> can't come up with a much better name.

Neither could I :(

> I also think that most of
> xfs_vnodeops.c really should go into xfs_iops.c as it's fairly Linux
> specific and should just be merged with the actual entry points.

Yes, that's definitely a good end goal (merging with entry points),
but what I'm trying to do now is create a solid obvious demarcation
line between kernel-only and shared source files. The current code

        xfs_iops.c ->  xfs_inode.c              (shared with libxfs)
                   ->  xfs_vnodeops.c           (kernel only)
                   ->  xfs_rename.c             (kernel only)
                   ->  xfs_util.c               (kernel only)

And there's a tricky maze of paths through those files for different
locking operations, inode allocation, and so on. The real issue is
that the code paths hop between shared and kernel only files, so
what I'm wanting to end up with for 3.11 is a linear progression
through the source files like this:

        xfs_iops.c -> xfs_inode_ops.c -> xfs_inode.c
                                      -> xfs_ialloc.c


        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.

FWIW, I seriously considering introducing a fs/xfs/libxfs/ subdir
for the kernel build to make this a very obvious boundary and place
all the shared headers in include/linux/uapi/xfs. This would make is
extremely obvious what code is shared with userspace, and make
future kernel/user srouce code syncs quite simple....

> Also once the big code move around starts xfs_rename.c really should
> die and rename should be treated the same as the other namespace
> operations.

Yup, I haven't got to xfs_rename.c and xfs_utils.c yet - I want them
to go away, too. :) I'm just going to pull them directly into
xfs_inode_ops.c to give a consistent structure for 3.11.

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.


Dave Chinner

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