On Tue, 9 Dec 2003 15:15:02 +0100
Rask Ingemann Lambertsen <rask@xxxxxxxxxx> wrote:
> On Fri, Dec 05, 2003 at 04:59:00PM -0800, David S. Miller wrote:
> > This is the classic case of doing disabling/enabling of software
> > interrupts with hardware interrupts disabled, which is a bug.
> > In this case pcnet32_set_multicast_list() is disabling hardware
> > interrupts, and the packet freeing of pcnet32_purge_tx_ring()
> > is what leads to the software interrupt disable/enable.
> I think the root cause of this problem is that pcnet32_set_multicast_list()
> dumps the entire TX ring on the floor (as a side effect of calling
> pcnet32_restart()). I don't think dev->set_multicast_list() is supposed to
> do that.
The task of dev->set_multicast_list() is to do whatever is necessary
to update the multicast filter.
If the pcnet32 hardware for some odd reason requires that you flush
the TX list, it is appropriate.
> I've been wondering about this too with the recent netpoll patches. Many
> (including pcnet32) implement the poll controller simply as
> disable_irq (dev->irq);
> driver_interrupt_handler (dev->irq, dev, NULL);
> enable_irq (dev->irq);
> If the interrupt handler calls dev_kfree_skb_any(), could you then run into
> this kind of problem? Or is it just if you call spin_lock_irq*() that you
> have a problem?
No, it would not be a problem because the thing that dev_kfree_skb_any()
_does_ test right now ('in_irq()') would trigger.