On 02/28, Michel Lespinasse wrote: > > On Thu, Feb 28, 2013 at 3:25 AM, Oleg Nesterov <oleg@xxxxxxxxxx> wrote: > > On 02/27, Michel Lespinasse wrote: > >> > >> +void lg_rwlock_local_read_lock(struct lgrwlock *lgrw) > >> +{ > >> + preempt_disable(); > >> + > >> + if (__this_cpu_read(*lgrw->local_refcnt) || > >> + arch_spin_trylock(this_cpu_ptr(lgrw->lglock->lock))) { > >> + __this_cpu_inc(*lgrw->local_refcnt); > > > > Please look at __this_cpu_generic_to_op(). You need this_cpu_inc() > > to avoid the race with irs. The same for _read_unlock. > > Hmmm, I was thinking that this was safe because while interrupts might > modify local_refcnt to acquire a nested read lock, they are expected > to release that lock as well which would set local_refcnt back to its > original value ??? Yes, yes, this is correct. I meant that (in general, x86 is fine) __this_cpu_inc() itself is not irq-safe. It simply does "pcp += 1". this_cpu_inc() is fine, _this_cpu_generic_to_op() does cli/sti around. I know this only because I did the same mistake recently, and Srivatsa explained the problem to me ;) Oleg. -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html