[PATCH 2/4] X event queue mutex

Simon Thum simon.thum at gmx.de
Thu Oct 2 17:01:46 PDT 2008


Keith Packard wrote:
> On Thu, 2008-10-02 at 18:58 -0300, Tiago Vignatti wrote:
>> Keith Packard escreveu:
>>> The input queue is written so that each user modifies only one of the
>>> two pointers (head and tail). There shouldn't be any need to have a
>>> mutex which protects both of these values together, and doing so
>>> prevents mouse motion while the server is processing events.
>> Yeah, but the input thread can change the tail pointer while the main
>> thread is reading that, doing dirty things.
> 
> That won't be a problem on any SMP machine where writes from one CPU are
> visible to the other CPU. The key is to have each thread writing only
> one of the two values, and we have that already.
It may be constructed, but IMO this means the queue size is not fully 
utilizable given head is 'old':
else {
	newtail = (oldtail + 1) % QUEUE_SIZE;
	
	if (newtail == miEventQueue.head) {
             ErrorF("[mi] EQ overflowing. The server is probably stuck "
                    "in an infinite loop.\n");
	    return;
         }
	miEventQueue.tail = newtail;
     }
It might make sense to increase QUEUE_SIZE when threaded (I'm unaware of 
any reasoning behind the current size).

BTW, given SMP visibility, isn't it a tiny bit risky to have 3 
'independent' miEventQueue.head reads in miEnqueue?


> So, we either need to fix the users of checkForInput along with all
> users of miEventQueue, or we need to assert that we're running on a
> sensible CPU that doesn't require a mutex to make memory changes visible
> across processors, in which case we don't need any mutex at all.
At XDS, we identified 2 scenarios:
  a) Full block of input processing (halt IT at defined state)
  b) block enqueuing on IT-side (so e.g. XTest can guarantee order)

(b) may suffice. Locking the queue in OsBlockSigs() should do it and fix 
most miEnqueue users.

Given that more input-related stuff is going to be done in input 
properties (which are processed on the main thread), (a) may be the only 
viable option in some circumstances (If I'm the only, let's forget it). 
Then we'd also need to assert visibility of writes when the IT is to 
continue. I don't know how to do that pthready however.

Processing input properties on the input thread may avoid that (a) crap 
altogether. (cc'ed peter for that). If I didn't miss something, this and 
(b) should get the job done.





More information about the xorg mailing list