Re: [PATCH 1/4] nfsd/blocklayout: always ignore loca_time_modify

[Jeff Layton <jlayton@kernel.org>]

On Tue, 2026-03-31 at 17:33 +0200, Christoph Hellwig wrote:
> RFC 8881 Section 18.42 makes it clear that the client provided timestamp
> is a "may" condition, and clients that want to force a specific timestamp
> should send a separate SETATTR in the compound.
> 
> Since commit b82f92d5dd1a ("fs: have setattr_copy handle multigrain
> timestamps appropriately") the ia_mtime value is ignored by file
> systems using multi-grain timestamps like XFS, which is the only
> file system supporting blocklayout exports right now, so make that
> explicit in NFSD as well.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/nfsd/blocklayout.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/nfsd/blocklayout.c b/fs/nfsd/blocklayout.c
> index a7cfba29990e..5ee5735b39bb 100644
> --- a/fs/nfsd/blocklayout.c
> +++ b/fs/nfsd/blocklayout.c
> @@ -179,15 +179,20 @@ static __be32
>  nfsd4_block_commit_blocks(struct inode *inode, struct nfsd4_layoutcommit *lcp,
>  		struct iomap *iomaps, int nr_iomaps)
>  {
> -	struct timespec64 mtime = inode_get_mtime(inode);
>  	struct iattr iattr = { .ia_valid = 0 };
>  	int error;
>  
> -	if (lcp->lc_mtime.tv_nsec == UTIME_NOW ||
> -	    timespec64_compare(&lcp->lc_mtime, &mtime) < 0)
> -		lcp->lc_mtime = current_time(inode);
> +	/*
> +	 * This ignores the client provided mtime in loca_time_modify, as a
> +	 * fully client specified mtime doesn't really fit into the Linux
> +	 * multi-grain timestamp architecture.
> +	 *
> +	 * RFC 8881 Section 18.42 makes it clear that the client provided
> +	 * 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 = lcp->lc_mtime;
> +	iattr.ia_atime = iattr.ia_ctime = iattr.ia_mtime = current_time(inode);

Technically, these fields are completely ignored by notify_change,
unless their counterpart ATTR_?TIME_SET flag is set, so you could just
leave them unset.
 
>  
>  	if (lcp->lc_size_chg) {
>  		iattr.ia_valid |= ATTR_SIZE;

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