linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff Layton <jlayton@kernel.org>
To: linux-fsdevel@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, hch@lst.de, neilb@suse.de,
	bfields@fieldses.org, amir73il@gmail.com, jack@suse.de,
	viro@zeniv.linux.org.uk, David Howells <dhowells@redhat.com>
Subject: Re: [PATCH 16/19] fs: only set S_VERSION when updating times if necessary
Date: Fri, 15 Dec 2017 07:59:51 -0500	[thread overview]
Message-ID: <1513342791.20336.4.camel@kernel.org> (raw)
In-Reply-To: <20171213142017.23653-17-jlayton@kernel.org>

On Wed, 2017-12-13 at 09:20 -0500, Jeff Layton wrote:
> From: Jeff Layton <jlayton@redhat.com>
> 
> We only really need to update i_version if someone has queried for it
> since we last incremented it. By doing that, we can avoid having to
> update the inode if the times haven't changed.
> 
> If the times have changed, then we go ahead and forcibly increment the
> counter, under the assumption that we'll be going to the storage
> anyway, and the increment itself is relatively cheap.
> 
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
>  fs/inode.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/inode.c b/fs/inode.c
> index 03102d6ef044..7f4215f4309c 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -1634,17 +1634,18 @@ static int relatime_need_update(const struct path *path, struct inode *inode,
>  int generic_update_time(struct inode *inode, struct timespec *time, int flags)
>  {
>  	int iflags = I_DIRTY_TIME;
> +	bool dirty = flags & ~S_VERSION;
>  
>  	if (flags & S_ATIME)
>  		inode->i_atime = *time;
> -	if (flags & S_VERSION)
> -		inode_inc_iversion(inode);
>  	if (flags & S_CTIME)
>  		inode->i_ctime = *time;
>  	if (flags & S_MTIME)
>  		inode->i_mtime = *time;
> +	if (flags & S_VERSION)
> +		dirty |= inode_maybe_inc_iversion(inode, dirty);
>  
> -	if (!(inode->i_sb->s_flags & SB_LAZYTIME) || (flags & S_VERSION))
> +	if (dirty || !(inode->i_sb->s_flags & SB_LAZYTIME))

David Howells pointed out that the logic for setting dirty in here will
break SB_LAZYTIME handling. The patch is now fixed in my tree.

>  		iflags |= I_DIRTY_SYNC;
>  	__mark_inode_dirty(inode, iflags);
>  	return 0;
> @@ -1863,7 +1864,7 @@ int file_update_time(struct file *file)
>  	if (!timespec_equal(&inode->i_ctime, &now))
>  		sync_it |= S_CTIME;
>  
> -	if (IS_I_VERSION(inode))
> +	if (IS_I_VERSION(inode) && inode_iversion_need_inc(inode))
>  		sync_it |= S_VERSION;
>  
>  	if (!sync_it)

-- 
Jeff Layton <jlayton@kernel.org>

  reply	other threads:[~2017-12-15 12:59 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 [this message]
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

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=1513342791.20336.4.camel@kernel.org \
    --to=jlayton@kernel.org \
    --cc=amir73il@gmail.com \
    --cc=bfields@fieldses.org \
    --cc=dhowells@redhat.com \
    --cc=hch@lst.de \
    --cc=jack@suse.de \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).