Date: Tue, 4 Nov 2003 17:54:31 -0800 (PST) From: Sam Leffler <sam@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 41404 for review Message-ID: <200311050154.hA51sVdo098812@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=41404 Change 41404 by sam@sam_ebb on 2003/11/04 17:53:41 o add an additional flags parameter to netisr_register to identify whether an isr thread is MPSAFE or not (those that are not have Giant acquired+released for them) o add NET_PICKUP_GIANT and NET_DROP_GIANT definitions to acquire and release Giant according to the setting of debug_mpsafenet o cleanup existing code to not explicitly acquire Giant when the netisr code does it for us o move debug_mpsafenet stuff to net/netisr.c and make the sysctl r/o since it's only really meaningful at boot Affected files ... .. //depot/projects/netperf/sys/dev/usb/usb_ethersubr.c#3 edit .. //depot/projects/netperf/sys/kern/kern_poll.c#7 edit .. //depot/projects/netperf/sys/net/if_ppp.c#6 edit .. //depot/projects/netperf/sys/net/netisr.c#6 edit .. //depot/projects/netperf/sys/net/netisr.h#2 edit .. //depot/projects/netperf/sys/netatalk/aarp.c#5 edit .. //depot/projects/netperf/sys/netatalk/ddp_input.c#3 edit .. //depot/projects/netperf/sys/netatalk/ddp_usrreq.c#2 edit .. //depot/projects/netperf/sys/netgraph/ng_base.c#3 edit .. //depot/projects/netperf/sys/netinet/if_ether.c#16 edit .. //depot/projects/netperf/sys/netinet/ip_divert.c#10 edit .. //depot/projects/netperf/sys/netinet/ip_input.c#18 edit .. //depot/projects/netperf/sys/netinet/ip_mroute.c#19 edit .. //depot/projects/netperf/sys/netinet/ip_output.c#13 edit .. //depot/projects/netperf/sys/netinet6/ip6_input.c#18 edit .. //depot/projects/netperf/sys/netipx/ipx_input.c#6 edit .. //depot/projects/netperf/sys/netnatm/natm.c#5 edit .. //depot/projects/netperf/sys/netnatm/natm_proto.c#3 edit .. //depot/projects/netperf/sys/sys/mutex.h#5 edit Differences ... ==== //depot/projects/netperf/sys/dev/usb/usb_ethersubr.c#3 (text+ko) ==== @@ -117,7 +117,7 @@ { if (mtx_inited) return; - netisr_register(NETISR_USB, (netisr_t *)usbintr, NULL); + netisr_register(NETISR_USB, (netisr_t *)usbintr, NULL, 0); mtx_init(&usbq_tx.ifq_mtx, "usbq_tx_mtx", NULL, MTX_DEF); mtx_init(&usbq_rx.ifq_mtx, "usbq_rx_mtx", NULL, MTX_DEF); mtx_inited++; ==== //depot/projects/netperf/sys/kern/kern_poll.c#7 (text+ko) ==== @@ -187,8 +187,8 @@ init_device_poll(void) { - netisr_register(NETISR_POLL, (netisr_t *)netisr_poll, NULL); - netisr_register(NETISR_POLLMORE, (netisr_t *)netisr_pollmore, NULL); + netisr_register(NETISR_POLL, (netisr_t *)netisr_poll, NULL, 0); + netisr_register(NETISR_POLLMORE, (netisr_t *)netisr_pollmore, NULL, 0); } SYSINIT(device_poll, SI_SUB_CLOCKS, SI_ORDER_MIDDLE, init_device_poll, NULL) ==== //depot/projects/netperf/sys/net/if_ppp.c#6 (text+ko) ==== @@ -244,7 +244,7 @@ case MOD_LOAD: if_clone_attach(&ppp_cloner); - netisr_register(NETISR_PPP, (netisr_t *)pppintr, NULL); + netisr_register(NETISR_PPP, (netisr_t *)pppintr, NULL, 0); /* * XXX layering violation - if_ppp can work over any lower * level transport that cares to attach to it. ==== //depot/projects/netperf/sys/net/netisr.c#6 (text+ko) ==== @@ -53,11 +53,22 @@ #include <net/if_var.h> #include <net/netisr.h> +/* + * XXX this is a temporary measure to allow folks to + * XXX disable Giant locking in the network code without + * XXX recompiling--in case of problems. + */ +int debug_mpsafenet = 1; +TUNABLE_INT("debug.mpsafenet", &debug_mpsafenet); +SYSCTL_INT(_debug, OID_AUTO, mpsafenet, CTLFLAG_RD, &debug_mpsafenet, 0, + "Enable/disable MPSAFE network support"); + volatile unsigned int netisr; /* scheduling bits for network */ struct netisr { netisr_t *ni_handler; struct ifqueue *ni_queue; + int ni_flags; } netisrs[32]; static void *net_ih; @@ -69,13 +80,16 @@ } void -netisr_register(int num, netisr_t *handler, struct ifqueue *inq) +netisr_register(int num, netisr_t *handler, struct ifqueue *inq, int flags) { KASSERT(!(num < 0 || num >= (sizeof(netisrs)/sizeof(*netisrs))), ("bad isr %d", num)); netisrs[num].ni_handler = handler; netisrs[num].ni_queue = inq; + if ((flags & NETISR_MPSAFE) && !debug_mpsafenet) + flags &= ~NETISR_MPSAFE; + netisrs[num].ni_flags = flags; } void @@ -166,8 +180,15 @@ * * Currently, we do a). Previously, we did c). */ - netisr_processqueue(ni); - ni->ni_handler(m); + if ((ni->ni_flags & NETISR_MPSAFE) == 0) { + mtx_lock(&Giant); + netisr_processqueue(ni); + ni->ni_handler(m); + mtx_unlock(&Giant); + } else { + netisr_processqueue(ni); + ni->ni_handler(m); + } } else { isrstat.isrs_deferred++; if (IF_HANDOFF(ni->ni_queue, m, NULL)) @@ -224,10 +245,19 @@ printf("swi_net: unregistered isr %d.\n", i); continue; } - if (ni->ni_queue == NULL) - ni->ni_handler(NULL); - else - netisr_processqueue(ni); + if ((ni->ni_flags & NETISR_MPSAFE) == 0) { + mtx_lock(&Giant); + if (ni->ni_queue == NULL) + ni->ni_handler(NULL); + else + netisr_processqueue(ni); + mtx_unlock(&Giant); + } else { + if (ni->ni_queue == NULL) + ni->ni_handler(NULL); + else + netisr_processqueue(ni); + } } } while (polling); } ==== //depot/projects/netperf/sys/net/netisr.h#2 (text+ko) ==== @@ -91,7 +91,8 @@ void netisr_dispatch(int, struct mbuf *); int netisr_queue(int, struct mbuf *); -void netisr_register(int, netisr_t *, struct ifqueue *); +#define NETISR_MPSAFE 0x0001 /* ISR does not need Giant */ +void netisr_register(int, netisr_t *, struct ifqueue *, int); void netisr_unregister(int); #endif ==== //depot/projects/netperf/sys/netatalk/aarp.c#5 (text+ko) ==== @@ -287,9 +287,7 @@ switch( ntohs( ar->ar_pro )) { case ETHERTYPE_AT : - mtx_lock(&Giant); at_aarpinput( ac, m ); - mtx_unlock(&Giant); return; default: @@ -314,6 +312,8 @@ int op; u_short net; + GIANT_REQUIRED(); + ea = mtod( m, struct ether_aarp *); /* Check to see if from my hardware address */ ==== //depot/projects/netperf/sys/netatalk/ddp_input.c#3 (text+ko) ==== @@ -39,13 +39,12 @@ void at2intr(struct mbuf *m) { + GIANT_REQUIRED(); /* * Phase 2 packet handling */ - mtx_lock(&Giant); ddp_input(m, m->m_pkthdr.rcvif, NULL, 2); - mtx_unlock(&Giant); return; } @@ -68,14 +67,14 @@ elhp = mtod(m, struct elaphdr *); m_adj(m, SZ_ELAPHDR); - mtx_lock(&Giant); + GIANT_REQUIRED(); + if (elhp->el_type == ELAP_DDPEXTEND) { ddp_input(m, m->m_pkthdr.rcvif, NULL, 1); } else { bcopy((caddr_t)elhp, (caddr_t)&elh, SZ_ELAPHDR); ddp_input(m, m->m_pkthdr.rcvif, &elh, 1); } - mtx_unlock(&Giant); return; } ==== //depot/projects/netperf/sys/netatalk/ddp_usrreq.c#2 (text+ko) ==== @@ -553,9 +553,9 @@ mtx_init(&atintrq1.ifq_mtx, "at1_inq", NULL, MTX_DEF); mtx_init(&atintrq2.ifq_mtx, "at2_inq", NULL, MTX_DEF); mtx_init(&aarpintrq.ifq_mtx, "aarp_inq", NULL, MTX_DEF); - netisr_register(NETISR_ATALK1, at1intr, &atintrq1); - netisr_register(NETISR_ATALK2, at2intr, &atintrq2); - netisr_register(NETISR_AARP, aarpintr, &aarpintrq); + netisr_register(NETISR_ATALK1, at1intr, &atintrq1, 0); + netisr_register(NETISR_ATALK2, at2intr, &atintrq2, 0); + netisr_register(NETISR_AARP, aarpintr, &aarpintrq, 0); } #if 0 ==== //depot/projects/netperf/sys/netgraph/ng_base.c#3 (text+ko) ==== @@ -2986,7 +2986,8 @@ mtx_init(&ng_idhash_mtx, "netgraph idhash mutex", NULL, 0); mtx_init(&ngq_mtx, "netgraph netisr mutex", NULL, 0); s = splimp(); - netisr_register(NETISR_NETGRAPH, (netisr_t *)ngintr, NULL); + /* XXX could use NETISR_MPSAFE but need to verify code */ + netisr_register(NETISR_NETGRAPH, (netisr_t *)ngintr, NULL, 0); splx(s); break; case MOD_UNLOAD: ==== //depot/projects/netperf/sys/netinet/if_ether.c#16 (text+ko) ==== @@ -980,6 +980,6 @@ mtx_init(&arpintrq.ifq_mtx, "arp_inq", NULL, MTX_DEF); LIST_INIT(&llinfo_arp); callout_init(&arp_callout, CALLOUT_MPSAFE); - netisr_register(NETISR_ARP, arpintr, &arpintrq); + netisr_register(NETISR_ARP, arpintr, &arpintrq, NETISR_MPSAFE); } SYSINIT(arp, SI_SUB_PROTO_DOMAIN, SI_ORDER_ANY, arp_init, 0); ==== //depot/projects/netperf/sys/netinet/ip_divert.c#10 (text+ko) ==== @@ -231,7 +231,7 @@ * the moment we're ignoring this. Once sockets are * locked this cruft can be removed. */ - mtx_lock(&Giant); /* XXX */ + NET_PICKUP_GIANT(); /* Put packet on socket queue, if any */ sa = NULL; nport = htons((u_int16_t)port); @@ -253,7 +253,7 @@ INP_UNLOCK(inp); } INP_INFO_RUNLOCK(&divcbinfo); - mtx_unlock(&Giant); /* XXX */ + NET_DROP_GIANT(); if (sa == NULL) { m_freem(m); ipstat.ips_noproto++; ==== //depot/projects/netperf/sys/netinet/ip_input.c#18 (text+ko) ==== @@ -323,7 +323,7 @@ #endif ipintrq.ifq_maxlen = ipqmaxlen; mtx_init(&ipintrq.ifq_mtx, "ip_inq", NULL, MTX_DEF); - netisr_register(NETISR_IP, ip_input, &ipintrq); + netisr_register(NETISR_IP, ip_input, &ipintrq, NETISR_MPSAFE); } /* @@ -999,7 +999,7 @@ * Switch out to protocol's input routine. */ ipstat.ips_delivered++; - mtx_lock(&Giant); /* XXX */ + NET_PICKUP_GIANT(); if (args.next_hop && ip->ip_p == IPPROTO_TCP) { /* TCP needs IPFORWARD info if available */ struct m_hdr tag; @@ -1013,7 +1013,7 @@ (struct mbuf *)&tag, hlen); } else (*inetsw[ip_protox[ip->ip_p]].pr_input)(m, hlen); - mtx_unlock(&Giant); /* XXX */ + NET_DROP_GIANT(); return; bad: m_freem(m); ==== //depot/projects/netperf/sys/netinet/ip_mroute.c#19 (text+ko) ==== @@ -1289,13 +1289,13 @@ socket_send(struct socket *s, struct mbuf *mm, struct sockaddr_in *src) { if (s) { - mtx_lock(&Giant); /* XXX no socket locking */ + NET_PICKUP_GIANT(); if (sbappendaddr(&s->so_rcv, (struct sockaddr *)src, mm, NULL) != 0) { sorwakeup(s); - mtx_unlock(&Giant); /* XXX */ + NET_DROP_GIANT(); return 0; } - mtx_unlock(&Giant); /* XXX */ + NET_DROP_GIANT(); } m_freem(mm); return -1; ==== //depot/projects/netperf/sys/netinet/ip_output.c#13 (text+ko) ==== @@ -206,6 +206,8 @@ KASSERT(ro != NULL, ("ip_output: no route, proto %d", mtod(m, struct ip *)->ip_p)); #endif + if (inp != NULL) + INP_LOCK_ASSERT(inp); if (args.rule != NULL) { /* dummynet already saw us */ ip = mtod(m, struct ip *); ==== //depot/projects/netperf/sys/netinet6/ip6_input.c#18 (text+ko) ==== @@ -196,7 +196,7 @@ #endif /* PFIL_HOOKS */ ip6intrq.ifq_maxlen = ip6qmaxlen; mtx_init(&ip6intrq.ifq_mtx, "ip6_inq", NULL, MTX_DEF); - netisr_register(NETISR_IPV6, ip6_input, &ip6intrq); + netisr_register(NETISR_IPV6, ip6_input, &ip6intrq, 0); scope6_init(); addrsel_policy_init(); nd6_init(); @@ -249,9 +249,7 @@ #endif int srcrt = 0; - mtx_assert(&Giant, MA_NOTOWNED); - mtx_lock(&Giant); - + GIANT_REQUIRED(); /* XXX for now */ #ifdef IPSEC /* * should the inner packet be considered authentic? @@ -331,7 +329,6 @@ if ((m = m_pullup(m, sizeof(struct ip6_hdr))) == NULL) { ip6stat.ip6s_toosmall++; in6_ifstat_inc(inifp, ifs6_in_hdrerr); - mtx_unlock(&Giant); return; } } @@ -353,14 +350,10 @@ * tell ip6_forward to do the right thing. */ odst = ip6->ip6_dst; - if (pfil_run_hooks(&inet6_pfil_hook, &m, m->m_pkthdr.rcvif, PFIL_IN)) { - mtx_unlock(&Giant); + if (pfil_run_hooks(&inet6_pfil_hook, &m, m->m_pkthdr.rcvif, PFIL_IN)) return; - } - if (m == NULL) { /* consumed by filter */ - mtx_unlock(&Giant); + if (m == NULL) /* consumed by filter */ return; - } ip6 = mtod(m, struct ip6_hdr *); srcrt = !IN6_ARE_ADDR_EQUAL(&odst, &ip6->ip6_dst); #endif /* PFIL_HOOKS */ @@ -378,10 +371,8 @@ m_freem(m); m = NULL; } - if (!m) { - mtx_unlock(&Giant); + if (!m) return; - } } /* @@ -641,7 +632,6 @@ #if 0 /*touches NULL pointer*/ in6_ifstat_inc(m->m_pkthdr.rcvif, ifs6_in_discard); #endif - mtx_unlock(&Giant); return; /* m have already been freed */ } @@ -665,7 +655,6 @@ icmp6_error(m, ICMP6_PARAM_PROB, ICMP6_PARAMPROB_HEADER, (caddr_t)&ip6->ip6_plen - (caddr_t)ip6); - mtx_unlock(&Giant); return; } #ifndef PULLDOWN_TEST @@ -676,7 +665,6 @@ sizeof(struct ip6_hbh)); if (hbh == NULL) { ip6stat.ip6s_tooshort++; - mtx_unlock(&Giant); return; } #endif @@ -725,17 +713,14 @@ if (ip6_mrouter && ip6_mforward(ip6, m->m_pkthdr.rcvif, m)) { ip6stat.ip6s_cantforward++; m_freem(m); - mtx_unlock(&Giant); return; } if (!ours) { m_freem(m); - mtx_unlock(&Giant); return; } } else if (!ours) { ip6_forward(m, srcrt); - mtx_unlock(&Giant); return; } @@ -794,11 +779,9 @@ #endif nxt = (*inet6sw[ip6_protox[nxt]].pr_input)(&m, &off, nxt); } - mtx_unlock(&Giant); return; bad: m_freem(m); - mtx_unlock(&Giant); } /* ==== //depot/projects/netperf/sys/netipx/ipx_input.c#6 (text+ko) ==== @@ -119,7 +119,7 @@ ipxintrq.ifq_maxlen = ipxqmaxlen; mtx_init(&ipxintrq.ifq_mtx, "ipx_inq", NULL, MTX_DEF); - netisr_register(NETISR_IPX, ipxintr, &ipxintrq); + netisr_register(NETISR_IPX, ipxintr, &ipxintrq, 0); } /* @@ -133,7 +133,8 @@ struct ipx_ifaddr *ia; int len; - mtx_lock(&Giant); + GIANT_REQUIRED(); + /* * If no IPX addresses have been set yet but the interfaces * are receiving, can't do anything with incoming packets yet. @@ -192,7 +193,6 @@ if (ipx->ipx_pt == IPXPROTO_NETBIOS) { if (ipxnetbios) { ipx_output_type20(m); - mtx_unlock(&Giant); return; } else goto bad; @@ -228,7 +228,6 @@ */ if (ipx->ipx_tc < IPX_MAXHOPS) { ipx_forward(m); - mtx_unlock(&Giant); return; } } @@ -244,7 +243,6 @@ if (ia == NULL) { ipx_forward(m); - mtx_unlock(&Giant); return; } } @@ -262,19 +260,16 @@ switch (ipx->ipx_pt) { case IPXPROTO_SPX: spx_input(m, ipxp); - mtx_unlock(&Giant); return; } ipx_input(m, ipxp); } else goto bad; - mtx_unlock(&Giant); return; bad: m_freem(m); - mtx_unlock(&Giant); } void ==== //depot/projects/netperf/sys/netnatm/natm.c#5 (text+ko) ==== @@ -685,7 +685,7 @@ struct socket *so; struct natmpcb *npcb; - mtx_lock(&Giant); + GIANT_REQUIRED(); #ifdef DIAGNOSTIC M_ASSERTPKTHDR(m); @@ -702,12 +702,12 @@ m_freem(m); if (npcb->npcb_inq == 0) FREE(npcb, M_PCB); /* done! */ - goto done; + return; } if (npcb->npcb_flags & NPCB_FREE) { m_freem(m); /* drop */ - goto done; + return; } #ifdef NEED_TO_RESTORE_IFP @@ -732,8 +732,6 @@ #endif m_freem(m); } -done: - mtx_unlock(&Giant); } /* ==== //depot/projects/netperf/sys/netnatm/natm_proto.c#3 (text+ko) ==== @@ -122,7 +122,7 @@ bzero(&natmintrq, sizeof(natmintrq)); natmintrq.ifq_maxlen = natmqmaxlen; mtx_init(&natmintrq.ifq_mtx, "natm_inq", NULL, MTX_DEF); - netisr_register(NETISR_NATM, natmintr, &natmintrq); + netisr_register(NETISR_NATM, natmintr, &natmintrq, 0); } #if defined(__FreeBSD__) ==== //depot/projects/netperf/sys/sys/mutex.h#5 (text+ko) ==== @@ -339,6 +339,27 @@ WITNESS_RESTORE(&Giant.mtx_object, Giant) #endif +/* + * Network MPSAFE temporary workarounds. When debug_mpsafenet + * is 1 the network is assumed to operate without Giant on the + * input path and protocols that require Giant must collect it + * on entry. When 0 Giant is grabbed in the network interface + * ISR's and in the netisr path and there is no need to grab + * the Giant lock. + * + * This mechanism is intended as temporary until everything of + * importance is properly locked. + */ +extern int debug_mpsafenet; /* defined in kern/subr_bus.c */ +#define NET_PICKUP_GIANT() do { \ + if (debug_mpsafenet) \ + mtx_lock(&Giant); \ +} while (0) +#define NET_DROP_GIANT() do { \ + if (debug_mpsafenet) \ + mtx_unlock(&Giant); \ +} while (0) + #define UGAR(rval) do { \ int _val = (rval); \ mtx_unlock(&Giant); \
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200311050154.hA51sVdo098812>