All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH 18/27] xfs: split out xfs inode operations into separate file
Date: Thu, 13 Jun 2013 18:00:09 +1000	[thread overview]
Message-ID: <20130613080009.GL29338@dastard> (raw)
In-Reply-To: <20130613011414.GB29338@dastard>

On Thu, Jun 13, 2013 at 11:14:14AM +1000, Dave Chinner wrote:
> On Wed, Jun 12, 2013 at 07:05:19AM -0700, Christoph Hellwig wrote:
> > On Wed, Jun 12, 2013 at 08:22:38PM +1000, Dave Chinner wrote:
> > > From: Dave Chinner <dchinner@redhat.com>
> > > 
> > > The core xfs inode operations such as locking, allocation, reading
> > > and writing are not shared with userspace. However, much of the
> > > remaining internal inode code such as the incore extent tracking and
> > > formatting is. Split the XFS inode operations out into a new
> > > xfs_inode_iops.c file to minimise the differences between the files
> > > shred with userspace.
> > 
> > Having struct xfs_inode in an xfs_inode_ops.h file while xfs_inode.h
> > is still around strikes me as extremely odd.
> 
> Consider it a first step in a longer process....
> 
> > It seems like icdinode
> > should be in xfs_inode_item.h as is part of the log format primarily.
> 
> Yes, I've definitely been looking at that, because xfs_inode_item.h is
> shared with userspace and there are some issues with cyclic
> inclusion and definitions in userspace based around the fact that
> the icdinode is defined in xfs_inode.h and so needs to be included
> before the struct xfs_inode is defined in include/libxfs.h. Yet
> there is static inline code in xfs_inode.h that needs to know the
> structure of the struct xfs_inode....

And this is where it gets sticky. The xfs_inode_ops.h header has a
dependency on struct xfs_icdinode. Moving that to xfs_inode_item.h
means that we now have a dependencies of:

	xfs_inode_ops.h requires
		xfs_inode_item.h requires:
			xfs_trans.h requires:
				xfs_log.h

which are new. And it also introduces a circular dependency as
xfs_inode_item.h has a dependency on the struct xfs_inode already
being defined, so it can't be included before struct xfs_inode is
defined.....

As it is, xfs_inode_item.h still has a section under #ifdef
__KERNEL__. It's the section dealing with the struct inode_log_item,
and all the other logitem header files have similar kernel only
sections.  libxfs has it's own definitions for these structures, so
what we've got here is the shared on-disk log format definitions and
kernel-only log item definitions sharing the same files in the
kernel tree.

So what I really think needs to happen here first is similar to the
dir2 header file re-org. That is, a header file to define the
format, and a header file to define the in-kernel structures and
APIs....

