All of lore.kernel.org
 help / color / mirror / Atom feed
* pvusb performance
@ 2009-09-12  3:57 James Harper
  2009-09-14  0:55 ` Noboru Iwamatsu
  0 siblings, 1 reply; 7+ messages in thread
From: James Harper @ 2009-09-12  3:57 UTC (permalink / raw)
  To: xen-devel, Noboru Iwamatsu

Noboru,

My pvUSB windows driver is now working at a very basic level (flash
memory stick), and I'm conscious of the fact that I only send one
request on the ring at a time and have to wait for it to complete before
I can put the next one on. Can this be improved?

I think maybe it can't with your current usbback driver - there would
need to be some mechanism to flush the ring of all subsequent requests
in the case of an error, eg when I get a 'Read 65536 bytes' request from
Windows, I do this:

. Put read request command on ring
. Wait for response
. Put data request on ring for first 512 bytes
. Wait for response
. Put data request on ring for next 512 bytes
. etc
. etc

If there is a buffer underrun, I can see no way for Linux to do
something with the subsequent data requests... it would need to do
something like put the ring into a error/underrun condition and 'eat'
all the requests until a clear error request came down the ring.

Or maybe this is already part of the design?

Thanks

James

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: pvusb performance
  2009-09-12  3:57 pvusb performance James Harper
@ 2009-09-14  0:55 ` Noboru Iwamatsu
  2009-09-14  1:19   ` James Harper
  0 siblings, 1 reply; 7+ messages in thread
From: Noboru Iwamatsu @ 2009-09-14  0:55 UTC (permalink / raw)
  To: james.harper, xen-devel

James,

> My pvUSB windows driver is now working at a very basic level (flash
> memory stick), and I'm conscious of the fact that I only send one
> request on the ring at a time and have to wait for it to complete before
> I can put the next one on. Can this be improved?

Possible.
RING accepts two or more requests, and queued requests are sent to
backend at a time.

> I think maybe it can't with your current usbback driver - there would
> need to be some mechanism to flush the ring of all subsequent requests
> in the case of an error, eg when I get a 'Read 65536 bytes' request from
> Windows, I do this:
> 
> . Put read request command on ring
> . Wait for response
> . Put data request on ring for first 512 bytes
> . Wait for response
> . Put data request on ring for next 512 bytes
> . etc
> . etc
> 
> If there is a buffer underrun, I can see no way for Linux to do
> something with the subsequent data requests... it would need to do
> something like put the ring into a error/underrun condition and 'eat'
> all the requests until a clear error request came down the ring.
> 
> Or maybe this is already part of the design?

Current pvusb doesn't care it.

You mean, when error occurred, HCD has to be responsible for flushing
the subsequent requests?

Should urb transferring errors be handled with USB device drivers
(the upper layer of HCD)?

If urb unlinking work properly, would it be solved?


Noboru.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: pvusb performance
  2009-09-14  0:55 ` Noboru Iwamatsu
@ 2009-09-14  1:19   ` James Harper
  2009-09-14  2:36     ` Noboru Iwamatsu
  0 siblings, 1 reply; 7+ messages in thread
From: James Harper @ 2009-09-14  1:19 UTC (permalink / raw)
  To: Noboru Iwamatsu, xen-devel

> 
> > I think maybe it can't with your current usbback driver - there
would
> > need to be some mechanism to flush the ring of all subsequent
requests
> > in the case of an error, eg when I get a 'Read 65536 bytes' request
from
> > Windows, I do this:
> >
> > . Put read request command on ring
> > . Wait for response
> > . Put data request on ring for first 512 bytes
> > . Wait for response
> > . Put data request on ring for next 512 bytes
> > . etc
> > . etc
> >
> > If there is a buffer underrun, I can see no way for Linux to do
> > something with the subsequent data requests... it would need to do
> > something like put the ring into a error/underrun condition and
'eat'
> > all the requests until a clear error request came down the ring.
> >
> > Or maybe this is already part of the design?
> 
> Current pvusb doesn't care it.
> 
> You mean, when error occurred, HCD has to be responsible for flushing
> the subsequent requests?
> 
> Should urb transferring errors be handled with USB device drivers
> (the upper layer of HCD)?
> 
> If urb unlinking work properly, would it be solved?
> 

