why spin_lock_bh() in match routine?

why spin_lock_bh() in match routine?

[Damon Gray]

If I'm not mistaken, and I possibly am, the match section of a netfilter 
module is run in softirq context, and the same softirq (not tasklet) can 
run on a *different* CPU but not the same CPU twice. So why is it being 
suggested here that we need to disable bottom halves for match targets.
Seems to me you will be safe with spin_lock() in match() and possibly need 
spin_lock_bh() in at least destroy() and maybe in checkentry(). Am I 
missing some?

-Damon-

Re: why spin_lock_bh() in match routine?

[Tobias DiPasquale]

On 6/30/05, Damon Gray <dgray@internap.com> wrote: 
> If I'm not mistaken, and I possibly am, the match section of a netfilter
> module is run in softirq context, and the same softirq (not tasklet) can
> run on a *different* CPU but not the same CPU twice. So why is it being
> suggested here that we need to disable bottom halves for match targets.
> Seems to me you will be safe with spin_lock() in match() and possibly need
> spin_lock_bh() in at least destroy() and maybe in checkentry(). Am I
> missing some?

You wouldn't need to disable BH's in the match() function itself, but
rather in other contexts where the protected data may be accessed.
This serves to prevent the softirq in the process of running the
match() function, from pre-empting another context and deadlocking by
attempting to acquire a lock that is already held.

An example of this kind of mistake would be to use
spin_lock()/unlock() in the match() function to access/modify a
variable x and to ALSO use spin_lock()/unlock() (i.e. NOT disabling
BH's) in a proc file read function to access the same variable x. A
softirq could preempt the proc read function after the lock has been
acquired and cause a deadlock b/c the match() function would be unable
to acquire the lock held by the proc read function and the proc read
function is unable to return to the CPU because it was preempted by a
function that will never return.

-- 
[ Tobias DiPasquale ]
0x636f6465736c696e67657240676d61696c2e636f6d