All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: "Theodore Ts'o" <tytso@mit.edu>
To: "J. Bruce Fields" <bfields@fieldses.org>
Cc: dsterba@suse.cz, Liu Bo <bo.li.liu@oracle.com>,
	linux-btrfs@vger.kernel.org, fdmanana@suse.com, kzak@redhat.com,
	linux-fsdevel@vger.kernel.org, viro@zeniv.linux.org.uk,
	linux-nfs@vger.kernel.org, chuck.lever@oracle.com,
	mingming.cao@oracle.com
Subject: Re: i_version vs iversion (Was: Re: [RFC PATCH v2 1/2] Btrfs: add noi_version option to disable MS_I_VERSION)
Date: Thu, 25 Jun 2015 18:12:57 -0400	[thread overview]
Message-ID: <20150625221257.GD13380@thunk.org> (raw)
In-Reply-To: <20150625184644.GA12300@fieldses.org>

On Thu, Jun 25, 2015 at 02:46:44PM -0400, J. Bruce Fields wrote:
> > Does this sound reasonable?
> 
> Just to make sure I understand, the logic is something like:
> 
> 	to read the i_version:
> 
> 		inode->i_version_seen = true;
> 		return inode->i_version
> 
> 	to update the i_version:
> 
> 		/*
> 		 * If nobody's seen this value of i_version then we can
> 		 * keep using it, otherwise we need a new one:
> 		 */
> 		if (inode->i_version_seen)
> 			inode->i_version++;
> 		inode->i_version_seen = false;

Yep, that's what I was proposing.

> Looks OK to me.  As I say I'd expect i_version_seen == true to end up
> being the common case in a lot of v4 workloads, so I'm more skeptical of
> the claim of a performance improvement in the v4 case.

Well, so long as we require i_version to be committed to disk on every
single disk write, we're going to be trading off:

	* client-side performance of the advanced NFSv4 cacheing for reads
	* server-side performance for writes
	* data robustness in case of the server crashing and the client-side cache
	  getting out of sync with the server after the crash

I don't see any way around that.  (So for example, with lazy mtime
updates we wouldn't be updating the inode after every single
non-allocating write; enabling i_version updates will trash that
optimization.)

I just want to reduce to a bare minimum the performance hit in the
case where NFSv4 exports are not being used (since that is true in a
very *large* number of ext4 deployments --- i.e., every single Android
handset using ext4 :-), such that we can leave i_version updates
turned on by default.

> Could maintaining the new flag be a significant drag in itself?  If not,
> then I guess we're not making things any worse there, so fine.

I don't think so; it's a bit in the in-memory inode, so I don't think
that should be an issue.

						- Ted

  reply	other threads:[~2015-06-25 22:13 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         ` i_version vs iversion (Was: Re: [RFC PATCH v2 1/2] Btrfs: add noi_version option to disable MS_I_VERSION) David Sterba
2015-06-19 11:44           ` 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 [this message]
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=20150625221257.GD13380@thunk.org \
    --to=tytso@mit.edu \
    --cc=bfields@fieldses.org \
    --cc=bo.li.liu@oracle.com \
    --cc=chuck.lever@oracle.com \
    --cc=dsterba@suse.cz \
    --cc=fdmanana@suse.com \
    --cc=kzak@redhat.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=mingming.cao@oracle.com \
    --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.