On Sat, May 01, 2010 at 03:02:42PM -0400, Alan Stern wrote:
> On Sat, 1 May 2010, Matthew Wilcox wrote:
> > Also, with just a couple of lines extra, I included
> > support for OHCI and UHCI controllers (because the USB midlayer handles
> > an SGL with one element), so this feels like a big win to me.
>
> Are you sure about this last part? As things stand currently, the only
> way to use SG with ohci-hcd or uhci-hcd is by way of usb_sg_init() and
> friends -- which has the disadvantage of being synchronous and
> requiring a process context. Although usbcore could be changed so that
> single-element SG lists would work directly, at the moment they don't
> (the DMA address needs to be copied from the scatterlist structure into
> the URB -- is that your couple of extra lines?).
Erm, your as1368 patch added support for single-element SG lists. This is
what I mean:
uas:
if (udev->bus->sg_tablesize) {
urb->num_sgs = sdb->table.nents;
} else {
urb->num_sgs = 0;
}
urb->sg = sdb->table.sgl;
hcd:
if (urb->num_sgs) {
[...]
} else if (urb->sg) {
struct scatterlist *sg = urb->sg;
urb->transfer_dma = dma_map_page(
hcd->self.controller,
sg_page(sg),
sg->offset,
urb->transfer_buffer_length,
dir);
> Also, there's a matter of efficiency. Right now usb-storage works fine
> with arbitrary-length SG lists on OHCI or UHCI, because it uses
> usb_sg_*(). If you change things so that the HCDs advertise native
> scatter-gather support with a maximum list size of 1, it will cause a
> big performance hit. Every single SG buffer would require its own SCSI
> command, whereas now one command can handle lots of buffers together.
> (We could avoid this performance hit by not using an HCD's native SG
> support if sg_tablesize is 1, but that's not an attractive hack.)
I'm not planning on changing usb-storage at all. Obviously, if you want
to change it, I'm not going to stand in your way. I don't think using
multiple SCSI commands with UAS will be as much of a performance hit as
using multiple SCSI commands with BOT because UAS does permit commands
to overlap.
> In principle, adding native scatterlist support to uhci-hcd and
> ohci-hcd wouldn't be very hard -- ading it to ehci-hcd was fairly easy.
> But I have yet to be convinced that it would be worth the effort. A
> better approach (and one that would help all the other USB-1.1 HCDs)
> would be to improve the usb_sg_*() family by adding an asynchronous
> mode of operation. Pete Zaitcev and I actually implemented this a
> while back, but it never was submitted or merged.
I'm not convinced we need to go to the effort of improving the UHCI and
OHCI drivers for the benefit of UAS. As far as I can tell, the only real
way you'd end up talking UAS through one of those controllers (on a PC
anyway) is either to find a system from 2001 (ICH4 onwards has support for
EHCI), or plug a USB 1.1 hub into the mix between the device and the host.
I don't care too much about performance in those situations, but it
is nice for it to work.
As far as an asynchronous version of usb_sg_* ... I don't really see the
advantage to doing that over improving the usb_submit_urb() support for
sg lists.
--
Matthew Wilcox Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
