Date: Wed, 18 Sep 2002 13:23:00 -0400 From: Bosko Milekic <bmilekic@unixdaemons.com> To: Poul-Henning Kamp <phk@freebsd.org> Cc: arch@freebsd.org Subject: Re: Trivial mbuf patch for review. Message-ID: <20020918132300.A34069@unixdaemons.com> In-Reply-To: <3185.1032361630@critter.freebsd.dk>; from phk@freebsd.org on Wed, Sep 18, 2002 at 05:07:10PM %2B0200 References: <3185.1032361630@critter.freebsd.dk>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Sep 18, 2002 at 05:07:10PM +0200, Poul-Henning Kamp wrote:
>
> This patch is a no-op which replaces local mbuf-chain counting
> loops with calls to m_length() and in one case m_fixhdr().
>
> Index: kern/uipc_socket2.c
> ===================================================================
> RCS file: /home/ncvs/src/sys/kern/uipc_socket2.c,v
> retrieving revision 1.103
> diff -u -r1.103 uipc_socket2.c
> --- kern/uipc_socket2.c 16 Aug 2002 18:41:48 -0000 1.103
> +++ kern/uipc_socket2.c 18 Sep 2002 14:08:34 -0000
> @@ -498,11 +498,11 @@
> #ifdef SOCKBUF_DEBUG
> void
> sbcheck(sb)
> - register struct sockbuf *sb;
> + struct sockbuf *sb;
> {
> - register struct mbuf *m;
> - register struct mbuf *n = 0;
> - register u_long len = 0, mbcnt = 0;
> + struct mbuf *m;
> + struct mbuf *n = 0;
> + u_long len = 0, mbcnt = 0;
>
> for (m = sb->sb_mb; m; m = n) {
> n = m->m_nextpkt;
> @@ -610,22 +610,18 @@
> */
> int
> sbappendaddr(sb, asa, m0, control)
> - register struct sockbuf *sb;
> + struct sockbuf *sb;
> struct sockaddr *asa;
> struct mbuf *m0, *control;
> {
> - register struct mbuf *m, *n;
> + struct mbuf *m, *n;
> int space = asa->sa_len;
>
> if (m0 && (m0->m_flags & M_PKTHDR) == 0)
> panic("sbappendaddr");
> if (m0)
> space += m0->m_pkthdr.len;
> - for (n = control; n; n = n->m_next) {
> - space += n->m_len;
> - if (n->m_next == 0) /* keep pointer to last control buf */
> - break;
> - }
> + space += m_length(control, &n);
> if (space > sbspace(sb))
> return (0);
> if (asa->sa_len > MLEN)
> @@ -657,19 +653,12 @@
> struct sockbuf *sb;
> struct mbuf *control, *m0;
> {
> - register struct mbuf *m, *n;
> - int space = 0;
> + struct mbuf *m, *n;
> + int space;
>
> if (control == 0)
> panic("sbappendcontrol");
> - for (m = control; ; m = m->m_next) {
> - space += m->m_len;
> - if (m->m_next == 0)
> - break;
> - }
> - n = m; /* save pointer to last control buffer */
> - for (m = m0; m; m = m->m_next)
> - space += m->m_len;
> + space = m_length(control, &n) + m_length(m0, NULL);
> if (space > sbspace(sb))
> return (0);
> n->m_next = m0; /* concatenate data to control */
Looks like there is a problem here, as you removed the 'n = m'
initialization.
> Index: net/bpf.c
> ===================================================================
> RCS file: /home/ncvs/src/sys/net/bpf.c,v
> retrieving revision 1.94
> diff -u -r1.94 bpf.c
> --- net/bpf.c 31 Jul 2002 16:11:32 -0000 1.94
> +++ net/bpf.c 18 Sep 2002 14:18:31 -0000
> @@ -1123,11 +1123,8 @@
> struct bpf_if *bp = ifp->if_bpf;
> struct bpf_d *d;
> u_int pktlen, slen;
> - struct mbuf *m0;
>
> - pktlen = 0;
> - for (m0 = m; m0 != 0; m0 = m0->m_next)
> - pktlen += m0->m_len;
> + pktlen = m_length(m, NULL);
>
> BPFIF_LOCK(bp);
> for (d = bp->bif_dlist; d != 0; d = d->bd_next) {
> Index: net/if_ppp.c
> ===================================================================
> RCS file: /home/ncvs/src/sys/net/if_ppp.c,v
> retrieving revision 1.83
> diff -u -r1.83 if_ppp.c
> --- net/if_ppp.c 19 Aug 2002 19:22:41 -0000 1.83
> +++ net/if_ppp.c 18 Sep 2002 14:19:07 -0000
> @@ -758,7 +758,6 @@
> struct ifqueue *ifq;
> enum NPmode mode;
> int len;
> - struct mbuf *m;
>
> #ifdef MAC
> error = mac_check_ifnet_transmit(ifp, m0);
> @@ -851,9 +850,7 @@
> *cp++ = protocol & 0xff;
> m0->m_len += PPP_HDRLEN;
>
> - len = 0;
> - for (m = m0; m != 0; m = m->m_next)
> - len += m->m_len;
> + len = m_length(m0, NULL);
>
> if (sc->sc_flags & SC_LOG_OUTPKT) {
> printf("ppp%d output: ", ifp->if_unit);
> @@ -1087,9 +1084,7 @@
> struct mbuf *mcomp = NULL;
> int slen, clen;
>
> - slen = 0;
> - for (mp = m; mp != NULL; mp = mp->m_next)
> - slen += mp->m_len;
> + slen = m_length(m, NULL);
> clen = (*sc->sc_xcomp->compress)
> (sc->sc_xc_state, &mcomp, m, slen, sc->sc_if.if_mtu + PPP_HDRLEN);
> if (mcomp != NULL) {
> @@ -1324,9 +1319,7 @@
> sc->sc_stats.ppp_ipackets++;
>
> if (sc->sc_flags & SC_LOG_INPKT) {
> - ilen = 0;
> - for (mp = m; mp != NULL; mp = mp->m_next)
> - ilen += mp->m_len;
> + ilen = m_length(m, NULL);
> printf("ppp%d: got %d bytes\n", ifp->if_unit, ilen);
> pppdumpm(m);
> }
> @@ -1389,9 +1382,7 @@
> }
> #endif
>
> - ilen = 0;
> - for (mp = m; mp != NULL; mp = mp->m_next)
> - ilen += mp->m_len;
> + ilen = m_length(m, NULL);
>
> #ifdef VJC
> if (sc->sc_flags & SC_VJ_RESET) {
> Index: netinet/ip_input.c
> ===================================================================
> RCS file: /home/ncvs/src/sys/netinet/ip_input.c,v
> retrieving revision 1.208
> diff -u -r1.208 ip_input.c
> --- netinet/ip_input.c 17 Sep 2002 11:20:02 -0000 1.208
> +++ netinet/ip_input.c 18 Sep 2002 13:47:10 -0000
> @@ -1071,12 +1071,8 @@
> m->m_len += (IP_VHL_HL(ip->ip_vhl) << 2);
> m->m_data -= (IP_VHL_HL(ip->ip_vhl) << 2);
> /* some debugging cruft by sklower, below, will go away soon */
> - if (m->m_flags & M_PKTHDR) { /* XXX this should be done elsewhere */
> - register int plen = 0;
> - for (t = m; t; t = t->m_next)
> - plen += t->m_len;
> - m->m_pkthdr.len = plen;
> - }
> + if (m->m_flags & M_PKTHDR) /* XXX this should be done elsewhere */
> + m_fixhdr(m);
> return (m);
>
> dropfrag:
> Index: netns/idp_usrreq.c
> ===================================================================
> RCS file: /home/ncvs/src/sys/netns/idp_usrreq.c,v
> retrieving revision 1.12
> diff -u -r1.12 idp_usrreq.c
> --- netns/idp_usrreq.c 31 May 2002 11:52:34 -0000 1.12
> +++ netns/idp_usrreq.c 18 Sep 2002 14:14:06 -0000
> @@ -144,18 +144,12 @@
> register struct mbuf *m;
> register struct idp *idp;
> register struct socket *so;
> - register int len = 0;
> + register int len;
> register struct route *ro;
> struct mbuf *mprev;
> extern int idpcksum;
>
> - /*
> - * Calculate data length.
> - */
> - for (m = m0; m; m = m->m_next) {
> - mprev = m;
> - len += m->m_len;
> - }
> + len = m_length(m0, &mprev);
> /*
> * Make sure packet is actually of even length.
> */
> Index: netns/spp_usrreq.c
> ===================================================================
> RCS file: /home/ncvs/src/sys/netns/spp_usrreq.c,v
> retrieving revision 1.16
> diff -u -r1.16 spp_usrreq.c
> --- netns/spp_usrreq.c 25 Aug 2002 13:17:35 -0000 1.16
> +++ netns/spp_usrreq.c 18 Sep 2002 14:13:27 -0000
> @@ -687,8 +687,7 @@
> firstbad = m;
> /*for (;;) {*/
> /* calculate length */
> - for (m0 = m, len = 0; m ; m = m->m_next)
> - len += m->m_len;
> + len = m_length(m);
> if (len > cb->s_mtu) {
> }
> /* FINISH THIS
> Index: netsmb/smb_rq.c
> ===================================================================
> RCS file: /home/ncvs/src/sys/netsmb/smb_rq.c,v
> retrieving revision 1.7
> diff -u -r1.7 smb_rq.c
> --- netsmb/smb_rq.c 16 Sep 2002 09:51:58 -0000 1.7
> +++ netsmb/smb_rq.c 18 Sep 2002 14:12:50 -0000
> @@ -421,9 +421,7 @@
> m0 = m_split(mtop, offset, M_TRYWAIT);
> if (m0 == NULL)
> return EBADRPC;
> - for(len = 0, m = m0; m->m_next; m = m->m_next)
> - len += m->m_len;
> - len += m->m_len;
> + len = m_length(m0, &m);
> m->m_len -= len - count;
> if (mdp->md_top == NULL) {
> md_initm(mdp, m0);
> Index: nfsclient/nfs_socket.c
> ===================================================================
> RCS file: /home/ncvs/src/sys/nfsclient/nfs_socket.c,v
> retrieving revision 1.86
> diff -u -r1.86 nfs_socket.c
> --- nfsclient/nfs_socket.c 8 Sep 2002 15:11:18 -0000 1.86
> +++ nfsclient/nfs_socket.c 18 Sep 2002 14:19:42 -0000
> @@ -869,13 +869,7 @@
> rep->r_vp = vp;
> rep->r_td = td;
> rep->r_procnum = procnum;
> - i = 0;
> - m = mrest;
> - while (m) {
> - i += m->m_len;
> - m = m->m_next;
> - }
> - mrest_len = i;
> + mrest_len = i = m_length(mrest, NULL);
>
> /*
> * Get the RPC header with authorization.
> Index: nfsserver/nfs_syscalls.c
> ===================================================================
> RCS file: /home/ncvs/src/sys/nfsserver/nfs_syscalls.c,v
> retrieving revision 1.80
> diff -u -r1.80 nfs_syscalls.c
> --- nfsserver/nfs_syscalls.c 24 Jul 2002 23:10:34 -0000 1.80
> +++ nfsserver/nfs_syscalls.c 18 Sep 2002 14:11:36 -0000
> @@ -451,12 +451,7 @@
> nfsrv_updatecache(nd, TRUE, mreq);
> nd->nd_mrep = NULL;
> case RC_REPLY:
> - m = mreq;
> - siz = 0;
> - while (m) {
> - siz += m->m_len;
> - m = m->m_next;
> - }
> + siz = m_length(mreq, NULL);
> if (siz <= 0 || siz > NFS_MAXPACKET) {
> printf("mbuf siz=%d\n",siz);
> panic("Bad nfs svc reply");
> --
> Poul-Henning Kamp | UNIX since Zilog Zeus 3.20
> phk@FreeBSD.ORG | TCP/IP since RFC 956
> FreeBSD committer | BSD since 4.3-tahoe
> Never attribute to malice what can adequately be explained by incompetence.
The rest looks OK, from a very quick glance. By the way, it's
good to see that you're doing some cleanups in this code (re:
m_length() implementation and m_fixhdr() movements). Thank you.
Regards,
--
Bosko Milekic * bmilekic@unixdaemons.com * bmilekic@FreeBSD.org
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-arch" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20020918132300.A34069>