> Date: Wed, 8 Dec 2010 06:07:30 +0000
> From: Jacob Meuser <jake...@sdf.lonestar.org>
> 
> I recently got a hp officejet 4500.  it's a 3-in-1 printer/scanner/fax.
> printing works great with the hplip packages.  scanning doesn't work at
> all.
> 
> I tracked the problem to read() failing in usb_bulk_read() in libusb.
> errno was EINTR.  so I made this function just continue when read() got
> EINTR.  I could then scan, but not very reliably.
> 
> so I went into the kernel, and tracked it to usbdi_util.c:
> 
> @ usbd_bulk_transfer(usbd_xfer_handle xfer, usbd_pipe_ha
> 
>       error = tsleep((caddr_t)xfer, PZERO | PCATCH, lbl, 0);
>       splx(s);
>       if (error) {
>               DPRINTF(("usbd_bulk_transfer: tsleep=%d\n", error));
>               usbd_abort_pipe(pipe);
>               return (USBD_INTERRUPTED);
>       }
> 
> on a hunch, I changed 'PZERO | PCATCH' to 'PWAIT'.  then scanning became
> 100% reliable.  even full-page 1200dpi (~525MB of data) worked
> perfectly.
> 
> great.  but, what if we need to interrupt the transfer.  we don't want
> to hang here.
> 
> well, this function takes a timeout.  so, it's possible to make it
> return, even if the transfer stalls.  but is this used?  I looked
> at the ports that use libusb.  setting 0/infinite timeout for bulk
> transfers is extremely rare, only set in code marked experimental
> or problematic.  in the kernel, there are however some callers that
> use USBD_NO_TIMEOUT.
> 
> so, I offer the following.  if there's no timeout, behave like we do
> now and catch signals, and if there's a timeout, ignore signals.
> 
> I did the same for interrupt transfers because the situation seems
> to be the same.
> 
> thoughts?

Sorry, but this very much feels like you're hacking around a bug in
the application you're using.  If it installs signal handler without
specifying the SA_RESTART flag, it has to deal with read(2) failing
with EINTR.

> Index: usbdi_util.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/usbdi_util.c,v
> retrieving revision 1.25
> diff -u -p usbdi_util.c
> --- usbdi_util.c      26 Jun 2008 05:42:19 -0000      1.25
> +++ usbdi_util.c      8 Dec 2010 04:58:49 -0000
> @@ -426,7 +426,7 @@ usbd_bulk_transfer(usbd_xfer_handle xfer, usbd_pipe_ha
>      u_int16_t flags, u_int32_t timeout, void *buf, u_int32_t *size, char 
> *lbl)
>  {
>       usbd_status err;
> -     int s, error;
> +     int s, error, pri;
>  
>       usbd_setup_xfer(xfer, pipe, 0, buf, *size, flags, timeout,
>           usbd_bulk_transfer_cb);
> @@ -437,7 +437,8 @@ usbd_bulk_transfer(usbd_xfer_handle xfer, usbd_pipe_ha
>               splx(s);
>               return (err);
>       }
> -     error = tsleep((caddr_t)xfer, PZERO | PCATCH, lbl, 0);
> +     pri = timeout == USBD_NO_TIMEOUT ? (PZERO | PCATCH) : PWAIT;
> +     error = tsleep((caddr_t)xfer, pri, lbl, 0);
>       splx(s);
>       if (error) {
>               DPRINTF(("usbd_bulk_transfer: tsleep=%d\n", error));
> @@ -467,7 +468,7 @@ usbd_intr_transfer(usbd_xfer_handle xfer, usbd_pipe_ha
>      u_int16_t flags, u_int32_t timeout, void *buf, u_int32_t *size, char 
> *lbl)
>  {
>       usbd_status err;
> -     int s, error;
> +     int s, error, pri;
>  
>       usbd_setup_xfer(xfer, pipe, 0, buf, *size, flags, timeout,
>           usbd_intr_transfer_cb);
> @@ -478,7 +479,8 @@ usbd_intr_transfer(usbd_xfer_handle xfer, usbd_pipe_ha
>               splx(s);
>               return (err);
>       }
> -     error = tsleep(xfer, PZERO | PCATCH, lbl, 0);
> +     pri = timeout == USBD_NO_TIMEOUT ? (PZERO | PCATCH) : PWAIT;
> +     error = tsleep(xfer, pri, lbl, 0);
>       splx(s);
>       if (error) {
>               DPRINTF(("usbd_intr_transfer: tsleep=%d\n", error));

Reply via email to