nfs: only issue commit in DIO codepath if we have uncommitted data
authorJeff Layton <jlayton@kernel.org>
Fri, 22 Jul 2022 18:12:20 +0000 (14:12 -0400)
committerTrond Myklebust <trond.myklebust@hammerspace.com>
Sat, 23 Jul 2022 19:28:59 +0000 (15:28 -0400)
Currently, we try to determine whether to issue a commit based on
nfs_write_need_commit which looks at the current verifier. In the case
where we got a short write and then tried to follow it up with one that
failed, the verifier can't be trusted.

What we really want to know is whether the pgio request had any
successful writes that came back as UNSTABLE. Add a new flag to the pgio
request, and use that to indicate that we've had a successful unstable
write. Only issue a commit if that flag is set.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
fs/nfs/direct.c
fs/nfs/write.c
include/linux/nfs_xdr.h

index a47d132961948c9f5476d4b99507877d46aba556..86df66bb14c5921084c760e5734d097d77fbc68a 100644 (file)
@@ -690,7 +690,7 @@ static void nfs_direct_write_completion(struct nfs_pgio_header *hdr)
        }
 
        nfs_direct_count_bytes(dreq, hdr);
-       if (hdr->good_bytes != 0 && nfs_write_need_commit(hdr)) {
+       if (test_bit(NFS_IOHDR_UNSTABLE_WRITES, &hdr->flags)) {
                if (!dreq->flags)
                        dreq->flags = NFS_ODIRECT_DO_COMMIT;
                flags = dreq->flags;
index 1c706465d090b095b98c900d8b5fe7cb407af57e..16d166bc4099d835cf06ca3f16311c05027abb57 100644 (file)
@@ -1576,25 +1576,37 @@ static int nfs_writeback_done(struct rpc_task *task,
        nfs_add_stats(inode, NFSIOS_SERVERWRITTENBYTES, hdr->res.count);
        trace_nfs_writeback_done(task, hdr);
 
-       if (hdr->res.verf->committed < hdr->args.stable &&
-           task->tk_status >= 0) {
-               /* We tried a write call, but the server did not
-                * commit data to stable storage even though we
-                * requested it.
-                * Note: There is a known bug in Tru64 < 5.0 in which
-                *       the server reports NFS_DATA_SYNC, but performs
-                *       NFS_FILE_SYNC. We therefore implement this checking
-                *       as a dprintk() in order to avoid filling syslog.
-                */
-               static unsigned long    complain;
+       if (task->tk_status >= 0) {
+               enum nfs3_stable_how committed = hdr->res.verf->committed;
+
+               if (committed == NFS_UNSTABLE) {
+                       /*
+                        * We have some uncommitted data on the server at
+                        * this point, so ensure that we keep track of that
+                        * fact irrespective of what later writes do.
+                        */
+                       set_bit(NFS_IOHDR_UNSTABLE_WRITES, &hdr->flags);
+               }
 
-               /* Note this will print the MDS for a DS write */
-               if (time_before(complain, jiffies)) {
-                       dprintk("NFS:       faulty NFS server %s:"
-                               " (committed = %d) != (stable = %d)\n",
-                               NFS_SERVER(inode)->nfs_client->cl_hostname,
-                               hdr->res.verf->committed, hdr->args.stable);
-                       complain = jiffies + 300 * HZ;
+               if (committed < hdr->args.stable) {
+                       /* We tried a write call, but the server did not
+                        * commit data to stable storage even though we
+                        * requested it.
+                        * Note: There is a known bug in Tru64 < 5.0 in which
+                        *       the server reports NFS_DATA_SYNC, but performs
+                        *       NFS_FILE_SYNC. We therefore implement this checking
+                        *       as a dprintk() in order to avoid filling syslog.
+                        */
+                       static unsigned long    complain;
+
+                       /* Note this will print the MDS for a DS write */
+                       if (time_before(complain, jiffies)) {
+                               dprintk("NFS:       faulty NFS server %s:"
+                                       " (committed = %d) != (stable = %d)\n",
+                                       NFS_SERVER(inode)->nfs_client->cl_hostname,
+                                       committed, hdr->args.stable);
+                               complain = jiffies + 300 * HZ;
+                       }
                }
        }
 
index 0e3aa0f5f324d4f24339b73e7c15e287c2855a43..e86cf6642d21240da2af685d5a8969cdbf453c19 100644 (file)
@@ -1600,6 +1600,7 @@ enum {
        NFS_IOHDR_STAT,
        NFS_IOHDR_RESEND_PNFS,
        NFS_IOHDR_RESEND_MDS,
+       NFS_IOHDR_UNSTABLE_WRITES,
 };
 
 struct nfs_io_completion;