Re: [PATCH 3/4] exportfs: don't pass struct iattr to ->commit_blocks

[Jeff Layton <jlayton@kernel.org>]

On Tue, 2026-03-31 at 17:33 +0200, Christoph Hellwig wrote:
> The only thing ->commit_blocks really needs is the new size, with a magic
> -1 placeholder 0 for "do not change the size" because it only ever
> extends the size.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/nfsd/blocklayout.c          | 12 ++----------
>  fs/xfs/xfs_pnfs.c              | 19 ++++++++++---------
>  include/linux/exportfs_block.h |  3 +--
>  3 files changed, 13 insertions(+), 21 deletions(-)
> 
> diff --git a/fs/nfsd/blocklayout.c b/fs/nfsd/blocklayout.c
> index 3cc3b47361e2..23c0e4d0ff34 100644
> --- a/fs/nfsd/blocklayout.c
> +++ b/fs/nfsd/blocklayout.c
> @@ -179,7 +179,6 @@ static __be32
>  nfsd4_block_commit_blocks(struct inode *inode, struct nfsd4_layoutcommit *lcp,
>  		struct iomap *iomaps, int nr_iomaps)
>  {
> -	struct iattr iattr = { .ia_valid = 0 };
>  	int error;
>  
>  	/*
> @@ -191,16 +190,9 @@ nfsd4_block_commit_blocks(struct inode *inode, struct nfsd4_layoutcommit *lcp,
>  	 * timestamp is a "may" condition, and clients that want to force a
>  	 * specific timestamp should send a separate SETATTR in the compound.
>  	 */
> -	iattr.ia_valid |= ATTR_ATIME | ATTR_CTIME | ATTR_MTIME;
> -	iattr.ia_atime = iattr.ia_ctime = iattr.ia_mtime = current_time(inode);
> -
> -	if (lcp->lc_size_chg) {
> -		iattr.ia_valid |= ATTR_SIZE;
> -		iattr.ia_size = lcp->lc_newsize;
> -	}
> -
>  	error = inode->i_sb->s_export_op->block_ops->commit_blocks(inode,
> -			iomaps, nr_iomaps, &iattr);
> +			iomaps, nr_iomaps,
> +			lcp->lc_size_chg ? lcp->lc_newsize : 0); 
>  	kfree(iomaps);
>  	return nfserrno(error);
>  }
> diff --git a/fs/xfs/xfs_pnfs.c b/fs/xfs/xfs_pnfs.c
> index a52978f6fb76..fee782a3edbe 100644
> --- a/fs/xfs/xfs_pnfs.c
> +++ b/fs/xfs/xfs_pnfs.c
> @@ -257,23 +257,22 @@ xfs_fs_commit_blocks(
>  	struct inode		*inode,
>  	struct iomap		*maps,
>  	int			nr_maps,
> -	struct iattr		*iattr)
> +	loff_t			new_size)
>  {
>  	struct xfs_inode	*ip = XFS_I(inode);
>  	struct xfs_mount	*mp = ip->i_mount;
>  	struct xfs_trans	*tp;
> +	struct timespec64	now;
>  	bool			update_isize = false;
>  	int			error, i;
>  	loff_t			size;
>  
> -	ASSERT(iattr->ia_valid & (ATTR_ATIME|ATTR_CTIME|ATTR_MTIME));
> -
>  	xfs_ilock(ip, XFS_IOLOCK_EXCL);
>  
>  	size = i_size_read(inode);
> -	if ((iattr->ia_valid & ATTR_SIZE) && iattr->ia_size > size) {
> +	if (new_size > size) {
>  		update_isize = true;
> -		size = iattr->ia_size;
> +		size = new_size;
>  	}
>  
>  	for (i = 0; i < nr_maps; i++) {
> @@ -318,11 +317,13 @@ xfs_fs_commit_blocks(
>  	xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
>  	xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
>  
> -	ASSERT(!(iattr->ia_valid & (ATTR_UID | ATTR_GID)));
> -	setattr_copy(&nop_mnt_idmap, inode, iattr);
> +	now = inode_set_ctime_current(inode);
> +	inode_set_atime_to_ts(inode, now);
> +	inode_set_mtime_to_ts(inode, now);
> +
>  	if (update_isize) {
> -		i_size_write(inode, iattr->ia_size);
> -		ip->i_disk_size = iattr->ia_size;
> +		i_size_write(inode, new_size);
> +		ip->i_disk_size = new_size;
>  	}
>  
>  	xfs_trans_set_sync(tp);
> diff --git a/include/linux/exportfs_block.h b/include/linux/exportfs_block.h
> index 1f52fea8e4dc..d1dec4689b14 100644
> --- a/include/linux/exportfs_block.h
> +++ b/include/linux/exportfs_block.h
> @@ -9,7 +9,6 @@
>  
>  #include <linux/types.h>
>  
> -struct iattr;
>  struct inode;
>  struct iomap;
>  struct super_block;
> @@ -33,7 +32,7 @@ struct exportfs_block_ops {
>  	 * the client.
>  	 */
>  	int (*commit_blocks)(struct inode *inode, struct iomap *iomaps,
> -			int nr_iomaps, struct iattr *iattr);
> +			int nr_iomaps, loff_t new_size);
>  };
>  
>  #endif /* LINUX_EXPORTFS_BLOCK_H */

I like this one. I think you can just fold the first patch in this
series into this one, since you're making that change moot (and the
ia_* times don't matter much anyway).

Reviewed-by: Jeff Layton <jlayton@kernel.org>