I don't think that unlink by itself would solve it. The failure scenario
I am think of is when Windows gives me a URB with 64k of data to read or
write and an error occurs. I need to break that up to send to usbback,
but if I have 128 x 512 bytes requests on the ring and the first one
fails, usbback will still continue to execute the remaining 127. I'm not
sure that that matters for a regular read error which is presumably a
rare occurance, but what about for a device which could do a 'short
read'?

Maybe we could have a sequence number in the req and so once the error
or short read occurs, usbback throws out all the remaining requests with
that sequence number (just returns them with a status to indicate that
they weren't used). The expected next sequence number would be kept by
usbback in the data structure for each and usbback would discard any
request with an unexpected sequence number...

Why do you allow up to 10 4K segments to be attached to the request? The
upper limit for usb packet size seems to be 512 bytes... or do ISOC
requests allow more (I don't know anything about those yet).

James

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: pvusb performance
  2009-09-14  1:19   ` James Harper
@ 2009-09-14  2:36     ` Noboru Iwamatsu
  2009-09-14  5:19       ` James Harper
  0 siblings, 1 reply; 7+ messages in thread
From: Noboru Iwamatsu @ 2009-09-14  2:36 UTC (permalink / raw)
  To: james.harper, xen-devel

James,

> I don't think that unlink by itself would solve it. The failure scenario
> I am think of is when Windows gives me a URB with 64k of data to read or
> write and an error occurs. I need to break that up to send to usbback,
> but if I have 128 x 512 bytes requests on the ring and the first one
> fails, usbback will still continue to execute the remaining 127. I'm not
> sure that that matters for a regular read error which is presumably a
> rare occurance, but what about for a device which could do a 'short
> read'?

We might have to understand the difference of linux and windows USB 
driver's behaviors.

In linux, I think,

- If 128 requests on the ring and the first one fails, usbback will
   still continue to the remaining 127.
   Subsequent requests may succeed or fail or not response. In ether
   case, USB device driver in the frontend can identify the urb failure,
   and re-initialize the device or unlink urbs.

- Reporting short-reads as errors or not is defined by the USB
   device driver itself.
   "USB_SHORT_NOT_OK" bit of the urb->transfer_flags is that.

I doubt host controller driver needs to clean up the error.

> Maybe we could have a sequence number in the req and so once the error
> or short read occurs, usbback throws out all the remaining requests with
> that sequence number (just returns them with a status to indicate that
> they weren't used). The expected next sequence number would be kept by
> usbback in the data structure for each and usbback would discard any
> request with an unexpected sequence number...
> 
> Why do you allow up to 10 4K segments to be attached to the request? The
> upper limit for usb packet size seems to be 512 bytes... or do ISOC
> requests allow more (I don't know anything about those yet).

USB packat size and urb->transfer_buffer_length are different.
The first is the actual USB transfer size that is defined in USB Spec,
the second is the data size handled by urb.

usbfront and usbback transfers urb.

Some device drivers that require over 10 * 4K segments can not be
supported by current pvusb.

Regards,
Noboru

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: pvusb performance
  2009-09-14  2:36     ` Noboru Iwamatsu
@ 2009-09-14  5:19       ` James Harper
  2009-09-14  5:45         ` Noboru Iwamatsu
  0 siblings, 1 reply; 7+ messages in thread
From: James Harper @ 2009-09-14  5:19 UTC (permalink / raw)
  To: Noboru Iwamatsu, xen-devel

> 
> > I don't think that unlink by itself would solve it. The failure
scenario
> > I am think of is when Windows gives me a URB with 64k of data to
read or
> > write and an error occurs. I need to break that up to send to
usbback,
> > but if I have 128 x 512 bytes requests on the ring and the first one
> > fails, usbback will still continue to execute the remaining 127. I'm
not
> > sure that that matters for a regular read error which is presumably
a
> > rare occurance, but what about for a device which could do a 'short
> > read'?
> 
> We might have to understand the difference of linux and windows USB
> driver's behaviors.
> 
> In linux, I think,
> 
> - If 128 requests on the ring and the first one fails, usbback will
>    still continue to the remaining 127.
>    Subsequent requests may succeed or fail or not response. In ether
>    case, USB device driver in the frontend can identify the urb
failure,
>    and re-initialize the device or unlink urbs.
> 
> - Reporting short-reads as errors or not is defined by the USB
>    device driver itself.
>    "USB_SHORT_NOT_OK" bit of the urb->transfer_flags is that.
> 
> I doubt host controller driver needs to clean up the error.
> 
> > Maybe we could have a sequence number in the req and so once the
error
> > or short read occurs, usbback throws out all the remaining requests
with
> > that sequence number (just returns them with a status to indicate
that
> > they weren't used). The expected next sequence number would be kept
by
> > usbback in the data structure for each and usbback would discard any
> > request with an unexpected sequence number...
> >
> > Why do you allow up to 10 4K segments to be attached to the request?
The
> > upper limit for usb packet size seems to be 512 bytes... or do ISOC
> > requests allow more (I don't know anything about those yet).
> 
> USB packat size and urb->transfer_buffer_length are different.
> The first is the actual USB transfer size that is defined in USB Spec,
> the second is the data size handled by urb.
> 
> usbfront and usbback transfers urb.
> 
> Some device drivers that require over 10 * 4K segments can not be
> supported by current pvusb.
> 

Okay maybe I'm a bit confused then.

When Windows gave me a URB with 65536 bytes of data, I set buffer_length
to 65536 and filled up the SG registers accordingly, but usbback gave me
an error. When I break it into 512 byte (packet size) chunks it works
okay. I just assumed that you have to give usbback requests with less
that 'max packet size' of data.

Maybe the cause of my error was somewhere else? I'll try it again.

Thanks

James

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: pvusb performance
  2009-09-14  5:19       ` James Harper
@ 2009-09-14  5:45         ` Noboru Iwamatsu
  2009-09-14  5:49           ` James Harper
  0 siblings, 1 reply; 7+ messages in thread
From: Noboru Iwamatsu @ 2009-09-14  5:45 UTC (permalink / raw)
  To: james.harper, xen-devel

James,

> Okay maybe I'm a bit confused then.
> 
> When Windows gave me a URB with 65536 bytes of data, I set buffer_length
> to 65536 and filled up the SG registers accordingly, but usbback gave me
> an error. When I break it into 512 byte (packet size) chunks it works
> okay. I just assumed that you have to give usbback requests with less
> that 'max packet size' of data.
> 
> Maybe the cause of my error was somewhere else? I'll try it again.

If you transfer 65536 bytes of data, you should change the
USBIF_MAX_SEGMENTS_PER_REQUESTS value to 16 or more.
It is defined in include/xen/interface/io/usbif.h
You need to recompile the drivers.

Request size and MAX_RING_SIZE are trade-off.
I defined this value empirically.
If the value is about up to 20, it doesn't affect the performance
of memory stick.

Regards,
Noboru

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: pvusb performance
  2009-09-14  5:45         ` Noboru Iwamatsu
@ 2009-09-14  5:49           ` James Harper
  0 siblings, 0 replies; 7+ messages in thread
From: James Harper @ 2009-09-14  5:49 UTC (permalink / raw)
  To: Noboru Iwamatsu, xen-devel

> 
> > Okay maybe I'm a bit confused then.
> >
> > When Windows gave me a URB with 65536 bytes of data, I set
buffer_length
> > to 65536 and filled up the SG registers accordingly, but usbback
gave me
> > an error. When I break it into 512 byte (packet size) chunks it
works
> > okay. I just assumed that you have to give usbback requests with
less
> > that 'max packet size' of data.
> >
> > Maybe the cause of my error was somewhere else? I'll try it again.
> 
> If you transfer 65536 bytes of data, you should change the
> USBIF_MAX_SEGMENTS_PER_REQUESTS value to 16 or more.
> It is defined in include/xen/interface/io/usbif.h
> You need to recompile the drivers.
> 
> Request size and MAX_RING_SIZE are trade-off.
> I defined this value empirically.
> If the value is about up to 20, it doesn't affect the performance
> of memory stick.
> 

D'oh! I feel like a complete fool :)

Yes, 10 * 4096 is much less than 65536 - that will explain it!

I don't think I can tell windows to send me less data, which might be a
bit of a pain.

Still, I can still get it working by breaking the request into 40960
byte chunks.

Thanks for clearing that up for me. Arithmetic has never been my strong
point :)

James

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2009-09-14  5:49 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-09-12  3:57 pvusb performance James Harper
2009-09-14  0:55 ` Noboru Iwamatsu
2009-09-14  1:19   ` James Harper
2009-09-14  2:36     ` Noboru Iwamatsu
2009-09-14  5:19       ` James Harper
2009-09-14  5:45         ` Noboru Iwamatsu
2009-09-14  5:49           ` James Harper

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.