From: David Sterba <dsterba@suse.cz>
To: Liu Bo <bo.li.liu@oracle.com>
Cc: linux-btrfs@vger.kernel.org, fdmanana@suse.com, kzak@redhat.com,
linux-fsdevel@vger.kernel.org, viro@zeniv.linux.org.uk
Subject: i_version vs iversion (Was: Re: [RFC PATCH v2 1/2] Btrfs: add noi_version option to disable MS_I_VERSION)
Date: Thu, 18 Jun 2015 16:38:56 +0200 [thread overview]
Message-ID: <20150618143856.GG6761@suse.cz> (raw)
In-Reply-To: <20150618024607.GA8530@localhost.localdomain>
Moving the discussion to fsdevel.
Summary: disabling MS_I_VERSION brings some speedups to btrfs, but the
generic 'noiversion' option cannot be used to achieve that. It is
processed before it reaches btrfs superblock callback, where
MS_I_VERSION is forced.
The proposed fix is to add btrfs-specific i_version/noi_version to btrfs,
to which I object.
Continued below.
On Thu, Jun 18, 2015 at 10:46:09AM +0800, Liu Bo wrote:
> On Wed, Jun 17, 2015 at 07:01:18PM +0200, David Sterba wrote:
> > On Wed, Jun 17, 2015 at 11:52:36PM +0800, Liu Bo wrote:
> > > On Wed, Jun 17, 2015 at 05:33:06PM +0200, David Sterba wrote:
> > > > On Wed, Jun 17, 2015 at 03:54:31PM +0800, Liu Bo wrote:
> > > > > MS_I_VERSION is enabled by default for btrfs, this adds an alternative
> > > > > option to toggle it off.
> > > >
> > > > There's an existing generic iversion/noiversion mount option pair, no
> > > > need to extra add it to btrfs.
> > >
> > > I know, it doesn't work though.
> >
> > Sigh, I see, btrfs forces MS_I_VERSION flag,
> > 0c4d2d95d06e920e0c61707e62c7fffc9c57f63a. I read 'enabled by default' as
> > that there's a standard way to override the defaults.
> >
> > So the right way is not to do that but this will break everyhing that
> > relies on that behaviour at the moment. This means to add the exception
> > to the upper layers, either VFS or 'mount', which is not very likely to
> > happen.
> >
> > The generic options do not reach the filesystem specific callbacks, so
> > we can't check it.
>
> Ext4 also makes its own "i_version" option, so I think we can do the
> same thing until more filesystems require to do it in a generic way.
AFAICS, ext4 had added it's own i_version before iversion was added to
mount:
ext4:
Commit: 25ec56b518257a56d2ff41a941d288e4b5ff9488
Commit date: Mon Jan 28 23:58:27 2008 -0500
Subject: ext4: Add inode version support in ext4
util-linux:
Commti: 4fa5e73d16828c94234ba0aeafaec2470f79011c
Commit date: Thu Nov 27 12:08:44 2008 +0100
Subject: mount: add i_version support
I don't know the history, this looks like adding the options was not
coordinated.
> The performance benefit with no_iversion is obvious for fsync
> related workloads since we would avoid some expensive log commits.
It is obviuos, but I'd like to avoid cluttering the mount options
interface further.
xfs also forces I_VERSION if it detects the superblock version 5, so it
could use the same fix that would work for btrfs.
I see two possibilities that pretend to be generic and clean:
1) the filesystem MS_I_* defaults would be exported and processed up the
mount call stack
2) pass the full mount options to the filesystem (if requested eg. by
file_system_type::fs_flags bits).
The other ideas contain 'make an exception to ... ' which does not sound
appealing.
next prev parent reply other threads:[~2015-06-18 14:38 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-17 7:54 [RFC PATCH v2 1/2] Btrfs: add noi_version option to disable MS_I_VERSION Liu Bo
2015-06-17 7:54 ` [RFC PATCH v2 2/2] Btrfs: improve fsync for nocow file Liu Bo
2015-06-17 15:58 ` David Sterba
2015-06-18 3:27 ` Liu Bo
2015-06-24 18:21 ` David Sterba
2015-06-25 2:24 ` Liu Bo
2015-06-25 16:10 ` David Sterba
2015-06-17 15:33 ` [RFC PATCH v2 1/2] Btrfs: add noi_version option to disable MS_I_VERSION David Sterba
2015-06-17 15:52 ` Liu Bo
2015-06-17 17:01 ` David Sterba
2015-06-18 2:46 ` Liu Bo
2015-06-18 14:38 ` David Sterba [this message]
2015-06-19 11:44 ` i_version vs iversion (Was: Re: [RFC PATCH v2 1/2] Btrfs: add noi_version option to disable MS_I_VERSION) Karel Zak
2015-06-19 11:44 ` Karel Zak
2015-06-22 20:42 ` Dave Chinner
2015-06-22 20:42 ` Dave Chinner
2015-06-24 17:28 ` David Sterba
2015-06-23 16:32 ` Theodore Ts'o
2015-06-24 8:23 ` Liu Bo
2015-06-24 18:02 ` David Sterba
2015-06-24 23:17 ` Theodore Ts'o
2015-06-24 23:17 ` Theodore Ts'o
2015-06-24 23:59 ` Dave Chinner
2015-06-24 23:59 ` Dave Chinner
2015-06-25 18:46 ` J. Bruce Fields
2015-06-25 18:46 ` J. Bruce Fields
2015-06-25 22:12 ` Theodore Ts'o
2015-06-26 13:32 ` J. Bruce Fields
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=20150618143856.GG6761@suse.cz \
--to=dsterba@suse.cz \
--cc=bo.li.liu@oracle.com \
--cc=fdmanana@suse.com \
--cc=kzak@redhat.com \
--cc=linux-btrfs@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=viro@zeniv.linux.org.uk \
/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.