[Click] NotifierQueue on multi-threaded Click problem.

[Eddie Kohler]

spin_lock_bh() actually increments the preemption count, causing in_softirq() 
to return true.

I forget what the problem is that we are trying to solve here.

Eddie


Joonwoo Park wrote:
> Ye, it was my fault.
> But I'm being stupid, I cannot understand why in_softirq() returns
> true with this fix.
> -
> diff --git a/elements/linuxmodule/todevice.cc b/elements/linuxmodule/todevice.cc
> index 8a694ce..4bab844 100644
> --- a/elements/linuxmodule/todevice.cc
> +++ b/elements/linuxmodule/todevice.cc
> @@ -286,6 +286,7 @@ ToDevice::run_task(Task *)
> 
>         _pulls++;
> 
> +     WARN_ON(in_softirq());          // in_softirq() returns true
>         Packet *p = input(0).pull();
>         if (!p)
>             break;
> -
> 
> And after below, It stops warning.
> -
> diff --git a/elements/linuxmodule/todevice.cc b/elements/linuxmodule/todevice.cc
> index 8a694ce..a0ce2ba 100644
> --- a/elements/linuxmodule/todevice.cc
> +++ b/elements/linuxmodule/todevice.cc
> @@ -286,7 +286,10 @@ ToDevice::run_task(Task *)
> 
>         _pulls++;
> 
> +     netif_tx_unlock_bh(_dev);
> +     WARN_ON(in_softirq());        // in_softirq() returns false
>         Packet *p = input(0).pull();
> +     netif_tx_lock_bh(_dev);
>         if (!p)
>             break;
> 
> -
> 
> Jason Park (Joonwoo Park)
> 
> 
> 2007/9/5, Eddie Kohler <kohler at cs.ucla.edu>:
>> ToDevice::run_task() is not run in softirq context, it is called from Click's
>> kernel thread like every other task.  ToDevice DOES call spin_trylock_bh
>> itself, however.
>>
>> Eddie
>>
>>
>> Joonwoo Park wrote:
>>> In addition about softirq.
>>>
>>>>> - "The spinlock_bh needed because of push and pull can be called
>>>>> concurrently from softirq." : What do you mean?  I may be being stupid,
>>>>> but I don't think ANY pushes and pulls should be called from softirq
>>>>> context.  What softirq context are you thinking of?  Click runs almost
>>>>> entirely in user context (i.e. the kernel threads), or at least it should.
>>> The ToDevice::run_task() is runs in sofrtirq context and it pulls packet.
>>> You may get warning WARN_ON(in_softirq()) at that function.
>>> Isn't it OK?
>>>
>>> Jason Park (Joonwoo Park)