On Tue, Mar 04, 2014 at 10:45:34AM +0800, Ming Lei wrote:
> On Tue, Mar 4, 2014 at 5:47 AM, Sarah Sharp <sarah.a.sharp@xxxxxxxxxxxxxxx> wrote:
> > Ok, we can't have SuperSpeed mass storage devices broken, so it looks
> > like we'll have to revert the last patch to add scatter-gather to the
> > ASIX driver to avoid that breakage. That means Mathias is going to need
> > to revert those two commits then, since he's taking over pushing xHCI
> > driver bug fixes this kernel.
> >
> > Greg, Dave, Freddy, how do you want to handle reverting commit
> > 3804fad45411? Should that come through Dave's networking tree or Greg's
> > USB tree?
>
> Sorry, could you explain why you want to revert commit 3804fad45411
> (USBNET: ax88179_178a)?
>
> If xHCI won't plan to support arbitrary-length scatter-gather any more, that
> is fine to revert the commit forever.
Scatter-gather under xHCI seems to work fine on mass storage, but it
fails with that particular ASIX device, because of how the xHCI driver
sets up the buffers on the endpoint rings. We need to implement the TD
fragment rules to avoid breaking this particular USB ethernet device.
In the meantime, mass storage works fine with scatter-gather, so we
don't want to lose hard drive performance.
> Otherwise, it should be better to just
> clear no_sg_constraint in xhcd, shouldn't it?
We tried that. That's what commit 247bf557273d does. It clears
no_sg_constraint for 1.0 xHCI hosts that need TD fragments (and thus
would cause the ASIX chipsets to drop packets). However, we've found
that commit breaks USB 3.0 mass storage devices.
The block layer may submit scatter-gather lists with entries that
are multiples of 512-byte blocks. That's fine for USB 2.0 devices,
where the bulk endpoint max packet size is 512 bytes. But USB 3.0
devices have bulk endpoints with a 1024 byte max packet size.
That means when the block layer submits a scatter-gather list with one
entry that includes, say, three 512-byte blocks, this code will reject
the URB if it's submitted to a USB 3.0 bulk endpoint:
int usb_submit_urb(struct urb *urb, gfp_t mem_flags)
{
...
max = usb_endpoint_maxp(&ep->desc);
...
} else if (urb->num_sgs && !urb->dev->bus->no_sg_constraint &&
dev->speed != USB_SPEED_WIRELESS) {
struct scatterlist *sg;
int i;
for_each_sg(urb->sg, sg, urb->num_sgs - 1, i)
if (sg->length % max)
return -EINVAL;
}
This results in failures with USB 3.0 drives. For me, a failure to
auto-mount the device. For others, a read or write SCSI command
failure. We can't introduce a regression in USB 3.0 mass storage in
order to get better performance on one USB ethernet adapter.
Until we get TD fragments implemented, we need to revert the commit
3804fad45411 and commit 247bf557273d. That will mean USB 3.0 mass
storage works again, and (unfortunately) the ASIX driver performance
won't be as good until we implement TD fragments.
Sarah Sharp
--
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
