> -----Original Message----- > From: Sarah Sharp [mailto:sarah.a.sharp@xxxxxxxxxxxxxxx] > Sent: Friday, June 17, 2011 4:47 AM > To: Alan Stern > Cc: Xu, Andiry; linux-usb@xxxxxxxxxxxxxxx; matthew.r.wilcox@xxxxxxxxx > Subject: Re: [PATCH 5/5] xHCI: assign devices to different interrupters > > On Thu, Jun 16, 2011 at 03:57:22PM -0400, Alan Stern wrote: > > On Thu, 16 Jun 2011, Sarah Sharp wrote: > > > > > > Adding a intr_target field to the URB? Then to set the field, usb > core > > > > not only need to know the number of interrupters enabled, but > also needs > > > > to find out the most suitable interrupter. Does every urb need to > > > > calculate > > > > the field before being submitted to the host driver? If so, I > think it's > > > > another performance lost. > > > > > > I think drivers probably know when they switch configurations or > > > alternate interface settings what latency requirements they need. > A > > > webcam driver needs a latency less than the periodic rate of the > > > isochronous endpoint. So we could have one call to allow the > driver to > > > set the interrupter once before it submits any URBs. > > > > > > The USB core could store that value in a per-driver structure, or > the > > > xHCI driver could store it in xhci_virt_ep. The xHCI driver could > set > > > up several different endpoint rings for periodic endpoints, and > > > advertise their latencies (IMODI values) to the USB core. The USB > core, > > > based on the latency the driver needs, could set the driver's > > > interrupter target once, or maybe once per alternate interface > setting > > > change. So that API design might avoid any per-URB performance hit. > > > > Drivers and the USB core should not have to care about interrupters. > > That is an internal implementation detail for xhci-hcd. > > Yeah, you're probably right. I'd mostly like the option of allowing > drivers to pick their interrupters for the UAS driver, but probably > xHCI > should have enough information to figure it out. I would still like to > see the patchset revised to direct different endpoint types to > different > event rings, if it turns out that different endpoints do benefit from > different latencies. > > > > If the drivers don't use this new API, the xHCI driver can try to > make > > > an informed choice (once per endpoint during the configure endpoint > > > command) to try and decide which interrupter would be best. At the > very > > > least, we should probably have separate event rings for different > device > > > speeds, and maybe two dedicated event rings for HS and SS > isochronous > > > traffic? > > > > This is all guesswork. You shouldn't spent much time on it before > > making some detailed measurements. > > We do need a way to experiment with the IMODI values in order to find > the right solution. Maybe we just need a smaller patch to set the > IMODI > value for the main event ring, and experiment with different devices to > see if different values help or hurt performance. I think I have one > laying around somewhere that I pointed Andiry to. It adds a module > parameter to xHCI to allow the IMODI value to be set. > > > > We probably want a separate event ring just for control transfers, > with > > > the IMODI field set to zero. There's no sense in making the > hardware > > > delay control transfers. Of course, control transfers are rarely > > > (perhaps never?) performance critical. > > > > > > Bulk endpoints are a separate issue. I think we always want to set > the > > > IMODI value to zero for those, because the drivers really want to > get > > > notified immediately when the transfers are done. Especially the > BoT > > > driver, that waits on each bulk transfer before submitting the rest. > So > > > we can probably direct control and bulk endpoints to the same event > ring > > > with IMODI set to zero. > > > > It's true that latency rarely matters for control endpoints. For all > > others, however, latency should be as small as possible. > > If we find that all types of devices just need to have the smallest > latency possible, I don't think the multiple event rings patchset is > going to be very useful. Maybe they would be helpful if some device > driver is spending too much time in their completion handler? Andiry, > can you think of any other reason to have multiple event rings? > I did this simply because xHCI driver uses MSI-X as default since 2.6.36, but only supports one interrupt vector, and other vectors allocated never have interrupts. To fully utilize the MSI-X vectors, interrupters are needed. What about the mechanism below? Interrupter 0: IMODI = 160 Assign all devices' ctrl transfer to it Interrupter 1-n: IMODI = 0 (interrupt ASAP) Assign devices' bulk/intr/isoc transfers. As Alan pointed out, all other transfers should have latency as small as possible. I've done some tests with USB3.0 storage device. IMODI=0 does help improve the performance, though not significantly. Thanks, Andiry -- 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
