[Bug 43125] Device reports coming in too fast

wine-bugs at winehq.org wine-bugs at winehq.org
Sat Jul 21 05:06:02 CDT 2018


https://bugs.winehq.org/show_bug.cgi?id=43125

--- Comment #7 from Kai Krakow <kai at kaishome.de> ---
(In reply to Konrad Rzepecki from comment #6)
> This code is in critical section and only setting ext->last_report_read to
> FALSE is after check. So other code should not affect this at all. Only
> possibility I see is that this "while" is never entered so TRUE is not set.
> I think this line with TRUE set should be moved outside while for two
> reasons.
> 1. If list is empty TRUE is never set however there is no report to process
> and it should (this bug).
> 2. When there are more than one report after process first it is set to TRUE
> however it should be after last.
> 
> But since all this appear in critical section i see no reason to have this
> handly made "last_report_read" semaphore in code at all. However I'm not
> familiar with wine code so my analyses may be wrong, but I hope it help
> someone fix this bug. This messages in console application are really
> annoying...

I'm not sure if you unterstood the code right - I did at first, too. There's no
such thing as "there are more than one report". The while loop just feeds the
readers, it's not delivering events down a queue, but up a subscription.
Readers subscribe to events (that's the irp queue).

But maybe you are true that it should set TRUE outside the while loop - but
only if anything was done. But since it's in a critical section, it doesn't
really matter, I think. The outcome would be the same.

The driver itself doesn't implement a queue: It just tries to instantly deliver
the report by calling process_hid_report(). If there's no reader queued yet,
next time the driver sends a report, the current one will be lost. That's when
you see the message.

OTOH, you probably don't want to implement a queue in the driver because
otherwise your HID reports may drift in time a lot.

So the only proper way would be to accelerate the readers, that is
deliver_last_report() which may have pretty high overhead as I was told.

I'm not sure what a HID report consists of: Is it single axes events and single
button events? Or is it a structure of whole device state (axes position and
button states)? In the latter case, an undelivered report could be coalesced
into the one still sitting there, and maybe just degrade the error to a warning
or remove it completely then.

I didn't have time to look into this completely but have some ideas.

Another idea of discussion: If the last report was not read yet: Should we
overwrite the queued report or should be throw away the incoming report?

-- 
Do not reply to this email, post in Bugzilla using the
above URL to reply.
You are receiving this mail because:
You are watching all bug changes.



More information about the wine-bugs mailing list