Re: NAPI note

[Jeff Garzik <jgarzik@pobox.com>]

jamal wrote:
> 
> On Tue, 18 Feb 2003, Manfred Spraul wrote:
> 
> 
>>David S. Miller wrote:
>>
>>
>>>  From: Jeff Garzik <jgarzik@pobox.com>
>>>  Date: Fri, 14 Feb 2003 18:58:13 -0500
>>>
>>>  Manfred Spraul wrote:
>>>  > It seems to be a generic NAPI restriction:
>>>  > The caller of netif_receive_skb() must not own a spinlock that is
>>>  > acquired from an interrupt handler.
>>>
>>>  Thanks much for noticing this, Manfred.
>>>
>>>I think this logic is buggy.
>>>
>>>In the example I've seen posted, only a NAPI implementation bug
>>>could cause the situation to occur.
>>>
>>>If cpu1 is in ->poll() for the driver, then by definition the
>>>device shall not cause interrupts.  The device's interrupts
>>>are disabled before we enter the ->poll() handler, and as such
>>>the "cpu2 take device interrupt and takes driver->lock" cannot
>>>occur.
>>>
>>>
>>
>>No. I think the rule is that drivers that use the NAPI interface must
>>not cause interrupts for packet receive and out-of-rx-buffers conditions.
> 
> 
> Ah, but that is only one of two rules.
> Theres other drivers which dont follow this rule and just shutdown
> all interupt sources. I know that the e1000 for example does this.
> I am not sure about the tg3. I think the doc says this but may not
> emphasize it as strongly.
> So if tg3 uses method 2 then its as Dave says - a bug.

tg3 shuts down all interrupt sources, and handles all interrupt events 
in dev->poll().

David and I hashed it out a bit on IRC.  The problem is that 
deliver_to_old_ones() waits, and thus the deadlock that Manfred 
described.  For 2.4.x, the solution is simply to avoid the deadlock in 
the driver.  For 2.5.x, David hinted that deliver_to_old_ones() may be 
going away.


>>But what about media error interrupts, or tx interrupts? Or MIB counter
>>overflow, etc. What about shared pci interrupts?
> 
> 
> Shared interupts should be interesting actually.
> However if you are in poll mode and you receive an interupt you should be
> able to quickly determine its not yours without much effect on shared
> locks, no?

Normally, yes.  However tg3 grabs a lock just about anytime it does 
anything.  ;-)  A long term project of mine is to slowly remove these 
locks, but that must wait until the driver stabilizes, and is overall a 
long process.  Most of the locks _are_ removeable, but we keep hit 
deadlock bugs like this, and hardware bugs which need workarounds, so 
those come first.


>>All of them could occur, and if they take a spinlock that is held across
>>netif_receive_skb(), then it can deadlock.
>>
> 
> 
> yes this could happen with method 1 of programming the driver; however,
> tx, receive, link are essentially separate threads and would hardly share
> locks.

They do in tg3's case.

The locks can be removed eventually, but such is the state of life right 
now.

	Jeff