From owner-svn-src-user@FreeBSD.ORG Mon Oct 11 20:38:22 2010 Return-Path: Delivered-To: svn-src-user@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 998C31065679 for ; Mon, 11 Oct 2010 20:38:22 +0000 (UTC) (envelope-from weongyo.jeong@gmail.com) Received: from mail-qw0-f54.google.com (mail-qw0-f54.google.com [209.85.216.54]) by mx1.freebsd.org (Postfix) with ESMTP id 3ECBB8FC1B for ; Mon, 11 Oct 2010 20:38:21 +0000 (UTC) Received: by qwe4 with SMTP id 4so1687174qwe.13 for ; Mon, 11 Oct 2010 13:38:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:received:from:date:to:cc :subject:message-id:reply-to:mail-followup-to:references :mime-version:content-type:content-disposition:in-reply-to :user-agent:organization:x-operation-sytem; bh=rJT96tVf8m7ECR33iQY59Jcz1gOq7HwkI+f+8rFM5qc=; b=s8u19WTfD/UZ3TTQzOgWdEB1N/Qgs7711PxpTLAvqMAaHaZWW8vaiKZ83QTHTRfdy5 ALXAnTKDOL8qzx+65+d/jxi/Sa8barDdOzo7b65dg9EGHbDzv1KPZ87qnDjuHroODBCq 2weXDv+okiOO6+PEMBVF138Pt53AvYt6UdYLw= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=from:date:to:cc:subject:message-id:reply-to:mail-followup-to :references:mime-version:content-type:content-disposition :in-reply-to:user-agent:organization:x-operation-sytem; b=Qryzqm9Tg7D2do7KWI0J6Zm6udIiRAXsebpTvIIA8pkRm3f57FuzhEFb+tt/ZJ3FUt HeBnPzRoBoHmNuu/kagfGhwVEjMNZraKGSKIla3ouH02htRxNgb+ja/ZOS7GLKP2gvO/ vtW9tqSjIfBnzDzoh872R4ty/2+iKOlXPNvDA= Received: by 10.229.229.83 with SMTP id jh19mr5461256qcb.76.1286827721164; Mon, 11 Oct 2010 13:08:41 -0700 (PDT) Received: from weongyo ([174.35.1.224]) by mx.google.com with ESMTPS id m7sm2744148qck.13.2010.10.11.13.08.39 (version=SSLv3 cipher=RC4-MD5); Mon, 11 Oct 2010 13:08:40 -0700 (PDT) Received: by weongyo (sSMTP sendmail emulation); Mon, 11 Oct 2010 13:08:45 -0700 From: Weongyo Jeong Date: Mon, 11 Oct 2010 13:08:45 -0700 To: Hans Petter Selasky Message-ID: <20101011200845.GA38869@weongyo> Mail-Followup-To: Hans Petter Selasky , "src-committers@freebsd.org" , "svn-src-user@freebsd.org" References: <201010080152.o981q1gJ074407@svn.freebsd.org> <201010111739.25082.hselasky@c2i.net> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201010111739.25082.hselasky@c2i.net> User-Agent: Mutt/1.4.2.3i Organization: CDNetworks. X-Operation-Sytem: FreeBSD Cc: "src-committers@freebsd.org" , "svn-src-user@freebsd.org" Subject: Re: svn commit: r213540 - user/weongyo/usb/sys/dev/usb/net X-BeenThere: svn-src-user@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Weongyo Jeong List-Id: "SVN commit messages for the experimental " user" src tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 11 Oct 2010 20:38:22 -0000 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