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?

-- 
jakemsr@sdf.lonestar.org
SDF Public Access UNIX System - http://sdf.lonestar.org

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 ...
[ message continues ]

This is really not my area of expertise but it seems like a safe
hack and it reads okay to me.

[ message continues ]

On Wed, 8 Dec 2010, Jacob Meuser wrote:


For what it's worth this diff fixed all remaining issues that were left 
with scanning and libusb for me. So I'll be happy if it (or a variarion) 
gets committed.

Thanks Jacob.

-- 
Antoine

[ message continues ]

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

[ message continues ]

that application would be libusb itself.  is a library supposed

-- 
jakemsr@sdf.lonestar.org
SDF Public Access UNIX System - http://sdf.lonestar.org

[ message continues ]

to be clear, the actual read() is in usb_bulk_read() in libusb.
so, the application doesn't really know that there's a read().

I tried making libusb continue, but the problem is that when
usbd_bulk_transfer catches the signal, it aborts everything in
the pipe.  restarting from there correctly is complicated,

-- 
jakemsr@sdf.lonestar.org
SDF Public Access UNIX System - http://sdf.lonestar.org

[ message continues ]

I looked at all users of libusb.  none deal with usb_{bulk,interrupt}_read
(the libusb "frontend") quitting with EINTR.  none set SA_RESTART.

usbd_{bulk,intr}_transfer() (in the usb(4) stack) do not obey
SA_RESTART.  neither do the ugen(4) ends of those functions.

usbd_transfer() does not catch signals.  in fact.  the only other
thing in the usb stack that does catch signals is usbread(), which
is an interface to read messages from the usb event queue.  the
event queue is stored in software.  it doesn't handle tsleep()
returning ERESTART either, btw.

figuring out an absolute max timeout seems fairly straight-forward.
timeouts are specified in miliseconds.  the usb frame rate is 1000 Hz.
an empty frame signals the end of a transfer.  timeout = bytes,
basically.

now, usbd_intr_transfer() may be a different story.  that will block
(if not set or non-blocking io) until there is data to read.  most
users of this in libusb do set a timeout though.  some also restart
the read if they get ETIMEDOUT.

I think, catching signals is wrong.  at least, it's not expected by
most of it's users and not even handled correctly in the kernel.

further, by not setting a timeout, it leaves the user to figure out

-- 
jakemsr@sdf.lonestar.org
SDF Public Access UNIX System - http://sdf.lonestar.org

[ message continues ]

any further thoughts on this?


-- 
jakemsr@sdf.lonestar.org
SDF Public Access UNIX System - http://sdf.lonestar.org

[ message continues ]

after talking with ratchov and deraadt, I am convinced the bug is that
we have a read() interface that can be interrupted but not restarted
reliably.  i.e.  even if the application deals with EINTR, it's
not reliable because data is lost in the kernel.

so I took a shot at making ugen's read() interface restartable.
diff is below.  unfortunately it only works about 90% of the time.
the original diff I sent works 100%.  this diff is also a bit
complicated, and still requires complicated userland code to
deal with EINTR, because of timeouts.  say you have a 4 second
timeout, and get interrupted after 3 seconds.  so you restart with
a 4 second timeout, then get interrupted again, so you restart
... ad infinitum.  otoh, you can gettimeofday and keep track
of how long you've been waiting, timersub and reset the timeout ...
but then you're potentially using a smaller timeout than is
required.  this timeout mess is exactly where the 10% failure rate
comes from.

considering that
a) I can't find any code that expects EINTR in this case,
b) no other transfers in our usb code are interruptable,
c) doing the "right" thing is complicated both in the kernel and in
   userland,
I am going to commit the original diff, minus the priority change.

I am just posting this here to let interested people know what the
situation is.  I agree it would be nice/correct if it were possible to
interrupt and restart read(), but I also think it's more important
to have things actually work reliably.

-- 
jakemsr@sdf.lonestar.org
SDF Public Access UNIX System - http://sdf.lonestar.org

Index: ugen.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/ugen.c,v
retrieving revision 1.62
diff -u -p ugen.c
--- ugen.c	24 Sep 2010 08:33:59 -0000	1.62
+++ ugen.c	17 Dec 2010 12:29:23 -0000
@@ -84,6 +84,9 @@ struct ugen_endpoint {
 	u_char *limit;		/* end of circular buffer (isoc) */
 	u_char *cur;		/* current read location (isoc) */
 	u_int32_t ...
[ message continues ]