From owner-freebsd-current@FreeBSD.ORG Sun Jun 4 19:12:26 2006 Return-Path: X-Original-To: freebsd-current@freebsd.org Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 0B30E16A9BE for ; Sun, 4 Jun 2006 19:12:26 +0000 (UTC) (envelope-from mistry.7@osu.edu) Received: from mail.united-ware.com (am-productions.biz [69.61.164.22]) by mx1.FreeBSD.org (Postfix) with ESMTP id 25FF643D70 for ; Sun, 4 Jun 2006 19:12:11 +0000 (GMT) (envelope-from mistry.7@osu.edu) Received: from [192.168.1.100] (am-productions.biz [69.61.164.22]) (authenticated bits=0) by mail.united-ware.com (8.13.4/8.13.6) with ESMTP id k54JEodU050651 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sun, 4 Jun 2006 15:14:56 -0400 (EDT) (envelope-from mistry.7@osu.edu) From: Anish Mistry To: thierry@herbelot.com Date: Sun, 4 Jun 2006 15:12:15 -0400 User-Agent: KMail/1.9.1 References: <200606010042.47193.thierry@herbelot.com> <200605311930.17675.mistry.7@osu.edu> <200606031135.52759.thierry@herbelot.com> In-Reply-To: <200606031135.52759.thierry@herbelot.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="nextPart10948918.mufkK5ULht"; protocol="application/pgp-signature"; micalg=pgp-sha1 Content-Transfer-Encoding: 7bit Message-Id: <200606041512.32879.mistry.7@osu.edu> X-Spam-Status: No, score=-2.5 required=5.0 tests=ALL_TRUSTED,BAYES_50, J_CHICKENPOX_23,J_CHICKENPOX_63,J_CHICKENPOX_73,J_CHICKENPOX_75, J_CHICKENPOX_91,MYFREEBSD3 autolearn=failed version=3.1.0 X-Spam-Checker-Version: SpamAssassin 3.1.0 (2005-09-13) on mail.united-ware.com X-Virus-Scanned: ClamAV 0.88.2/1511/Sun Jun 4 02:50:01 2006 on mail.united-ware.com X-Virus-Status: Clean Cc: freebsd-current@freebsd.org Subject: Re: panic while playing with a ugen X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 04 Jun 2006 19:12:33 -0000 --nextPart10948918.mufkK5ULht Content-Type: multipart/mixed; boundary="Boundary-01=_QCzgEPRlQhrBUrJ" Content-Transfer-Encoding: 7bit Content-Disposition: inline --Boundary-01=_QCzgEPRlQhrBUrJ Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Content-Disposition: inline On Saturday 03 June 2006 05:35, Thierry Herbelot wrote: > Le Thursday 1 June 2006 01:30, Anish Mistry a =E9crit : > > On Wednesday 31 May 2006 18:42, Thierry Herbelot wrote: > > > the panic occured when closing one endpoint of a ugen device > > > (the device was disconnecting from the USB bus after being > > > reseted). > > > > I haven't seen this particular panic with ugen before. > > Try the patch in PR: usb/97271. If you've got a test program and > > instructions that can reproduce this panic after applying that > > patch let me know. > > > > Thanks, > > indeed, after applying the patch from usb/97271, the behaviour > seems to be more stable (multiple runs of mytest program, in a > loop, and not one panic so far). Would you try the attached patch? This is modified version that is=20 closer to the version that might be committed. =2D-=20 Anish Mistry --Boundary-01=_QCzgEPRlQhrBUrJ Content-Type: text/x-diff; charset="iso-8859-1"; name="ugen.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="ugen.patch" =2D-- ugen.c.orig Sat Jun 3 15:59:24 2006 +++ ugen.c Sun Jun 4 14:55:25 2006 @@ -1,4 +1,4 @@ =2D/* $NetBSD: ugen.c,v 1.59 2002/07/11 21:14:28 augustss Exp $ */ +/* $NetBSD: ugen.c,v 1.79 2006/03/01 12:38:13 yamt Exp $ */ =20 /* Also already merged from NetBSD: * $NetBSD: ugen.c,v 1.61 2002/09/23 05:51:20 simonb Exp $ @@ -284,6 +284,9 @@ ugen_make_devnodes(sc); #endif =20 + usbd_add_drv_event(USB_EVENT_DRIVER_ATTACH, sc->sc_udev, + USBDEV(sc->sc_dev)); + USB_ATTACH_SUCCESS_RETURN; } =20 @@ -322,9 +325,11 @@ Static void ugen_destroy_devnodes(struct ugen_softc *sc) { =2D int endptno; + int endptno, prev_sc_dying; struct cdev *dev; =20 + prev_sc_dying =3D sc->sc_dying; + sc->sc_dying =3D 1; /* destroy all devices for the other (existing) endpoints as well */ for (endptno =3D 1; endptno < USB_MAX_ENDPOINTS; endptno++) { if (sc->sc_endpoints[endptno][IN].sc !=3D NULL || @@ -341,9 +346,16 @@ dev =3D sc->sc_endpoints[endptno][IN].dev; else dev =3D sc->sc_endpoints[endptno][OUT].dev; =2D destroy_dev(dev); + + KASSERT(dev !=3D NULL, ("ugen_destroy_devnodes: NULL dev")); + if(dev !=3D NULL) + destroy_dev(dev); + + sc->sc_endpoints[endptno][IN].sc =3D NULL; + sc->sc_endpoints[endptno][OUT].sc =3D NULL; } } + sc->sc_dying =3D prev_sc_dying; } #endif =20 @@ -378,9 +390,9 @@ return (err); /* store an array of endpoint descriptors to clear if the configuration * change succeeds - these aren't available afterwards */ =2D nendpt_cache =3D malloc(sizeof(u_int8_t) * niface, M_TEMP, M_WAITOK); + nendpt_cache =3D malloc(sizeof(u_int8_t) * niface, M_TEMP, M_WAITOK|M_ZER= O); sce_cache_arr =3D malloc(sizeof(struct ugen_endpoint **) * niface, M_TEMP, =2D M_WAITOK); + M_WAITOK|M_ZERO); niface_cache =3D niface; =20 for (ifaceno =3D 0; ifaceno < niface; ifaceno++) { @@ -727,13 +739,12 @@ sce->state |=3D UGEN_ASLP; DPRINTFN(5, ("ugenread: sleep on %p\n", sce)); error =3D tsleep(sce, PZERO | PCATCH, "ugenri", 0); + sce->state &=3D ~UGEN_ASLP; DPRINTFN(5, ("ugenread: woke, error=3D%d\n", error)); if (sc->sc_dying) error =3D EIO; =2D if (error) { =2D sce->state &=3D ~UGEN_ASLP; + if (error) break; =2D } } splx(s); =20 @@ -791,13 +802,12 @@ sce->state |=3D UGEN_ASLP; DPRINTFN(5, ("ugenread: sleep on %p\n", sce)); error =3D tsleep(sce, PZERO | PCATCH, "ugenri", 0); + sce->state &=3D ~UGEN_ASLP; DPRINTFN(5, ("ugenread: woke, error=3D%d\n", error)); if (sc->sc_dying) error =3D EIO; =2D if (error) { =2D sce->state &=3D ~UGEN_ASLP; + if (error) break; =2D } } =20 while (sce->cur !=3D sce->fill && uio->uio_resid > 0 && !error) { @@ -835,6 +845,9 @@ =20 USB_GET_SC(ugen, UGENUNIT(dev), sc); =20 + if (sc->sc_dying) + return (EIO); + UGEN_DEV_REF(dev, sc); error =3D ugen_do_read(sc, endpt, uio, flag); UGEN_DEV_RELE(dev, sc); @@ -933,6 +946,9 @@ =20 USB_GET_SC(ugen, UGENUNIT(dev), sc); =20 + if (sc->sc_dying) + return (EIO); + UGEN_DEV_REF(dev, sc); error =3D ugen_do_write(sc, endpt, uio, flag); UGEN_DEV_RELE(dev, sc); @@ -971,6 +987,20 @@ sce =3D &sc->sc_endpoints[endpt][IN]; if (sce->pipeh) usbd_abort_pipe(sce->pipeh); + if (sce->state & UGEN_ASLP) { + DPRINTFN(5, ("ugenpurge: waking %p\n", sce)); + wakeup(sce); + } + selwakeuppri(&sce->rsel, PZERO); + + sce =3D &sc->sc_endpoints[endpt][OUT]; + if (sce->pipeh) + usbd_abort_pipe(sce->pipeh); + if (sce->state & UGEN_ASLP) { + DPRINTFN(5, ("ugenpurge: waking %p\n", sce)); + wakeup(sce); + } + selwakeuppri(&sce->rsel, PZERO); } #endif =20 @@ -996,6 +1026,7 @@ sce =3D &sc->sc_endpoints[i][dir]; if (sce->pipeh) usbd_abort_pipe(sce->pipeh); + selwakeuppri(&sce->rsel, PZERO); } } =20 @@ -1035,6 +1066,9 @@ destroy_dev(sc->dev); #endif =20 + usbd_add_drv_event(USB_EVENT_DRIVER_DETACH, sc->sc_udev, + USBDEV(sc->sc_dev)); + return (0); } =20 @@ -1333,7 +1367,7 @@ switch (err) { case USBD_NORMAL_COMPLETION: #if defined(__FreeBSD__) =2D ugen_make_devnodes(sc); + ugen_make_devnodes(sc); #endif break; case USBD_IN_USE: @@ -1542,6 +1576,9 @@ =20 USB_GET_SC(ugen, UGENUNIT(dev), sc); =20 + if (sc->sc_dying) + return (EIO); + UGEN_DEV_REF(dev, sc); error =3D ugen_do_ioctl(sc, endpt, cmd, addr, flag, p); UGEN_DEV_RELE(dev, sc); @@ -1552,43 +1589,57 @@ ugenpoll(struct cdev *dev, int events, usb_proc_ptr p) { struct ugen_softc *sc; =2D struct ugen_endpoint *sce; + struct ugen_endpoint *sce_in, *sce_out; + usb_endpoint_descriptor_t *edesc; int revents =3D 0; int s; =20 USB_GET_SC(ugen, UGENUNIT(dev), sc); =20 if (sc->sc_dying) =2D return (EIO); + return ((events & (POLLIN | POLLOUT | POLLRDNORM | + POLLWRNORM)) | POLLHUP); + /* Do not allow to poll a control endpoint */ + if (UGENENDPOINT(dev) =3D=3D USB_CONTROL_ENDPOINT) + return (0); + + sce_in =3D &sc->sc_endpoints[UGENENDPOINT(dev)][IN]; + sce_out =3D &sc->sc_endpoints[UGENENDPOINT(dev)][OUT]; + edesc =3D (sce_in->edesc !=3D NULL) ? sce_in->edesc : sce_out->edesc; + KASSERT(edesc !=3D NULL, ("ugenpoll: NULL edesc")); + if (sce_in->edesc =3D=3D NULL || sce_in->pipeh =3D=3D NULL) + sce_in =3D NULL; + if (sce_out->edesc =3D=3D NULL || sce_out->pipeh =3D=3D NULL) + sce_out =3D NULL; =20 =2D /* XXX always IN */ =2D sce =3D &sc->sc_endpoints[UGENENDPOINT(dev)][IN]; =2D#ifdef DIAGNOSTIC =2D if (!sce->edesc) { =2D printf("ugenpoll: no edesc\n"); =2D return (EIO); =2D } =2D if (!sce->pipeh) { =2D printf("ugenpoll: no pipe\n"); =2D return (EIO); =2D } =2D#endif s =3D splusb(); =2D switch (sce->edesc->bmAttributes & UE_XFERTYPE) { + switch (edesc->bmAttributes & UE_XFERTYPE) { case UE_INTERRUPT: =2D if (events & (POLLIN | POLLRDNORM)) { =2D if (sce->q.c_cc > 0) + if (sce_in !=3D NULL && (events & (POLLIN | POLLRDNORM))) { + if (sce_in->q.c_cc > 0) revents |=3D events & (POLLIN | POLLRDNORM); else =2D selrecord(p, &sce->rsel); + selrecord(p, &sce_in->rsel); + } + if (sce_out !=3D NULL && (events & (POLLOUT | POLLWRNORM))) { + if (sce_out->q.c_cc > 0) + revents |=3D events & (POLLIN | POLLRDNORM); + else + selrecord(p, &sce_out->rsel); } break; case UE_ISOCHRONOUS: =2D if (events & (POLLIN | POLLRDNORM)) { =2D if (sce->cur !=3D sce->fill) + if (sce_in !=3D NULL && (events & (POLLIN | POLLRDNORM))) { + if (sce_in->cur !=3D sce_in->fill) + revents |=3D events & (POLLIN | POLLRDNORM); + else + selrecord(p, &sce_in->rsel); + } + if (sce_out !=3D NULL && (events & (POLLOUT | POLLWRNORM))) { + if (sce_out->cur !=3D sce_out->fill) revents |=3D events & (POLLIN | POLLRDNORM); else =2D selrecord(p, &sce->rsel); + selrecord(p, &sce_out->rsel); } break; case UE_BULK: --Boundary-01=_QCzgEPRlQhrBUrJ-- --nextPart10948918.mufkK5ULht Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.3 (FreeBSD) iD8DBQBEgzCgxqA5ziudZT0RAi7VAKDUAf2Bykk051sNJm/PnJh36MCe4QCeOakt jFr2KiUV772qfQ3/Z121MPQ= =nkYA -----END PGP SIGNATURE----- --nextPart10948918.mufkK5ULht--