From 74fb0ba732c0470e4cae21d0a7af01715bd77bc3 Mon Sep 17 00:00:00 2001 From: John Baldwin Date: Mon, 1 Jun 2009 21:17:03 +0000 Subject: Rework socket upcalls to close some races with setup/teardown of upcalls. - Each socket upcall is now invoked with the appropriate socket buffer locked. It is not permissible to call soisconnected() with this lock held; however, so socket upcalls now return an integer value. The two possible values are SU_OK and SU_ISCONNECTED. If an upcall returns SU_ISCONNECTED, then the soisconnected() will be invoked on the socket after the socket buffer lock is dropped. - A new API is provided for setting and clearing socket upcalls. The API consists of soupcall_set() and soupcall_clear(). - To simplify locking, each socket buffer now has a separate upcall. - When a socket upcall returns SU_ISCONNECTED, the upcall is cleared from the receive socket buffer automatically. Note that a SO_SND upcall should never return SU_ISCONNECTED. - All this means that accept filters should now return SU_ISCONNECTED instead of calling soisconnected() directly. They also no longer need to explicitly clear the upcall on the new socket. - The HTTP accept filter still uses soupcall_set() to manage its internal state machine, but other accept filters no longer have any explicit knowlege of socket upcall internals aside from their return value. - The various RPC client upcalls currently drop the socket buffer lock while invoking soreceive() as a temporary band-aid. The plan for the future is to add a new flag to allow soreceive() to be called with the socket buffer locked. - The AIO callback for socket I/O is now also invoked with the socket buffer locked. Previously sowakeup() would drop the socket buffer lock only to call aio_swake() which immediately re-acquired the socket buffer lock for the duration of the function call. Discussed with: rwatson, rmacklem --- sys/nfsserver/nfs.h | 2 +- sys/nfsserver/nfs_srvsock.c | 4 ++-- sys/nfsserver/nfs_syscalls.c | 10 ++++------ 3 files changed, 7 insertions(+), 9 deletions(-) (limited to 'sys/nfsserver') diff --git a/sys/nfsserver/nfs.h b/sys/nfsserver/nfs.h index b47ffea89340..25fe8ae2d8bf 100644 --- a/sys/nfsserver/nfs.h +++ b/sys/nfsserver/nfs.h @@ -367,7 +367,7 @@ void nfsrv_timer(void *); int nfsrv_getcache(struct nfsrv_descript *, struct mbuf **); void nfsrv_updatecache(struct nfsrv_descript *, int, struct mbuf *); void nfsrv_cleancache(void); -void nfsrv_rcv(struct socket *so, void *arg, int waitflag); +int nfsrv_rcv(struct socket *so, void *arg, int waitflag); void nfsrv_slpderef(struct nfssvc_sock *slp); void nfsrv_wakenfsd(struct nfssvc_sock *slp); int nfsrv_writegather(struct nfsrv_descript **, struct nfssvc_sock *, diff --git a/sys/nfsserver/nfs_srvsock.c b/sys/nfsserver/nfs_srvsock.c index 639772b8a3b5..db48869184d9 100644 --- a/sys/nfsserver/nfs_srvsock.c +++ b/sys/nfsserver/nfs_srvsock.c @@ -406,7 +406,7 @@ nfsmout: * Essentially do as much as possible non-blocking, else punt and it will * be called with M_WAIT from an nfsd. */ -void +int nfsrv_rcv(struct socket *so, void *arg, int waitflag) { struct nfssvc_sock *slp = (struct nfssvc_sock *)arg; @@ -420,7 +420,7 @@ nfsrv_rcv(struct socket *so, void *arg, int waitflag) /* XXXRW: Unlocked read. */ if ((slp->ns_flag & SLP_VALID) == 0) - return; + return (SU_OK); /* * We can't do this in the context of a socket callback diff --git a/sys/nfsserver/nfs_syscalls.c b/sys/nfsserver/nfs_syscalls.c index 59a30a3b0a7a..29c9643ee4eb 100644 --- a/sys/nfsserver/nfs_syscalls.c +++ b/sys/nfsserver/nfs_syscalls.c @@ -263,11 +263,11 @@ nfssvc_addsock(struct file *fp, struct sockaddr *mynam) slp->ns_nam = mynam; fhold(fp); slp->ns_fp = fp; + NFSD_UNLOCK(); SOCKBUF_LOCK(&so->so_rcv); - so->so_upcallarg = (caddr_t)slp; - so->so_upcall = nfsrv_rcv; - so->so_rcv.sb_flags |= SB_UPCALL; + soupcall_set(so, SO_RCV, nfsrv_rcv, slp); SOCKBUF_UNLOCK(&so->so_rcv); + NFSD_LOCK(); slp->ns_flag = (SLP_VALID | SLP_NEEDQ); nfsrv_wakenfsd(slp); NFSD_UNLOCK(); @@ -585,9 +585,7 @@ nfsrv_zapsock(struct nfssvc_sock *slp) slp->ns_fp = NULL; so = slp->ns_so; SOCKBUF_LOCK(&so->so_rcv); - so->so_rcv.sb_flags &= ~SB_UPCALL; - so->so_upcall = NULL; - so->so_upcallarg = NULL; + soupcall_clear(so, SO_RCV); SOCKBUF_UNLOCK(&so->so_rcv); soshutdown(so, SHUT_RDWR); closef(fp, NULL); -- cgit v1.3