Hi, On Fri, 17 May 2013 14:06:49 +0200 Martin Pieuchot <mpieuc...@nolizard.org> wrote: > On 17/05/13(Fri) 19:25, YASUOKA Masahiko wrote: >> On Fri, 1 Mar 2013 13:43:00 +0000 >> "Wade, Daniel" <dw...@meridium.com> wrote: >> > -----Original Message----- >> > From: owner-t...@openbsd.org [mailto:owner-t...@openbsd.org] On Behalf Of >> > Stefan Sperling >> > Sent: Thursday, February 28, 2013 12:16 PM >> > To: Edd Barrett >> > Cc: tech@openbsd.org >> > Subject: Re: Wake from zzz causes panic on thinkpad x60 >> > >> > On Thu, Feb 28, 2013 at 04:59:12PM +0000, Edd Barrett wrote: >> >> Went to run some TESTS for release and I am seeing panics when waking >> >> my thinkpad x60 from zzz. >> >> >> >> I didn't have a serial line attached to get trace and ps, so I have >> >> taken pictures of the kernel debugger. Sorry about that. >> >> >> >> http://farm9.staticflickr.com/8505/8516467142_1f3580e87a_c.jpg >> > >> > I've seen this panic in usb_abort_task_thread() on an x60s before. >> > It doesn't happen often. It's not a new issue in recent snaps. >> >> Same problem happens on my sony vaio vgn-sz94s. >> Attached diff will fix the problem. >> >> Remove `abort_task' from usb task queue before recycling a `struct >> usbd_xfer object' which includes the `abort_task'. Otherwise >> usb_abort_task_thread() may try to dequeue the recycled task then it >> causes panic with page fault. > > Good analysis, but what about the less intrusive diff below from FreeBSD? > > It looks like when isochronous support has been imported, task > cancellation were forgotten from the abort path.
Your diff seem to fix the problem more exactly. > Does this also fix your panic? Yes, it fixed the panic. But I needed to apply same changes to uhci.c like attached diff. Index: sys/dev/usb/ehci.c =================================================================== RCS file: /cvs/openbsd/src/sys/dev/usb/ehci.c,v retrieving revision 1.131 diff -u -p -r1.131 ehci.c --- sys/dev/usb/ehci.c 19 Apr 2013 08:58:53 -0000 1.131 +++ sys/dev/usb/ehci.c 17 May 2013 15:56:30 -0000 @@ -800,6 +800,7 @@ ehci_check_itd_intr(struct ehci_softc *s done: DPRINTFN(12, ("ehci_check_itd_intr: ex=%p done\n", ex)); timeout_del(&ex->xfer.timeout_handle); + usb_rem_task(ex->xfer.pipe->device, &ex->abort_task); ehci_idone(ex); } @@ -2859,6 +2860,7 @@ ehci_abort_isoc_xfer(struct usbd_xfer *x s = splusb(); xfer->status = status; timeout_del(&xfer->timeout_handle); + usb_rem_task(epipe->pipe.device, &exfer->abort_task); usb_transfer_complete(xfer); splx(s); return; @@ -2883,6 +2885,7 @@ ehci_abort_isoc_xfer(struct usbd_xfer *x xfer->status = status; timeout_del(&xfer->timeout_handle); + usb_rem_task(epipe->pipe.device, &exfer->abort_task); s = splusb(); for (itd = exfer->itdstart; itd != NULL; itd = itd->xfer_next) { Index: sys/dev/usb/uhci.c =================================================================== RCS file: /cvs/openbsd/src/sys/dev/usb/uhci.c,v retrieving revision 1.96 diff -u -p -r1.96 uhci.c --- sys/dev/usb/uhci.c 19 Apr 2013 08:58:53 -0000 1.96 +++ sys/dev/usb/uhci.c 17 May 2013 15:57:26 -0000 @@ -1264,6 +1264,7 @@ uhci_check_intr(struct uhci_softc *sc, s done: DPRINTFN(12, ("uhci_check_intr: ii=%p done\n", ii)); timeout_del(&ii->xfer->timeout_handle); + usb_rem_task(ii->xfer->pipe->device, &UXFER(ii->xfer)->abort_task); uhci_idone(ii); } @@ -1856,6 +1857,7 @@ uhci_abort_xfer(struct usbd_xfer *xfer, s = splusb(); xfer->status = status; /* make software ignore it */ timeout_del(&xfer->timeout_handle); + usb_rem_task(xfer->pipe->device, &UXFER(xfer)->abort_task); usb_transfer_complete(xfer); splx(s); return; @@ -1870,6 +1872,7 @@ uhci_abort_xfer(struct usbd_xfer *xfer, s = splusb(); xfer->status = status; /* make software ignore it */ timeout_del(&xfer->timeout_handle); + usb_rem_task(xfer->pipe->device, &UXFER(xfer)->abort_task); DPRINTFN(1,("uhci_abort_xfer: stop ii=%p\n", ii)); for (std = ii->stdstart; std != NULL; std = std->link.std) std->td.td_status &= htole32(~(UHCI_TD_ACTIVE | UHCI_TD_IOC));