>  
> > Matt, any ideas on this?
> 
> Not at the moment.

how about this for a solution?  It doesn't make netpoll any more robust, but I
think in the interests of efficiency it would be fair to require that, when
netpolled, a driver must receive frames on the same net device for which it was
polled.  With this patch we detect that condition and handle it accordingly in
e1000_intr.  This eliminates the need for us to call the clean_rx method from
the poll_controller when napi is configured, instead allowing the poll method to
be called from napi_poll, as the netpoll model currently does.  This fixes the
netdump regression, and eliminates the layering violation and the potential race
that we've been discussing.  I've just tested it with netdump here and it works
quite well.

Thoughts appreciated.

Thanks & Regards
Neil

Signed-off-by: Neil Horman <[EMAIL PROTECTED]>


 e1000_main.c |   54 +++++++++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 45 insertions(+), 9 deletions(-)


--- linux-2.6.9/drivers/net/e1000/e1000_main.c.neil     2006-06-06 
10:37:42.000000000 -0400
+++ linux-2.6.9/drivers/net/e1000/e1000_main.c  2006-06-07 10:48:22.000000000 
-0400
@@ -3207,8 +3207,9 @@
  * @pt_regs: CPU registers structure
  **/
 
+
 static irqreturn_t
-e1000_intr(int irq, void *data, struct pt_regs *regs)
+__e1000_intr(int irq, void *data, struct pt_regs *regs, int netpoll_op)
 {
        struct net_device *netdev = data;
        struct e1000_adapter *adapter = netdev_priv(netdev);
@@ -3217,6 +3218,7 @@
 #ifndef CONFIG_E1000_NAPI
        int i;
 #else
+       struct net_device *dev_to_sched;
        /* Interrupt Auto-Mask...upon reading ICR,
         * interrupts are masked.  No need for the
         * IMC write, but it does mean we should
@@ -3255,8 +3257,22 @@
                E1000_WRITE_REG(hw, IMC, ~0);
                E1000_WRITE_FLUSH(hw);
        }
-       if (likely(netif_rx_schedule_prep(&adapter->polling_netdev[0])))
-               __netif_rx_schedule(&adapter->polling_netdev[0]);
+
+       /*
+        * netpoll operations, in the interests of efficiency 
+        * only do napi polling on the device passed to the 
+        * poll_controller.  Therefore, if we are preforming 
+        * a netpoll operation, then we can't schedule a receive
+        * to one of the dummy net devices that exist for sole
+        * purpose of spreading out rx schedules
+        */     
+       if (netpoll_op)
+               dev_to_sched = netdev;
+       else
+               dev_to_sched = &adapter->polling_netdev[0];
+
+       if (likely(netif_rx_schedule_prep(dev_to_sched)))
+               __netif_rx_schedule(dev_to_sched);
        else
                e1000_irq_enable(adapter);
 #else
@@ -3288,6 +3304,13 @@
        return IRQ_HANDLED;
 }
 
+static irqreturn_t
+e1000_intr(int irq, void *data, struct pt_regs *regs)
+{
+       return __e1000_intr(irq, data, regs, 0);
+}
+
+
 #ifdef CONFIG_E1000_NAPI
 /**
  * e1000_clean - NAPI Rx polling callback
@@ -3300,7 +3323,6 @@
        struct e1000_adapter *adapter;
        int work_to_do = min(*budget, poll_dev->quota);
        int tx_cleaned = 0, i = 0, work_done = 0;
-
        /* Must NOT use netdev_priv macro here. */
        adapter = poll_dev->priv;
 
@@ -3308,10 +3330,24 @@
        if (!netif_carrier_ok(adapter->netdev))
                goto quit_polling;
 
-       while (poll_dev != &adapter->polling_netdev[i]) {
-               i++;
-               if (unlikely(i == adapter->num_rx_queues))
-                       BUG();
+       /*
+        * only search for a matching polling_netdev in the event
+        * that this isn't a real registered net_device
+        * A real net device can be passed in here in the event
+        * that netdump has been activated (this comes through 
+        * netpoll_poll_dev).  We detect this by virtue of the 
+        * fact that each polling_netdev->priv points to the private
+        * data of its parent (registered) netdev.  So if:
+        * poll_dev->priv == netdev_priv(poll_dev), its a real device
+        * otherwise its a polling_netdev.
+        */
+       if (adapter != netdev_priv(poll_dev)) {
+               while (poll_dev != &adapter->polling_netdev[i]) {
+                       i++;
+                       if (unlikely(i == adapter->num_rx_queues))
+                               BUG();
+               }
+
        }
 
        if (likely(adapter->num_tx_queues == 1)) {
@@ -4624,7 +4660,7 @@
 {
        struct e1000_adapter *adapter = netdev_priv(netdev);
        disable_irq(adapter->pdev->irq);
-       e1000_intr(adapter->pdev->irq, netdev, NULL);
+       __e1000_intr(adapter->pdev->irq, netdev, NULL, 1);
        e1000_clean_tx_irq(adapter, adapter->tx_ring);
 #ifndef CONFIG_E1000_NAPI
        adapter->clean_rx(adapter, adapter->rx_ring);
-- 
/***************************************************
 *Neil Horman
 *Software Engineer
 *gpg keyid: 1024D / 0x92A74FA1 - http://pgp.mit.edu
 ***************************************************/
-
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