The header files in XFS have never been properly sorted out - it's
about time we cleaned up this mess ;)

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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

  reply	other threads:[~2013-06-13  8:00 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-12 10:22 [PATCH 00/27] xfs: current patch queue for 3.11 Dave Chinner
2013-06-12 10:22 ` [PATCH 01/27] xfs: update mount options documentation Dave Chinner
2013-06-13 13:34   ` Eric Sandeen
2013-06-14  0:40     ` Dave Chinner
2013-06-14  0:53       ` Eric Sandeen
2013-06-12 10:22 ` [PATCH 02/27] xfs: add pluging for bulkstat readahead Dave Chinner
2013-06-12 10:22 ` [PATCH 03/27] xfs: plug directory buffer readahead Dave Chinner
2013-06-12 10:22 ` [PATCH 04/27] xfs: don't use speculative prealloc for small files Dave Chinner
2013-06-12 16:10   ` Brian Foster
2013-06-13  0:50     ` Dave Chinner
2013-06-12 10:22 ` [PATCH 05/27] xfs: don't do IO when creating an new inode Dave Chinner
2013-06-12 10:22 ` [PATCH 06/27] xfs: xfs_ifree doesn't need to modify the inode buffer Dave Chinner
2013-06-12 10:22 ` [PATCH 07/27] xfs: Introduce ordered log vector support Dave Chinner
2013-06-12 10:22 ` [PATCH 08/27] xfs: Introduce an ordered buffer item Dave Chinner
2013-06-12 10:22 ` [PATCH 09/27] xfs: Inode create log items Dave Chinner
2013-06-12 10:22 ` [PATCH 10/27] xfs: Inode create transaction reservations Dave Chinner
2013-06-12 10:22 ` [PATCH 11/27] xfs: Inode create item recovery Dave Chinner
2013-06-12 10:22 ` [PATCH 12/27] xfs: Use inode create transaction Dave Chinner
2013-06-12 10:22 ` [PATCH 13/27] xfs: remove local fork format handling from xfs_bmapi_write() Dave Chinner
2013-06-12 10:22 ` [PATCH 14/27] xfs: move getdents code into it's own file Dave Chinner
2013-06-12 10:22 ` [PATCH 15/27] xfs: reshuffle dir2 definitions around for userspace Dave Chinner
2013-06-17 16:05   ` Christoph Hellwig
2013-06-18 21:12     ` Dave Chinner
2013-06-18 21:35       ` Dave Chinner
2013-06-12 10:22 ` [PATCH 16/27] xfs: split out attribute listing code into separate file Dave Chinner
2013-06-12 10:22 ` [PATCH 17/27] xfs: split out attribute fork truncation " Dave Chinner
2013-06-12 10:22 ` [PATCH 18/27] xfs: split out xfs inode operations " Dave Chinner
2013-06-12 14:05   ` Christoph Hellwig
2013-06-13  1:14     ` Dave Chinner
2013-06-13  8:00       ` Dave Chinner [this message]
2013-06-17 15:56         ` Christoph Hellwig
2013-06-17 18:14           ` Ben Myers
2013-06-18 20:40             ` Dave Chinner
2013-06-18 21:37               ` Ben Myers
2013-06-18 22:02                 ` Dave Chinner
2013-06-12 10:22 ` [PATCH 19/27] xfs: consolidate xfs_vnodeops.c into xfs_inode_ops.c Dave Chinner
2013-06-12 13:59   ` Christoph Hellwig
2013-06-13  1:39     ` Dave Chinner
2013-06-17 16:02       ` Christoph Hellwig
2013-06-18 20:55         ` Dave Chinner
2013-06-12 10:22 ` [PATCH 20/27] xfs: move xfs_getbmap to xfs_extent_ops.c Dave Chinner
2013-06-12 10:22 ` [PATCH 21/27] xfs: introduce xfs_sb.c for sharing with libxfs Dave Chinner
2013-06-12 10:22 ` [PATCH 22/27] xfs: move xfs_trans_reservations to xfs_trans.h Dave Chinner
2013-06-12 10:22 ` [PATCH 23/27] xfs: sync minor header differences needed by userspace Dave Chinner
2013-06-12 10:22 ` [PATCH 24/27] xfs: move xfs_bmap_punch_delalloc() to xfs_aops.c Dave Chinner
2013-06-12 14:06   ` Christoph Hellwig
2013-06-13  1:39     ` Dave Chinner
2013-06-12 10:22 ` [PATCH 25/27] xfs: split out transaction reservation code Dave Chinner
2013-06-12 10:22 ` [PATCH 26/27] xfs: minor cleanups Dave Chinner
2013-06-12 10:22 ` [PATCH 27/27] xfs: fix issues that cause userspace warnings Dave Chinner
2013-06-17 19:32   ` Brian Foster
2013-06-18 21:42     ` Dave Chinner
2013-06-12 13:06 ` [PATCH 00/27] xfs: current patch queue for 3.11 Brian Foster
2013-06-13  1:40   ` Dave Chinner
2013-06-12 14:17 ` Ben Myers
2013-06-13  1:58   ` Dave Chinner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130613080009.GL29338@dastard \
    --to=david@fromorbit.com \
    --cc=hch@infradead.org \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.