From: YASUOKA Masahiko (yasuokayasuoka.net)
Date: Fri May 17 2013 - 11:08:15 CDT

• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi,

On Fri, 17 May 2013 14:06:49 +0200
Martin Pieuchot <mpieuchotnolizard.org> wrote:
> On 17/05/13(Fri) 19:25, YASUOKA Masahiko wrote:
>> On Fri, 1 Mar 2013 13:43:00 +0000
>> "Wade, Daniel" <DWademeridium.com> wrote:
>> > -----Original Message-----
>> > From: owner-techopenbsd.org [mailto:owner-techopenbsd.org] On Behalf Of Stefan Sperling
>> > Sent: Thursday, February 28, 2013 12:16 PM
>> > To: Edd Barrett
>> > Cc: techopenbsd.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));

• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]