Re: Network-related Oopses on 2.2.13pre14

Andi Kleen (ak@muc.de)
Mon, 4 Oct 1999 02:23:09 +0200

On Mon, Oct 04, 1999 at 02:00:52AM +0200, Andrea Arcangeli wrote:
> On Mon, 4 Oct 1999, Andi Kleen wrote:
>
> >I don't like your fix though. skb_queue_lock is far too fundamental to
> >be taken that long, especially over a kfree_skb(). e.g. if the skb destructor
> >does any skb list operations it'll deadlock. so it would be better to enforce
> >the ordering in skb_queue_tail.
>
> The path where we take the spinlock that long is definetly not a
> fast-path. During production the performance would be the same as now. And
> more important my patch is more efficient than an ordered-write solution
> as I avoid lots of additional SMP lock on the bus on i386 to do the
> ordered writes for each skb_queue_tail.

I am not worried about the performance, just about the robustness.
skb_queue_lock is really an internal lock to the skb module. That dev.c plays
with it is already not nice, but holding it for longer times while calling
other subsystems (kfree_skb -> skb->destruct etc.) is just nasty and violates
the abstraction barriers. And it is dangerous because it makes every skb_*
list operation a death trap.

Maybe the fix is simple, but the implications on the rest of the code are not :)

> I checked all the usage of the destructor and none seems to recurse on the
> spinlock.

You never know, there are drivers doing clever things with it.

-Andi

-- 
This is like TV. I don't like TV.


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/