Messages in this thread | | | From | Arnd Bergmann <> | Subject | Re: [PATCH 1/2] lkdtm: use atomic_t to replace count_lock | Date | Wed, 1 Feb 2012 15:27:35 +0000 |
| |
On Wednesday 01 February 2012, Cong Wang wrote: > static void lkdtm_handler(void) > { > - unsigned long flags; > - > - spin_lock_irqsave(&count_lock, flags); > - count--; > printk(KERN_INFO "lkdtm: Crash point %s of type %s hit, trigger in %d rounds\n", > - cp_name_to_str(cpoint), cp_type_to_str(cptype), count); > + cp_name_to_str(cpoint), cp_type_to_str(cptype), atomic_dec_return(&count)); > > - if (count == 0) { > + if (!atomic_cmpxchg(&count, 0, cpoint_count)) > lkdtm_do_action(cptype); > - count = cpoint_count; > - } > - spin_unlock_irqrestore(&count_lock, flags); > }
This use is not atomic, you could have two threads doing atomic_dec_return at the same time, and after that the value will be -1 so the atomic_cmpxchg does not trigger.
In order to have an atomic here, you have to use a loop around atomic_cmpxchg, like
int old, new; old = atomic_read(&count); do { new = old ? old - 1 : cpoint_count; old = cmpxchg(&count, old, new); } while (old != new);
I suppose you could also just keep the spinlock and move lkdtm_do_action() outside of it?
Arnd
| |