From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751305AbdLMWsm (ORCPT ); Wed, 13 Dec 2017 17:48:42 -0500 Received: from ipmail07.adl2.internode.on.net ([150.101.137.131]:33110 "EHLO ipmail07.adl2.internode.on.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750748AbdLMWsk (ORCPT ); Wed, 13 Dec 2017 17:48:40 -0500 Date: Thu, 14 Dec 2017 09:48:37 +1100 From: Dave Chinner To: Jeff Layton Cc: linux-fsdevel@vger.kernel.org, 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 Subject: Re: [PATCH 14/19] xfs: convert to new i_version API Message-ID: <20171213224837.GB5858@dastard> References: <20171213142017.23653-1-jlayton@kernel.org> <20171213142017.23653-15-jlayton@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171213142017.23653-15-jlayton@kernel.org> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Dec 13, 2017 at 09:20:12AM -0500, Jeff Layton wrote: > From: Jeff Layton > > Signed-off-by: Jeff Layton > --- > fs/xfs/libxfs/xfs_inode_buf.c | 5 +++-- > fs/xfs/xfs_icache.c | 4 ++-- > fs/xfs/xfs_inode.c | 2 +- > fs/xfs/xfs_inode_item.c | 2 +- > fs/xfs/xfs_trans_inode.c | 2 +- > 5 files changed, 8 insertions(+), 7 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c > index 6b7989038d75..6b47de201391 100644 > --- a/fs/xfs/libxfs/xfs_inode_buf.c > +++ b/fs/xfs/libxfs/xfs_inode_buf.c > @@ -264,7 +264,8 @@ xfs_inode_from_disk( > to->di_flags = be16_to_cpu(from->di_flags); > > if (to->di_version == 3) { > - inode->i_version = be64_to_cpu(from->di_changecount); > + inode_set_iversion_queried(inode, > + be64_to_cpu(from->di_changecount)); So we use the "kernel managed" (really not sure what that means) set function here to read it off disk, but... > to->di_crtime.t_sec = be32_to_cpu(from->di_crtime.t_sec); > to->di_crtime.t_nsec = be32_to_cpu(from->di_crtime.t_nsec); > to->di_flags2 = be64_to_cpu(from->di_flags2); > @@ -314,7 +315,7 @@ xfs_inode_to_disk( > to->di_flags = cpu_to_be16(from->di_flags); > > if (from->di_version == 3) { > - to->di_changecount = cpu_to_be64(inode->i_version); > + to->di_changecount = cpu_to_be64(inode_peek_iversion_raw(inode)); ... use the raw access mode to put it back on disk. > to->di_crtime.t_sec = cpu_to_be32(from->di_crtime.t_sec); > to->di_crtime.t_nsec = cpu_to_be32(from->di_crtime.t_nsec); > to->di_flags2 = cpu_to_be64(from->di_flags2); > diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c > index 43005fbe8b1e..4838462616fd 100644 > --- a/fs/xfs/xfs_icache.c > +++ b/fs/xfs/xfs_icache.c > @@ -293,14 +293,14 @@ xfs_reinit_inode( > int error; > uint32_t nlink = inode->i_nlink; > uint32_t generation = inode->i_generation; > - uint64_t version = inode->i_version; > + uint64_t version = inode_peek_iversion_raw(inode); > umode_t mode = inode->i_mode; > > error = inode_init_always(mp->m_super, inode); > > set_nlink(inode, nlink); > inode->i_generation = generation; > - inode->i_version = version; > + inode_set_iversion_queried(inode, version); Again - raw mode to read, kernel managed to set. > inode->i_mode = mode; > return error; > } > diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c > index 801274126648..be6d87980dd5 100644 > --- a/fs/xfs/xfs_inode.c > +++ b/fs/xfs/xfs_inode.c > @@ -833,7 +833,7 @@ xfs_ialloc( > ip->i_d.di_flags = 0; > > if (ip->i_d.di_version == 3) { > - inode->i_version = 1; > + inode_set_iversion(inode, 1); But here you are using the "filesystem managed" mdoe to set the new value. Why? How is this any different from reading the value off disk and setting it? > +++ b/fs/xfs/xfs_trans_inode.c > @@ -117,7 +117,7 @@ xfs_trans_log_inode( > */ > if (!(ip->i_itemp->ili_item.li_desc->lid_flags & XFS_LID_DIRTY) && > IS_I_VERSION(VFS_I(ip))) { > - VFS_I(ip)->i_version++; > + inode_inc_iversion(VFS_I(ip)); > flags |= XFS_ILOG_CORE; > } And isn't this a case of "filesystem managed" iversion behaviour? Basically, I can't make head or tail of why the different API functions are used here.... Cheers, Dave. -- Dave Chinner david@fromorbit.com