David S. Miller wrote: > From: Geoff Levand <[EMAIL PROTECTED]> > Date: Thu, 25 Aug 2005 22:13:57 -0700 > > >>@@ -970,7 +970,8 @@ >> /* gem_interrupt is safe to reentrance so no need >> * to disable_irq here. >> */ >>- gem_interrupt(dev->irq, dev, NULL); >>+ if(gem_interrupt(dev->irq, dev, NULL) == IRQ_NONE) >>+ __netif_rx_schedule(dev); >> } >> #endif > > > It might be cleaner to just open-code the parts of gem_interrupt() > we want, making the necessary modifications to make sure > __netif_rx_schedule() occurs and a snapshot of gp->gem_status > is always sampled. >
I took a look at your suggestion, but it turned ugly, as there are many code paths to gem_poll(), and the netif routines don't export enough information to determine the state from inside gem_poll(). My solution is to keep a state variable in the device structure. This should give a 100% proper solution (my original patch didn't cover all cases). -Geoff This fixes a major bug in the Sun GEM Ether driver's netpoll implementation. When using kgdb over Ether the 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. 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-26 10:27:07.000000000 -0700 +++ linux-2.6.12/drivers/net/sungem.c 2005-08-26 17:38:54.000000000 -0700 @@ -885,6 +885,15 @@ */ spin_lock_irqsave(&gp->lock, flags); + /* + * This takes care of the case when gem_interrupt() + * failed to call __netif_rx_schedule(). + */ + if(!gp->rx_scheduled) { + netif_rx_schedule_prep(dev); + __netif_rx_schedule(dev); + } + do { int work_to_do, work_done; @@ -922,6 +931,7 @@ } while (gp->status & GREG_STAT_NAPI); __netif_rx_complete(dev); + gp->rx_scheduled = 0; gem_enable_ints(gp); spin_unlock_irqrestore(&gp->lock, flags); @@ -945,14 +955,15 @@ if (netif_rx_schedule_prep(dev)) { u32 gem_status = readl(gp->regs + GREG_STAT); + gp->status = gem_status; - if (gem_status == 0) { + if ((gem_status & ~GREG_STAT_TXNR) == 0) { spin_unlock_irqrestore(&gp->lock, flags); return IRQ_NONE; } - gp->status = gem_status; gem_disable_ints(gp); __netif_rx_schedule(dev); + gp->rx_scheduled = 1; } spin_unlock_irqrestore(&gp->lock, flags); @@ -3047,6 +3058,7 @@ gp->lstate = link_down; gp->timer_ticks = 0; + gp->rx_scheduled = 0; netif_carrier_off(dev); gp->regs = ioremap(gemreg_base, gemreg_len); Index: linux-2.6.12/drivers/net/sungem.h =================================================================== --- linux-2.6.12.orig/drivers/net/sungem.h 2005-08-26 10:27:07.000000000 -0700 +++ linux-2.6.12/drivers/net/sungem.h 2005-08-26 16:54:34.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; @@ -1028,6 +1028,7 @@ #ifdef CONFIG_PPC_PMAC struct device_node *of_node; #endif + int rx_scheduled; }; #define found_mii_phy(gp) ((gp->phy_type == phy_mii_mdio0 || gp->phy_type == phy_mii_mdio1) \ - 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