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));
