Re: [PATCH 03/11] nfs: split nfs_update_timestamps

Jan Kara <[email protected]>
Newsgroups dev.linux.lists.gfs2,org.infradead.lists.linux-mtd,org.kernel.vger.io-uring,org.kernel.vger.linux-btrfs,org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-kernel,org.kernel.vger.linux-nfs,org.kernel.vger.linux-unionfs,org.kernel.vger.linux-xfs
Message-ID <wct2d2jrwausxgwusbnepzuegelkcl3s2veaixgrzehl6qcjyp@affaed5rinqo>
On Tue 06-01-26 08:49:57, Christoph Hellwig wrote:
> The VFS paths update either the atime or ctime and mtime but never mix
> between atime and the others.  Split nfs_update_timestamps to match this
> to prepare for cleaning up the VFS interfaces.
> 
> Signed-off-by: Christoph Hellwig <[email protected]>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <[email protected]>

								Honza

> ---
>  fs/nfs/inode.c | 31 +++++++++++++++----------------
>  1 file changed, 15 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> index 84049f3cd340..3be8ba7b98c5 100644
> --- a/fs/nfs/inode.c
> +++ b/fs/nfs/inode.c
> @@ -669,35 +669,31 @@ static void nfs_set_timestamps_to_ts(struct inode *inode, struct iattr *attr)
>  	NFS_I(inode)->cache_validity &= ~cache_flags;
>  }
>  
> -static void nfs_update_timestamps(struct inode *inode, unsigned int ia_valid)
> +static void nfs_update_atime(struct inode *inode)
>  {
> -	enum file_time_flags time_flags = 0;
> -	unsigned int cache_flags = 0;
> +	inode_update_timestamps(inode, S_ATIME);
> +	NFS_I(inode)->cache_validity &= ~NFS_INO_INVALID_ATIME;
> +}
>  
> -	if (ia_valid & ATTR_MTIME) {
> -		time_flags |= S_MTIME | S_CTIME;
> -		cache_flags |= NFS_INO_INVALID_CTIME | NFS_INO_INVALID_MTIME;
> -	}
> -	if (ia_valid & ATTR_ATIME) {
> -		time_flags |= S_ATIME;
> -		cache_flags |= NFS_INO_INVALID_ATIME;
> -	}
> -	inode_update_timestamps(inode, time_flags);
> -	NFS_I(inode)->cache_validity &= ~cache_flags;
> +static void nfs_update_mtime(struct inode *inode)
> +{
> +	inode_update_timestamps(inode, S_MTIME | S_CTIME);
> +	NFS_I(inode)->cache_validity &=
> +		~(NFS_INO_INVALID_CTIME | NFS_INO_INVALID_MTIME);
>  }
>  
>  void nfs_update_delegated_atime(struct inode *inode)
>  {
>  	spin_lock(&inode->i_lock);
>  	if (nfs_have_delegated_atime(inode))
> -		nfs_update_timestamps(inode, ATTR_ATIME);
> +		nfs_update_atime(inode);
>  	spin_unlock(&inode->i_lock);
>  }
>  
>  void nfs_update_delegated_mtime_locked(struct inode *inode)
>  {
>  	if (nfs_have_delegated_mtime(inode))
> -		nfs_update_timestamps(inode, ATTR_MTIME);
> +		nfs_update_mtime(inode);
>  }
>  
>  void nfs_update_delegated_mtime(struct inode *inode)
> @@ -747,7 +743,10 @@ nfs_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
>  						ATTR_ATIME|ATTR_ATIME_SET);
>  			}
>  		} else {
> -			nfs_update_timestamps(inode, attr->ia_valid);
> +			if (attr->ia_valid & ATTR_MTIME)
> +				nfs_update_mtime(inode);
> +			if (attr->ia_valid & ATTR_ATIME)
> +				nfs_update_atime(inode);
>  			attr->ia_valid &= ~(ATTR_MTIME|ATTR_ATIME);
>  		}
>  		spin_unlock(&inode->i_lock);
> -- 
> 2.47.3
> 
-- 
Jan Kara <[email protected]>
SUSE Labs, CR
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.