Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 4 Jun 2006 15:12:15 -0400
From:      Anish Mistry <mistry.7@osu.edu>
To:        thierry@herbelot.com
Cc:        freebsd-current@freebsd.org
Subject:   Re: panic while playing with a ugen
Message-ID:  <200606041512.32879.mistry.7@osu.edu>
In-Reply-To: <200606031135.52759.thierry@herbelot.com>
References:  <200606010042.47193.thierry@herbelot.com> <200605311930.17675.mistry.7@osu.edu> <200606031135.52759.thierry@herbelot.com>

next in thread | previous in thread | raw e-mail | index | archive | help
--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--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200606041512.32879.mistry.7>