Commit bc760564 authored by Gleb Smirnoff's avatar Gleb Smirnoff
Browse files

sockets: use positive flag for file descriptor socket reference

Rename SS_NOFDREF to SS_FDREF and flip all bitwise operations.
Mark sockets created by socreate() with SS_FDREF.

This change is mostly illustrative. With it we see that SS_FDREF
is a debugging flag, since:
* socreate() takes a reference with soref().
* on accept path solisten_dequeue() takes a reference
  with soref() and then soaccept() sets SS_FDREF.
* soclose() checks SS_FDREF, removes it and does sorele().

Reviewed by:		tuexen
Differential revision:	https://reviews.freebsd.org/D35678
parent 74703901
...@@ -158,8 +158,8 @@ db_print_sostate(short so_state) ...@@ -158,8 +158,8 @@ db_print_sostate(short so_state)
int comma; int comma;
comma = 0; comma = 0;
if (so_state & SS_NOFDREF) { if (so_state & SS_FDREF) {
db_printf("%sSS_NOFDREF", comma ? ", " : ""); db_printf("%sSS_FDREF", comma ? ", " : "");
comma = 1; comma = 1;
} }
if (so_state & SS_ISCONNECTED) { if (so_state & SS_ISCONNECTED) {
......
...@@ -498,8 +498,8 @@ sodealloc(struct socket *so) ...@@ -498,8 +498,8 @@ sodealloc(struct socket *so)
} }
/* /*
* socreate returns a socket with a ref count of 1. The socket should be * socreate returns a socket with a ref count of 1 and a file descriptor
* closed with soclose(). * reference. The socket should be closed with soclose().
*/ */
int int
socreate(int dom, struct socket **aso, int type, int proto, socreate(int dom, struct socket **aso, int type, int proto,
...@@ -540,6 +540,7 @@ socreate(int dom, struct socket **aso, int type, int proto, ...@@ -540,6 +540,7 @@ socreate(int dom, struct socket **aso, int type, int proto,
return (ENOBUFS); return (ENOBUFS);
so->so_type = type; so->so_type = type;
so->so_state = SS_FDREF;
so->so_cred = crhold(cred); so->so_cred = crhold(cred);
if ((prp->pr_domain->dom_family == PF_INET) || if ((prp->pr_domain->dom_family == PF_INET) ||
(prp->pr_domain->dom_family == PF_INET6) || (prp->pr_domain->dom_family == PF_INET6) ||
...@@ -740,7 +741,7 @@ sonewconn(struct socket *head, int connstatus) ...@@ -740,7 +741,7 @@ sonewconn(struct socket *head, int connstatus)
so->so_type = head->so_type; so->so_type = head->so_type;
so->so_options = head->so_options & ~SO_ACCEPTCONN; so->so_options = head->so_options & ~SO_ACCEPTCONN;
so->so_linger = head->so_linger; so->so_linger = head->so_linger;
so->so_state = head->so_state | SS_NOFDREF; so->so_state = head->so_state & ~SS_FDREF;
so->so_fibnum = head->so_fibnum; so->so_fibnum = head->so_fibnum;
so->so_proto = head->so_proto; so->so_proto = head->so_proto;
so->so_cred = crhold(head->so_cred); so->so_cred = crhold(head->so_cred);
...@@ -1126,7 +1127,7 @@ solisten_dequeue(struct socket *head, struct socket **ret, int flags) ...@@ -1126,7 +1127,7 @@ solisten_dequeue(struct socket *head, struct socket **ret, int flags)
* - There are no outstanding file descriptor references or related consumers * - There are no outstanding file descriptor references or related consumers
* (so_count == 0). * (so_count == 0).
* *
* - The socket has been closed by user space, if ever open (SS_NOFDREF). * - 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 * - The protocol does not have an outstanding strong reference on the socket
* (SS_PROTOREF). * (SS_PROTOREF).
...@@ -1143,7 +1144,7 @@ sofree(struct socket *so) ...@@ -1143,7 +1144,7 @@ sofree(struct socket *so)
SOCK_LOCK_ASSERT(so); SOCK_LOCK_ASSERT(so);
if ((so->so_state & (SS_NOFDREF | SS_PROTOREF)) != SS_NOFDREF || if ((so->so_state & (SS_FDREF | SS_PROTOREF)) != 0 ||
refcount_load(&so->so_count) != 0 || so->so_qstate == SQ_COMP) { refcount_load(&so->so_count) != 0 || so->so_qstate == SQ_COMP) {
SOCK_UNLOCK(so); SOCK_UNLOCK(so);
return; return;
...@@ -1159,7 +1160,7 @@ sofree(struct socket *so) ...@@ -1159,7 +1160,7 @@ sofree(struct socket *so)
* To solve race between close of a listening socket and * To solve race between close of a listening socket and
* a socket on its incomplete queue, we need to lock both. * a socket on its incomplete queue, we need to lock both.
* The order is first listening socket, then regular. * The order is first listening socket, then regular.
* Since we don't have SS_NOFDREF neither SS_PROTOREF, this * Since we don't have SS_FDREF neither SS_PROTOREF, this
* function and the listening socket are the only pointers * function and the listening socket are the only pointers
* to so. To preserve so and sol, we reference both and then * to so. To preserve so and sol, we reference both and then
* relock. * relock.
...@@ -1254,7 +1255,8 @@ soclose(struct socket *so) ...@@ -1254,7 +1255,8 @@ soclose(struct socket *so)
int error = 0; int error = 0;
bool listening, last __diagused; bool listening, last __diagused;
KASSERT(!(so->so_state & SS_NOFDREF), ("soclose: SS_NOFDREF on enter")); KASSERT(so->so_state & SS_FDREF,
("%s: %p no SS_FDREF on enter", __func__, so));
CURVNET_SET(so->so_vnet); CURVNET_SET(so->so_vnet);
funsetown(&so->so_sigio); funsetown(&so->so_sigio);
...@@ -1306,8 +1308,9 @@ soclose(struct socket *so) ...@@ -1306,8 +1308,9 @@ soclose(struct socket *so)
__func__, so)); __func__, so));
} }
} }
KASSERT((so->so_state & SS_NOFDREF) == 0, ("soclose: NOFDREF")); KASSERT(so->so_state & SS_FDREF,
so->so_state |= SS_NOFDREF; ("%s: %p no SS_FDREF upon lock", __func__, so));
so->so_state &= ~SS_FDREF;
sorele_locked(so); sorele_locked(so);
if (listening) { if (listening) {
struct socket *sp, *tsp; struct socket *sp, *tsp;
...@@ -1352,9 +1355,8 @@ soabort(struct socket *so) ...@@ -1352,9 +1355,8 @@ soabort(struct socket *so)
* current thread is responsible for arranging for no references, but * current thread is responsible for arranging for no references, but
* is as close as we can get for now. * is as close as we can get for now.
*/ */
KASSERT(so->so_count == 0, ("soabort: so_count")); KASSERT((so->so_state & (SS_FDREF | SS_PROTOREF)) == 0 &&
KASSERT((so->so_state & SS_PROTOREF) == 0, ("soabort: SS_PROTOREF")); so->so_count == 0, ("%s: so %p has references", __func__, so));
KASSERT(so->so_state & SS_NOFDREF, ("soabort: !SS_NOFDREF"));
VNET_SO_ASSERT(so); VNET_SO_ASSERT(so);
if (so->so_proto->pr_usrreqs->pru_abort != NULL) if (so->so_proto->pr_usrreqs->pru_abort != NULL)
...@@ -1369,8 +1371,9 @@ soaccept(struct socket *so, struct sockaddr **nam) ...@@ -1369,8 +1371,9 @@ soaccept(struct socket *so, struct sockaddr **nam)
int error; int error;
SOCK_LOCK(so); SOCK_LOCK(so);
KASSERT((so->so_state & SS_NOFDREF) != 0, ("soaccept: !NOFDREF")); KASSERT((so->so_state & SS_FDREF) == 0,
so->so_state &= ~SS_NOFDREF; ("%s: %p has SS_FDREF", __func__, so));
so->so_state |= SS_FDREF;
SOCK_UNLOCK(so); SOCK_UNLOCK(so);
CURVNET_SET(so->so_vnet); CURVNET_SET(so->so_vnet);
......
...@@ -210,7 +210,7 @@ struct socket { ...@@ -210,7 +210,7 @@ struct socket {
* Many fields will be read without locks to improve performance and avoid * Many fields will be read without locks to improve performance and avoid
* lock order issues. However, this approach must be used with caution. * lock order issues. However, this approach must be used with caution.
*/ */
#define SS_NOFDREF 0x0001 /* no file table ref any more */ #define SS_FDREF 0x0001 /* strong file descriptor reference */
#define SS_ISCONNECTED 0x0002 /* socket connected to a peer */ #define SS_ISCONNECTED 0x0002 /* socket connected to a peer */
#define SS_ISCONNECTING 0x0004 /* in process of connecting to peer */ #define SS_ISCONNECTING 0x0004 /* in process of connecting to peer */
#define SS_ISDISCONNECTING 0x0008 /* in process of disconnecting */ #define SS_ISDISCONNECTING 0x0008 /* in process of disconnecting */
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment