[PATCH] mieq: Provide better adaptability and diagnostics during mieq overflow
Jeremy Huddleston
jeremyhu at apple.com
Sun Oct 16 13:47:42 PDT 2011
On Oct 16, 2011, at 8:44 AM, Keith Packard wrote:
> On Sat, 15 Oct 2011 23:09:30 -0700, Jeremy Huddleston <jeremyhu at apple.com> wrote:
>
>> + if (new_queue == NULL)
>> + FatalError("Unable to allocate memory for the event queue.\n");
>
> This function should be boolean and return whether the queue was
> resized, instead of failing when out of memory.
Ok.
>> + miEventQueue.tail = (miEventQueue.tail - miEventQueue.head) % miEventQueue.q_size;
>
> Shouldn't this just be miEventQueue.q_size? If not, head and tail are
> both 'int' type, so the subtraction can be negative, in which case the %
> operator is undefined (thanks, dmr!).
No. miEventQueue.q_size is the number of buckets. (miEventQueue.tail - miEventQueue.head) is the number of filled buckets (modulo number of buckets).
% is undefined for negative numbers? WTF? That's the first time I've heard that. It's certainly well defined in mathematics.
>
>
>> + ErrorF("[mi] EQ overflowing. Increasing queue size to %lu to accomidate.\n", miEventQueue.q_size << 1);
>
> accommodate.
That was removed in v2 already.
>
>> + if (miEventQueue.dropped == 1) {
>> + ErrorF("[mi] EQ overflowing. Maximum queue size reached. Events will be discarded.\n");
>> + xorg_backtrace();
>> + } else if (miEventQueue.dropped % DROP_BACKTRACE_FREQUENCY == 0 &&
>> + miEventQueue.dropped / DROP_BACKTRACE_FREQUENCY <= DROP_BACKTRACE_MAX) {
>> + ErrorF("[mi] EQ overflow continuing. %lu events have been dropped.\n", miEventQueue.dropped);
>> + if (miEventQueue.dropped / DROP_BACKTRACE_FREQUENCY == DROP_BACKTRACE_MAX) {
>> + ErrorF("[mi] No further overflow reports will be reported until the clog is cleared.\n");
>> + }
>> + xorg_backtrace();
>
> I'm not sure we need to be even this verbose; just a single warning when
> the queue is blocked and another one when it starts working again should
> be sufficient. I fear flooding the disk with errors if this happens
> because your cat falls asleep on the keyboard while watching dancing
> mice on the screen.
That's why we shut up after DROP_BACKTRACE_MAX reports. What about lowering DROP_BACKTRACE_MAX to 5 rather than 20?
More information about the xorg-devel
mailing list