[RFC] Should vfs.nfsrv.async be implemented for new NFS server?

[Rick Macklem]

Hi,

Josh Paetzel pointed out that vfs.nfsrv.async doesn't exist
for the new NFS server.

I don't think I had spotted this before, but when I looked I
saw that, when vfs.nfsrv.async is set non-zero in the old server,
it returns FILESYNC (which means the write has been committed to
non-volatile storage) even when it hasn't actually done that.

This can improve performance, but has some negative implications:
- If the server crashes before the write is committed to
  non-volatile storage, the file modification will be lost.
  (When a server replies UNSTABLE to a write, the client holds
   onto the data in its cache and does the write again if the
   server crashes/reboots before the client does a Commit RPC
   for the file. However, a reply of FILESYNC tells the client
   it can forget about the write, because it is done.)
- Because of the above, replying FILESYNC when the data is not
  yet committed to non-volatile (also referred to as stable)
  storage, this is a violation of RFC1813.

I wouldn't want this to be the default, but am willing to
patch head based on the "backwards compatibility" argument.
My concern with these types of patches is that some people
will enable them without realizing the risk of data loss
that they introduce.

So, how do others feel with respect to whether or not this
patch should be committed to head?

Thanks in advance for any comments, rick
ps: Here's the patch, just in case anyone is interested.
--- fs/nfsserver/nfs_nfsdserv.c.sav	2011-11-05 10:29:38.000000000 -0400
+++ fs/nfsserver/nfs_nfsdserv.c	2011-11-05 10:37:40.000000000 -0400
@@ -55,6 +55,11 @@ extern int nfs_rootfhset;
 extern int nfsrv_enable_crossmntpt;
 #endif	/* !APPLEKEXT */
 
+static int	nfs_async = 0;
+SYSCTL_DECL(_vfs_nfsd);
+SYSCTL_INT(_vfs_nfsd, OID_AUTO, async, CTLFLAG_RW, &nfs_async, 0,
+    "Tell client that writes were synced even though they were not");
+
 /*
  * This list defines the GSS mechanisms supported.
  * (Don't ask me how you get these strings from the RFC stuff like
@@ -912,7 +917,13 @@ nfsrvd_write(struct nfsrv_descript *nd, 
 			goto out;
 		NFSM_BUILD(tl, u_int32_t *, 4 * NFSX_UNSIGNED);
 		*tl++ = txdr_unsigned(retlen);
-		if (stable == NFSWRITE_UNSTABLE)
+		/*
+		 * If nfs_async is set, then pretend the write was FILESYNC.
+		 * Warning: Doing this violates RFC1813 and runs a risk
+		 * of data written by a client being lost when the server
+		 * crashes/reboots.
+		 */
+		if (stable == NFSWRITE_UNSTABLE && nfs_async == 0)
 			*tl++ = txdr_unsigned(stable);
 		else
 			*tl++ = txdr_unsigned(NFSWRITE_FILESYNC);