On Thu, Jan 12, 2012 at 10:35:28AM -0500, Alan Stern wrote: > On Thu, 12 Jan 2012, Andiry Xu wrote: > > > Sarah, > > > > Thanks for the information! I'll check it. > > Somewhat out of the blue... > > I haven't paid too much attention to the ring-expansion patches, but > it's obvious that they were difficult and complex. What was the main > problem? There were a couple problems. The first is that the xHCI driver doesn't keep track of how many free TRBs are on a ring, so the enqueueing code had to walk across the ring, seeing if each TRB needed was free before it enqueued any transfers. To implement your suggestion of not putting TRBs in the segment where the dequeue pointer is, we'd still need to know how many free TRBs are leftover in the enqueue segment. So either way, that code has to change. That code change is what's currently buggy. The other difficulty was inserting a new segment in the middle of the ring, while making sure that the hardware-owned cycle bit was set correctly. Otherwise the hardware would think that it owned TRBs in the new segment before we enqueued any to that segment. > Was it that you could sometimes run into situations where the CPU had > advanced almost all the way around the ring, so that the enqueue > pointer was in the same segment as the dequeue pointer but somewhere > behind it? That certainly would make it difficult to link an extra > segment into the unused region. Yes, that was one of the problems. We can actually insert a link TRB anywhere in the segment (it doesn't have to be at the end of the segment), but the xHCI driver was architected with the simplification that link TRBs would only ever be at the end of the segment. So we thought we would wait until the dequeue pointer moved past the link TRB at the end of the enqueue segment to expand the ring. The trouble with that is the xHCI driver is sometimes a bit uncertain as to where the actual dequeue pointer is. We don't get an interrupt when a link TRB at the end of a segment is processed by the host controller and it starts fetching TRBs in the next segment. We can't get an interrupt on link TRBs because it would mess up the cancellation code. Therefore, when we get an event for a TD that ended just before the link TRB, we don't know if the hardware has processed the link TRB, and therefore we don't know if we can overwrite it to expand the ring. > If that was the main problem, there's an easy way around it. Simply > never advance the enqueue pointer into the segment currently occupied > by the dequeue pointer. Of course, in practice this means every ring > will end up using a minimum of two segments; the tradeoff ought to be > worthwhile. > > But maybe you already know all this... It's true, we could make sure there's always one link TRB free to expand the ring. That solution was just too simple for me to think of, I guess. :-P Andiry, what do you think? 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
