All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Mark Tinguely <tinguely@sgi.com>
Cc: Eric Sandeen <sandeen@sandeen.net>, xfs@oss.sgi.com
Subject: Re: [PATCH 51/50] xfs: add xfs sb v4 support for dirent filetype field
Date: Wed, 21 Aug 2013 09:19:03 +1000	[thread overview]
Message-ID: <20130820231903.GN6023@dastard> (raw)
In-Reply-To: <52137D3D.8060205@sgi.com>

On Tue, Aug 20, 2013 at 09:29:17AM -0500, Mark Tinguely wrote:
> On 08/19/13 18:28, Eric Sandeen wrote:
> >On 8/19/13 3:19 PM, Mark Tinguely wrote:
> >
> ><an attachment that doesn't show up on reply, moving d_type support to v4 superblocks ;)>
> >
> >Thanks, Mark!
> >
> >Has you been able to test this at all?
> 
> There is no test for this feature. Yes I did my version of testing.
> First adding each type of inode type and verifying it. Then fsstress
> testing using the same seed for sb v4+feature, v4 plain, v5+feature.
> The resulting directory and checked with xfs_db. fsstress was chosen

You would have had to have modified xfs_db to do this - can you send
the patches out for review?

> because how it manipulate directory items.

But fsstress doesn't build large directories, so I don't think
you've done anywhere near enough testing to say that you'd done
anything more than smoke tested it.

Indeed, in testing the dirent code on v5 superblocks, Michael Semon
triggered a corner case failure in the v5 directory code that I
haven't ever triggered in all my testin.

Run this on a v5 filesystem:

# seq 200000 | xargs touch

And watch it fail when splitting a leaf in a node format directory.
It's taken several months of testing to uncover this problem, and it
is almost certain to be a bug that causes directory corruption.
fsstress doesn't get anywhere near the per-directory file count
necessary to exercise these sorts of directory operations.

The moral of the story: the XFS directory code is *very hard to
validate*.

This is an clear demonstration of why I want to be extremely
conservative in bringing this feature to v4 filesystems. The risk of
there being an undiscovered corruption bug in the dtype code, or it
exposes a pre-existing corruption bug in the directory code is
still significant.

> >I do still owe a promised xfstest - but for that, we'll need at least mkfs
> >&  xfs_repair support.
> >
> 
> Dave made changes so that xfs_repair will run (find the correct
> directory items) but the feature verification and repairs has not
> been done, so technically this is an incomplete feature.

The patches I sent won't support a new v4 superblock feature bit, so
you had had to write code to do that. Can you post your userspace
patches at well?

> >* XFS_IOC_FSGEOM support so that xfs_info can report the difference
> >* xfs_repair needs to know that it's a valid feature on V4
> 
> okay, it will run xfs_repair to the same level as v5. AND ...As
> pointed out, there is no xfs_repair support to verify/correct the
> feature in v5 and therefore v4 - (again it is the same directory
> code). As is, this feature is incomplete. That could keep the kernel
> portion from moving forward.

That's not what XFS_IOC_FSGEOM is for. Adding the feature bit to the
XFS_IOC_FSGEOM ioctl is to allow us to find out if the user has
enabled the feature via xfs_info....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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

  parent reply	other threads:[~2013-08-20 23:19 UTC|newest]

