Re: Problem with dev_kfree_skb_any() in 2.6.0

From: David S. Miller
Date: Tue Dec 30 2003 - 01:08:51 EST


On Tue, 30 Dec 2003 00:15:19 -0500
Jeff Garzik <jgarzik@xxxxxxxxx> wrote:

> OK, agreed. But fixing it in the driver is still incorrect, also.
>
> We need a single solution in the net stack, not a per-driver solution.

I totally disagree.

Let's quickly review, this is illegal:

local_irq_disable();
{
local_bh_disable();
... do kfree_skb work ...
local_bh_enable();
}
local_irq_enable();

as is this:

local_irq_disable();
{
... queue to softirq TX work ...
}
local_irq_enable();
... oops this won't make softirq TX work get run ...

The driver must therefore recognize that it may only free packets
in it's IRQ handler or in situations where BH protection has occurred
at a higher level or BH protection is the only protection it uses
from base context.

This is similar to how the driver must be aware that
netif_receive_skb() can cause it's ->hard_start_xmit() method to run
and therefore it must prevent deadlocks that might occur as a result
of locks held during the netif_receive_skb() call.

So let's fix the drivers. :)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/