Hi. On Tue, Apr 21, 2009 at 02:52:30PM +0800, Lai Jiangshan (laijs@xxxxxxxxxxxxxx) wrote: > > +void xt_info_rdlock_bh(void) > > > +{ > > > + struct xt_info_lock *lock; > > > + > > > + preempt_disable(); > > > + lock = &__get_cpu_var(xt_info_locks); > > > + if (likely(++lock->depth == 0)) > > So what happen when xt_info_rdlock_bh() called recursively here? > > > > + spin_lock_bh(&lock->lock); > > > + preempt_enable_no_resched(); > > > +} > > > +EXPORT_SYMBOL_GPL(xt_info_rdlock_bh); > > > + > > ---------- > Is this OK? (Now I suppose we can enter the read-side critical region > in irq context) > > void xt_info_rdlock_bh(void) > { > unsigned long flags; > struct xt_info_lock *lock; > > local_irq_save(flags); > lock = &__get_cpu_var(xt_info_locks); > if (likely(++lock->depth == 0)) > spin_lock_bh(&lock->lock); > local_irq_restore(flags); > } Netfilter as long as other generic network pathes are never accessed from interrupt context, but your analysis looks right for the softirq case. Stephen, should preempt_disable() be replaced with local_bh_disable() to prevent softirq to race on the same cpu for the lock's depth field? Or can it be made atomic? -- Evgeniy Polyakov -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html