Re: [PATCH 1/4] NFS/localio: Stop further I/O upon hitting an error

[Trond Myklebust <trondmy@kernel.org>]

On Mon, 2026-01-05 at 12:19 -0500, Mike Snitzer wrote:
> On Sat, Jan 03, 2026 at 12:14:57PM -0500, Trond Myklebust wrote:
> > From: Trond Myklebust <trond.myklebust@hammerspace.com>
> > 
> > If the call into the filesystem results in an I/O error, then the
> > next
> > chunk of data won't be contiguous with the end of the last
> > successful
> > chunk. So break out of the I/O loop and report the results.
> > Currently the localio code will do this for a short read/write, but
> > not
> > for an error.
> > 
> > Fixes: 6a218b9c3183 ("nfs/localio: do not issue misaligned DIO out-
> > of-order")
> > Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> 
> Thanks, definitely cleaner to not have the awkward force_done flag,
> sorry for that nastiness.
> 
> But this one needs to be rebased on tip of linus' master due to
> commit
> 3af870aedbff ("nfs/localio: fix regression due to out-of-order
> __put_cred") which landed after the 6.19 merge.
> 
> Like so:
> 
> From: Trond Myklebust <trond.myklebust@hammerspace.com>
> Date: Sat, 3 Jan 2026 12:14:57 -0500
> Subject: [PATCH] NFS/localio: Stop further I/O upon hitting an error
> 
> If the call into the filesystem results in an I/O error, then the
> next
> chunk of data won't be contiguous with the end of the last successful
> chunk. So break out of the I/O loop and report the results.
> Currently the localio code will do this for a short read/write, but
> not
> for an error.
> 
> Fixes: 6a218b9c3183 ("nfs/localio: do not issue misaligned DIO out-
> of-order")
> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> Reviewed-by: Mike Snitzer <snitzer@kernel.org>
> Signed-off-by: Mike Snitzer <snitzer@kernel.org>
> ---
>  fs/nfs/localio.c | 30 ++++++++++++++----------------
>  1 file changed, 14 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/nfs/localio.c b/fs/nfs/localio.c
> index ed2a7efaf8f20..97e4733d04714 100644
> --- a/fs/nfs/localio.c
> +++ b/fs/nfs/localio.c
> @@ -619,7 +619,6 @@ static void nfs_local_call_read(struct
> work_struct *work)
>  		container_of(work, struct nfs_local_kiocb, work);
>  	struct file *filp = iocb->kiocb.ki_filp;
>  	const struct cred *save_cred;
> -	bool force_done = false;
>  	ssize_t status;
>  	int n_iters;
>  
> @@ -639,13 +638,13 @@ static void nfs_local_call_read(struct
> work_struct *work)
>  		status = filp->f_op->read_iter(&iocb->kiocb, &iocb-
> >iters[i]);
>  		revert_creds(save_cred);
>  
> -		if (status != -EIOCBQUEUED) {
> -			if (unlikely(status >= 0 && status < iocb-
> >iters[i].count))
> -				force_done = true; /* Partial read
> */
> -			if (nfs_local_pgio_done(iocb, status,
> force_done)) {
> -				nfs_local_read_iocb_done(iocb);
> -				break;
> -			}
> +		if (status == -EIOCBQUEUED)
> +			continue;
> +		/* Break on completion, errors, or short reads */
> +		if (nfs_local_pgio_done(iocb, status, false) ||
> status < 0 ||
> +		    (size_t)status < iov_iter_count(&iocb-
> >iters[i])) {
> +			nfs_local_read_iocb_done(iocb);
> +			break;
>  		}
>  	}
>  }
> @@ -824,7 +823,6 @@ static void nfs_local_call_write(struct
> work_struct *work)
>  	struct file *filp = iocb->kiocb.ki_filp;
>  	unsigned long old_flags = current->flags;
>  	const struct cred *save_cred;
> -	bool force_done = false;
>  	ssize_t status;
>  	int n_iters;
>  
> @@ -847,13 +845,13 @@ static void nfs_local_call_write(struct
> work_struct *work)
>  		status = filp->f_op->write_iter(&iocb->kiocb, &iocb-
> >iters[i]);
>  		revert_creds(save_cred);
>  
> -		if (status != -EIOCBQUEUED) {
> -			if (unlikely(status >= 0 && status < iocb-
> >iters[i].count))
> -				force_done = true; /* Partial write
> */
> -			if (nfs_local_pgio_done(iocb, status,
> force_done)) {
> -				nfs_local_write_iocb_done(iocb);
> -				break;
> -			}
> +		if (status == -EIOCBQUEUED)
> +			continue;
> +		/* Break on completion, errors, or short writes */
> +		if (nfs_local_pgio_done(iocb, status, false) ||
> status < 0 ||
> +		    (size_t)status < iov_iter_count(&iocb-
> >iters[i])) {
> +			nfs_local_write_iocb_done(iocb);
> +			break;
>  		}
>  	}
>  	file_end_write(filp);

Yep. I rebased my testing branch yesterday, and caught that.

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trondmy@kernel.org, trond.myklebust@hammerspace.com