diff --git a/sys/kern/uipc_debug.c b/sys/kern/uipc_debug.c --- a/sys/kern/uipc_debug.c +++ b/sys/kern/uipc_debug.c @@ -158,10 +158,6 @@ int comma; comma = 0; - if (so_state & SS_FDREF) { - db_printf("%sSS_FDREF", comma ? ", " : ""); - comma = 1; - } if (so_state & SS_ISCONNECTED) { db_printf("%sSS_ISCONNECTED", comma ? ", " : ""); comma = 1; @@ -186,10 +182,6 @@ db_printf("%sSS_ISCONFIRMING", comma ? ", " : ""); comma = 1; } - if (so_state & SS_PROTOREF) { - db_printf("%sSS_PROTOREF", comma ? ", " : ""); - comma = 1; - } } static void diff --git a/sys/kern/uipc_socket.c b/sys/kern/uipc_socket.c --- a/sys/kern/uipc_socket.c +++ b/sys/kern/uipc_socket.c @@ -540,7 +540,6 @@ return (ENOBUFS); so->so_type = type; - so->so_state = SS_FDREF; so->so_cred = crhold(cred); if ((prp->pr_domain->dom_family == PF_INET) || (prp->pr_domain->dom_family == PF_INET6) || @@ -741,7 +740,7 @@ so->so_type = head->so_type; so->so_options = head->so_options & ~SO_ACCEPTCONN; so->so_linger = head->so_linger; - so->so_state = head->so_state & ~SS_FDREF; + so->so_state = head->so_state; so->so_fibnum = head->so_fibnum; so->so_proto = head->so_proto; so->so_cred = crhold(head->so_cred); @@ -763,7 +762,10 @@ so->so_snd.sb_mtx = &so->so_snd_mtx; so->so_rcv.sb_mtx = &so->so_rcv_mtx; } + /* Socket starts with a reference from the listen queue. */ + refcount_init(&so->so_count, 1); if ((*so->so_proto->pr_usrreqs->pru_attach)(so, 0, NULL)) { + MPASS(refcount_release(&so->so_count)); sodealloc(so); log(LOG_DEBUG, "%s: pcb %p: pru_attach() failed\n", __func__, head->so_pcb); @@ -1062,7 +1064,8 @@ /* * Return single connection off a listening socket queue. Main consumer of * the function is kern_accept4(). Some modules, that do their own accept - * management also use the function. + * management also use the function. The socket reference held by the + * listen queue is handed to the caller. * * Listening socket must be locked on entry and is returned unlocked on * return. @@ -1100,7 +1103,6 @@ SOCK_LOCK(so); KASSERT(so->so_qstate == SQ_COMP, ("%s: so %p not SQ_COMP", __func__, so)); - soref(so); head->sol_qlen--; so->so_qstate = SQ_NONE; so->so_listen = NULL; @@ -1117,86 +1119,19 @@ } /* - * Evaluate the reference count and named references on a socket; if no - * references remain, free it. This should be called whenever a reference is - * released, such as in sorele(), but also when named reference flags are - * cleared in socket or protocol code. - * - * sofree() will free the socket if: - * - * - There are no outstanding file descriptor references or related consumers - * (so_count == 0). - * - * - The socket has been closed by user space, if ever open (no SS_FDREF). - * - * - The protocol does not have an outstanding strong reference on the socket - * (SS_PROTOREF). - * - * - The socket is not in a completed connection queue, so a process has been - * notified that it is present. If it is removed, the user process may - * block in accept() despite select() saying the socket was ready. + * Free socket upon release of the very last reference. */ -void +static void sofree(struct socket *so) { struct protosw *pr = so->so_proto; - bool last __diagused; SOCK_LOCK_ASSERT(so); + KASSERT(refcount_load(&so->so_count) == 0, + ("%s: so %p has references", __func__, so)); + KASSERT(SOLISTENING(so) || so->so_qstate == SQ_NONE, + ("%s: so %p is on listen queue", __func__, so)); - if ((so->so_state & (SS_FDREF | SS_PROTOREF)) != 0 || - refcount_load(&so->so_count) != 0 || so->so_qstate == SQ_COMP) { - SOCK_UNLOCK(so); - return; - } - - if (!SOLISTENING(so) && so->so_qstate == SQ_INCOMP) { - struct socket *sol; - - sol = so->so_listen; - KASSERT(sol, ("%s: so %p on incomp of NULL", __func__, so)); - - /* - * To solve race between close of a listening socket and - * a socket on its incomplete queue, we need to lock both. - * The order is first listening socket, then regular. - * Since we don't have SS_FDREF neither SS_PROTOREF, this - * function and the listening socket are the only pointers - * to so. To preserve so and sol, we reference both and then - * relock. - * After relock the socket may not move to so_comp since it - * doesn't have PCB already, but it may be removed from - * so_incomp. If that happens, we share responsiblity on - * freeing the socket, but soclose() has already removed - * it from queue. - */ - soref(sol); - soref(so); - SOCK_UNLOCK(so); - SOLISTEN_LOCK(sol); - SOCK_LOCK(so); - if (so->so_qstate == SQ_INCOMP) { - KASSERT(so->so_listen == sol, - ("%s: so %p migrated out of sol %p", - __func__, so, sol)); - TAILQ_REMOVE(&sol->sol_incomp, so, so_list); - sol->sol_incqlen--; - last = refcount_release(&sol->so_count); - KASSERT(!last, ("%s: released last reference for %p", - __func__, sol)); - so->so_qstate = SQ_NONE; - so->so_listen = NULL; - } else - KASSERT(so->so_listen == NULL, - ("%s: so %p not on (in)comp with so_listen", - __func__, so)); - sorele_locked(sol); - KASSERT(refcount_load(&so->so_count) == 1, - ("%s: so %p count %u", __func__, so, so->so_count)); - so->so_count = 0; - } - if (SOLISTENING(so)) - so->so_error = ECONNABORTED; SOCK_UNLOCK(so); if (so->so_dtor != NULL) @@ -1255,9 +1190,6 @@ int error = 0; bool listening, last __diagused; - KASSERT(so->so_state & SS_FDREF, - ("%s: %p no SS_FDREF on enter", __func__, so)); - CURVNET_SET(so->so_vnet); funsetown(&so->so_sigio); if (so->so_state & SS_ISCONNECTED) { @@ -1308,24 +1240,12 @@ __func__, so)); } } - KASSERT(so->so_state & SS_FDREF, - ("%s: %p no SS_FDREF upon lock", __func__, so)); - so->so_state &= ~SS_FDREF; sorele_locked(so); if (listening) { struct socket *sp, *tsp; - TAILQ_FOREACH_SAFE(sp, &lqueue, so_list, tsp) { - SOCK_LOCK(sp); - if (refcount_load(&sp->so_count) == 0) { - SOCK_UNLOCK(sp); - soabort(sp); - } else { - /* See the handling of queued sockets - in sofree(). */ - SOCK_UNLOCK(sp); - } - } + TAILQ_FOREACH_SAFE(sp, &lqueue, so_list, tsp) + soabort(sp); } CURVNET_RESTORE(); return (error); @@ -1338,31 +1258,30 @@ * * This interface is tricky, because it is called on an unreferenced socket, * and must be called only by a thread that has actually removed the socket - * from the listen queue it was on, or races with other threads are risked. + * from the listen queue it was on. Likely this thread holds the last + * reference on the socket and soabort() will proceed with sofree(). But + * it might be not the last, as the sockets on the listen queues are seen + * from the protocol side. * * This interface will call into the protocol code, so must not be called * with any socket locks held. Protocols do call it while holding their own * recursible protocol mutexes, but this is something that should be subject * to review in the future. + * + * Usually socket should have a single reference left, but this is not a + * requirement. In the past, when we have had named references for file + * descriptor and protocol, we asserted that none of them are being held. */ void soabort(struct socket *so) { - /* - * In as much as is possible, assert that no references to this - * socket are held. This is not quite the same as asserting that the - * current thread is responsible for arranging for no references, but - * is as close as we can get for now. - */ - KASSERT((so->so_state & (SS_FDREF | SS_PROTOREF)) == 0 && - so->so_count == 0, ("%s: so %p has references", __func__, so)); VNET_SO_ASSERT(so); if (so->so_proto->pr_usrreqs->pru_abort != NULL) (*so->so_proto->pr_usrreqs->pru_abort)(so); SOCK_LOCK(so); - sofree(so); + sorele_locked(so); } int @@ -1370,12 +1289,6 @@ { int error; - SOCK_LOCK(so); - KASSERT((so->so_state & SS_FDREF) == 0, - ("%s: %p has SS_FDREF", __func__, so)); - so->so_state |= SS_FDREF; - SOCK_UNLOCK(so); - CURVNET_SET(so->so_vnet); error = (*so->so_proto->pr_usrreqs->pru_accept)(so, nam); CURVNET_RESTORE(); diff --git a/sys/netinet/tcp_subr.c b/sys/netinet/tcp_subr.c --- a/sys/netinet/tcp_subr.c +++ b/sys/netinet/tcp_subr.c @@ -2501,13 +2501,9 @@ so = inp->inp_socket; soisdisconnected(so); if (inp->inp_flags & INP_SOCKREF) { - KASSERT(so->so_state & SS_PROTOREF, - ("tcp_close: !SS_PROTOREF")); inp->inp_flags &= ~INP_SOCKREF; INP_WUNLOCK(inp); - SOCK_LOCK(so); - so->so_state &= ~SS_PROTOREF; - sofree(so); + sorele(so); return (NULL); } return (tp); diff --git a/sys/netinet/tcp_timewait.c b/sys/netinet/tcp_timewait.c --- a/sys/netinet/tcp_timewait.c +++ b/sys/netinet/tcp_timewait.c @@ -370,13 +370,9 @@ * detach and free the socket as it is not needed in time wait. */ if (inp->inp_flags & INP_SOCKREF) { - KASSERT(so->so_state & SS_PROTOREF, - ("tcp_twstart: !SS_PROTOREF")); inp->inp_flags &= ~INP_SOCKREF; INP_WUNLOCK(inp); - SOCK_LOCK(so); - so->so_state &= ~SS_PROTOREF; - sofree(so); + sorele(so); } else INP_WUNLOCK(inp); } @@ -573,11 +569,7 @@ if (inp->inp_flags & INP_SOCKREF) { inp->inp_flags &= ~INP_SOCKREF; INP_WUNLOCK(inp); - SOCK_LOCK(so); - KASSERT(so->so_state & SS_PROTOREF, - ("tcp_twclose: INP_SOCKREF && !SS_PROTOREF")); - so->so_state &= ~SS_PROTOREF; - sofree(so); + sorele(so); } else { /* * If we don't own the only reference, the socket and diff --git a/sys/netinet/tcp_usrreq.c b/sys/netinet/tcp_usrreq.c --- a/sys/netinet/tcp_usrreq.c +++ b/sys/netinet/tcp_usrreq.c @@ -1370,9 +1370,7 @@ TCP_PROBE2(debug__user, tp, PRU_ABORT); } if (!(inp->inp_flags & INP_DROPPED)) { - SOCK_LOCK(so); - so->so_state |= SS_PROTOREF; - SOCK_UNLOCK(so); + soref(so); inp->inp_flags |= INP_SOCKREF; } INP_WUNLOCK(inp); @@ -1413,9 +1411,7 @@ TCP_PROBE2(debug__user, tp, PRU_CLOSE); } if (!(inp->inp_flags & INP_DROPPED)) { - SOCK_LOCK(so); - so->so_state |= SS_PROTOREF; - SOCK_UNLOCK(so); + soref(so); inp->inp_flags |= INP_SOCKREF; } INP_WUNLOCK(inp); diff --git a/sys/ofed/drivers/infiniband/ulp/sdp/sdp_main.c b/sys/ofed/drivers/infiniband/ulp/sdp/sdp_main.c --- a/sys/ofed/drivers/infiniband/ulp/sdp/sdp_main.c +++ b/sys/ofed/drivers/infiniband/ulp/sdp/sdp_main.c @@ -308,13 +308,9 @@ so = ssk->socket; soisdisconnected(so); if (ssk->flags & SDP_SOCKREF) { - KASSERT(so->so_state & SS_PROTOREF, - ("sdp_closed: !SS_PROTOREF")); ssk->flags &= ~SDP_SOCKREF; SDP_WUNLOCK(ssk); - SOCK_LOCK(so); - so->so_state &= ~SS_PROTOREF; - sofree(so); + sorele(so); return (NULL); } return (ssk); @@ -1472,10 +1468,8 @@ * holding on to the socket and pcb for a while. */ if (!(ssk->flags & SDP_DROPPED)) { - SOCK_LOCK(so); - so->so_state |= SS_PROTOREF; - SOCK_UNLOCK(so); ssk->flags |= SDP_SOCKREF; + soref(so); } SDP_WUNLOCK(ssk); } diff --git a/sys/sys/socketvar.h b/sys/sys/socketvar.h --- a/sys/sys/socketvar.h +++ b/sys/sys/socketvar.h @@ -210,7 +210,6 @@ * Many fields will be read without locks to improve performance and avoid * lock order issues. However, this approach must be used with caution. */ -#define SS_FDREF 0x0001 /* strong file descriptor reference */ #define SS_ISCONNECTED 0x0002 /* socket connected to a peer */ #define SS_ISCONNECTING 0x0004 /* in process of connecting to peer */ #define SS_ISDISCONNECTING 0x0008 /* in process of disconnecting */ @@ -219,15 +218,6 @@ #define SS_ISCONFIRMING 0x0400 /* deciding to accept connection req */ #define SS_ISDISCONNECTED 0x2000 /* socket disconnected from peer */ -/* - * Protocols can mark a socket as SS_PROTOREF to indicate that, following - * pru_detach, they still want the socket to persist, and will free it - * themselves when they are done. Protocols should only ever call sofree() - * following setting this flag in pru_detach(), and never otherwise, as - * sofree() bypasses socket reference counting. - */ -#define SS_PROTOREF 0x4000 /* strong protocol reference */ - #ifdef _KERNEL #define SOCK_MTX(so) (&(so)->so_lock) @@ -479,7 +469,6 @@ int sodisconnect(struct socket *so); void sodtor_set(struct socket *, so_dtor_t *); struct sockaddr *sodupsockaddr(const struct sockaddr *sa, int mflags); -void sofree(struct socket *so); void sohasoutofband(struct socket *so); int solisten(struct socket *so, int backlog, struct thread *td); void solisten_proto(struct socket *so, int backlog);