Re: [PATCH 1/2] NFSv4/pNFS: prevent i_size regression while layoutcommit is outstanding

[Trond Myklebust]

On Thu, 2026-10-08 at 22:28 +0000, Lucheng Bao wrote:
> Commit ac46bd374c9a ("pNFS: Ensure we layoutcommit before
> revalidating
> attributes") replaced outstanding-layoutcommit attribute filtering
> with
> synchronization before explicit revalidation. For layouts requiring
> LAYOUTCOMMIT, OPEN attributes can still shrink i_size before metadata
> synchronization completes.

The client can't police OPEN. It has no idea which file will be
affected (particularly when doing an open-by-filename) so unlike the
case of an explicit truncate() call, it can't serialise with the
truncate.
It is therefore up to the server to resolve any ambiguity, either by
recalling or revoking the layout before allowing the OPEN truncate to
proceed.

> 
> Commit d8c951c313ed ("NFSv4.1: Don't trust attributes if a pNFS
> LAYOUTCOMMIT is outstanding") moved LAYOUTCOMMIT post-op attribute
> processing after cleanup, leaving another unprotected window after
> NFS_INO_LAYOUTCOMMITTING is cleared.

This is unrelated to the above problem, so fixes for this issue need to
be in a separate patch.

> 
> Extend the existing writeback checks to reject smaller sizes while a
> LAYOUTCOMMIT is pending or in flight, preserving size increases and
> WCC
> checks. Process the reply's attributes and establish their generation
> barrier before cleanup removes that protection.
> 
> Fixes: d8c951c313ed ("NFSv4.1: Don't trust attributes if a pNFS
> LAYOUTCOMMIT is outstanding")
> Fixes: ac46bd374c9a ("pNFS: Ensure we layoutcommit before
> revalidating attributes")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Lucheng Bao <lubao@xxxxxxxxxxxxxxxx>
> ---
>  fs/nfs/inode.c    | 8 ++++++--
>  fs/nfs/nfs4proc.c | 2 +-
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> index 3022454f7698..11432f521b04 100644
> --- a/fs/nfs/inode.c
> +++ b/fs/nfs/inode.c
> @@ -1636,7 +1636,9 @@ static void nfs_wcc_update_inode(struct inode
> *inode, struct nfs_fattr *fattr)
>  	if ((fattr->valid & NFS_ATTR_FATTR_PRESIZE)
>  			&& (fattr->valid & NFS_ATTR_FATTR_SIZE)
>  			&& i_size_read(inode) ==
> nfs_size_to_loff_t(fattr->pre_size)
> -			&& !nfs_have_writebacks(inode)) {
> +			&& !nfs_have_writebacks(inode)
> +			&& (nfs_size_to_loff_t(fattr->size) >=
> i_size_read(inode)
> +				||
> !pnfs_layoutcommit_outstanding(inode))) {

Under what circumstance is the client legally supposed to be able to
call nfs_wcc_update_inode() with NFS_ATTR_FATTR_PRESIZE set, while
there is an outstanding layoutcommit?

>  		trace_nfs_size_wcc(inode, fattr->size);
>  		i_size_write(inode, nfs_size_to_loff_t(fattr-
> >size));
>  	}
> @@ -2392,7 +2394,9 @@ static int nfs_update_inode(struct inode
> *inode, struct nfs_fattr *fattr)
>  		if (new_isize != cur_isize && !have_delegation) {
>  			/* Do we perhaps have any outstanding
> writes, or has
>  			 * the file grown beyond our last write? */
> -			if (!nfs_have_writebacks(inode) || new_isize
> > cur_isize) {
> +			if ((!nfs_have_writebacks(inode) &&
> +			     !pnfs_layoutcommit_outstanding(inode))
> ||
> +			    new_isize > cur_isize) {
>  				trace_nfs_size_update(inode,
> new_isize);
>  				i_size_write(inode, new_isize);
>  				if (!have_writers)
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 04b1987115d5..1ae047a062b8 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -10093,9 +10093,9 @@ static void nfs4_layoutcommit_release(void
> *calldata)
>  {
>  	struct nfs4_layoutcommit_data *data = calldata;
>  
> -	pnfs_cleanup_layoutcommit(data);
>  	nfs_post_op_update_inode_force_wcc(data->args.inode,
>  					   data->res.fattr);
> +	pnfs_cleanup_layoutcommit(data);
>  	put_cred(data->cred);
>  	nfs_iput_and_deactive(data->inode);
>  	kfree(data);

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trondmy@xxxxxxxxxx, trond.myklebust@xxxxxxxxxxxxxxx