Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 9 Feb 2010 23:45:14 +0000 (UTC)
From:      Marius Strobl <marius@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r203732 - in head/sys: nfs nfsclient nfsserver
Message-ID:  <201002092345.o19NjFoW017367@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: marius
Date: Tue Feb  9 23:45:14 2010
New Revision: 203732
URL: http://svn.freebsd.org/changeset/base/203732

Log:
  - Move nfs_realign() from the NFS client to the shared NFS code and
    remove the NFS server version in order to reduce code duplication.
    The shared version now uses a second parameter how, which is passed
    on to m_get(9) and m_getcl(9) as the server used M_WAIT while the
    client requires M_DONTWAIT, and replaces the the previously unused
    parameter hsiz.
  - Change nfs_realign() to use nfsm_aligned() so as with other NFS code
    the alignment check isn't actually performed on platforms without
    strict alignment requirements for performance reasons because as the
    comment suggests unaligned data only occasionally occurs with TCP.
  - Change fha_extract_info() to use nfs_realign() with M_DONTWAIT rather
    than M_WAIT because it's called with the RPC sp_lock held.
  
  Reviewed by:	jhb, rmacklem
  MFC after:	1 week

Modified:
  head/sys/nfs/nfs_common.c
  head/sys/nfs/nfs_common.h
  head/sys/nfsclient/nfs_krpc.c
  head/sys/nfsserver/nfs.h
  head/sys/nfsserver/nfs_fha.c
  head/sys/nfsserver/nfs_srvkrpc.c

Modified: head/sys/nfs/nfs_common.c
==============================================================================
--- head/sys/nfs/nfs_common.c	Tue Feb  9 23:40:07 2010	(r203731)
+++ head/sys/nfs/nfs_common.c	Tue Feb  9 23:45:14 2010	(r203732)
@@ -56,6 +56,7 @@ __FBSDID("$FreeBSD$");
 #include <sys/malloc.h>
 #include <sys/sysent.h>
 #include <sys/syscall.h>
+#include <sys/sysctl.h>
 
 #include <vm/vm.h>
 #include <vm/vm_object.h>
