Date: Thu, 13 Dec 2012 23:33:19 -0600 From: Bryan Venteicher <bryanv@freebsd.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r244200 - in head/sys/dev/virtio: block network scsi Message-ID: <CAGaYwLdQV7qq8mY917C2f--Vi-zn7pfZXTbqS4BDun_Ln6r8Rg@mail.gmail.com> In-Reply-To: <201212140527.qBE5Rvue097151@svn.freebsd.org> References: <201212140527.qBE5Rvue097151@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Dec 13, 2012 at 11:27 PM, Bryan Venteicher <bryanv@freebsd.org> wrote: > > Author: bryanv > Date: Fri Dec 14 05:27:56 2012 > New Revision: 244200 > URL: http://svnweb.freebsd.org/changeset/base/244200 > > Log: > virtio: Start taskqueues threads after attach cannot fail > > If virtio_setup_intr() failed during boot, we would hang in > taskqueue_free() -> taskqueue_terminate() for all the taskq > threads to terminate. This will never happen since the > scheduler is not running by this point. > FWIW, this seems to be a pretty common bug across dozen of drivers. bus_setup_intr() is not very likely to fail though. For VirtIO, it is somewhat a moot point because the taskqueues are about to be removed and replaced with ithreads as I should have done in the beginning. > Reported by: neel, grehan > Approved by: grehan (mentor) > > Modified: > head/sys/dev/virtio/block/virtio_blk.c > head/sys/dev/virtio/network/if_vtnet.c > head/sys/dev/virtio/scsi/virtio_scsi.c > > Modified: head/sys/dev/virtio/block/virtio_blk.c > > ============================================================================== > --- head/sys/dev/virtio/block/virtio_blk.c Fri Dec 14 00:13:29 2012 > (r244199) > +++ head/sys/dev/virtio/block/virtio_blk.c Fri Dec 14 05:27:56 2012 > (r244200) > @@ -338,8 +338,6 @@ vtblk_attach(device_t dev) > device_printf(dev, "cannot allocate taskqueue\n"); > goto fail; > } > - taskqueue_start_threads(&sc->vtblk_tq, 1, PI_DISK, "%s taskq", > - device_get_nameunit(dev)); > > error = virtio_setup_intr(dev, INTR_TYPE_BIO | INTR_ENTROPY); > if (error) { > @@ -347,6 +345,9 @@ vtblk_attach(device_t dev) > goto fail; > } > > + taskqueue_start_threads(&sc->vtblk_tq, 1, PI_DISK, "%s taskq", > + device_get_nameunit(dev)); > + > vtblk_create_disk(sc); > > virtqueue_enable_intr(sc->vtblk_vq); > > Modified: head/sys/dev/virtio/network/if_vtnet.c > > ============================================================================== > --- head/sys/dev/virtio/network/if_vtnet.c Fri Dec 14 00:13:29 2012 > (r244199) > +++ head/sys/dev/virtio/network/if_vtnet.c Fri Dec 14 05:27:56 2012 > (r244200) > @@ -439,18 +439,17 @@ vtnet_attach(device_t dev) > ether_ifdetach(ifp); > goto fail; > } > - taskqueue_start_threads(&sc->vtnet_tq, 1, PI_NET, "%s taskq", > - device_get_nameunit(dev)); > > error = virtio_setup_intr(dev, INTR_TYPE_NET); > if (error) { > device_printf(dev, "cannot setup virtqueue interrupts\n"); > - taskqueue_free(sc->vtnet_tq); > - sc->vtnet_tq = NULL; > ether_ifdetach(ifp); > goto fail; > } > > + taskqueue_start_threads(&sc->vtnet_tq, 1, PI_NET, "%s taskq", > + device_get_nameunit(dev)); > + > /* > * Device defaults to promiscuous mode for backwards > * compatibility. Turn it off if possible. > > Modified: head/sys/dev/virtio/scsi/virtio_scsi.c > > ============================================================================== > --- head/sys/dev/virtio/scsi/virtio_scsi.c Fri Dec 14 00:13:29 2012 > (r244199) > +++ head/sys/dev/virtio/scsi/virtio_scsi.c Fri Dec 14 05:27:56 2012 > (r244200) > @@ -347,12 +347,6 @@ vtscsi_attach(device_t dev) > device_printf(dev, "cannot allocate taskqueue\n"); > goto fail; > } > - error = taskqueue_start_threads(&sc->vtscsi_tq, 1, PI_DISK, "%s > taskq", > - device_get_nameunit(dev)); > - if (error) { > - device_printf(dev, "cannot start taskqueue threads\n"); > - goto fail; > - } > > error = virtio_setup_intr(dev, INTR_TYPE_CAM); > if (error) { > @@ -360,6 +354,9 @@ vtscsi_attach(device_t dev) > goto fail; > } > > + taskqueue_start_threads(&sc->vtscsi_tq, 1, PI_DISK, "%s taskq", > + device_get_nameunit(dev)); > + > vtscsi_enable_vqs_intr(sc); > > /*
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAGaYwLdQV7qq8mY917C2f--Vi-zn7pfZXTbqS4BDun_Ln6r8Rg>