Re: Convert bridge span & int. list to SLIST()
On 09/01/19(Wed) 10:11, Martin Pieuchot wrote: > On 23/12/18(Sun) 14:41, Martin Pieuchot wrote: > > Using SLIST() instead of TAILQ() is a step towards using lock-less > > lists. > > > > As found in my previous attempts the interface list cannot be protected > > by a mutex as long a if_enqueue() might grab the KERNEL_LOCK(). So I'm > > heading towards using SRPL_*(). Revised diff that plugs a leak and use SLIST_REMOVE() instead of rerolling it, as pointed out by visa@. ok? Index: net/if_bridge.c === RCS file: /cvs/src/sys/net/if_bridge.c,v retrieving revision 1.315 diff -u -p -r1.315 if_bridge.c --- net/if_bridge.c 12 Dec 2018 14:19:15 - 1.315 +++ net/if_bridge.c 9 Jan 2019 18:10:45 - @@ -108,6 +108,8 @@ voidbridgeattach(int); intbridge_ioctl(struct ifnet *, u_long, caddr_t); void bridge_ifdetach(void *); void bridge_spandetach(void *); +intbridge_ifremove(struct bridge_iflist *); +intbridge_spanremove(struct bridge_iflist *); intbridge_input(struct ifnet *, struct mbuf *, void *); void bridge_process(struct ifnet *, struct mbuf *); void bridgeintr_frame(struct bridge_softc *, struct ifnet *, struct mbuf *); @@ -170,8 +172,8 @@ bridge_clone_create(struct if_clone *ifc sc->sc_brtmax = BRIDGE_RTABLE_MAX; sc->sc_brttimeout = BRIDGE_RTABLE_TIMEOUT; timeout_set(&sc->sc_brtimeout, bridge_rtage, sc); - TAILQ_INIT(&sc->sc_iflist); - TAILQ_INIT(&sc->sc_spanlist); + SLIST_INIT(&sc->sc_iflist); + SLIST_INIT(&sc->sc_spanlist); for (i = 0; i < BRIDGE_RTABLE_SIZE; i++) LIST_INIT(&sc->sc_rts[i]); arc4random_buf(&sc->sc_hashkey, sizeof(sc->sc_hashkey)); @@ -216,10 +218,12 @@ bridge_clone_destroy(struct ifnet *ifp) bridge_stop(sc); bridge_rtflush(sc, IFBF_FLUSHALL); - while ((bif = TAILQ_FIRST(&sc->sc_iflist)) != NULL) + while ((bif = SLIST_FIRST(&sc->sc_iflist)) != NULL) { + SLIST_REMOVE_HEAD(&sc->sc_iflist, bif_next); bridge_delete(sc, bif); - while ((bif = TAILQ_FIRST(&sc->sc_spanlist)) != NULL) - bridge_spandetach(bif); + } + while ((bif = SLIST_FIRST(&sc->sc_spanlist)) != NULL) + bridge_spanremove(bif); bstp_destroy(sc->sc_stp); @@ -249,7 +253,6 @@ bridge_delete(struct bridge_softc *sc, s hook_disestablish(bif->ifp->if_detachhooks, bif->bif_dhcookie); if_ih_remove(bif->ifp, bridge_input, NULL); - TAILQ_REMOVE(&sc->sc_iflist, bif, next); bridge_rtdelete(sc, bif->ifp, 0); bridge_flushrule(bif); free(bif, M_DEVBUF, sizeof *bif); @@ -297,7 +300,7 @@ bridge_ioctl(struct ifnet *ifp, u_long c } /* If it's in the span list, it can't be a member. */ - TAILQ_FOREACH(bif, &sc->sc_spanlist, next) { + SLIST_FOREACH(bif, &sc->sc_spanlist, bif_next) { if (bif->ifp == ifs) break; } @@ -338,7 +341,7 @@ bridge_ioctl(struct ifnet *ifp, u_long c bif->bif_dhcookie = hook_establish(ifs->if_detachhooks, 0, bridge_ifdetach, bif); if_ih_insert(bif->ifp, bridge_input, NULL); - TAILQ_INSERT_TAIL(&sc->sc_iflist, bif, next); + SLIST_INSERT_HEAD(&sc->sc_iflist, bif, bif_next); break; case SIOCBRDGDEL: if ((error = suser(curproc)) != 0) @@ -353,7 +356,7 @@ bridge_ioctl(struct ifnet *ifp, u_long c error = ESRCH; break; } - error = bridge_delete(sc, bif); + error = bridge_ifremove(bif); break; case SIOCBRDGIFS: error = bridge_bifconf(sc, (struct ifbifconf *)data); @@ -375,7 +378,7 @@ bridge_ioctl(struct ifnet *ifp, u_long c error = EBUSY; break; } - TAILQ_FOREACH(bif, &sc->sc_spanlist, next) { + SLIST_FOREACH(bif, &sc->sc_spanlist, bif_next) { if (bif->ifp == ifs) break; } @@ -395,7 +398,7 @@ bridge_ioctl(struct ifnet *ifp, u_long c bridge_spandetach, bif); SIMPLEQ_INIT(&bif->bif_brlin); SIMPLEQ_INIT(&bif->bif_brlout); - TAILQ_INSERT_TAIL(&sc->sc_spanlist, bif, next); + SLIST_INSERT_HEAD(&sc->sc_spanlist, bif, bif_next); break; case SIOCBRDGDELS: if ((error = suser(curproc)) != 0) @@ -405,15 +408,15 @@ bridge_ioctl(struct ifnet *ifp, u_long c error = ENOENT; break; } - TAILQ_FOREACH(bif, &sc->sc_spanlist, next) { + SLIST_FOREACH(bif, &s
Re: Convert bridge span & int. list to SLIST()
On 23/12/18(Sun) 14:41, Martin Pieuchot wrote: > Using SLIST() instead of TAILQ() is a step towards using lock-less > lists. > > As found in my previous attempts the interface list cannot be protected > by a mutex as long a if_enqueue() might grab the KERNEL_LOCK(). So I'm > heading towards using SRPL_*(). > > Ok? Anyone? > Index: net/if_bridge.c > === > RCS file: /cvs/src/sys/net/if_bridge.c,v > retrieving revision 1.315 > diff -u -p -r1.315 if_bridge.c > --- net/if_bridge.c 12 Dec 2018 14:19:15 - 1.315 > +++ net/if_bridge.c 12 Dec 2018 14:27:21 - > @@ -108,6 +108,8 @@ void bridgeattach(int); > int bridge_ioctl(struct ifnet *, u_long, caddr_t); > void bridge_ifdetach(void *); > void bridge_spandetach(void *); > +int bridge_ifremove(struct bridge_iflist *); > +int bridge_spanremove(struct bridge_iflist *); > int bridge_input(struct ifnet *, struct mbuf *, void *); > void bridge_process(struct ifnet *, struct mbuf *); > void bridgeintr_frame(struct bridge_softc *, struct ifnet *, struct mbuf *); > @@ -170,8 +172,8 @@ bridge_clone_create(struct if_clone *ifc > sc->sc_brtmax = BRIDGE_RTABLE_MAX; > sc->sc_brttimeout = BRIDGE_RTABLE_TIMEOUT; > timeout_set(&sc->sc_brtimeout, bridge_rtage, sc); > - TAILQ_INIT(&sc->sc_iflist); > - TAILQ_INIT(&sc->sc_spanlist); > + SLIST_INIT(&sc->sc_iflist); > + SLIST_INIT(&sc->sc_spanlist); > for (i = 0; i < BRIDGE_RTABLE_SIZE; i++) > LIST_INIT(&sc->sc_rts[i]); > arc4random_buf(&sc->sc_hashkey, sizeof(sc->sc_hashkey)); > @@ -216,10 +218,14 @@ bridge_clone_destroy(struct ifnet *ifp) > > bridge_stop(sc); > bridge_rtflush(sc, IFBF_FLUSHALL); > - while ((bif = TAILQ_FIRST(&sc->sc_iflist)) != NULL) > + while ((bif = SLIST_FIRST(&sc->sc_iflist)) != NULL) { > + SLIST_REMOVE_HEAD(&sc->sc_iflist, bif_next); > bridge_delete(sc, bif); > - while ((bif = TAILQ_FIRST(&sc->sc_spanlist)) != NULL) > + } > + while ((bif = SLIST_FIRST(&sc->sc_spanlist)) != NULL) { > + SLIST_REMOVE_HEAD(&sc->sc_spanlist, bif_next); > bridge_spandetach(bif); > + } > > bstp_destroy(sc->sc_stp); > > @@ -249,7 +255,6 @@ bridge_delete(struct bridge_softc *sc, s > hook_disestablish(bif->ifp->if_detachhooks, bif->bif_dhcookie); > > if_ih_remove(bif->ifp, bridge_input, NULL); > - TAILQ_REMOVE(&sc->sc_iflist, bif, next); > bridge_rtdelete(sc, bif->ifp, 0); > bridge_flushrule(bif); > free(bif, M_DEVBUF, sizeof *bif); > @@ -297,7 +302,7 @@ bridge_ioctl(struct ifnet *ifp, u_long c > } > > /* If it's in the span list, it can't be a member. */ > - TAILQ_FOREACH(bif, &sc->sc_spanlist, next) { > + SLIST_FOREACH(bif, &sc->sc_spanlist, bif_next) { > if (bif->ifp == ifs) > break; > } > @@ -338,7 +343,7 @@ bridge_ioctl(struct ifnet *ifp, u_long c > bif->bif_dhcookie = hook_establish(ifs->if_detachhooks, 0, > bridge_ifdetach, bif); > if_ih_insert(bif->ifp, bridge_input, NULL); > - TAILQ_INSERT_TAIL(&sc->sc_iflist, bif, next); > + SLIST_INSERT_HEAD(&sc->sc_iflist, bif, bif_next); > break; > case SIOCBRDGDEL: > if ((error = suser(curproc)) != 0) > @@ -353,7 +358,7 @@ bridge_ioctl(struct ifnet *ifp, u_long c > error = ESRCH; > break; > } > - error = bridge_delete(sc, bif); > + error = bridge_ifremove(bif); > break; > case SIOCBRDGIFS: > error = bridge_bifconf(sc, (struct ifbifconf *)data); > @@ -375,7 +380,7 @@ bridge_ioctl(struct ifnet *ifp, u_long c > error = EBUSY; > break; > } > - TAILQ_FOREACH(bif, &sc->sc_spanlist, next) { > + SLIST_FOREACH(bif, &sc->sc_spanlist, bif_next) { > if (bif->ifp == ifs) > break; > } > @@ -395,7 +400,7 @@ bridge_ioctl(struct ifnet *ifp, u_long c > bridge_spandetach, bif); > SIMPLEQ_INIT(&bif->bif_brlin); > SIMPLEQ_INIT(&bif->bif_brlout); > - TAILQ_INSERT_TAIL(&sc->sc_spanlist, bif, next); > + SLIST_INSERT_HEAD(&sc->sc_spanlist, bif, bif_next); > break; > case SIOCBRDGDELS: > if ((error = suser(curproc)) != 0) > @@ -405,15 +410,15 @@ bridge_ioctl(struct ifnet *ifp, u_long c > error = ENOENT; > break; > } > - TAILQ_FOREACH(bif, &sc->sc_spanlist, next) { > + SLIST_FOREACH(bif, &sc->sc_spanlist, bif_next) { > if (bif->ifp == ifs) >
Convert bridge span & int. list to SLIST()
Using SLIST() instead of TAILQ() is a step towards using lock-less lists. As found in my previous attempts the interface list cannot be protected by a mutex as long a if_enqueue() might grab the KERNEL_LOCK(). So I'm heading towards using SRPL_*(). Ok? Index: net/if_bridge.c === RCS file: /cvs/src/sys/net/if_bridge.c,v retrieving revision 1.315 diff -u -p -r1.315 if_bridge.c --- net/if_bridge.c 12 Dec 2018 14:19:15 - 1.315 +++ net/if_bridge.c 12 Dec 2018 14:27:21 - @@ -108,6 +108,8 @@ voidbridgeattach(int); intbridge_ioctl(struct ifnet *, u_long, caddr_t); void bridge_ifdetach(void *); void bridge_spandetach(void *); +intbridge_ifremove(struct bridge_iflist *); +intbridge_spanremove(struct bridge_iflist *); intbridge_input(struct ifnet *, struct mbuf *, void *); void bridge_process(struct ifnet *, struct mbuf *); void bridgeintr_frame(struct bridge_softc *, struct ifnet *, struct mbuf *); @@ -170,8 +172,8 @@ bridge_clone_create(struct if_clone *ifc sc->sc_brtmax = BRIDGE_RTABLE_MAX; sc->sc_brttimeout = BRIDGE_RTABLE_TIMEOUT; timeout_set(&sc->sc_brtimeout, bridge_rtage, sc); - TAILQ_INIT(&sc->sc_iflist); - TAILQ_INIT(&sc->sc_spanlist); + SLIST_INIT(&sc->sc_iflist); + SLIST_INIT(&sc->sc_spanlist); for (i = 0; i < BRIDGE_RTABLE_SIZE; i++) LIST_INIT(&sc->sc_rts[i]); arc4random_buf(&sc->sc_hashkey, sizeof(sc->sc_hashkey)); @@ -216,10 +218,14 @@ bridge_clone_destroy(struct ifnet *ifp) bridge_stop(sc); bridge_rtflush(sc, IFBF_FLUSHALL); - while ((bif = TAILQ_FIRST(&sc->sc_iflist)) != NULL) + while ((bif = SLIST_FIRST(&sc->sc_iflist)) != NULL) { + SLIST_REMOVE_HEAD(&sc->sc_iflist, bif_next); bridge_delete(sc, bif); - while ((bif = TAILQ_FIRST(&sc->sc_spanlist)) != NULL) + } + while ((bif = SLIST_FIRST(&sc->sc_spanlist)) != NULL) { + SLIST_REMOVE_HEAD(&sc->sc_spanlist, bif_next); bridge_spandetach(bif); + } bstp_destroy(sc->sc_stp); @@ -249,7 +255,6 @@ bridge_delete(struct bridge_softc *sc, s hook_disestablish(bif->ifp->if_detachhooks, bif->bif_dhcookie); if_ih_remove(bif->ifp, bridge_input, NULL); - TAILQ_REMOVE(&sc->sc_iflist, bif, next); bridge_rtdelete(sc, bif->ifp, 0); bridge_flushrule(bif); free(bif, M_DEVBUF, sizeof *bif); @@ -297,7 +302,7 @@ bridge_ioctl(struct ifnet *ifp, u_long c } /* If it's in the span list, it can't be a member. */ - TAILQ_FOREACH(bif, &sc->sc_spanlist, next) { + SLIST_FOREACH(bif, &sc->sc_spanlist, bif_next) { if (bif->ifp == ifs) break; } @@ -338,7 +343,7 @@ bridge_ioctl(struct ifnet *ifp, u_long c bif->bif_dhcookie = hook_establish(ifs->if_detachhooks, 0, bridge_ifdetach, bif); if_ih_insert(bif->ifp, bridge_input, NULL); - TAILQ_INSERT_TAIL(&sc->sc_iflist, bif, next); + SLIST_INSERT_HEAD(&sc->sc_iflist, bif, bif_next); break; case SIOCBRDGDEL: if ((error = suser(curproc)) != 0) @@ -353,7 +358,7 @@ bridge_ioctl(struct ifnet *ifp, u_long c error = ESRCH; break; } - error = bridge_delete(sc, bif); + error = bridge_ifremove(bif); break; case SIOCBRDGIFS: error = bridge_bifconf(sc, (struct ifbifconf *)data); @@ -375,7 +380,7 @@ bridge_ioctl(struct ifnet *ifp, u_long c error = EBUSY; break; } - TAILQ_FOREACH(bif, &sc->sc_spanlist, next) { + SLIST_FOREACH(bif, &sc->sc_spanlist, bif_next) { if (bif->ifp == ifs) break; } @@ -395,7 +400,7 @@ bridge_ioctl(struct ifnet *ifp, u_long c bridge_spandetach, bif); SIMPLEQ_INIT(&bif->bif_brlin); SIMPLEQ_INIT(&bif->bif_brlout); - TAILQ_INSERT_TAIL(&sc->sc_spanlist, bif, next); + SLIST_INSERT_HEAD(&sc->sc_spanlist, bif, bif_next); break; case SIOCBRDGDELS: if ((error = suser(curproc)) != 0) @@ -405,15 +410,15 @@ bridge_ioctl(struct ifnet *ifp, u_long c error = ENOENT; break; } - TAILQ_FOREACH(bif, &sc->sc_spanlist, next) { + SLIST_FOREACH(bif, &sc->sc_spanlist, bif_next) { if (bif->ifp == ifs) break; } if (bif == NULL) { -