@@ -78,6 +79,16 @@ nfstype nfsv3_type[9] = {
 static void *nfsm_dissect_xx_sub(int s, struct mbuf **md, caddr_t *dpos,
     int how);
 
+SYSCTL_DECL(_vfs_nfs);
+
+static int nfs_realign_test;
+SYSCTL_INT(_vfs_nfs, OID_AUTO, realign_test, CTLFLAG_RD, &nfs_realign_test,
+    0, "Number of realign tests done");
+
+static int nfs_realign_count;
+SYSCTL_INT(_vfs_nfs, OID_AUTO, realign_count, CTLFLAG_RD, &nfs_realign_count,
+    0, "Number of mbuf realignments done");
+
 u_quad_t
 nfs_curusec(void)
 {
@@ -337,3 +348,59 @@ nfsm_adv_xx(int s, struct mbuf **md, cad
 		return (t1);
 	return (0);
 }
+
+/*
+ * Check for badly aligned mbuf data and realign by copying the unaligned
+ * portion of the data into a new mbuf chain and freeing the portions of the
+ * old chain that were replaced.
+ *
+ * We cannot simply realign the data within the existing mbuf chain because
+ * the underlying buffers may contain other rpc commands and we cannot afford
+ * to overwrite them.
+ *
+ * We would prefer to avoid this situation entirely.  The situation does not
+ * occur with NFS/UDP and is supposed to only occassionally occur with TCP.
+ * Use vfs.nfs.realign_count and realign_test to check this.
+ */
+int
+nfs_realign(struct mbuf **pm, int how)
+{
+	struct mbuf *m, *n;
+	int off;
+
+	++nfs_realign_test;
+	while ((m = *pm) != NULL) {
+		if (!nfsm_aligned(m->m_len, u_int32_t) ||
+		    !nfsm_aligned(mtod(m, intptr_t), u_int32_t)) {
+			/*
+			 * NB: we can't depend on m_pkthdr.len to help us
+			 * decide what to do here.  May not be worth doing
+			 * the m_length calculation as m_copyback will
+			 * expand the mbuf chain below as needed.
+			 */
+			if (m_length(m, NULL) >= MINCLSIZE) {
+				/* NB: m_copyback handles space > MCLBYTES */
+				n = m_getcl(how, MT_DATA, 0);
+			} else
+				n = m_get(how, MT_DATA);
+			if (n == NULL)
+				return (ENOMEM);
+			/*
+			 * Align the remainder of the mbuf chain.
+			 */
+			n->m_len = 0;
+			off = 0;
+			while (m != NULL) {
+				m_copyback(n, off, m->m_len, mtod(m, caddr_t));
+				off += m->m_len;
+				m = m->m_next;
+			}
+			m_freem(*pm);
+			*pm = n;
+			++nfs_realign_count;
+			break;
+		}
+		pm = &m->m_next;
+	}
+	return (0);
+}

Modified: head/sys/nfs/nfs_common.h
==============================================================================
--- head/sys/nfs/nfs_common.h	Tue Feb  9 23:40:07 2010	(r203731)
+++ head/sys/nfs/nfs_common.h	Tue Feb  9 23:45:14 2010	(r203732)
@@ -48,6 +48,7 @@ extern nfstype nfsv3_type[];
 int	nfs_adv(struct mbuf **, caddr_t *, int, int);
 u_quad_t nfs_curusec(void);
 void	*nfsm_disct(struct mbuf **, caddr_t *, int, int, int);
+int	nfs_realign(struct mbuf **, int);
 
 /* ****************************** */
 /* Build request/reply phase macros */

Modified: head/sys/nfsclient/nfs_krpc.c
==============================================================================
--- head/sys/nfsclient/nfs_krpc.c	Tue Feb  9 23:40:07 2010	(r203731)
+++ head/sys/nfsclient/nfs_krpc.c	Tue Feb  9 23:45:14 2010	(r203732)
@@ -87,8 +87,6 @@ uint32_t	nfsclient_nfs3_start_probes[NFS
 uint32_t	nfsclient_nfs3_done_probes[NFS_NPROCS];
 #endif
 
-static int	nfs_realign_test;
-static int	nfs_realign_count;
 static int	nfs_bufpackets = 4;
 static int	nfs_reconnects;
 static int	nfs3_jukebox_delay = 10;
@@ -97,10 +95,6 @@ static int	fake_wchan;
 
 SYSCTL_DECL(_vfs_nfs);
 
-SYSCTL_INT(_vfs_nfs, OID_AUTO, realign_test, CTLFLAG_RW, &nfs_realign_test, 0,
-    "Number of realign tests done");
-SYSCTL_INT(_vfs_nfs, OID_AUTO, realign_count, CTLFLAG_RW, &nfs_realign_count, 0,
-    "Number of mbuf realignments done");
 SYSCTL_INT(_vfs_nfs, OID_AUTO, bufpackets, CTLFLAG_RW, &nfs_bufpackets, 0,
     "Buffer reservation size 2 < x < 64");
 SYSCTL_INT(_vfs_nfs, OID_AUTO, reconnects, CTLFLAG_RD, &nfs_reconnects, 0,
@@ -403,65 +397,6 @@ nfs_feedback(int type, int proc, void *a
 }
 
 /*
- *	nfs_realign:
- *
- *	Check for badly aligned mbuf data and realign by copying the unaligned
- *	portion of the data into a new mbuf chain and freeing the portions
- *	of the old chain that were replaced.
- *
- *	We cannot simply realign the data within the existing mbuf chain
- *	because the underlying buffers may contain other rpc commands and
- *	we cannot afford to overwrite them.
- *
- *	We would prefer to avoid this situation entirely.  The situation does
- *	not occur with NFS/UDP and is supposed to only occassionally occur
- *	with TCP.  Use vfs.nfs.realign_count and realign_test to check this.
- *
- */
-static int
-nfs_realign(struct mbuf **pm, int hsiz)
-{
-	struct mbuf *m, *n;
-	int off, space;
-
-	++nfs_realign_test;
-	while ((m = *pm) != NULL) {
-		if ((m->m_len & 0x3) || (mtod(m, intptr_t) & 0x3)) {
-			/*
-			 * NB: we can't depend on m_pkthdr.len to help us
-			 * decide what to do here.  May not be worth doing
-			 * the m_length calculation as m_copyback will
-			 * expand the mbuf chain below as needed.
-			 */
-			space = m_length(m, NULL);
-			if (space >= MINCLSIZE) {
-				/* NB: m_copyback handles space > MCLBYTES */
-				n = m_getcl(M_DONTWAIT, MT_DATA, 0);
-			} else
-				n = m_get(M_DONTWAIT, MT_DATA);
-			if (n == NULL)
-				return (ENOMEM);
-			/*
-			 * Align the remainder of the mbuf chain.
-			 */
-			n->m_len = 0;
-			off = 0;
-			while (m != NULL) {
-				m_copyback(n, off, m->m_len, mtod(m, caddr_t));
-				off += m->m_len;
-				m = m->m_next;
-			}
-			m_freem(*pm);
-			*pm = n;
-			++nfs_realign_count;
-			break;
-		}
-		pm = &m->m_next;
-	}
-	return (0);
-}
-
-/*
  * nfs_request - goes something like this
  *	- fill in request struct
  *	- links it into list
@@ -589,7 +524,7 @@ tryagain:
 	 * These could cause pointer alignment problems, so copy them to
 	 * well aligned mbufs.
 	 */
-	error = nfs_realign(&mrep, 2 * NFSX_UNSIGNED);
+	error = nfs_realign(&mrep, M_DONTWAIT);
 	if (error == ENOMEM) {
 		m_freem(mrep);
 		AUTH_DESTROY(auth);

Modified: head/sys/nfsserver/nfs.h
==============================================================================
--- head/sys/nfsserver/nfs.h	Tue Feb  9 23:40:07 2010	(r203731)
+++ head/sys/nfsserver/nfs.h	Tue Feb  9 23:45:14 2010	(r203732)
@@ -239,7 +239,6 @@ extern int nfs_debug;
 
 #endif
 
-void	nfs_realign(struct mbuf **);
 struct mbuf *nfs_rephead(int, struct nfsrv_descript *, int, struct mbuf **,
 	    caddr_t *);
 void	nfsm_srvfattr(struct nfsrv_descript *, struct vattr *,

Modified: head/sys/nfsserver/nfs_fha.c
==============================================================================
--- head/sys/nfsserver/nfs_fha.c	Tue Feb  9 23:40:07 2010	(r203731)
+++ head/sys/nfsserver/nfs_fha.c	Tue Feb  9 23:45:14 2010	(r203732)
@@ -202,7 +202,9 @@ fha_extract_info(struct svc_req *req, st
 	    procnum == NFSPROC_NULL)
 		goto out;
 
-	nfs_realign(&req->rq_args);
+	error = nfs_realign(&req->rq_args, M_DONTWAIT);
+	if (error)
+		goto out;
 	md = req->rq_args;
 	dpos = mtod(md, caddr_t);
 

Modified: head/sys/nfsserver/nfs_srvkrpc.c
==============================================================================
--- head/sys/nfsserver/nfs_srvkrpc.c	Tue Feb  9 23:40:07 2010	(r203731)
+++ head/sys/nfsserver/nfs_srvkrpc.c	Tue Feb  9 23:45:14 2010	(r203732)
@@ -96,8 +96,6 @@ SYSCTL_DECL(_vfs_nfsrv);
 SVCPOOL		*nfsrv_pool;
 int		nfsd_waiting = 0;
 int		nfsrv_numnfsd = 0;
-static int	nfs_realign_test;
-static int	nfs_realign_count;
 struct callout	nfsrv_callout;
 static eventhandler_tag nfsrv_nmbclusters_tag;
 
@@ -111,10 +109,6 @@ SYSCTL_INT(_vfs_nfsrv, OID_AUTO, gatherd
 SYSCTL_INT(_vfs_nfsrv, OID_AUTO, gatherdelay_v3, CTLFLAG_RW,
     &nfsrvw_procrastinate_v3, 0,
     "Delay in seconds for NFSv3 write gathering");
-SYSCTL_INT(_vfs_nfsrv, OID_AUTO, realign_test, CTLFLAG_RW,
-	    &nfs_realign_test, 0, "");
-SYSCTL_INT(_vfs_nfsrv, OID_AUTO, realign_count, CTLFLAG_RW,
-	    &nfs_realign_count, 0, "");
 
 static int	nfssvc_addsock(struct file *, struct thread *);
 static int	nfssvc_nfsd(struct thread *, struct nfsd_nfsd_args *);
@@ -250,57 +244,6 @@ nfs_rephead(int siz, struct nfsrv_descri
 	return (mreq);
 }
 
-/*
- *	nfs_realign:
- *
- *	Check for badly aligned mbuf data and realign by copying the unaligned
- *	portion of the data into a new mbuf chain and freeing the portions
- *	of the old chain that were replaced.
- *
- *	We cannot simply realign the data within the existing mbuf chain
- *	because the underlying buffers may contain other rpc commands and
- *	we cannot afford to overwrite them.
- *
- *	We would prefer to avoid this situation entirely.  The situation does
- *	not occur with NFS/UDP and is supposed to only occassionally occur
- *	with TCP.  Use vfs.nfs.realign_count and realign_test to check this.
- */
-void
-nfs_realign(struct mbuf **pm)	/* XXX COMMON */
-{
-	struct mbuf *m;
-	struct mbuf *n = NULL;
-	int off = 0;
-
-	++nfs_realign_test;
-	while ((m = *pm) != NULL) {
-		if ((m->m_len & 0x3) || (mtod(m, intptr_t) & 0x3)) {
-			MGET(n, M_WAIT, MT_DATA);
-			if (m->m_len >= MINCLSIZE) {
-				MCLGET(n, M_WAIT);
-			}
-			n->m_len = 0;
-			break;
-		}
-		pm = &m->m_next;
-	}
-
-	/*
-	 * If n is non-NULL, loop on m copying data, then replace the
-	 * portion of the chain that had to be realigned.
-	 */
-	if (n != NULL) {
-		++nfs_realign_count;
-		while (m) {
-			m_copyback(n, off, m->m_len, mtod(m, caddr_t));
-			off += m->m_len;
-			m = m->m_next;
-		}
-		m_freem(*pm);
-		*pm = n;
-	}
-}
-
 static void
 nfssvc_program(struct svc_req *rqst, SVCXPRT *xprt)
 {
@@ -334,7 +277,7 @@ nfssvc_program(struct svc_req *rqst, SVC
 	mreq = mrep = NULL;
 	mreq = rqst->rq_args;
 	rqst->rq_args = NULL;
-	nfs_realign(&mreq);
+	(void)nfs_realign(&mreq, M_WAIT);
 
 	/*
 	 * Note: we want rq_addr, not svc_getrpccaller for nd_nam2 -



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201002092345.o19NjFoW017367>