re: kern/46648: disconnecting usb mouse crashes system

[matthew green <mrg%eterna.com.au@localhost>]

The following reply was made to PR kern/46648; it has been noted by GNATS.

From: matthew green <mrg%eterna.com.au@localhost>
To: kern-bug-people%netbsd.org@localhost, gnats-admin%netbsd.org@localhost,
    netbsd-bugs%netbsd.org@localhost, gnats-bugs%NetBSD.org@localhost,
    christos%zoulas.com@localhost (Christos Zoulas)
Cc: 
Subject: re: kern/46648: disconnecting usb mouse crashes system
Date: Fri, 06 Jul 2012 09:15:44 +1000

 FWIW, the patch below fixes the crash and things work again.
 after chatting with jmcneill a little, i think the right way
 is to move all the xfer callbacks to be called from the task
 thread.  i may commit the patch below in the interim.
 
 
 .mrg.
 
 
 Index: usb_subr.c
 ===================================================================
 RCS file: /cvsroot/src/sys/dev/usb/usb_subr.c,v
 retrieving revision 1.182
 diff -p -u -r1.182 usb_subr.c
 --- usb_subr.c 10 Jun 2012 06:15:54 -0000      1.182
 +++ usb_subr.c 4 Jul 2012 00:50:51 -0000
 @@ -765,6 +765,7 @@ usbd_setup_pipe(usbd_device_handle dev, 
                free(p, M_USB);
                return (err);
        }
 +      usb_init_task(&p->async_task, usbd_clear_endpoint_stall_async_cb, p);
        *pipe = p;
        return (USBD_NORMAL_COMPLETION);
  }
 @@ -779,6 +780,7 @@ usbd_kill_pipe(usbd_pipe_handle pipe)
        usbd_lock_pipe(pipe);
        pipe->methods->close(pipe);
        usbd_unlock_pipe(pipe);
 +      usb_rem_task(pipe->device, &pipe->async_task);
        pipe->endpoint->refcnt--;
        free(pipe, M_USB);
  }
 Index: usbdi.c
 ===================================================================
 RCS file: /cvsroot/src/sys/dev/usb/usbdi.c,v
 retrieving revision 1.138
 diff -p -u -r1.138 usbdi.c
 --- usbdi.c    10 Jun 2012 06:15:55 -0000      1.138
 +++ usbdi.c    4 Jul 2012 00:50:51 -0000
 @@ -599,12 +599,12 @@ XXX should we do this?
        return (err);
  }
  
 -usbd_status
 -usbd_clear_endpoint_stall_async(usbd_pipe_handle pipe)
 +void
 +usbd_clear_endpoint_stall_async_cb(void *arg)
  {
 +      usbd_pipe_handle pipe = arg;
        usbd_device_handle dev = pipe->device;
        usb_device_request_t req;
 -      usbd_status err;
  
        pipe->methods->cleartoggle(pipe);
  
 @@ -613,8 +613,14 @@ usbd_clear_endpoint_stall_async(usbd_pip
        USETW(req.wValue, UF_ENDPOINT_HALT);
        USETW(req.wIndex, pipe->endpoint->edesc->bEndpointAddress);
        USETW(req.wLength, 0);
 -      err = usbd_do_request_async(dev, &req, 0);
 -      return (err);
 +      (void)usbd_do_request_async(dev, &req, 0);
 +}
 +
 +void
 +usbd_clear_endpoint_stall_async(usbd_pipe_handle pipe)
 +{
 +
 +      usb_add_task(pipe->device, &pipe->async_task, USB_TASKQ_DRIVER);
  }
  
  void
 Index: usbdi.h
 ===================================================================
 RCS file: /cvsroot/src/sys/dev/usb/usbdi.h,v
 retrieving revision 1.83
 diff -p -u -r1.83 usbdi.h
 --- usbdi.h    10 Jun 2012 06:15:55 -0000      1.83
 +++ usbdi.h    4 Jul 2012 00:50:51 -0000
 @@ -113,12 +113,12 @@ usb_endpoint_descriptor_t *usbd_interfac
  usbd_status usbd_abort_pipe(usbd_pipe_handle);
  usbd_status usbd_abort_default_pipe(usbd_device_handle);
  usbd_status usbd_clear_endpoint_stall(usbd_pipe_handle);
 -usbd_status usbd_clear_endpoint_stall_async(usbd_pipe_handle);
 +void usbd_clear_endpoint_stall_async(usbd_pipe_handle);
  void usbd_clear_endpoint_toggle(usbd_pipe_handle);
  usbd_status usbd_endpoint_count(usbd_interface_handle, u_int8_t *);
  usbd_status usbd_interface_count(usbd_device_handle, u_int8_t *);
  void usbd_interface2device_handle(usbd_interface_handle,
 -                                       usbd_device_handle *);
 +                                usbd_device_handle *);
  usbd_status usbd_device2interface_handle(usbd_device_handle,
                              u_int8_t, usbd_interface_handle *);
  
 @@ -188,6 +188,9 @@ typedef struct {
  void usb_desc_iter_init(usbd_device_handle, usbd_desc_iter_t *);
  const usb_descriptor_t *usb_desc_iter_next(usbd_desc_iter_t *);
  
 +/* Used to clear endpoint stalls from the softint */
 +void usbd_clear_endpoint_stall_async_cb(void *);
 +
  /*
   * The usb_task structs form a queue of things to run in the USB event
   * thread.  Normally this is just device discovery when a connect/disconnect
 Index: usbdivar.h
 ===================================================================
 RCS file: /cvsroot/src/sys/dev/usb/usbdivar.h,v
 retrieving revision 1.97
 diff -p -u -r1.97 usbdivar.h
 --- usbdivar.h 10 Jun 2012 06:15:55 -0000      1.97
 +++ usbdivar.h 4 Jul 2012 00:50:51 -0000
 @@ -212,6 +212,7 @@ struct usbd_pipe {
        char                    aborting;
        SIMPLEQ_HEAD(, usbd_xfer) queue;
        LIST_ENTRY(usbd_pipe)   next;
 +      struct usb_task         async_task;
  
        usbd_xfer_handle        intrxfer; /* used for repeating requests */
        char                    repeat;