svn commit: r249623 - in head/sys: fs/nfsclient nfsclient

[Rick Macklem]

Author: rmacklem
Date: Thu Apr 18 13:09:04 2013
New Revision: 249623
URL: http://svnweb.freebsd.org/changeset/base/249623

Log:
  Both NFS clients can deadlock when using the "rdirplus" mount
  option. This can occur when an nfsiod thread that already holds
  a buffer lock attempts to acquire a vnode lock on an entry in
  the directory (a LOR) when another thread holding the vnode lock
  is waiting on an nfsiod thread. This patch avoids the deadlock by disabling
  readahead for this case, so the nfsiod threads never do readdirplus.
  Since readaheads for directories need the directory offset cookie
  from the previous read, they cannot normally happen in parallel.
  As such, testing by jhb@ and myself didn't find any performance
  degredation when this patch is applied. If there is a case where
  this results in a significant performance degradation, mounting
  without the "rdirplus" option can be done to re-enable readahead
  for directories.
  
  Reported and tested by:	jhb
  Reviewed by:	jhb
  MFC after:	2 weeks

Modified:
  head/sys/fs/nfsclient/nfs_clbio.c
  head/sys/nfsclient/nfs_bio.c

Modified: head/sys/fs/nfsclient/nfs_clbio.c
==============================================================================
--- head/sys/fs/nfsclient/nfs_clbio.c	Thu Apr 18 12:43:06 2013	(r249622)
+++ head/sys/fs/nfsclient/nfs_clbio.c	Thu Apr 18 13:09:04 2013	(r249623)
@@ -1404,10 +1404,18 @@ ncl_asyncio(struct nfsmount *nmp, struct
 	 * Commits are usually short and sweet so lets save some cpu and
 	 * leave the async daemons for more important rpc's (such as reads
 	 * and writes).
+	 *
+	 * Readdirplus RPCs do vget()s to acquire the vnodes for entries
+	 * in the directory in order to update attributes. This can deadlock
+	 * with another thread that is waiting for async I/O to be done by
+	 * an nfsiod thread while holding a lock on one of these vnodes.
+	 * To avoid this deadlock, don't allow the async nfsiod threads to
+	 * perform Readdirplus RPCs.
 	 */
 	mtx_lock(&ncl_iod_mutex);
-	if (bp->b_iocmd == BIO_WRITE && (bp->b_flags & B_NEEDCOMMIT) &&
-	    (nmp->nm_bufqiods > ncl_numasync / 2)) {
+	if ((bp->b_iocmd == BIO_WRITE && (bp->b_flags & B_NEEDCOMMIT) &&
+	     (nmp->nm_bufqiods > ncl_numasync / 2)) ||
+	    (bp->b_vp->v_type == VDIR && (nmp->nm_flag & NFSMNT_RDIRPLUS))) {
 		mtx_unlock(&ncl_iod_mutex);
 		return(EIO);
 	}

Modified: head/sys/nfsclient/nfs_bio.c
==============================================================================
--- head/sys/nfsclient/nfs_bio.c	Thu Apr 18 12:43:06 2013	(r249622)
+++ head/sys/nfsclient/nfs_bio.c	Thu Apr 18 13:09:04 2013	(r249623)
@@ -1345,10 +1345,18 @@ nfs_asyncio(struct nfsmount *nmp, struct
 	 * Commits are usually short and sweet so lets save some cpu and
 	 * leave the async daemons for more important rpc's (such as reads
 	 * and writes).
+	 *
+	 * Readdirplus RPCs do vget()s to acquire the vnodes for entries
+	 * in the directory in order to update attributes. This can deadlock
+	 * with another thread that is waiting for async I/O to be done by
+	 * an nfsiod thread while holding a lock on one of these vnodes.
+	 * To avoid this deadlock, don't allow the async nfsiod threads to
+	 * perform Readdirplus RPCs.
 	 */
 	mtx_lock(&nfs_iod_mtx);
-	if (bp->b_iocmd == BIO_WRITE && (bp->b_flags & B_NEEDCOMMIT) &&
-	    (nmp->nm_bufqiods > nfs_numasync / 2)) {
+	if ((bp->b_iocmd == BIO_WRITE && (bp->b_flags & B_NEEDCOMMIT) &&
+	     (nmp->nm_bufqiods > nfs_numasync / 2)) ||
+	    (bp->b_vp->v_type == VDIR && (nmp->nm_flag & NFSMNT_RDIRPLUS))) {
 		mtx_unlock(&nfs_iod_mtx);
 		return(EIO);
 	}