On Sun, 2006-09-07 at 01:46 +0200, Thomas Graf wrote:
> * Jamal Hadi Salim <[EMAIL PROTECTED]> 2006-07-08 10:14
[..]
> > There is no easy way to detect such a deadlock. I think it reduces to
> > the same case as eth0->eth0, i.e:
>
> It's very simple. Just use the same method and add a flag if a mirred
> is currently in use as Herbert did to qdisc_run().
>
But that would make it more complex in the sense it would require a lot
more code.
Another simpler approach is to check for recursion right in
dev_queue_xmit() - but i did not dare to do that because i could not
justify it for any other code other than loops created by the action
code. i.e something along the lines of:
if (q->enqueue) {
if (!spin_trylock(&dev->queue_lock)) {
printk("yikes recursed device %s\n",dev->name);
goto tx_recursion_detected;
}
.
.
.
recursion_detected:
rc = -ENETDOWN;
local_bh_enable();
out_kfree_skb:
kfree_skb(skb);
return rc;
out:
local_bh_enable();
return rc;
}
[BTW, notice how i used code to describe my view above ;->]
Again, I was hoping that Herbert's stuff may change this view (and
therefore give it more legitimacy) - but if the above can be accepted
then we can forget touching mirred.
The above change if acceptable will catch all sorts of scenarios:
A->A, A->B->A, A->B->C->B etc
> > I have a dated patch to mirred (may not apply cleanly) that i believe
> > will fix this specific one. Try to see if it also fixes this case you
> > have.
>
> This is complete nonsense, fixing a case out of a generic
> problem is useless.
>
No Thomas - this is a valid check. As valid as mirred checking if device
is up first. I dont like it, like i said, because it adds one more check
in the first path. I was contemplating at some point even not doing the
> > Its the second pathological case tx deadlock essentially and mirred
> > could help - it is not the mirred lock really if the attached patch
> > fixes it, but i could be wrong.
>
> __LINK_STATE_QDISC_RUNNING will prevent an eventual tx deadlock,
> it will not prevent the mirred deadlock.
>
By "mirred deadlock" i think you mean the deadlock that happens in
dev_queue_xmit()?
> I think everything has been said about this, since I'm still not
> getting the intend of many parts I'm not able to fix these, sorry.
>
What is still not clear above?
cheers,
jamal
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
