Eric Lemoine wrote:
> On 8/27/05, Eric Lemoine <[EMAIL PROTECTED]> wrote:
>
>>On 8/27/05, Eric Lemoine <[EMAIL PROTECTED]> wrote:
>>
>>>On 8/26/05, Geoff Levand <[EMAIL PROTECTED]> wrote:
>>>
>>>>This fixes a major bug in the Sun GEM Ether
>>>>driver's netpoll implementation. When both polled
>>>>and interrupt driven i/o are used simultaneously,
>>>>for example when using kgdb over Ether with active
>>>>NFS mounts, a condition easily arises where the bug
>>>>is hit.
>>>>
>>>>The problem is that gem_poll() expects an rx softnet_data
>>>>structure to have been allocated (via __netif_rx_schedule())
>>>>prior to its being called. The existing gem driver code
>>>>uses gem_interrupt() to set things up for gem_poll(),
>>>>but unfortunately, gem_interrupt() doesn't know about
>>>>this, and so doesn't allocate the required structure
>>>>when it finds no bits set in the interrupt status
>>>>register. gem_poll() then move on its way and tries to
>>>>delete the non-existent softnet_data structure.
>>>
>>>I may be missing something here but I don't see how gem_poll() can
>>>move on its way if __netif_rx_schedule() hasen't been called.
>
> Wait...
>
>>>in netpoll.c:poll_napi() because bit __LINK_STATE_RX_SCHED is set,
>>>correct?
>>>
>>>To me the bug is that __LINK_STATE_RX_SCHED can be set while
>>>__netif_rx_schedule() hasen't be called. Why don't fix it in the
>>>simplest way ? See attached patch (absolutely untested).
>>
>>It doesn't even compile :-)
>>
>>[new patch attached]
>
>
> I just realized that my previous patch isn't correct: gem_interrupt()
> cannot read from
> GREG_STAT without making sure gem_poll() isn't currently running -
> gem_poll() must have exclusive access to GREG_STAT. One solution would
> be to clear __LINK_STATE_RX_SCHED (i.e. reenable polling) before
> returning IRQ_NONE. See attached patch.
>
Sure, your fix works, and seems to be the best way to do it.
-Geoff
Signed-off-by: Geoff Levand <[EMAIL PROTECTED]>
Index: linux-2.6.12/drivers/net/sungem.c
===================================================================
--- linux-2.6.12.orig/drivers/net/sungem.c 2005-08-29 14:28:17.000000000
-0700
+++ linux-2.6.12/drivers/net/sungem.c 2005-08-29 14:50:51.000000000 -0700
@@ -947,6 +947,7 @@
u32 gem_status = readl(gp->regs + GREG_STAT);
if (gem_status == 0) {
+ netif_poll_enable(dev);
spin_unlock_irqrestore(&gp->lock, flags);
return IRQ_NONE;
}
Index: linux-2.6.12/drivers/net/sungem.h
===================================================================
--- linux-2.6.12.orig/drivers/net/sungem.h 2005-08-29 14:28:17.000000000
-0700
+++ linux-2.6.12/drivers/net/sungem.h 2005-08-29 15:00:10.000000000 -0700
@@ -1020,7 +1020,7 @@
struct gem_init_block *init_block;
struct sk_buff *rx_skbs[RX_RING_SIZE];
- struct sk_buff *tx_skbs[RX_RING_SIZE];
+ struct sk_buff *tx_skbs[TX_RING_SIZE];
dma_addr_t gblock_dvma;
struct pci_dev *pdev;
-
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
