Fix at least one source of the continued 'NFS append race'. close()

    was calling nfs_flush() and then clearing the NMODIFIED bit.  This is
    not legal since there might still be dirty buffers after the nfs_flush
    (for example, pending commits).  The clearing of this bit in turn prevented
    a necessary vinvalbuf() from occuring leaving left over dirty buffers
    even after truncating the file in a new operation.  The fix is to
    simply not clear NMODIFIED.

    Also added a sysctl vfs.nfs.nfsv3_commit_on_close which, if set to 1,
    will cause close() to do a stage 1 write AND a stage 2 commit
    synchronously.  By default only the stage 1 write is done synchronously.

Reviewed by:	Alfred Perlstein <bright@wintelcom.net>

1 files changed, 21 insertions, 3 deletions

diff --git a/sys/nfs/nfs_vnops.c b/sys/nfs/nfs_vnops.c
index 4097d96..0a6de2f 100644
--- a/sys/nfs/nfs_vnops.c
+++ b/sys/nfs/nfs_vnops.c
@@ -251,6 +251,9 @@ static int	nfsaccess_cache_timeout = NFS_MAXATTRTIMO;
 SYSCTL_INT(_vfs_nfs, OID_AUTO, access_cache_timeout, CTLFLAG_RW, 
 	   &nfsaccess_cache_timeout, 0, "NFS ACCESS cache timeout");
 
+static int	nfsv3_commit_on_close = 0;
+SYSCTL_INT(_vfs_nfs, OID_AUTO, nfsv3_commit_on_close, CTLFLAG_RW, 
+	   &nfsv3_commit_on_close, 0, "write+commit on close, else only write");
 #if 0
 SYSCTL_INT(_vfs_nfs, OID_AUTO, access_cache_hits, CTLFLAG_RD, 
 	   &nfsstats.accesscache_hits, 0, "NFS ACCESS cache hit count");
@@ -561,10 +564,25 @@ nfs_close(ap)
 	    if ((VFSTONFS(vp->v_mount)->nm_flag & NFSMNT_NQNFS) == 0 &&
 		(np->n_flag & NMODIFIED)) {
 		if (NFS_ISV3(vp)) {
-		    error = nfs_flush(vp, ap->a_cred, MNT_WAIT, ap->a_p, 0);
-		    np->n_flag &= ~NMODIFIED;
-		} else
+		    /*
+		     * Under NFSv3 we have dirty buffers to dispose of.  We
+		     * must flush them to the NFS server.  We have the option
+		     * of waiting all the way through the commit rpc or just
+		     * waiting for the initial write.  The default is to only
+		     * wait through the initial write so the data is in the
+		     * server's cache, which is roughly similar to the state
+		     * a standard disk subsystem leaves the file in on close().
+		     *
+		     * We cannot clear the NMODIFIED bit in np->n_flag due to
+		     * potential races with other processes, and certainly
+		     * cannot clear it if we don't commit.
+		     */
+		    int cm = nfsv3_commit_on_close ? 1 : 0;
+		    error = nfs_flush(vp, ap->a_cred, MNT_WAIT, ap->a_p, cm);
+		    /* np->n_flag &= ~NMODIFIED; */
+		} else {
 		    error = nfs_vinvalbuf(vp, V_SAVE, ap->a_cred, ap->a_p, 1);
+		}
 		np->n_attrstamp = 0;
 	    }
 	    if (np->n_flag & NWRITEERR) {