All of lore.kernel.org
 help / color / mirror / Atom feed
From: "J. Bruce Fields" <bfields@fieldses.org>
To: Jeff Layton <jlayton@kernel.org>
Cc: Dave Chinner <david@fromorbit.com>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	hch@lst.de, neilb@suse.de, amir73il@gmail.com, jack@suse.de,
	viro@zeniv.linux.org.uk
Subject: Re: [PATCH 00/19] fs: rework and optimize i_version handling in filesystems
Date: Fri, 15 Dec 2017 10:26:31 -0500	[thread overview]
Message-ID: <20171215152631.GD12428@fieldses.org> (raw)
In-Reply-To: <1513350929.20336.21.camel@kernel.org>

On Fri, Dec 15, 2017 at 10:15:29AM -0500, Jeff Layton wrote:
> On Thu, 2017-12-14 at 10:14 -0500, J. Bruce Fields wrote:
> > On Thu, Dec 14, 2017 at 09:14:47AM -0500, Jeff Layton wrote:
> > > There is some clear peformance impact when you are running frequent
> > > queries of the i_version.
> > > 
> > > My gut feeling is that you could probably make the new code perform
> > > worse than the old if you were to _really_ hammer the inode with queries
> > > for the i_version (probably from many threads in parallel) while doing a
> > > lot of small writes to it.
> > > 
> > > That'd be a pretty unusual workload though.
> > 
> > It may be pretty common for NFS itself: if I'm understanding the client
> > code right (mainly nfs4_write_need_cache_consistency()), our client will
> > request the change attribute in every WRITE that isn't a pNFS write, an
> > O_DIRECT write, or associated with a delegation.
> > 
> > The goal of this series isn't to improve NFS performance, it's to save
> > non-NFS users from paying a performance penalty for something that NFS
> > requires for correctness.  Probably this series doesn't make much
> > difference in the NFS write case, and that's fine.  Still, might be
> > worth confirming that a workload with lots of small NFS writes is mostly
> > unaffected.
> 
> Just for yuks, I ran such a test this morning. I used the same fio
> jobfile, but changed it to have:
> 
>     direct=1
> 
> ...to eliminate client-side caching effects:
> 
> old:
>    WRITE: bw=1146KiB/s (1174kB/s), 143KiB/s-143KiB/s (147kB/s-147kB/s), io=672MiB (705MB), run=600075-600435msec
> 
> patched:
>    WRITE: bw=1253KiB/s (1283kB/s), 156KiB/s-157KiB/s (160kB/s-161kB/s), io=735MiB (770MB), run=600089-600414msec
> 
> So still seems to be a bit faster -- maybe because we're using an
> atomic64_t instead of a spinlock now? Probably I should profile that at
> some point...

That would be interesting!

But it looks like this and all your results are about what we expect,
and all the evidence so far is that the series is doing what we need it
to.

--b.

      reply	other threads:[~2017-12-15 15:26 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-13 14:19 [PATCH 00/19] fs: rework and optimize i_version handling in filesystems Jeff Layton
2017-12-13 14:19 ` [PATCH 01/19] fs: new API for handling inode->i_version Jeff Layton
2017-12-13 22:04   ` NeilBrown
2017-12-14  0:27     ` Jeff Layton
2017-12-16  4:17       ` NeilBrown
2017-12-17 13:01         ` Jeff Layton
2017-12-18 14:03         ` Jeff Layton
2017-12-13 14:20 ` [PATCH 02/19] fs: don't take the i_lock in inode_inc_iversion Jeff Layton
2017-12-13 21:52   ` Jeff Layton
2017-12-13 22:07     ` NeilBrown
2017-12-13 14:20 ` [PATCH 03/19] fat: convert to new i_version API Jeff Layton
2017-12-13 14:20 ` [PATCH 04/19] affs: " Jeff Layton
2017-12-13 14:20 ` [PATCH 05/19] afs: " Jeff Layton
2017-12-13 14:20 ` [PATCH 06/19] btrfs: " Jeff Layton
2017-12-13 14:20 ` [PATCH 07/19] exofs: switch " Jeff Layton
2017-12-13 14:20 ` [PATCH 08/19] ext2: convert " Jeff Layton
2017-12-18 12:47   ` Jan Kara
2017-12-13 14:20 ` [PATCH 09/19] ext4: " Jeff Layton
2017-12-14 21:52   ` Theodore Ts'o
2017-12-13 14:20 ` [PATCH 10/19] nfs: " Jeff Layton
2017-12-13 14:20 ` [PATCH 11/19] nfsd: " Jeff Layton
2017-12-13 14:20 ` [PATCH 12/19] ocfs2: " Jeff Layton
2017-12-18 12:49   ` Jan Kara
2017-12-13 14:20 ` [PATCH 13/19] ufs: use " Jeff Layton
2017-12-13 14:20 ` [PATCH 14/19] xfs: convert to " Jeff Layton
2017-12-13 22:48   ` Dave Chinner
2017-12-13 23:25     ` Dave Chinner
2017-12-14  0:10       ` Jeff Layton
2017-12-14  2:17         ` Dave Chinner
2017-12-14 11:16           ` Jeff Layton
2017-12-13 14:20 ` [PATCH 15/19] IMA: switch IMA over " Jeff Layton
2017-12-13 14:20 ` [PATCH 16/19] fs: only set S_VERSION when updating times if necessary Jeff Layton
2017-12-15 12:59   ` Jeff Layton
2017-12-13 14:20 ` [PATCH 17/19] xfs: avoid setting XFS_ILOG_CORE if i_version doesn't need incrementing Jeff Layton
2017-12-13 14:20 ` [PATCH 18/19] btrfs: only dirty the inode in btrfs_update_time if something was changed Jeff Layton
2017-12-15 13:03   ` Jeff Layton
2017-12-13 14:20 ` [PATCH 19/19] fs: handle inode->i_version more efficiently Jeff Layton
2017-12-13 15:05 ` [PATCH 00/19] fs: rework and optimize i_version handling in filesystems J. Bruce Fields
2017-12-13 20:14   ` Jeff Layton
2017-12-13 22:10     ` Jeff Layton
2017-12-13 23:03     ` Dave Chinner
2017-12-14  0:02       ` Jeff Layton
2017-12-14 14:14         ` Jeff Layton
2017-12-14 15:14           ` J. Bruce Fields
2017-12-15 15:15             ` Jeff Layton
2017-12-15 15:26               ` J. Bruce Fields [this message]

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=20171215152631.GD12428@fieldses.org \
    --to=bfields@fieldses.org \
    --cc=amir73il@gmail.com \
    --cc=david@fromorbit.com \
    --cc=hch@lst.de \
    --cc=jack@suse.de \
    --cc=jlayton@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=neilb@suse.de \
    --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.