Re: [Qemu-devel] [PATCH 04/12] usb: Add support for input pipelining

[Gerd Hoffmann]

On 10/08/12 09:51, Hans de Goede wrote:
> Currently we effectively only do pipelining for output endpoints, since
> controllers will stop filling the queue for a packet with stop the queue
> on a short read semantics enabled.

> This causes large input transfers to get split into multiple packets, which
> comes with a serious performance penalty.
> 
> This patch makes it possible to do pipelining for input endpoints, by
> re-combining packets which belong to 1 transfer at the guest device-driver
> level into 1 large packet before sending them to the device, this is mostly
> useful as a significant performance improvement for redirection of real USB
> devices.

This splitting and recombining is too much magic in the core layer for
my taste.

I think the core work flow should stay as-is.  Instead we should add
more meta data to USBPacket (stop-queue-on-short-read bit, maybe
interrupt-on-complete too).  Continue queuing packets even with
stop-queue-on-short-read set.  Maybe add a callback to notify USBDevice
when the host controller is done filling the queue, so USBDevices can
process all queued packets in one go (bottom half would work too
though).  Then expect USBDevices to get things right based on the
USBPacket flags passed on (i.e. have host-linux use
USBFS_URB_BULK_CONTINUATION as needed).

This way usb-host and usbredir will see what is really going on instead
of having the usb core magically fixing up stuff for them.  I think this
will serve us better long-term.  Maybe you are seeing this "data *and*
stall" issue (patch 1) exactly because of all this combining & splitting
logic?

cheers,
  Gerd