Date: Mon, 22 Dec 2003 21:25:03 -0800 (PST) From: Sam Leffler <sam@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 44227 for review Message-ID: <200312230525.hBN5P2jK083859@repoman.freebsd.org>
index | next in thread | raw e-mail
http://perforce.freebsd.org/chv.cgi?CH=44227 Change 44227 by sam@sam_ebb on 2003/12/22 21:24:29 Fix braino: we can drop the mutex because we have the sockbuf marked locked (via sblock). This means the previous change to move the externalization of MT_CONTROL mbufs doesn't need to be moved to the bottom of soreceive--we can do it in the same place it was before by just dropping the sockbuf lock. This means we don't change the order of processing and so don't change any of the semantics of error handling. With this re-realization we can also fix the calls to dup sockaddr's and copy data when peeking to use M_WAITOK/M_TRYWAIT. It may not be worthwhile for performance reasons as doing so requires that we drop+reaquire the sockbuf mutex but we can revisit that. At the least the peek case is unlikely to be noticed and the cost is probably down in the noice for recvfrom/recvmsg too. Need to move local variable decls back up to the top of the function before committing to keep bde happy. Affected files ... .. //depot/projects/netperf+sockets/sys/kern/uipc_socket.c#9 edit Differences ... ==== //depot/projects/netperf+sockets/sys/kern/uipc_socket.c#9 (text+ko) ==== @@ -775,19 +775,14 @@ int flags, len, error, offset; struct protosw *pr = so->so_proto; struct mbuf *nextrecord; - struct mbuf *cm, **cme; int moff, type = 0; int orig_resid = uio->uio_resid; mp = mp0; if (psa) *psa = 0; - cm = NULL; - if (controlp) { + if (controlp) *controlp = 0; - cme = &cm; - } else - cme = &cm; /* XXX to silence gcc */ if (flagsp) flags = *flagsp &~ MSG_EOR; else @@ -908,12 +903,10 @@ KASSERT(m->m_type == MT_SONAME, ("m->m_type == %d", m->m_type)); if (psa) { + SOCKBUF_UNLOCK(&so->so_rcv); *psa = sodupsockaddr(mtod(m, struct sockaddr *), - M_NOWAIT); /* XXX */ - if (*psa == NULL) { - error = ENOMEM; - goto release; - } + M_WAITOK); + SOCKBUF_LOCK(&so->so_rcv); } if (flags & MSG_PEEK) { m = m->m_next; @@ -925,10 +918,16 @@ orig_resid = 0; } if (m && m->m_type == MT_CONTROL) { + struct mbuf *cm = NULL; + struct mbuf **cme = &cm; + do { if (flags & MSG_PEEK) { if (controlp) { - *controlp = m_copy(m, 0, m->m_len); + SOCKBUF_UNLOCK(&so->so_rcv); + *controlp = m_copym(m, 0, m->m_len, + M_TRYWAIT); + SOCKBUF_LOCK(&so->so_rcv); if (*controlp == NULL) { error = ENOBUFS; goto release; @@ -942,10 +941,7 @@ m->m_next = NULL; if (controlp) { /* - * Link mbufs together for processing - * below. See the comments there for - * an explanation of why we delay the - * work. + * Collect mbufs for processing below. */ *cme = m; cme = &(*cme)->m_next; @@ -954,6 +950,20 @@ m = so->so_rcv.sb_mb; } } while (m && m->m_type == MT_CONTROL); + if (cm != NULL) { + if (pr->pr_domain->dom_externalize) { + /* + * NB: drop the lock to avoid potential LORs; + * in particular unix domain sockets grab the + * file descriptor lock which would be a LOR. + */ + SOCKBUF_UNLOCK(&so->so_rcv); + error = (*pr->pr_domain->dom_externalize) + (cm, controlp); + SOCKBUF_LOCK(&so->so_rcv); + } else + m_freem(cm); + } orig_resid = 0; } if (m) { @@ -1159,20 +1169,6 @@ sbunlock(&so->so_rcv); out: SOCKBUF_UNLOCK(&so->so_rcv); - if (cm != NULL) { - /* - * Deal with control data now that we've dopped the - * sockbuf lock. This is important as otherwise, for - * unix domain sockets, we create a LOR between so_rcv - * and the file descriptor lock. Note we assume the - * externalize method handles a list of mbufs. - */ - if (error == 0 && pr->pr_domain->dom_externalize) { - /* XXX ignore error? */ - error = (*pr->pr_domain->dom_externalize)(cm, controlp); - } else - m_freem(cm); - } return (error); }help
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200312230525.hBN5P2jK083859>