Page MenuHomeFreeBSD
Files F137595005

D35679.diff
No OneTemporary
Actions

D35679.diff
View Options

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);

File Metadata

Mime Type
text/plain
Expires
Tue, Nov 25, 12:32 PM (5 h, 39 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
26134225
Default Alt Text
D35679.diff (12 KB)

Event Timeline