Subject: Re: Add "last record" and "last mbuf" pointers to sockbuf
To: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
From: Jason R Thorpe <thorpej@wasabisystems.com>
List: tech-kern
Date: 07/03/2002 08:37:04
On Wed, Jul 03, 2002 at 06:30:58PM +0900, YAMAMOTO Takashi wrote:
> i made a patch. (attached)
> both tcp and udp seems to work with it.
Wow, great!
I have some questions for you, though...
> --- bak/uipc_socket.c Wed Jul 3 13:40:41 2002
> +++ uipc_socket.c Wed Jul 3 18:07:00 2002
> @@ -894,6 +894,8 @@
> error = EWOULDBLOCK;
> goto release;
> }
> + SBLASTRECORDCHK(&so->so_rcv, "soreceive sbwait 1");
> + SBLASTMBUFCHK(&so->so_rcv, "soreceive sbwait 1");
> sbunlock(&so->so_rcv);
> error = sbwait(&so->so_rcv);
> splx(s);
> @@ -931,12 +933,10 @@
> if (paddr) {
> *paddr = m;
> so->so_rcv.sb_mb = m->m_next;
> - SB_UPDATE_TAIL(&so->so_rcv);
> m->m_next = 0;
> m = so->so_rcv.sb_mb;
> } else {
> MFREE(m, so->so_rcv.sb_mb);
> - SB_UPDATE_TAIL(&so->so_rcv);
> m = so->so_rcv.sb_mb;
> }
> }
> @@ -956,12 +956,10 @@
> error = (*pr->pr_domain->dom_externalize)(m);
> *controlp = m;
> so->so_rcv.sb_mb = m->m_next;
> - SB_UPDATE_TAIL(&so->so_rcv);
> m->m_next = 0;
> m = so->so_rcv.sb_mb;
> } else {
> MFREE(m, so->so_rcv.sb_mb);
> - SB_UPDATE_TAIL(&so->so_rcv);
> m = so->so_rcv.sb_mb;
> }
> }
> @@ -984,6 +982,13 @@
> if (type == MT_OOBDATA)
> flags |= MSG_OOB;
> }
> + else {
> + if ((flags & MSG_PEEK) == 0) {
> + KASSERT(so->so_rcv.sb_mb == m);
> + so->so_rcv.sb_mb = nextrecord;
> + SB_UPDATE_TAIL(&so->so_rcv);
> + }
> + }
With this change, I think the next block of:
/*
* If nextrecord == NULL (this is a single chain), then
* sb_lastrecord may not be valid here if m was changed
* earlier.
*/
if (nextrecord == NULL && (flags & MSG_PEEK) == 0) {
KASSERT(so->so_rcv.sb_mb == m);
so->so_rcv.sb_lastrecord = m;
}
is partially redundant. I think it can be removed, and the the previous
block changed to:
if (m) {
if ((flags & MSG_PEEK) == 0) {
m->m_nextpkt = nextrecord;
/*
* If nextrecord == NULL (this is a single chain),
* then sb_lastrecord may not be valid here if m
* was changed earlier.
*/
if (nextrecord == NULL) {
KASSERT(so->so_rcv.sb_mb == m);
so->so_rcv.sb_lastrecord = m;
}
}
type = m->m_type;
if (type == MT_OOBDATA)
flags |= MSG_OOB;
} else {
if ((flags & MSG_PEEK) == 0) {
KASSERT(so->so_rcv.sb_mb == m);
so->so_rcv.sb_mb = nextrecord;
SB_UPDATE_TAIL(&so->so_rcv);
}
}
Do you agree?
--
-- Jason R. Thorpe <thorpej@wasabisystems.com>