On 01.06.2012 08:30, Michel Dänzer wrote:
On Fre, 2012-06-01 at 08:19 +0200, Michel Dänzer wrote:
I think this might introduce a race condition:

Thread 0 Thread 1
-------- --------
atomic_inc_return() returns 1
spin_lock_irqsave()
atomic_dec_and_test()
radeon_irq_set()

=>  the interrupt won't be enabled.
Hrmm, I messed up the formatting there, let me try one more time:

Thread 0                                Thread 1
--------                                --------
atomic_inc_return() returns 1
spin_lock_irqsave()
                                        atomic_dec_and_test()
radeon_irq_set()


Nope that isn't a problem, cause what you really get in your example is:

Thread 0                                Thread 1
--------                                --------
atomic_inc_return() returns 1
spin_lock_irqsave()
                                        atomic_dec_and_test()
radeon_irq_set()
spin_unlock_irqrestore()
                                        spin_lock_irqsave()
                                        radeon_irq_set()
                                        spin_unlock_irqrestore()


So testing the atomic counters just determines if we need an update of the irq registers or not, and since a significant change will always trigger an update we can make sure that the irq regs are always set to the last known state. We might call radeon_irq_set more often than necessary, but that won't hurt us and is really unlikely.

Also I have found the real reason why using the atomic for preventing ih recursion didn't worked as expected - it was just a stupid typo in my patch. But thanks for the comment anyway, it got me to look into the right direction for the bug.

Cheers,
Christian.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to