On Fri, 2024-03-01 at 11:49 -0500, Josef Bacik wrote:
> We protect accesses to the nfs_direct_req fields with the dreq->lock
> ever where except nfs_direct_commit_complete. This isn't a huge deal,
> but it does lead to confusion, and we could potentially end up setting
> NFS_ODIRECT_RESCHED_WRITES in one thread where we've had an error in
> another. Clean this up to properly protect ->error and ->flags in the
> commit completion path.
>
> Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
> ---
> fs/nfs/direct.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfs/direct.c b/fs/nfs/direct.c
> index c03926a1cc73..befcc167e25f 100644
> --- a/fs/nfs/direct.c
> +++ b/fs/nfs/direct.c
> @@ -606,6 +606,7 @@ static void nfs_direct_commit_complete(struct nfs_commit_data *data)
>
> trace_nfs_direct_commit_complete(dreq);
>
> + spin_lock(&dreq->lock);
> if (status < 0) {
> /* Errors in commit are fatal */
> dreq->error = status;
> @@ -613,6 +614,7 @@ static void nfs_direct_commit_complete(struct nfs_commit_data *data)
> } else {
> status = dreq->error;
> }
> + spin_unlock(&dreq->lock);
>
> nfs_init_cinfo_from_dreq(&cinfo, dreq);
>
> @@ -625,7 +627,10 @@ static void nfs_direct_commit_complete(struct nfs_commit_data *data)
> spin_unlock(&dreq->lock);
> nfs_release_request(req);
> } else if (!nfs_write_match_verf(verf, req)) {
> - dreq->flags = NFS_ODIRECT_RESCHED_WRITES;
> + spin_lock(&dreq->lock);
> + if (dreq->flags == 0)
> + dreq->flags = NFS_ODIRECT_RESCHED_WRITES;
> + spin_unlock(&dreq->lock);
> /*
> * Despite the reboot, the write was successful,
> * so reset wb_nio.
Seems reasonable, even if just for consistency's sake:
Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>
