On Tue, 24 Aug 2010, David Brownell wrote: > --- On Mon, 8/23/10, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > > > From: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> > > > > > > > Wouldn't it make more sense to have > > scan_periodic() > > > > terminate its inner > > > > loop whenever it encounters a Q_TYPE_QH entry? > > And > > > > then do a separate > > > > pass over all the interrupt QHs, much like > > scan_async() > > > > does a pass > > > > over all the QHs in the async ring? > > > > > > Probably better to group by period not type. > > Although it > > > should be a NOP to scan an IRQ QH that's done > > nothing. > > > > An expensive NOP. qh_completions() is a complex > > function > > Isn't there logic to skip a QH that's done nothing? > Cheap logic for that not-uncommon case? Yes, but it's still a reasonably expensive operation. That function has a lot of overhead. > > > No > > > point in skewing the timings even more, though, by > > adding > > > any type of sort-by-type. > > > > I don't follow. Can you spell this out in greater > > detail? > > I don't have such detail. I was just concerned > with keeping the amount of work down, by using > the pre-sort (by period) and avoiding those > (as you say) expensive NOPs). Oh. I don't see any way to do that, offhand. > My thought > > was just to put all the interrupt QHs on a big list, kind > > of like the > > async ring, and have scan_periodic check them all. > > (Or maybe split > > the function up into scan_isochronous and scan_interrupt, > > since the two > > activities aren't very similar.) > > That's the sort of sort-by-type to which I just > reacted negatively. Note also that if the host > initiates ISO then INTR, it might expect them to > complete in that sequence (to the extent that it > can detect ordering). There's no need to reorder > via sort-by-type. No benefit either, AFAICT. The benefit would be that each interrupt QH is scanned only once. > > The drawback, of course, is that we would also end up > > scanning QHs that > > _couldn't_ have made any progress because they weren't > > reachable from > > any of the frames we're looking at. A different > > approach that would > > solve this problem is to keep the current scanning code, > > but store in > > the QH the last frame it was scanned for and skip over QHs > > that were > > just scanned. > > > ISTR that was the original scheme ... It was? I don't see any trace of it in the existing code. Regardless, I'll go ahead and implement this and fly it by you for approval. Alan Stern -- 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
