On Sun, 27 Nov 2005, Chris Humbert wrote:

> I want to implement URB queueing, where URB submission succeeds
> if the URB could be mapped later.  This would prevent sporadic
> submission failures, usb_submit_urb() could return a better error
> like -EMSGSIZE for too large URBs, and the URB scheduler can
> round-robin EPs to prevent one device from hogging the bus.

So far as I know, the USB API doesn't allow for size limits on URB 
transfer lengths.  There's no mechanism to report back that an URB is too 
large, and no driver tries to break URBs up into smaller pieces.

> echo 8 > /sys/block/sda/device/max_sectors; works for now, but
> I'd like a better solution for when this HCD is included in a
> distribution.  Preferably, usb-storage would learn that it can't
> submit large URBs and wouldn't map more than it submits.

usb-storage doesn't control the size of all its URBs and doesn't map them.  
The mapping is done by usbcore, and the size is determined by the size of 
the elements in a scatter-gather list.

> max_sectors could take care of this, but it looks like
> usb-storage does some 16KB requests as four 4KB URBs.  Is it
> possible to use a larger max_sector with 4KB URBs?  That would
> help performance immensely.

That question doesn't make sense.  max_sectors is a property of a request
queue and is controlled through the block layer.  There are several other
related properties of block request queues; you would interested in the 
one that controls the maximum size of a scatter_gather hardware segment.  
Unfortunately usb-storage doesn't provide any way to set that property.

> usb-storage doesn't check for dma mapping failures either.  After

Nonsense.  What about this code from usb_sg_init():

        /* not all host controllers use DMA (like the mainstream pci ones);
         * they can use PIO (sl811) or be software over another transport.
         */
        dma = (dev->dev.dma_mask != NULL);
        if (dma)
                io->entries = usb_buffer_map_sg (dev, pipe, sg, nents);
        else
                io->entries = nents;

        /* initialize all the urbs we'll use */
        if (io->entries <= 0)
                return io->entries;


> Since the usb-storage maps its own dma-able memory, it would
> cause problems with URB queueing.  When I was queueing TDs, the
> HCD had dma_mask == NULL, so the HCD handled all dma operations.
> I could do this again, as it has a few advantages:
>  * The generic allocator manages the 28KB not used by the TD and
>    ED dma pool, resulting in more efficient memory usage.
>  * Increased performance?  With usb-storage, I'm currently
>    getting 568kB/s @ 32% CPU (max_sectors=24) as opposed to
>    900kB/s @ 8% CPU (max_sectors=240) with TD queueing.
>  * This method doesn't use dmabounce, which is more acceptable
>    upstream.
> 
> This wouldn't require modification to the USB core if I
> temporarily replace the USB completion handler, but this seems
> like a hack.  Ideally, struct usb_operations would have a
> 'complete_urb' or 'giveback_urb' function I would use to unmap
> dma memory.

Why do you need to replace anything?  Can't you do the unmapping within 
the HCD's URB-completion routine?

Alan Stern



-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to