From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755559AbdLOM74 (ORCPT ); Fri, 15 Dec 2017 07:59:56 -0500 Received: from mail.kernel.org ([198.145.29.99]:35206 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755331AbdLOM7y (ORCPT ); Fri, 15 Dec 2017 07:59:54 -0500 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3D38B21877 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=jlayton@kernel.org Message-ID: <1513342791.20336.4.camel@kernel.org> Subject: Re: [PATCH 16/19] fs: only set S_VERSION when updating times if necessary From: Jeff Layton 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 Date: Fri, 15 Dec 2017 07:59:51 -0500 In-Reply-To: <20171213142017.23653-17-jlayton@kernel.org> References: <20171213142017.23653-1-jlayton@kernel.org> <20171213142017.23653-17-jlayton@kernel.org> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.26.2 (3.26.2-1.fc27) Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 2017-12-13 at 09:20 -0500, Jeff Layton wrote: > From: Jeff Layton > > 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 > --- > 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