public inbox for linux-xfs@vger.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: [RFC] libxfs kernel infrastructure (was [XFS updates] XFS development tree branch, xfs-libxfs-in-kernel-RFC, created. xfs-for-linus-3.15-rc2-52-g6579dd8)
Date: Tue, 6 May 2014 19:00:56 +1000	[thread overview]
Message-ID: <20140506090056.GH5421@dastard> (raw)
In-Reply-To: <20140506083744.GA9976@infradead.org>

On Tue, May 06, 2014 at 01:37:44AM -0700, Christoph Hellwig wrote:
> I like this in general, but one major and one minor issue with
> the include files:
> 
>  - headers that just include other headers are a bad idea in general.
>    Either they are dependent enough that they should be merged, or
>    they are not, in which case they shouldn't.
> 
>    In this case it seems like we should temporarily provide a
>    xfs_mount.h stub in userspace, and just leave all the includes
>    for things in libxfs.h as they were.  That doesn't preclude further
>    merging of the headers into more sensible ones as we've started
>    with the disk formats.

I did this because I'm sick of having to edit 50+ files whenever a
single header dependency changes. There are almost all cookie cutter
duplicates because of the dependencies - if it were code, we'd
factor it in an instant.  I just don't see why we should treat 50
copies of the same header includes any differently....

>  - do we really need the separate include/ dir?  That always annoys
>    me when editing code.  It makes sense for something that is a real
>    public interface, which this is not.

It's for simplicity of updates with the userspace code. Both
userspace and kernel need the same code layout, and userspace
currently has a separate include directory for all the header files
(and they get installed that way, too). If we want to change the
userspace source layout and commingle all the headers with the C
code, then that's a lot more work on the userspace side (i.e. it's
more than just pointing the include/xfs symlink to libxfs/include).

I don't mind which approach we take - it's trivial to rework the
patchset to place all the headers in the libxfs/ directory - I just
took the one that matched the current userspace infrastructure...

> Also is libxfs/ really the right name?  libxfs in userspace has quite
> a bit more code than this, so maybe we should just called this "shared"
> for the shared user/kernel code?

I'd like to have this kernel code define libxfs/, while in userspace
we separate out all the support code (i.e. libxfs/rdwr.c, etc) into
a different directory that builds the userspace libraries. i.e.
libxfs/ is a static object archive that is wrapped by the userspace
infrastructure, just like the kernel wraps it with infrastructure to
make it useful...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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

  reply	other threads:[~2014-05-06  9:01 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-06  7:18 [XFS updates] XFS development tree branch, xfs-libxfs-in-kernel-RFC, created. xfs-for-linus-3.15-rc2-52-g6579dd8 xfs
2014-05-06  7:59 ` [RFC] libxfs kernel infrastructure (was [XFS updates] XFS development tree branch, xfs-libxfs-in-kernel-RFC, created. xfs-for-linus-3.15-rc2-52-g6579dd8) Dave Chinner
2014-05-06  8:37   ` Christoph Hellwig
2014-05-06  9:00     ` Dave Chinner [this message]
2014-05-09  7:29       ` Christoph Hellwig
2014-05-09 21:45         ` Dave Chinner
2014-05-06  8:43   ` Christoph Hellwig
2014-05-06  9:05     ` Dave Chinner
2014-05-07 14:48   ` Brian Foster
2014-05-07 22:47     ` Dave Chinner
2014-05-08  1:12       ` Dave Chinner
2014-05-08 12:02         ` Brian Foster
2014-05-08 12:54           ` Christoph Hellwig
2014-05-08 13:45             ` Brian Foster
2014-05-08 21:21               ` Dave Chinner
2014-05-09  7:21                 ` Christoph Hellwig

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=20140506090056.GH5421@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox