| Message ID | 20240318225406.3378998-2-david@fromorbit.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | xfs: drop SB_I_VERSION support | expand |
On Tue, Mar 19, 2024 at 09:51:00AM +1100, Dave Chinner wrote: > From: Dave Chinner <dchinner@redhat.com> > > The redefinition of how NFS wants inode->i_version to be updated is > incomaptible with the XFS i_version mechanism. The VFS now wants > inode->i_version to only change when ctime changes (i.e. it has > become a ctime change counter, not an inode change counter). XFS has > fine grained timestamps, so it can just use ctime for the NFS change > cookie like it still does for V4 XFS filesystems. > > We still want XFS to update the inode change counter as it currently > does, so convert all the code that checks SB_I_VERSION to check for > v5 format support. Then we can remove the SB_I_VERSION flag from the > VFS superblock to indicate that inode->i_version is not a valid > change counter and should not be used as such. > > Signed-off-by: Dave Chinner <dchinner@redhat.com> > Acked-by: Jeff Layton <jlayton@kernel.org> Looks good to me, Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > --- > fs/xfs/libxfs/xfs_trans_inode.c | 15 +++++---------- > fs/xfs/xfs_iops.c | 16 +++------------- > fs/xfs/xfs_super.c | 8 -------- > 3 files changed, 8 insertions(+), 31 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_trans_inode.c b/fs/xfs/libxfs/xfs_trans_inode.c > index 69fc5b981352..b82f9c7ff2d5 100644 > --- a/fs/xfs/libxfs/xfs_trans_inode.c > +++ b/fs/xfs/libxfs/xfs_trans_inode.c > @@ -97,17 +97,12 @@ xfs_trans_log_inode( > > /* > * First time we log the inode in a transaction, bump the inode change > - * counter if it is configured for this to occur. While we have the > - * inode locked exclusively for metadata modification, we can usually > - * avoid setting XFS_ILOG_CORE if no one has queried the value since > - * the last time it was incremented. If we have XFS_ILOG_CORE already > - * set however, then go ahead and bump the i_version counter > - * unconditionally. > + * counter if it is configured for this to occur. > */ > - if (!test_and_set_bit(XFS_LI_DIRTY, &iip->ili_item.li_flags)) { > - if (IS_I_VERSION(inode) && > - inode_maybe_inc_iversion(inode, flags & XFS_ILOG_CORE)) > - flags |= XFS_ILOG_IVERSION; > + if (!test_and_set_bit(XFS_LI_DIRTY, &iip->ili_item.li_flags) && > + xfs_has_crc(ip->i_mount)) { > + atomic64_inc(&inode->i_version); > + flags |= XFS_ILOG_IVERSION; > } > > iip->ili_dirty_flags |= flags; > diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c > index 66f8c47642e8..3940ad1ee66e 100644 > --- a/fs/xfs/xfs_iops.c > +++ b/fs/xfs/xfs_iops.c > @@ -584,11 +584,6 @@ xfs_vn_getattr( > } > } > > - if ((request_mask & STATX_CHANGE_COOKIE) && IS_I_VERSION(inode)) { > - stat->change_cookie = inode_query_iversion(inode); > - stat->result_mask |= STATX_CHANGE_COOKIE; > - } > - > /* > * Note: If you add another clause to set an attribute flag, please > * update attributes_mask below. > @@ -1043,16 +1038,11 @@ xfs_vn_update_time( > struct timespec64 now; > > trace_xfs_update_time(ip); > + ASSERT(!(flags & S_VERSION)); > > if (inode->i_sb->s_flags & SB_LAZYTIME) { > - if (!((flags & S_VERSION) && > - inode_maybe_inc_iversion(inode, false))) { > - generic_update_time(inode, flags); > - return 0; > - } > - > - /* Capture the iversion update that just occurred */ > - log_flags |= XFS_ILOG_CORE; > + generic_update_time(inode, flags); > + return 0; > } > > error = xfs_trans_alloc(mp, &M_RES(mp)->tr_fsyncts, 0, 0, 0, &tp); > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c > index c21f10ab0f5d..bfc5f3a27451 100644 > --- a/fs/xfs/xfs_super.c > +++ b/fs/xfs/xfs_super.c > @@ -1690,10 +1690,6 @@ xfs_fs_fill_super( > > set_posix_acl_flag(sb); > > - /* version 5 superblocks support inode version counters. */ > - if (xfs_has_crc(mp)) > - sb->s_flags |= SB_I_VERSION; > - > if (xfs_has_dax_always(mp)) { > error = xfs_setup_dax_always(mp); > if (error) > @@ -1915,10 +1911,6 @@ xfs_fs_reconfigure( > int flags = fc->sb_flags; > int error; > > - /* version 5 superblocks always support version counters. */ > - if (xfs_has_crc(mp)) > - fc->sb_flags |= SB_I_VERSION; > - > error = xfs_fs_validate_params(new_mp); > if (error) > return error; > -- > 2.43.0 > >
A bit sad, but with the curren tstate of affairs it's the right thing
to do:
Reviewed-by: Christoph Hellwig <hch@lst.de>
diff --git a/fs/xfs/libxfs/xfs_trans_inode.c b/fs/xfs/libxfs/xfs_trans_inode.c index 69fc5b981352..b82f9c7ff2d5 100644 --- a/fs/xfs/libxfs/xfs_trans_inode.c +++ b/fs/xfs/libxfs/xfs_trans_inode.c @@ -97,17 +97,12 @@ xfs_trans_log_inode( /* * First time we log the inode in a transaction, bump the inode change - * counter if it is configured for this to occur. While we have the - * inode locked exclusively for metadata modification, we can usually - * avoid setting XFS_ILOG_CORE if no one has queried the value since - * the last time it was incremented. If we have XFS_ILOG_CORE already - * set however, then go ahead and bump the i_version counter - * unconditionally. + * counter if it is configured for this to occur. */ - if (!test_and_set_bit(XFS_LI_DIRTY, &iip->ili_item.li_flags)) { - if (IS_I_VERSION(inode) && - inode_maybe_inc_iversion(inode, flags & XFS_ILOG_CORE)) - flags |= XFS_ILOG_IVERSION; + if (!test_and_set_bit(XFS_LI_DIRTY, &iip->ili_item.li_flags) && + xfs_has_crc(ip->i_mount)) { + atomic64_inc(&inode->i_version); + flags |= XFS_ILOG_IVERSION; } iip->ili_dirty_flags |= flags; diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c index 66f8c47642e8..3940ad1ee66e 100644 --- a/fs/xfs/xfs_iops.c +++ b/fs/xfs/xfs_iops.c @@ -584,11 +584,6 @@ xfs_vn_getattr( } } - if ((request_mask & STATX_CHANGE_COOKIE) && IS_I_VERSION(inode)) { - stat->change_cookie = inode_query_iversion(inode); - stat->result_mask |= STATX_CHANGE_COOKIE; - } - /* * Note: If you add another clause to set an attribute flag, please * update attributes_mask below. @@ -1043,16 +1038,11 @@ xfs_vn_update_time( struct timespec64 now; trace_xfs_update_time(ip); + ASSERT(!(flags & S_VERSION)); if (inode->i_sb->s_flags & SB_LAZYTIME) { - if (!((flags & S_VERSION) && - inode_maybe_inc_iversion(inode, false))) { - generic_update_time(inode, flags); - return 0; - } - - /* Capture the iversion update that just occurred */ - log_flags |= XFS_ILOG_CORE; + generic_update_time(inode, flags); + return 0; } error = xfs_trans_alloc(mp, &M_RES(mp)->tr_fsyncts, 0, 0, 0, &tp); diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c index c21f10ab0f5d..bfc5f3a27451 100644 --- a/fs/xfs/xfs_super.c +++ b/fs/xfs/xfs_super.c @@ -1690,10 +1690,6 @@ xfs_fs_fill_super( set_posix_acl_flag(sb); - /* version 5 superblocks support inode version counters. */ - if (xfs_has_crc(mp)) - sb->s_flags |= SB_I_VERSION; - if (xfs_has_dax_always(mp)) { error = xfs_setup_dax_always(mp); if (error) @@ -1915,10 +1911,6 @@ xfs_fs_reconfigure( int flags = fc->sb_flags; int error; - /* version 5 superblocks always support version counters. */ - if (xfs_has_crc(mp)) - fc->sb_flags |= SB_I_VERSION; - error = xfs_fs_validate_params(new_mp); if (error) return error;