Thread overview: 86+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-12 10:49 ***** SUSPECTED SPAM ***** [PATCH 00/50] xfs: patches for 3.12 Dave Chinner
2013-08-12 10:49 ` ***** SUSPECTED SPAM ***** [PATCH 01/50] xfs: separate out log format definitions Dave Chinner
2013-08-12 10:49 ` ***** SUSPECTED SPAM ***** [PATCH 02/50] xfs: split out inode log item format definition Dave Chinner
2013-08-12 10:49 ` ***** SUSPECTED SPAM ***** [PATCH 03/50] xfs: split out buf log item format definitions Dave Chinner
2013-08-12 10:49 ` ***** SUSPECTED SPAM ***** [PATCH 04/50] xfs: split out EFI/EFD log item format definition Dave Chinner
2013-08-12 10:49 ` ***** SUSPECTED SPAM ***** [PATCH 05/50] xfs: separate dquot on disk format definitions out of xfs_quota.h Dave Chinner
2013-08-12 10:49 ` ***** SUSPECTED SPAM ***** [PATCH 06/50] xfs: separate icreate log format definitions from xfs_icreate_item.h Dave Chinner
2013-08-12 10:49 ` ***** SUSPECTED SPAM ***** [PATCH 07/50] xfs: split out on-disk transaction definitions Dave Chinner
2013-08-12 10:49 ` ***** SUSPECTED SPAM ***** [PATCH 08/50] xfs: introduce xfs_rtalloc_defs.h Dave Chinner
2013-08-12 10:49 ` ***** SUSPECTED SPAM ***** [PATCH 09/50] xfs: introduce xfs_quota_defs.h Dave Chinner
2013-08-12 10:49 ` ***** SUSPECTED SPAM ***** [PATCH 10/50] xfs: sync minor header differences needed by userspace Dave Chinner
2013-08-12 10:49 ` ***** SUSPECTED SPAM ***** [PATCH 11/50] xfs: split out transaction reservation code Dave Chinner
2013-08-12 10:49 ` ***** SUSPECTED SPAM ***** [PATCH 12/50] xfs: move inode fork definitions to a new header file Dave Chinner
2013-08-12 10:49 ` ***** SUSPECTED SPAM ***** [PATCH 13/50] xfs: move unrelated definitions out of xfs_inode.h Dave Chinner
2013-08-12 10:49 ` ***** SUSPECTED SPAM ***** [PATCH 14/50] xfs: introduce xfs_inode_buf.c for inode buffer operations Dave Chinner
2013-08-12 10:49 ` ***** SUSPECTED SPAM ***** [PATCH 15/50] xfs: move getdents code into it's own file Dave Chinner
2013-08-12 10:49 ` ***** SUSPECTED SPAM ***** [PATCH 16/50] xfs: reshuffle dir2 definitions around for userspace Dave Chinner
2013-08-12 10:49 ` ***** SUSPECTED SPAM ***** [PATCH 17/50] xfs: split out attribute listing code into separate file Dave Chinner
2013-08-12 10:49 ` ***** SUSPECTED SPAM ***** [PATCH 18/50] xfs: split out attribute fork truncation " Dave Chinner
2013-08-12 10:49 ` ***** SUSPECTED SPAM ***** [PATCH 19/50] xfs: split out the remote symlink handling Dave Chinner
2013-08-12 10:49 ` ***** SUSPECTED SPAM ***** [PATCH 20/50] xfs: introduce xfs_sb.c for sharing with libxfs Dave Chinner
2013-08-12 10:49 ` ***** SUSPECTED SPAM ***** [PATCH 21/50] xfs: create xfs_bmap_util.[ch] Dave Chinner
2013-08-12 10:49 ` ***** SUSPECTED SPAM ***** [PATCH 22/50] xfs: minor cleanups Dave Chinner
2013-08-12 10:49 ` ***** SUSPECTED SPAM ***** [PATCH 23/50] xfs: fix issues that cause userspace warnings Dave Chinner
2013-08-12 10:49 ` ***** SUSPECTED SPAM ***** [PATCH 24/50] xfs: kill xfs_vnodeops.[ch] Dave Chinner
2013-08-12 10:49 ` ***** SUSPECTED SPAM ***** [PATCH 25/50] xfs: consolidate xfs_rename.c Dave Chinner
2013-08-12 10:49 ` ***** SUSPECTED SPAM ***** [PATCH 26/50] xfs: consolidate xfs_utils.c Dave Chinner
2013-08-12 10:49 ` ***** SUSPECTED SPAM ***** [PATCH 27/50] xfs: consolidate extent swap code Dave Chinner
2013-08-12 10:49 ` ***** SUSPECTED SPAM ***** [PATCH 28/50] xfs: don't special case shared superblock mounts Dave Chinner
2013-08-12 10:49 ` ***** SUSPECTED SPAM ***** [PATCH 29/50] xfs: kill __KERNEL__ check for debug code in allocation code Dave Chinner
2013-08-12 10:49 ` ***** SUSPECTED SPAM ***** [PATCH 30/50] xfs: remove __KERNEL__ from debug code Dave Chinner
2013-08-12 10:49 ` ***** SUSPECTED SPAM ***** [PATCH 31/50] xfs: remove __KERNEL__ check from xfs_dir2_leaf.c Dave Chinner
2013-08-12 10:49 ` ***** SUSPECTED SPAM ***** [PATCH 32/50] xfs: xfs_filestreams.h doesn't need __KERNEL__ Dave Chinner
2013-08-12 10:49 ` ***** SUSPECTED SPAM ***** [PATCH 33/50] xfs: move kernel specific type definitions to xfs.h Dave Chinner
2013-08-12 10:49 ` ***** SUSPECTED SPAM ***** [PATCH 34/50] xfs: make struct xfs_perag kernel only Dave Chinner
2013-08-12 10:49 ` ***** SUSPECTED SPAM ***** [PATCH 35/50] xfs: Introduce a new structure to hold transaction reservation items Dave Chinner
2013-08-12 10:49 ` ***** SUSPECTED SPAM ***** [PATCH 36/50] xfs: Introduce tr_fsyncts to m_reservation Dave Chinner
2013-08-12 10:49 ` ***** SUSPECTED SPAM ***** [PATCH 37/50] xfs: Make writeid transaction use tr_writeid Dave Chinner
2013-08-12 10:49 ` ***** SUSPECTED SPAM ***** [PATCH 38/50] xfs: refactor xfs_trans_reserve() interface Dave Chinner
2013-08-12 10:50 ` ***** SUSPECTED SPAM ***** [PATCH 39/50] xfs: Get rid of all XFS_XXX_LOG_RES() macro Dave Chinner
2013-08-12 10:50 ` ***** SUSPECTED SPAM ***** [PATCH 40/50] xfs: Refactor xfs_ticket_alloc() to extract a new helper Dave Chinner
2013-08-12 10:50 ` ***** SUSPECTED SPAM ***** [PATCH 41/50] xfs: Add xfs_log_rlimit.c Dave Chinner
2013-08-12 10:50 ` ***** SUSPECTED SPAM ***** [PATCH 42/50] xfs: Validate log space at mount time Dave Chinner
2013-08-12 18:46   ` Mark Tinguely
2013-08-12 10:50 ` ***** SUSPECTED SPAM ***** [PATCH 43/50] xfs: return log item size in IOP_SIZE Dave Chinner
2013-08-12 10:50 ` ***** SUSPECTED SPAM ***** [PATCH 44/50] xfs: Reduce allocations during CIL insertion Dave Chinner
2013-08-13 13:28   ` Mark Tinguely
2013-08-12 10:50 ` ***** SUSPECTED SPAM ***** [PATCH 45/50] xfs: avoid CIL allocation during insert Dave Chinner
2013-08-13 13:37   ` Mark Tinguely
2013-08-12 10:50 ` ***** SUSPECTED SPAM ***** [PATCH 46/50] xfs: Combine CIL insert and prepare passes Dave Chinner
2013-08-13 14:02   ` Mark Tinguely
2013-08-12 10:50 ` ***** SUSPECTED SPAM ***** [PATCH 47/50] xfs: split the CIL lock Dave Chinner
2013-08-12 10:50 ` ***** SUSPECTED SPAM ***** [PATCH 48/50] xfs: Add read-only support for dirent filetype field Dave Chinner
2013-08-12 10:50 ` ***** SUSPECTED SPAM ***** [PATCH 49/50] xfs: Add write " Dave Chinner
2013-08-12 10:50 ` ***** SUSPECTED SPAM ***** [PATCH 50/50] xfs: use reference counts to free clean buffer items Dave Chinner
2013-08-13 15:03   ` Mark Tinguely
2013-08-13 21:46     ` Dave Chinner
2013-08-13 22:00       ` Mark Tinguely
2013-08-14  3:57         ` Dave Chinner
2013-08-14  4:12           ` Zhi Yong Wu
2013-08-14  6:41             ` Dave Chinner
2013-08-14 13:26           ` Mark Tinguely
2013-08-14 17:49             ` Mark Tinguely
2013-08-15  0:48               ` Dave Chinner
2013-08-15 21:43   ` ***** SUSPECTED SPAM ***** " Ben Myers
2013-08-12 22:55 ` ***** SUSPECTED SPAM ***** [PATCH 00/50] xfs: patches for 3.12 Ben Myers
2013-08-13 21:28   ` Ben Myers
2013-08-19 20:19 ` [PATCH 51/50] xfs: add xfs sb v4 support for dirent filetype field Mark Tinguely
2013-08-19 23:28   ` Eric Sandeen
2013-08-20 14:29     ` Mark Tinguely
2013-08-20 14:45       ` Eric Sandeen
2013-08-20 18:50         ` Ben Myers
2013-08-20 21:00           ` Mark Tinguely
2013-08-20 21:05             ` Ben Myers
2013-08-20 23:19       ` Dave Chinner [this message]
2013-08-21  0:06       ` Dave Chinner
2013-08-21 17:03         ` Ben Myers
2013-08-22  2:02           ` Dave Chinner
2013-08-22 16:14             ` Geoffrey Wehrman
2013-08-22 18:19               ` Ben Myers
2013-08-25  5:18                 ` Michael L. Semon
2013-08-25 23:21                   ` Michael L. Semon
2013-08-26 15:40                   ` Mark Tinguely
2013-08-19 23:40   ` Dave Chinner
2013-08-20 19:57   ` Geoffrey Wehrman
2013-08-22 15:59 ` ***** SUSPECTED SPAM ***** [PATCH 00/50] xfs: patches for 3.12 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=20130820231903.GN6023@dastard \
    --to=david@fromorbit.com \
    --cc=sandeen@sandeen.net \
    --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 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.