On Fre, 2012-06-01 at 12:44 +0200, Christian König wrote: > 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. Yeah, I also realized in the meantime that the race can't happen. I blame it on lack of caffeine. :) > 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. Glad to hear that! -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Debian, X and DRI developer _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
