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

Reply via email to