All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: "J. Bruce Fields" <bfields@fieldses.org>
Cc: Masayoshi Mizuma <msys.mizuma@gmail.com>,
	Eric Sandeen <sandeen@sandeen.net>,
	"Darrick J. Wong" <darrick.wong@oracle.com>,
	Christoph Hellwig <hch@infradead.org>,
	Theodore Ts'o <tytso@mit.edu>,
	Andreas Dilger <adilger.kernel@dilger.ca>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>,
	linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-xfs <linux-xfs@vger.kernel.org>,
	jlayton@redhat.com
Subject: Re: [PATCH] fs: i_version mntopt gets visible through /proc/mounts
Date: Sat, 20 Jun 2020 11:49:57 +1000	[thread overview]
Message-ID: <20200620014957.GQ2005@dread.disaster.area> (raw)
In-Reply-To: <20200619222843.GB2650@fieldses.org>

On Fri, Jun 19, 2020 at 06:28:43PM -0400, J. Bruce Fields wrote:
> On Sat, Jun 20, 2020 at 08:10:44AM +1000, Dave Chinner wrote:
> > On Fri, Jun 19, 2020 at 04:40:33PM -0400, J. Bruce Fields wrote:
> > > On Fri, Jun 19, 2020 at 12:44:55PM +1000, Dave Chinner wrote:
> > > > On Thu, Jun 18, 2020 at 10:20:05PM -0400, J. Bruce Fields wrote:
> > > > > My memory was that after Jeff Layton's i_version patches, there wasn't
> > > > > really a significant performance hit any more, so the ability to turn it
> > > > > off is no longer useful.
> > > > 
> > > > Yes, I completely agree with you here. However, with some
> > > > filesystems allowing it to be turned off, we can't just wave our
> > > > hands and force enable the option. Those filesystems - if the
> > > > maintainers chose to always enable iversion - will have to go
> > > > through a mount option deprecation period before permanently
> > > > enabling it.
> > > 
> > > I don't understand why.
> > > 
> > > The filesystem can continue to let people set iversion or noiversion as
> > > they like, while under the covers behaving as if iversion is always set.
> > > I can't see how that would break any application.  (Or even how an
> > > application would be able to detect that the filesystem was doing this.)
> > 
> > It doesn't break functionality, but it affects performance.
> 
> I thought you just agreed above that any performance hit was not
> "significant".

Yes, but that's just /my opinion/.

However, other people have different opinions on this matter (and we
know that from the people who considered XFS v4 -> v5 going slower
because iversion a major regression), and so we must acknowledge
those opinions even if we don't agree with them.

That is, if people are using noiversion for performance reasons on
filesystems that are not designed/intended to have it permanently
enabled, then yanking that functionality out from under them without
warning is a Bad Thing To Do.

If we want to change this behaviour, the mount option needs to be
deprecated and a date/kernel release placed on when it will no
longer function (at least a year in the future).  When the mount
option is used, it needs to log a deprecation warning to the syslog
so that users are informed that the option is going away. Then when
the deprecation date passes, the mount option can then be ignored by
the kernel.

> > IOWs, it can make certain workloads go a lot slower in some
> > circumstances.  And that can result in unexectedly breaking SLAs or
> > slow down a complex, finely tuned data center wide workload to the
> > point it no longer meets requirements.  Such changes in behaviour are
> > considered a regression, especially if they result from a change that
> > just ignores the mount option that turned off that specific feature.
> 
> I get that, but, what's the threshhold here for a significant risk of
> regression?

<shrug>

I have no real idea because the filesystems that are affected are
not ones I'm actively involved in supporting or developing for.
That's for the people with domain specific expertise - the
individual filesystem maintainers - to decide.

> The "noiversion" behavior is kinda painful for NFS.

Sure, but that's all you ever get on XFS v4 filesystems and any
other filesystem that doesn't support persistent iversion storage.
So the NFS implemenations are going to have to live with filesystems
without iversion support at all for many more years to come.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

  reply	other threads:[~2020-06-20  1:50 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-16 20:21 [PATCH] fs: i_version mntopt gets visible through /proc/mounts Masayoshi Mizuma
2020-06-17  8:03 ` Christoph Hellwig
2020-06-17 13:33   ` Masayoshi Mizuma
2020-06-17 15:58   ` J. Bruce Fields
2020-06-17 17:14     ` Eric Sandeen
2020-06-17 17:24       ` Darrick J. Wong
2020-06-17 17:55         ` Eric Sandeen
2020-06-17 18:18           ` J. Bruce Fields
2020-06-17 18:28             ` Eric Sandeen
2020-06-17 18:45               ` J. Bruce Fields
2020-06-18  1:30                 ` Masayoshi Mizuma
2020-06-18  1:44                   ` Darrick J. Wong
2020-06-18  3:33                     ` Masayoshi Mizuma
2020-06-18  3:05                   ` Dave Chinner
2020-06-18  3:45                     ` Masayoshi Mizuma
2020-06-18 22:39                       ` Dave Chinner
2020-06-19  2:20                         ` J. Bruce Fields
2020-06-19  2:44                           ` Dave Chinner
2020-06-19 12:04                             ` Jeff Layton
2020-06-19 20:40                             ` J. Bruce Fields
2020-06-19 22:10                               ` Dave Chinner
2020-06-19 22:28                                 ` J. Bruce Fields
2020-06-20  1:49                                   ` Dave Chinner [this message]
2020-06-20  1:56                                     ` J. Bruce Fields
2020-06-20 17:00                                       ` Eric Sandeen
2020-06-20 17:09                                         ` J. Bruce Fields
2020-06-20 23:54                                       ` Dave Chinner
2020-06-22 21:26                                         ` J. Bruce Fields
2020-06-22 22:03                                           ` Dave Chinner
2020-06-19 13:17                         ` Christoph Hellwig
2020-07-13 23:45                         ` Eric Sandeen
2020-07-14  8:30                           ` Christoph Hellwig
2020-07-14 20:26                             ` Eric Sandeen
2020-06-27  8:29 ` [fs] 089633dad0: kmsg.xfs:Unknown_parameter'i_version' kernel test robot

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=20200620014957.GQ2005@dread.disaster.area \
    --to=david@fromorbit.com \
    --cc=adilger.kernel@dilger.ca \
    --cc=bfields@fieldses.org \
    --cc=darrick.wong@oracle.com \
    --cc=hch@infradead.org \
    --cc=jlayton@redhat.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=m.mizuma@jp.fujitsu.com \
    --cc=msys.mizuma@gmail.com \
    --cc=sandeen@sandeen.net \
    --cc=tytso@mit.edu \
    --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.