svn commit: r213540 - user/weongyo/usb/sys/dev/usb/net

[Weongyo Jeong]

On Mon, Oct 11, 2010 at 05:39:25PM +0200, Hans Petter Selasky wrote:
> On Friday 08 October 2010 03:52:01 Weongyo Jeong wrote:
> > Author: weongyo
> > Date: Fri Oct  8 01:52:01 2010
> > New Revision: 213540
> > URL: http://svn.freebsd.org/changeset/base/213540
> > 
> > Log:
> >   o fixes a regression that setting the promiscuous mode should be
> >     happened at the taskqueue.  It's to avoid a `sleepable after
> >     non-sleepable' because ioctl handler could be called with holding bpf
> >     mtx which is a default mutex.
> >   o defines SLEEPOUT_DRAIN_TASK helper.
> > 
> > Modified:
> >   user/weongyo/usb/sys/dev/usb/net/if_aue.c
> >   user/weongyo/usb/sys/dev/usb/net/if_auereg.h
> >   user/weongyo/usb/sys/dev/usb/net/if_axe.c
> >   user/weongyo/usb/sys/dev/usb/net/if_axereg.h
> >   user/weongyo/usb/sys/dev/usb/net/if_cue.c
> >   user/weongyo/usb/sys/dev/usb/net/if_cuereg.h
> >   user/weongyo/usb/sys/dev/usb/net/if_kue.c
> >   user/weongyo/usb/sys/dev/usb/net/if_rue.c
> >   user/weongyo/usb/sys/dev/usb/net/if_ruereg.h
> >   user/weongyo/usb/sys/dev/usb/net/if_udav.c
> >   user/weongyo/usb/sys/dev/usb/net/if_udavreg.h
> > 
> > Modified: user/weongyo/usb/sys/dev/usb/net/if_aue.c
> > ===========================================================================
> > === --- user/weongyo/usb/sys/dev/usb/net/if_aue.c	Fri Oct  8 01:47:14
> > 2010	(r213539) +++ user/weongyo/usb/sys/dev/usb/net/if_aue.c	Fri Oct  8
> > 01:52:01 2010	(r213540) @@ -230,7 +230,8 @@ static
> > void	aue_setmulti_locked(struct a
> >  static void	aue_rxflush(struct aue_softc *);
> >  static int	aue_rxbuf(struct aue_softc *, struct usb_page_cache *,
> >  		    unsigned int, unsigned int);
> > -static void	aue_setpromisc(struct aue_softc *);
> > +static void	aue_setpromisc(void *, int);
> > +static void	aue_setpromisc_locked(struct aue_softc *);
> >  static void	aue_init_locked(struct aue_softc *);
> >  static void	aue_watchdog(void *);
> > 
> > @@ -709,6 +710,7 @@ aue_attach(device_t dev)
> >  	sleepout_create(&sc->sc_sleepout, "aue sleepout");
> >  	sleepout_init_mtx(&sc->sc_sleepout, &sc->sc_watchdog, &sc->sc_mtx, 0);
> >  	TASK_INIT(&sc->sc_setmulti, 0, aue_setmulti, sc);
> > +	TASK_INIT(&sc->sc_setpromisc, 0, aue_setpromisc, sc);
> > 
> >  	iface_index = AUE_IFACE_IDX;
> >  	error = usbd_transfer_setup(uaa->device, &iface_index,
> > @@ -764,7 +766,8 @@ aue_detach(device_t dev)
> >  	struct ifnet *ifp = sc->sc_ifp;
> > 
> >  	sleepout_drain(&sc->sc_watchdog);
> > -	taskqueue_drain(sc->sc_sleepout.s_taskqueue, &sc->sc_setmulti);
> > +	SLEEPOUT_DRAIN_TASK(&sc->sc_sleepout, &sc->sc_setpromisc);
> > +	SLEEPOUT_DRAIN_TASK(&sc->sc_sleepout, &sc->sc_setmulti);
> >  	usbd_transfer_unsetup(sc->sc_xfer, AUE_N_TRANSFER);
> > 
> >  	if (sc->sc_miibus != NULL)
> > @@ -1032,7 +1035,7 @@ aue_init_locked(struct aue_softc *sc)
> >  		aue_csr_write_1(sc, AUE_PAR0 + i, IF_LLADDR(ifp)[i]);
> > 
> >  	/* update promiscuous setting */
> > -	aue_setpromisc(sc);
> > +	aue_setpromisc_locked(sc);
> > 
> >  	/* Load the multicast filter. */
> >  	aue_setmulti_locked(sc);
> > @@ -1050,7 +1053,17 @@ aue_init_locked(struct aue_softc *sc)
> >  }
> > 
> >  static void
> > -aue_setpromisc(struct aue_softc *sc)
> > +aue_setpromisc(void *arg, int npending)
> > +{
> > +	struct aue_softc *sc = arg;
> > +
> > +	AUE_LOCK(sc);
> > +	aue_setpromisc_locked(sc);
> > +	AUE_UNLOCK(sc);
> > +}
> > +
> > +static void
> > +aue_setpromisc_locked(struct aue_softc *sc)
> >  {
> >  	struct ifnet *ifp = sc->sc_ifp;
> > 
> > @@ -1140,7 +1153,8 @@ aue_ioctl(struct ifnet *ifp, u_long comm
> >  		AUE_LOCK(sc);
> >  		if (ifp->if_flags & IFF_UP) {
> >  			if (ifp->if_drv_flags & IFF_DRV_RUNNING)
> > -				aue_setpromisc(sc);
> > +				SLEEPOUT_RUN_TASK(&sc->sc_sleepout,
> > +				    &sc->sc_setpromisc);
> >  			else
> >  				aue_init_locked(sc);
> >  		} else
> > 
> > Modified: user/weongyo/usb/sys/dev/usb/net/if_auereg.h
> > ===========================================================================
> > === --- user/weongyo/usb/sys/dev/usb/net/if_auereg.h	Fri Oct  8 01:47:14
> > 2010	(r213539) +++ user/weongyo/usb/sys/dev/usb/net/if_auereg.h	Fri Oct  
> 8
> > 01:52:01 2010	(r213540) @@ -222,6 +222,7 @@ struct aue_softc {
> >  	struct sleepout		sc_sleepout;
> >  	struct sleepout_task	sc_watchdog;
> >  	struct task		sc_setmulti;
> > +	struct task		sc_setpromisc;
> >  };
> > 
> >  #define	AUE_LOCK(_sc)		mtx_lock(&(_sc)->sc_mtx)
>
> These taskqueues belong in the network stack and not the USB drivers! And 

No it's not in the network stack.  The taskqueue is created from USB
driver and it's under control.

> please understand that you cannot use taskqueues for these commands, because 
> the events can be executed out of order!!!

Please share your story with all when the events are executed out of
order.  I'm ready to fix it.

regards,
Weongyo Jeong