Re: [PATCH 2/4] X event queue mutex

2008-11-07 Thread Tiago Vignatti
Hi Jeremy, Jeremy Huddleston escreveu: Well the problem was that mieqProcessInputEvents DOESN'T use those locks, so it can read the incremented value of miEventQueue.tail thinking that the item at head=tail-1 is ready to read when in fact mieqEnqueue is still in the process of writing to

Re: [PATCH 2/4] X event queue mutex

2008-11-07 Thread Jeremy Huddleston
On Nov 7, 2008, at 12:23, Tiago Vignatti wrote: For this reason, I went back to locking inside mieqProcessInputEvents with XQUARTZ. Your mutex will lags your cursor update on screen because the input thread will block before enqueuing while the main thread pops events. On this case try to

Re: [PATCH 2/4] X event queue mutex

2008-11-06 Thread Tiago Vignatti
Peter Hutterer escreveu: From: Peter Hutterer [EMAIL PROTECTED] Date: Tue, 4 Nov 2008 15:27:30 +1030 Subject: [PATCH] mi: clean up mieqProcessInputEvents, copy all events before processing. Copy the EventRec's information into local variables before processing them, this should make it

Re: [PATCH 2/4] X event queue mutex

2008-11-06 Thread Tiago Vignatti
Hey, Jeremy Huddleston escreveu: Looks good! A recent bug report has surfaced for us since I got rid of the locking in mieqProcessInputEvents. We need to update miEventQueue.tail only after the data has actually been pushed into the tail. This should take care of that problem on master,

Re: [PATCH 2/4] X event queue mutex

2008-11-04 Thread Jeremy Huddleston
On Nov 3, 2008, at 22:08, Peter Hutterer wrote: ... righty-o. hunk 2 is different, with the change you suggested. To make the flow cleaner, I moved all the EQ and memory alloc stuff together and the copies into local variables below it. Almost... you need to move the pop down 4 lines.

Re: [PATCH 2/4] X event queue mutex

2008-11-04 Thread Peter Hutterer
On Tue, Nov 04, 2008 at 09:34:23AM -0800, Jeremy Huddleston wrote: good +dev = e-pDev; +screen = e-pScreen; + miEventQueue.head = (miEventQueue.head + 1) % QUEUE_SIZE; +type= event-u.u.type; +master = (!dev-isMaster dev-u.master) ?

Re: [PATCH 2/4] X event queue mutex

2008-11-04 Thread Jeremy Huddleston
Looks good! A recent bug report has surfaced for us since I got rid of the locking in mieqProcessInputEvents. We need to update miEventQueue.tail only after the data has actually been pushed into the tail. This should take care of that problem on master, but I haven't tested it: diff

Re: [PATCH 2/4] X event queue mutex

2008-11-03 Thread Peter Hutterer
Sorry for the late reply, I was tied up and missed the mail. On Thu, Oct 23, 2008 at 02:47:30PM -0700, Jeremy Huddleston wrote: And here's a stab at setting up mieqProcessInputEvents in master to be more friendly towards this locking. master doesn't work for us on OSX, so I can't really

Re: [PATCH 2/4] X event queue mutex

2008-11-03 Thread Peter Hutterer
On Mon, Nov 03, 2008 at 09:41:48PM -0800, Jeremy Huddleston wrote: That looks much better (and is much more readable), but it's still open to data-thrashing when the queue is full. Move this: miEventQueue.head = (miEventQueue.head + 1) % QUEUE_SIZE; after this: +for (i = 0; i

Re: [PATCH 2/4] X event queue mutex

2008-11-03 Thread Jeremy Huddleston
That looks much better (and is much more readable), but it's still open to data-thrashing when the queue is full. Move this: miEventQueue.head = (miEventQueue.head + 1) % QUEUE_SIZE; after this: +for (i = 0; i nevents; i++) +memcpy(event[i], e-events[i].event, evlen);

Re: [PATCH 2/4] X event queue mutex

2008-10-23 Thread Jeremy Huddleston
Hey Tiago, 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

Re: [PATCH 2/4] X event queue mutex

2008-10-23 Thread Jeremy Huddleston
And here's a stab at setting up mieqProcessInputEvents in master to be more friendly towards this locking. master doesn't work for us on OSX, so I can't really verify that it works... I may have missed an e- to e. or e-events[i]-event to event[i] somewhere... This also fixes what I think

Re: [PATCH 2/4] X event queue mutex

2008-10-23 Thread Jeremy Huddleston
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

Re: [PATCH 2/4] X event queue mutex

2008-10-06 Thread Keith Packard
On Tue, 2008-10-07 at 00:46 -0300, Tiago Vignatti wrote: BTW, all mieqEnqueue() calls isn't needed to be wrapped by OsBlockSignals() and OsReleaseSignals()? This is not what is happening in our code. OsBlockSignals is only required when queuing events other than from the SIGIO handler as

Re: [PATCH 2/4] X event queue mutex

2008-10-06 Thread Tiago Vignatti
Simon Thum escreveu: Keith Packard wrote: Why does inserting events do anything but pull events from the kernel, insert them to the queue and update the sprite location on the screen? All event processing should happen in the main server thread; the only latency we're looking to reduce is the

Re: [PATCH 2/4] X event queue mutex

2008-10-03 Thread Simon Thum
On Fri, 2008-10-03 at 05:57 +0300, Daniel Stone wrote: Except if the lock is held across the entire event processing, because we need to queue events from event processing. I guess that's what I meant with guaranteeing order. That makes it more important that the mutex cover precisely the

Re: [PATCH 2/4] X event queue mutex

2008-10-02 Thread Simon Thum
Keith Packard wrote: On Wed, 2008-10-01 at 21:39 -0300, Tiago Vignatti wrote: A mutex is needed because the X event queue is a critical region. Though the X event queue is re-entrant, we cannot guarantee the simultaneous processing by both main and input threads. The input queue is

Re: [PATCH 2/4] X event queue mutex

2008-10-02 Thread Simon Thum
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,

Re: [PATCH 2/4] X event queue mutex

2008-10-02 Thread Keith Packard
On Fri, 2008-10-03 at 02:01 +0200, Simon Thum wrote: It may be constructed, but IMO this means the queue size is not fully utilizable given head is 'old': Yes, that's fairly common in queues -- you often can't use the last entry. Not a huge deal if you make the queue big enough. BTW, given

Re: [PATCH 2/4] X event queue mutex

2008-10-02 Thread Daniel Stone
On Thu, Oct 02, 2008 at 06:53:09PM -0700, Keith Packard wrote: On Fri, 2008-10-03 at 02:01 +0200, Simon Thum wrote: (b) may suffice. Locking the queue in OsBlockSigs() should do it and fix most miEnqueue users. Or just lock the queue in mieqEnqueue itself; keeping the lock near the code

Re: [PATCH 2/4] X event queue mutex

2008-10-02 Thread Keith Packard
On Fri, 2008-10-03 at 05:57 +0300, Daniel Stone wrote: On Thu, Oct 02, 2008 at 06:53:09PM -0700, Keith Packard wrote: On Fri, 2008-10-03 at 02:01 +0200, Simon Thum wrote: (b) may suffice. Locking the queue in OsBlockSigs() should do it and fix most miEnqueue users. Or just lock the

Re: [PATCH 2/4] X event queue mutex

2008-10-01 Thread Keith Packard
On Wed, 2008-10-01 at 21:39 -0300, Tiago Vignatti wrote: A mutex is needed because the X event queue is a critical region. Though the X event queue is re-entrant, we cannot guarantee the simultaneous processing by both main and input threads. The input queue is written so that each user