Paolo Bonzini writes ("Re: [PATCH v2] add event queueing to USB HID"): > It's all pretty academic as in practice it worked well. The queue-full > code would never trigger in usb_pointer_event, and instead the queue > would be instantly emptied when a 17th event arrived. This is lucky > actually, because the device would also stop unqueuing from a full queue > if it had SET_IDLE 0. Both bugs happened because head==tail could mean > both "empty queue" and "full queue". I'm not even sure it's possible to > fill the queue in practice; even with a very high latency you'd have to > do 8 clicks in say half a second. I didn't try with a shorter queue, > which of course would have made the problems evident.
Ah. head==tail was supposed to mean empty (as is conventional); I guess my "full" test just had an off-by-one error. > I don't disagree, but I think this is better left for a separate patch. > I see three reasons for this: > > 1) I would like to understand better the relation between GET_REPORT and > TOKEN_IN. If an event occurs between two TOKEN_IN messages, and the OS > sends a GET_REPORT in between, should the second TOKEN_IN return > USB_RET_NAK or not? With your patch it will, with current QEMU and my > patch it won't. In this sense, the "changed" flag is recording slightly > different information at least in my version of the code. Hrm. > 2) if buffering is implemented for the keyboard device (and it probably > should) most of the differences would go away again. True. > 3) Secondarily, this is the only part of your patch that would need > adjustments, since finite idle delays are now implemented. Right. OK. Thanks for your work. Acked-by: Ian Jackson <ian.jack...@eu.citrix.com> Ian.