From: Theodore Tso <tytso@MIT.EDU>
To: Christoph Hellwig <hch@infradead.org>
Cc: Mark Fasheh <mfasheh@suse.com>,
Andrew Morton <akpm@linux-foundation.org>,
Andreas Dilger <adilger@sun.com>,
Eric Sandeen <esandeen@redhat.com>,
linux-ext4@vger.kernel.org
Subject: Re: [PATCH 0/3] Fiemap, an extent mapping ioctl
Date: Wed, 10 Sep 2008 10:07:19 -0400 [thread overview]
Message-ID: <20080910140719.GQ21071@mit.edu> (raw)
In-Reply-To: <20080910134727.GA17498@infradead.org>
On Wed, Sep 10, 2008 at 09:47:27AM -0400, Christoph Hellwig wrote:
> On Wed, Sep 10, 2008 at 05:49:34AM -0700, Mark Fasheh wrote:
> > * FIEMAP_FLAG_XATTR
> > If this flag is set, the extents returned will describe the inodes
> > extended attribute lookup tree, instead of it's data tree.
>
> So does this actually make sense for any filesystem but XFS? Still
> seems like a not too useful option for a highlevel generic interface.
Yes, Andreas has submitted a proposed extension which would make this
make sense for ext4.
> > __u32 fe_device; /* device number for extent */
>
> As sayd before please kill thise one. It doesn't make any sense at all
> for any merged or near mainline FS. It'd be an utterly stupid
> lustre-specific hack that still doesn't make much sense.
As a suggestion, how about adding some __u32 and __u64 reserved
fields, some of which are reserved for use by the filesystem (i.e.,
you have to check f_type as returned by statfs to know what can be
used), and some for future generic expansions to the generic FIEMAP
interface. When designing a userspace interfaces, leaving room in
data structures for future expansion is a good thing, since it can
avoid needing to have multiple versions of an interface when we need
to expand the data structure.
While I can understand not wanting to have any new kernel functions
without any mainline users for that interface, leaving a few reserved
fields in data structures is just a smart thing to do, and has little
or no downsides.
Whether or not we explicitly define bitfields like
FIEMAP_EXTENT_SECONDARY is much less of an issue, since we can always
grab a bit position later without changing the size of the interface.
But if it turns out we want to add something like some way of
identifying which physical device is used by for a particular extent
--- something which btrfs will need, by the way --- not having room in
the data structure will just bite us later. So why not reserve some
room now?
Interface minimalism to allow for flexibility later is one thing; but
taken to extremes, it it's just stupid. For example, if we didn't
have any filesystems that needed 64-bit fields in mainline, would that
be a justification for making data structures to use 32-bit fields and
forcing a flag data interface change in the future. Sometimes we can
look ahead just a little bit and design interfaces which are foward
compatible. And it's pretty clear that btrfs will need something like
fe_device, although whether it should be a 32-bit index or something
else like a 128-bit UUID admittedly might not be clear at the moment.
But leaving room in the data structure for future growth is just a
intelligent thing to do.
Regards,
- Ted
next prev parent reply other threads:[~2008-09-10 14:07 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-08-25 20:22 FIEMAP patches Andreas Dilger
2008-08-26 6:22 ` Mark Fasheh
2008-08-26 6:57 ` Andreas Dilger
2008-09-10 12:40 ` Mark Fasheh
2008-09-10 12:49 ` [PATCH 0/3] Fiemap, an extent mapping ioctl Mark Fasheh
2008-09-10 13:47 ` Christoph Hellwig
2008-09-10 14:07 ` Theodore Tso [this message]
2008-09-10 14:11 ` Christoph Hellwig
2008-09-10 20:33 ` Andreas Dilger
2008-09-10 16:10 ` Mark Fasheh
2008-09-10 16:20 ` Christoph Hellwig
2008-09-10 18:46 ` Andrew Morton
2008-09-10 19:46 ` Andreas Dilger
2008-09-10 12:49 ` [PATCH 1/3] vfs: vfs-level fiemap interface Mark Fasheh
2008-09-10 12:50 ` [PATCH 2/3] generic block based fiemap implementation Mark Fasheh
2008-09-10 12:50 ` [PATCH 3/3] ocfs2: fiemap support Mark Fasheh
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=20080910140719.GQ21071@mit.edu \
--to=tytso@mit.edu \
--cc=adilger@sun.com \
--cc=akpm@linux-foundation.org \
--cc=esandeen@redhat.com \
--cc=hch@infradead.org \
--cc=linux-ext4@vger.kernel.org \
--cc=mfasheh@suse.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