[Click] NotifierQueue on multi-threaded Click problem.

[Joonwoo Park]

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)
>