Re: Is usb_buffer_alloc a generic URB buffer allocator ? (was Split Bulk Transfers)

On Thursday 17 May 2007, Alan Stern wrote:
> On Thu, 17 May 2007, Laurent Pinchart wrote:
> > Ok. A driver should obviously not allocate a lot of DMA cache coherent
> > memory if it's not going to use it. For buffers that are allocated at
> > initialisation time and not used often, usb_buffer_alloc() should not be
> > used.
>
> Yes.  Similarly, buffers that are going to be used once and then thrown
> away shouldn't be obtained from usb_buffer_alloc().
>
> > There's something I don't get. The documentation states that USB buffer
> > memory should be allocated using kmalloc or get_free_pages. This means
> > that allocated memory will be physically contiguous. However, not all
> > controllers should require that. There are, from what I can tell, three
> > classes of USB host controllers:
> >
> > - PIO host controllers don't use DMA at all, and should thus not require
> > physically contiguous memory. The device dma_mask field is set to 0, and
> > usb_hcd_submit_urb doesn't allocate DMA mappings for URB buffers when
> > using such a controller.
> >
> > - DMA-able host controllers without hardware scatter-gather require
> > physically contiguous memory. For those controllers, usb_hcd_submit_urb
> > allocates DMA mappings if the URB buffers haven't been mapped already
> > (URB_NO_*_DMA_MAP flags not set). usb_hcd_giveback_urb unmaps the buffers
> > if they have been mapped by usb_hcd_submit_urb. As those controllers
> > require physically contiguous memory, kmalloc or get_free_pages must be
> > used.
> >
> > - DMA-able host controllers with hardware scatter-gather don't require
> > physically contiguous memory. EHCI controllers seem to have
> > scatter-gather capabilities. For those controllers, pressure on the
> > memory allocator could be lowered by implementing scatter-gather.
> >
> > USB device drivers currently either allocate URB buffers with
> > kmalloc/get_free_pages or call usb_buffer_alloc. When using the former,
> > the DMA mapping is created and deleted at URB submission/completion. When
> > using the later, DMA mappings are created once and kept for the entire
> > life of the buffer.
> >
> > I suppose we should keep kmalloc/get_free_pages for buffers that are
> > seldom used. From what I've seen so far, usb_buffer_alloc would help for
> > buffers used for isochronous or bulk streaming, where the same URBs are
> > submitted again and again. That would remove the DMA map/unmap overhead.
>
> That is an appropriate use of usb_buffer_alloc, yes.
>
> > Shouldn't we implement scatter-gather for EHCI (and possibly other)
> > controllers ? Is it planned ? We would then need a generic URB buffer
> > allocator, as the device driver should not care about what kind of memory
> > the USB host controller needs. The allocator would be passed a parameter
> > to DMA-map the buffer for streaming bulk/isochronous transfers.
>
> This is a little complicated.
>
> First, let's break it down according to the transfer type.  Control
> messages tend to be quite short; there's no advantage to using
> scatter-gather for them.  Likewise with interrupt transfers (they
> usually are limited to a maxpacket-sized buffer).

Agreed.

> Iso transfers involve lots of small sub-buffers inside a larger buffer,
> where the exact arrangement doesn't much matter.  You want the large
> buffer to be big enough to maintain streaming while minimizing
> interrupt overhead, but it doesn't have to be terribly big.  Generally
> multiple URBs are submitted anyway, each with its own buffer, so again
> s-g isn't needed.

Agreed too. s-g might help, but the isochronous URB buffers are usually not 
too big.

> Bulk transfers are the only ones which could really benefit.  However
> nothing says that large bulk transfers have to be done using a single
> URB.  They can be streamed just like Iso transfers, thereby making s-g
> at the HCD level unnecessary.  That's how the s-g library already
> present in message.c works.

Right. That's what I implemented in my driver and it seems to work fine. 
Physically contiguous pages allocation dropped from order 22 to order 3.

> So on the whole, there's no pressing reason to add s-g support to the
> controller drivers.  What might be useful would be a bulk s-g library
> that could be used outside of process context -- unlike the current
> one.  (The current one also assumes that all the buffers have already
> been allocated; it doesn't allocate them for you.)  At one point Pete
> Zaitcev spent some time working on such a thing, but then he dropped
> it.

If not overly difficult, it might be interesting to implement transparent s-g, 
where the URB submission handler would cut buffers in pieces if needed. That 
way buffers could be allocated with kmalloc/get_free_pages for small buffers 
used infrequently, with usb_buffer_alloc for small buffers used in data 
bulk/isochronous streaming or with vmalloc for big buffers.

Thinking about it, scatter-gathering buffers transparently would make it 
difficult to allocate coherent DMA mappings for data streaming. It would make 
life easier for driver writers, but performances might be impacted.

Laurent Pinchart

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel