From c6bf5f18c04e033a967879c0daff9538a856df0b Mon Sep 17 00:00:00 2001 From: Marius Strobl Date: Mon, 22 Feb 2010 21:45:20 +0000 Subject: [PATCH] - Factor out the code shared between NFS client and server into its own module so it's not present twice. - 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. --- sys/modules/Makefile | 1 + sys/modules/nfs_common/Makefile | 8 ++++ sys/modules/nfsclient/Makefile | 4 +- sys/modules/nfsserver/Makefile | 4 +- sys/nfs/nfs_common.c | 77 +++++++++++++++++++++++++++++++++ sys/nfs/nfs_common.h | 1 + sys/nfsclient/nfs_krpc.c | 67 +--------------------------- sys/nfsclient/nfs_vfsops.c | 1 + sys/nfsserver/nfs.h | 1 - sys/nfsserver/nfs_fha.c | 4 +- sys/nfsserver/nfs_srvkrpc.c | 59 +------------------------ sys/nfsserver/nfs_srvsubs.c | 1 + 12 files changed, 98 insertions(+), 130 deletions(-) create mode 100644 sys/modules/nfs_common/Makefile diff --git a/sys/modules/Makefile b/sys/modules/Makefile index e76d63ad39a..07e4bfbacd1 100644 --- a/sys/modules/Makefile +++ b/sys/modules/Makefile @@ -193,6 +193,7 @@ SUBDIR= ${_3dfx} \ ${_ndis} \ ${_netgraph} \ ${_nfe} \ + nfs_common \ nfscl \ nfsclient \ nfscommon \ diff --git a/sys/modules/nfs_common/Makefile b/sys/modules/nfs_common/Makefile new file mode 100644 index 00000000000..8f51f185d8d --- /dev/null +++ b/sys/modules/nfs_common/Makefile @@ -0,0 +1,8 @@ +# $FreeBSD$ + +.PATH: ${.CURDIR}/../../nfs + +KMOD= nfs_common +SRCS= nfs_common.c opt_nfs.h vnode_if.h + +.include diff --git a/sys/modules/nfsclient/Makefile b/sys/modules/nfsclient/Makefile index c53e3d5f98a..5cd3556821f 100644 --- a/sys/modules/nfsclient/Makefile +++ b/sys/modules/nfsclient/Makefile @@ -1,11 +1,11 @@ # $FreeBSD$ -.PATH: ${.CURDIR}/../../nfsclient ${.CURDIR}/../../nfs ${.CURDIR}/../../rpc +.PATH: ${.CURDIR}/../../nfsclient ${.CURDIR}/../../rpc KMOD= nfsclient SRCS= vnode_if.h \ nfs_bio.c nfs_lock.c nfs_node.c nfs_subs.c nfs_nfsiod.c \ - nfs_vfsops.c nfs_vnops.c nfs_common.c nfs_krpc.c \ + nfs_vfsops.c nfs_vnops.c nfs_krpc.c \ opt_inet.h opt_nfs.h opt_bootp.h opt_nfsroot.h SRCS+= opt_inet6.h opt_kdtrace.h opt_kgssapi.h diff --git a/sys/modules/nfsserver/Makefile b/sys/modules/nfsserver/Makefile index 692179b916c..58ab2bc8fbb 100644 --- a/sys/modules/nfsserver/Makefile +++ b/sys/modules/nfsserver/Makefile @@ -1,9 +1,9 @@ # $FreeBSD$ -.PATH: ${.CURDIR}/../../nfsserver ${.CURDIR}/../../nfs +.PATH: ${.CURDIR}/../../nfsserver KMOD= nfsserver SRCS= vnode_if.h \ - nfs_fha.c nfs_serv.c nfs_srvkrpc.c nfs_srvsubs.c nfs_common.c \ + nfs_fha.c nfs_serv.c nfs_srvkrpc.c nfs_srvsubs.c \ opt_mac.h \ opt_kgssapi.h \ opt_nfs.h diff --git a/sys/nfs/nfs_common.c b/sys/nfs/nfs_common.c index b2c231fc61e..fc767c16f6a 100644 --- a/sys/nfs/nfs_common.c +++ b/sys/nfs/nfs_common.c @@ -54,8 +54,10 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include #include +#include #include #include @@ -77,6 +79,16 @@ nfstype nfsv3_type[9] = { static void *nfsm_dissect_xx_sub(int s, struct mbuf **md, caddr_t *dpos, int how); +SYSCTL_NODE(_vfs, OID_AUTO, nfs_common, CTLFLAG_RD, 0, "NFS common support"); + +static int nfs_realign_test; +SYSCTL_INT(_vfs_nfs_common, OID_AUTO, realign_test, CTLFLAG_RD, + &nfs_realign_test, 0, "Number of realign tests done"); + +static int nfs_realign_count; +SYSCTL_INT(_vfs_nfs_common, OID_AUTO, realign_count, CTLFLAG_RD, + &nfs_realign_count, 0, "Number of mbuf realignments done"); + u_quad_t nfs_curusec(void) { @@ -334,3 +346,68 @@ nfsm_adv_xx(int s, struct mbuf **md, caddr_t *dpos) 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); +} + +static moduledata_t nfs_common_mod = { + "nfs_common", + NULL, + NULL +}; + +DECLARE_MODULE(nfs_common, nfs_common_mod, SI_SUB_VFS, SI_ORDER_ANY); +MODULE_VERSION(nfs_common, 1); diff --git a/sys/nfs/nfs_common.h b/sys/nfs/nfs_common.h index 7c061309593..97b8c5ffea3 100644 --- a/sys/nfs/nfs_common.h +++ b/sys/nfs/nfs_common.h @@ -49,6 +49,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 */ diff --git a/sys/nfsclient/nfs_krpc.c b/sys/nfsclient/nfs_krpc.c index 098c46b06a9..43bfe640945 100644 --- a/sys/nfsclient/nfs_krpc.c +++ b/sys/nfsclient/nfs_krpc.c @@ -87,8 +87,6 @@ uint32_t nfsclient_nfs3_start_probes[NFS_NPROCS]; 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 *arg) } } -/* - * 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 @@ -592,7 +527,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); diff --git a/sys/nfsclient/nfs_vfsops.c b/sys/nfsclient/nfs_vfsops.c index 72ca2ce3fe3..a8f32da0188 100644 --- a/sys/nfsclient/nfs_vfsops.c +++ b/sys/nfsclient/nfs_vfsops.c @@ -147,6 +147,7 @@ MODULE_DEPEND(nfs, krpc, 1, 1, 1); #ifdef KGSSAPI MODULE_DEPEND(nfs, kgssapi, 1, 1, 1); #endif +MODULE_DEPEND(nfs, nfs_common, 1, 1, 1); static struct nfs_rpcops nfs_rpcops = { nfs_readrpc, diff --git a/sys/nfsserver/nfs.h b/sys/nfsserver/nfs.h index 00dbe5bcf5a..bb1893e18c0 100644 --- a/sys/nfsserver/nfs.h +++ b/sys/nfsserver/nfs.h @@ -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 *, diff --git a/sys/nfsserver/nfs_fha.c b/sys/nfsserver/nfs_fha.c index 25e930fe413..76537d7ef94 100644 --- a/sys/nfsserver/nfs_fha.c +++ b/sys/nfsserver/nfs_fha.c @@ -202,7 +202,9 @@ fha_extract_info(struct svc_req *req, struct fha_info *i) 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); diff --git a/sys/nfsserver/nfs_srvkrpc.c b/sys/nfsserver/nfs_srvkrpc.c index bdfe4245b06..512373b30e2 100644 --- a/sys/nfsserver/nfs_srvkrpc.c +++ b/sys/nfsserver/nfs_srvkrpc.c @@ -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, gatherdelay, CTLFLAG_RW, 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_descript *nd, int err, 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, SVCXPRT *xprt) 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 - diff --git a/sys/nfsserver/nfs_srvsubs.c b/sys/nfsserver/nfs_srvsubs.c index ee0614b4dc9..d84261e8249 100644 --- a/sys/nfsserver/nfs_srvsubs.c +++ b/sys/nfsserver/nfs_srvsubs.c @@ -560,6 +560,7 @@ DECLARE_MODULE(nfsserver, nfsserver_mod, SI_SUB_VFS, SI_ORDER_ANY); MODULE_VERSION(nfsserver, 1); MODULE_DEPEND(nfsserver, nfssvc, 1, 1, 1); MODULE_DEPEND(nfsserver, krpc, 1, 1, 1); +MODULE_DEPEND(nfsserver, nfs_common, 1, 1, 1); /* * Set up nameidata for a lookup() call and do it.