From: Dave Chinner <david@fromorbit.com>
To: Mark Tinguely <tinguely@sgi.com>
Cc: xfs@oss.sgi.com
Subject: Re: ***** SUSPECTED SPAM ***** Re: [PATCH 48/49] xfs: Add read-only support for dirent filetype field
Date: Wed, 14 Aug 2013 17:50:42 +1000 [thread overview]
Message-ID: <20130814075042.GE6023@dastard> (raw)
In-Reply-To: <520A53E8.6030604@sgi.com>
On Tue, Aug 13, 2013 at 10:42:32AM -0500, Mark Tinguely wrote:
> On 08/12/13 19:50, Dave Chinner wrote:
> >On Mon, Aug 12, 2013 at 08:25:23AM -0500, Mark Tinguely wrote:
> >>On 08/11/13 19:59, Dave Chinner wrote:
> >>>On Tue, Jul 30, 2013 at 02:10:32PM -0500, Mark Tinguely wrote:
> >>>>On 07/19/13 01:25, Dave Chinner wrote:
> >>>>>From: Dave Chinner<dchinner@redhat.com>
> >>>>>
> >>>>>Add support for the file type field in directory entries so that
> >>>>>readdir can return the type of the inode the dirent points to to
> >>>>>userspace without first having to read the inode off disk.
.....
> >>>>>Signed-off-by: Dave Chinner<dchinner@redhat.com>
> >>>>>---
> >>>>>
> >>>>
> >>>>>+static inline int xfs_sb_version_hasftype(struct xfs_sb *sbp)
> >>>>>+{
> >>>>>+ return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5&&
> >>>>>+ xfs_sb_has_incompat_feature(sbp, XFS_SB_FEAT_INCOMPAT_FTYPE);
> >>>>> }
> >>>>>
> >>>>
> >>>>This feature should support inode version 2 and 3.
> >>>
> >>>Has nothing to do with the inode version number - it has to do with
> >>>the directory structure being modified.
> >>>
> >>>We're changing the directory structure for CRCs, and this builds on
> >>>top of that. It is essentially part of the V3 directory format, and
> >>>should be treated as such. Suggesting that we retrofit and support a
> >>>modified v2 directory format is close to insane - instead of only
> >>>having to test v2 vs v3 directory formats, you're suggesting we
> >>>support v2 vs v2+dtype vs v3 vs v3+dtype. We simply do not have the
> >>>resources to adequately test and support such an explosion of
> >>>filesystem configurations.
> >>>
> >>>We've had this discussion before - new on-disk features go into the
> >>>v5 superblock format - the v4 superblock format from this point
> >>>onwards is essentially legacy support from an upstream development
> >>>perspective.
....
> >>>That said, there's nothing to stop anyone from backporting such a
> >>>feature to an older kernel and maintaining it themselves - it's open
> >>>source software. But the idea that development should be constrained
> >>>by having to support both old and new formats is wrong - the old v4
> >>>format should be considered stable and we need think very hard about
> >>>changing it at all now, especially as much of the development focus
> >>>is now shifting to taking advantage of the additions to the v5
> >>>format....
> >>
> >>I guess we need more time to argue this out. It is not going into
> >>Linux 3.12 as a crc feature only.
> >
> >Seriously?
>
> yes seriously.
Great, another random roadblock from the XFS maintainers to deal
with.
Mark, this is not a question of how complex the code is to implement it,
it's a question of testing and support. I simply do not have the
resources available to me to adequately test on-disk format changes
to both v4 and v5 format filesystems. I do not require the
functionality on v4 format filesystems, nor do I have the time to
support users that will make use of such functionality on v4
filesystems. Let it stabilise in v5 filesystems first, then we can
backport it in 6-12 months time.
> >This was *always* intended as a feature that went along with the v3
> >directory modifications. For example, see slide 40 of this
> >presentation:
> >
> >http://xfs.org/images/d/d1/Xfs-scalability-lca2012.pdf
> >
> >the d_type field is listed alongside version counters, inode create
> >times, and unlinked list logging via inode items....
> >
> >The only reason this has been done as a feature bit is because I
> >didn't have it done in time to included it under the v5 superblock
> >version change.
> >
> >The directory format for v4 filesystems is not going to change. We
> >do not have the resources to test it adequately, nor do we have the
> >resources to support it adequately. We're already taking on a huge
> >amount of risk with the v5 superblock changes, and our fallback
> >position is that v4 superblock formats are unchanged and so
> >unaffected by the format changes made for v5. Pushing format changes
> >intended only for v5 fielsystems back into v4 filesystems jeopodises
> >that fallback position and exposes all existing filesystems to
> >increased risk.
> >
> >And that is a risk I'm not willing to take on behalf of upstream
> >kernel users. Those that move to CRC enabled filesystems know the
> >risks of being early adopters and accept that there may be problems
> >encountered. THose that are using v4 filesystems expect that there
> >are no risky, destabilising changes made to the on-disk format, yet
> >that's exactly what you are suggesting we do.
....
> >This is why I'm pushing back hard on any suggestions that we target
> >new features on v4 superblocks - it's a development dead end and
> >therefore essentially a waste of upstream development resources....
>
> v4 is hardly dead end. Feature bits can keep the filesystem stable.
Features bits don't keep a filesystem stable. Adequate testing and
enough resources to support users is what keeps the filesystem
stable. I don't scale to supporting an exponential feature matrix.
I'm providing features appropriate to what I can support. If SGI
needs this feature on v4 superblocks *right now*, then they are
welcome to write the patch and supply the testing and community
level support necessary. I'd appreciate some help writing the
userspace code needed to support it properly...
> v5 superblock is experimental and is not the automatically the
> default and only version.
Sure, but you are saying that the only way to get a brand new
experimental feature into the experimental on-disk format is to also
provide, test and support that experimental format change in the
stable format?
What happened to the ultimatum I was given for getting CRC support
into the kernel? i.e "thou shalt not regress existing filesystems
with code to support new on disk formats". I mean, that's the rule
you guys laid down for the 3.10 CRC merge and other work. it's the
rule I've been working to for the past year or so while developing
all this stuff.
Why has the story has changed now that we're starting to roll
out all sorts of interesting performance improvements based on new
on-disk formats? Somebody doesn't want the CRC features because they
are "overhead" and "bloat", but they want performance features
because "faster is better"?
Sheesh!
> Geoffrey has been expressing concerns about v5 and I agree with
> them. We came to the party too late, and despite our concerns, SGI
> has worked hard to get the crc pieces reviewed, tested and
> committed. The concerns are still there.
I've addressed all of Geoffrey's stated concerns more than once. If
he's got any new ones, then I'm more than willing to discuss them
with him. For better or for worse, v5 filesystems are here to stay
so but please stop using nebulous "concerns" as a reason for
stopping us from making forwards progress.
> Yes, I am serious. v4 is not dead and should get new features where
> appropriate.
And it's not appropriate right now. In 6 months to a year - once
we've stabilised the feature on v5 superblocks and have robust
userspace tool support - we can add the 5 lines of code to support
the feature bit on v4 superblocks. But right now it's asking way too
much.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
next prev parent reply other threads:[~2013-08-14 7:50 UTC|newest]
Thread overview: 158+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-07-19 6:24 [PATCH 00/49] current patch queue for 3.12 Dave Chinner
2013-07-19 6:24 ` [PATCH 01/49] xfs: separate out log format definitions Dave Chinner
2013-07-23 14:00 ` Brian Foster
2013-07-19 6:24 ` [PATCH 02/49] xfs: split out inode log item format definition Dave Chinner
2013-07-23 14:00 ` Brian Foster
2013-07-30 16:20 ` Mark Tinguely
2013-08-01 8:50 ` Christoph Hellwig
2013-07-19 6:24 ` [PATCH 03/49] xfs: split out buf log item format definitions Dave Chinner
2013-07-23 14:01 ` Brian Foster
2013-07-19 6:24 ` [PATCH 04/49] xfs: split out inode log item format definition Dave Chinner
2013-07-23 14:01 ` Brian Foster
2013-07-19 6:24 ` [PATCH 05/49] xfs: separate dquot on disk format definitions out of xfs_quota.h Dave Chinner
2013-07-24 12:09 ` Brian Foster
2013-08-01 8:51 ` Christoph Hellwig
2013-08-02 1:44 ` Dave Chinner
2013-07-19 6:24 ` [PATCH 06/49] xfs: separate icreate log format definitions from xfs_icreate_item.h Dave Chinner
2013-07-24 12:09 ` Brian Foster
2013-07-19 6:24 ` [PATCH 07/49] xfs: split out on-disk transaction definitions Dave Chinner
2013-07-24 12:09 ` Brian Foster
2013-07-19 6:24 ` [PATCH 08/49] xfs: introduce xfs_rtalloc_defs.h Dave Chinner
2013-07-24 12:09 ` Brian Foster
2013-07-19 6:24 ` [PATCH 09/49] xfs: introduce xfs_quota_defs.h Dave Chinner
2013-07-25 12:54 ` Brian Foster
2013-07-19 6:24 ` [PATCH 10/49] xfs: sync minor header differences needed by userspace Dave Chinner
2013-07-25 12:54 ` Brian Foster
2013-07-19 6:24 ` [PATCH 11/49] xfs: split out transaction reservation code Dave Chinner
2013-07-25 12:55 ` Brian Foster
2013-07-19 6:24 ` [PATCH 12/49] xfs: move inode fork definitions to a new header file Dave Chinner
2013-07-25 20:40 ` Mark Tinguely
2013-07-19 6:24 ` [PATCH 13/49] xfs: move unrealted definitions out of xfs_inode.h Dave Chinner
2013-07-25 19:24 ` Mark Tinguely
2013-07-19 6:24 ` [PATCH 14/49] xfs: introduce xfs_inode_buf.c for inode buffer operations Dave Chinner
2013-07-25 19:17 ` Mark Tinguely
2013-07-26 0:24 ` Dave Chinner
2013-07-26 12:41 ` Brian Foster
2013-07-19 6:24 ` [PATCH 15/49] xfs: move getdents code into it's own file Dave Chinner
2013-07-26 13:00 ` Brian Foster
2013-07-19 6:24 ` [PATCH 16/49] xfs: reshuffle dir2 definitions around for userspace Dave Chinner
2013-07-26 13:18 ` Brian Foster
2013-07-19 6:24 ` [PATCH 17/49] xfs: split out attribute listing code into separate file Dave Chinner
2013-07-27 20:23 ` Mark Tinguely
2013-07-19 6:24 ` [PATCH 18/49] xfs: split out attribute fork truncation " Dave Chinner
2013-07-27 19:25 ` Mark Tinguely
2013-07-19 6:24 ` [PATCH 19/49] xfs: split out the remote symlink handling Dave Chinner
2013-07-27 19:48 ` Mark Tinguely
2013-07-19 6:24 ` [PATCH 20/49] xfs: introduce xfs_sb.c for sharing with libxfs Dave Chinner
2013-07-27 17:54 ` Mark Tinguely
2013-07-28 1:08 ` Dave Chinner
2013-07-19 6:24 ` [PATCH 21/49] xfs: create xfs_bmap_util.[ch] Dave Chinner
2013-07-27 17:57 ` Mark Tinguely
2013-07-19 6:24 ` [PATCH 22/49] xfs: minor cleanups Dave Chinner
2013-07-27 18:00 ` Mark Tinguely
2013-07-28 1:07 ` Dave Chinner
2013-07-19 6:24 ` [PATCH 23/49] xfs: fix issues that cause userspace warnings Dave Chinner
2013-07-27 18:02 ` Mark Tinguely
2013-07-19 6:24 ` [PATCH 24/49] xfs: kill xfs_vnodeops.[ch] Dave Chinner
2013-07-26 19:18 ` Mark Tinguely
2013-07-27 1:55 ` Dave Chinner
2013-07-27 18:58 ` Mark Tinguely
2013-07-19 6:24 ` [PATCH 25/49] xfs: consolidate xfs_rename.c Dave Chinner
2013-07-26 19:33 ` Mark Tinguely
2013-07-19 6:24 ` [PATCH 26/49] xfs: consolidate xfs_utils.c Dave Chinner
2013-07-26 20:16 ` Mark Tinguely
2013-07-19 6:24 ` [PATCH 27/49] xfs: consolidate extent swap code Dave Chinner
2013-07-26 21:16 ` Mark Tinguely
2013-07-19 6:24 ` [PATCH 28/49] xfs: don't special case shared superblock mounts Dave Chinner
2013-07-26 15:32 ` Mark Tinguely
2013-07-19 6:25 ` [PATCH 29/49] xfs: kill __KERNEL__ check for debug code in allocation code Dave Chinner
2013-07-26 15:07 ` Mark Tinguely
2013-07-19 6:25 ` [PATCH 30/49] xfs: remove __KERNEL__ from debug code Dave Chinner
2013-07-26 15:03 ` Mark Tinguely
2013-07-19 6:25 ` [PATCH 31/49] xfs: remove __KERNEL__ check from xfs_dir2_leaf.c Dave Chinner
2013-07-26 14:16 ` Mark Tinguely
2013-07-19 6:25 ` [PATCH 32/49] xfs: xfs_filestreams.h doesn't need __KERNEL__ Dave Chinner
2013-07-26 14:10 ` Mark Tinguely
2013-07-19 6:25 ` [PATCH 33/49] xfs: move kernel specific type definitions to xfs.h Dave Chinner
2013-07-26 13:51 ` Mark Tinguely
2013-07-19 6:25 ` [PATCH 34/49] xfs: make struct xfs_perag kernel only Dave Chinner
2013-07-26 13:32 ` Mark Tinguely
2013-07-19 6:25 ` [PATCH 35/49] xfs: Introduce a new structure to hold transaction reservation items Dave Chinner
2013-07-22 13:05 ` Mark Tinguely
2013-07-19 6:25 ` [PATCH 36/49] xfs: Introduce tr_fsyncts to m_reservation Dave Chinner
2013-07-22 13:22 ` Mark Tinguely
2013-07-19 6:25 ` [PATCH 37/49] xfs: Make writeid transaction use tr_writeid Dave Chinner
2013-07-22 13:23 ` Mark Tinguely
2013-07-19 6:25 ` [PATCH 38/49] xfs: refactor xfs_trans_reserve() interface Dave Chinner
2013-07-22 13:27 ` Mark Tinguely
2013-07-22 23:37 ` Dave Chinner
2013-07-19 6:25 ` [PATCH 39/49] xfs: Get rid of all XFS_XXX_LOG_RES() macro Dave Chinner
2013-07-22 13:31 ` Mark Tinguely
2013-07-19 6:25 ` [PATCH 40/49] xfs: Refactor xfs_ticket_alloc() to extract a new helper Dave Chinner
2013-07-22 13:49 ` Mark Tinguely
2013-07-19 6:25 ` [PATCH 41/49] xfs: Add xfs_log_rlimit.c Dave Chinner
2013-07-23 15:15 ` Mark Tinguely
2013-07-19 6:25 ` [PATCH 42/49] xfs: Validate log space at mount time Dave Chinner
2013-07-22 13:55 ` Mark Tinguely
2013-07-25 4:11 ` Dave Chinner
2013-07-19 6:25 ` [PATCH 43/49] xfs: return log item size in IOP_SIZE Dave Chinner
2013-07-23 18:22 ` Mark Tinguely
2013-08-01 8:18 ` Christoph Hellwig
2013-07-19 6:25 ` [PATCH 44/49] xfs: Reduce allocations during CIL insertion Dave Chinner
2013-07-23 21:15 ` Mark Tinguely
2013-07-23 21:44 ` Michael L. Semon
2013-07-24 13:28 ` Mark Tinguely
2013-07-24 19:20 ` Michael L. Semon
2013-07-25 0:21 ` Dave Chinner
2013-07-25 15:02 ` Mark Tinguely
2013-07-26 0:32 ` Dave Chinner
2013-07-26 20:46 ` Michael L. Semon
2013-07-26 21:06 ` Mark Tinguely
2013-07-26 22:19 ` Michael L. Semon
2013-07-27 1:58 ` Dave Chinner
2013-07-27 18:32 ` Mark Tinguely
2013-07-28 1:12 ` Dave Chinner
2013-07-29 14:15 ` Mark Tinguely
2013-07-30 0:30 ` Dave Chinner
2013-07-30 13:31 ` Mark Tinguely
2013-07-30 22:19 ` Dave Chinner
2013-07-19 6:25 ` [PATCH 45/49] xfs: avoid CIL allocation during insert Dave Chinner
2013-07-29 18:13 ` Mark Tinguely
2013-07-19 6:25 ` [PATCH 46/49] xfs: Combine CIL insert and prepare passes Dave Chinner
2013-07-23 21:21 ` Mark Tinguely
2013-07-25 0:23 ` Dave Chinner
2013-07-29 21:07 ` Mark Tinguely
2013-07-19 6:25 ` [PATCH 47/49] xfs: split the CIL lock Dave Chinner
2013-07-29 22:24 ` Mark Tinguely
2013-07-19 6:25 ` [PATCH 48/49] xfs: Add read-only support for dirent filetype field Dave Chinner
2013-07-30 19:10 ` Mark Tinguely
2013-08-12 0:59 ` ***** SUSPECTED SPAM ***** " Dave Chinner
2013-08-12 13:25 ` Mark Tinguely
2013-08-13 0:50 ` Dave Chinner
2013-08-13 15:42 ` Mark Tinguely
2013-08-13 15:57 ` Christoph Hellwig
2013-08-14 7:50 ` Dave Chinner [this message]
2013-08-14 18:47 ` Geoffrey Wehrman
2013-08-15 4:22 ` Dave Chinner
2013-08-15 17:39 ` Geoffrey Wehrman
2013-08-15 5:59 ` Ric Wheeler
2013-08-15 18:04 ` Geoffrey Wehrman
2013-08-15 21:41 ` Ric Wheeler
2013-08-16 14:08 ` Geoffrey Wehrman
2013-08-19 5:28 ` Dave Chinner
2013-08-19 18:48 ` Geoffrey Wehrman
2013-08-20 2:23 ` Dave Chinner
2013-08-20 14:30 ` Geoffrey Wehrman
2013-08-20 18:27 ` Ric Wheeler
2013-08-20 19:47 ` Eric Sandeen
2013-08-15 16:50 ` Eric Sandeen
2013-08-15 18:32 ` Geoffrey Wehrman
2013-08-15 18:41 ` Eric Sandeen
2013-08-15 19:06 ` Geoffrey Wehrman
2013-08-15 18:54 ` Ben Myers
2013-08-15 22:40 ` Ben Myers
2013-07-19 6:25 ` [PATCH 49/49] xfs: Add write " Dave Chinner
2013-07-21 6:23 ` [PATCH 00/49] current patch queue for 3.12 Michael L. Semon
2013-07-22 23:43 ` Dave Chinner
2013-07-23 1:00 ` Michael L. Semon
2013-08-01 21:21 ` Ben Myers
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=20130814075042.GE6023@dastard \
--to=david@fromorbit.com \
--cc=tinguely@sgi.com \
--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