[PATCH 2/4] X event queue mutex

Jeremy Huddleston jeremyhu at freedesktop.org
Thu Oct 23 16:19:57 PDT 2008


On Oct 23, 2008, at 16:05, Tiago Vignatti wrote:

> Hey Jeremy, I'm going well. Thanks my friend.
>
> Jeremy Huddleston escreveu:
>> I hope things are going well for you.  I've recently hit an issue  
>> using locks in miEq.  We're doing it the same way in mieq.c as your  
>> proposal (patch 2/4) and this causes us to hit a deadlock when the  
>> enqueueing thread is waiting for the lock to push an event and the  
>> dequeuing thread (the server thread) is processing an event that  
>> requires a message to be sent to the enqueueing thread.
>
> I posted another version of that patch that doesn't requires a lock  
> in mieqProcessInputEvents().

Hmm... I must've missed that version... I thought I was looking at the  
Oct 1 one, but maybe I misclicked... sorry

> We just need to guarantee order of events enqueued (to protect  
> writes to the tail pointer in mieqEnqueue) and don't need to  
> serialize dequeuing events. So no locks in the server thread is  
> needed.
> (well, this is the theory of the X server with Xorg DDX and the  
> input thread craziness. Maybe your problem is a bit different from  
> mine)

I think by not locking, we open ourselves up to clobber because we  
reference the event on the queue after we pop the event off the event  
queue:

server thread:
e = miEventQueue.events[miEventQueue.head];
miEventQueue.head = (miEventQueue.head + 1) % QUEUE_SIZE;

-- context switch to input thread with a 1-open-slot event queue  
(miEventQueue.head = miEventQueue.head - 1) --
miEventQueue.events[miEventQueue.tail] gets set, but this is what 'e'  
is.

-- context switch to server thread --
e was just clobbered by the input thread

-----

I think you're right, actually, that we don't need to actually lock  
here.  We just need to copy the data locally then pop the queue.  If  
we keep the data in the queue and point to it, then we need to either  
lock (bad IMO), pop on return (possibly a good solution), or cache it  
locally (what I did in the patch I sent a few hours ago).



-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 3040 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg/attachments/20081023/fcd95a48/attachment.bin>


More information about the xorg mailing list