> > > 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