Patchwork [3/4] forcedeth: napi schedule lock fix

login
register
mail settings
Submitter Ayaz Abdulla
Date Jan. 9, 2009, 9:03 p.m.
Message ID <4967BBB5.9080803@nvidia.com>
Download mbox | patch
Permalink /patch/17691/
State Accepted
Delegated to: David Miller
Headers show

Comments

Ayaz Abdulla - Jan. 9, 2009, 9:03 p.m.
This patch fixes a potential race condition between scheduling napi and 
completing napi poll. The call to netif_rx_schedule should be under 
protection of the lock (as is the completion), otherwise, interrupts 
could be masked off.

Signed-off-by: Ayaz Abdulla <aabdulla@nvidia.com>
David Miller - Jan. 10, 2009, 7:13 a.m.
From: Ayaz Abdulla <aabdulla@nvidia.com>
Date: Fri, 09 Jan 2009 16:03:49 -0500

> This patch fixes a potential race condition between scheduling napi and completing napi poll. The call to netif_rx_schedule should be under protection of the lock (as is the completion), otherwise, interrupts could be masked off.
> 
> Signed-off-by: Ayaz Abdulla <aabdulla@nvidia.com>

Another patch I had to apply by hand.

The 'dev' argument was removed from the netif_*() NAPI interfaces
and then were converted to napi_*() in the net-2.6 tree.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vitaliy Gusev - Jan. 27, 2009, 4:28 p.m.
On 10 January 2009 00:03:49 Ayaz Abdulla wrote:
> This patch fixes a potential race condition between scheduling napi and 
> completing napi poll. The call to netif_rx_schedule should be under 
> protection of the lock (as is the completion), otherwise, interrupts 
> could be masked off.

Can you explain how this issue or race can occur (interrupts can be masked off) ?

netif_rx_schedule() merely mark soft irq as waked on current CPU. So dev->poll
will not be called until irq_exit() or local_bh_enable. As nv_nic_irq() is run
under interrupt context, so napi poll will be called only during irq_exit.


> 
> Signed-off-by: Ayaz Abdulla <aabdulla@nvidia.com>
> 
>

Patch

--- old/drivers/net/forcedeth.c	2009-01-09 15:11:55.000000000 -0800
+++ new/drivers/net/forcedeth.c	2009-01-09 15:17:27.000000000 -0800
@@ -3407,10 +3407,10 @@ 
 
 #ifdef CONFIG_FORCEDETH_NAPI
 		if (events & NVREG_IRQ_RX_ALL) {
+			spin_lock(&np->lock);
 			netif_rx_schedule(dev, &np->napi);
 
 			/* Disable furthur receive irq's */
-			spin_lock(&np->lock);
 			np->irqmask &= ~NVREG_IRQ_RX_ALL;
 
 			if (np->msi_flags & NV_MSI_X_ENABLED)
@@ -3524,10 +3524,10 @@ 
 
 #ifdef CONFIG_FORCEDETH_NAPI
 		if (events & NVREG_IRQ_RX_ALL) {
+			spin_lock(&np->lock);
 			netif_rx_schedule(dev, &np->napi);
 
 			/* Disable furthur receive irq's */
-			spin_lock(&np->lock);
 			np->irqmask &= ~NVREG_IRQ_RX_ALL;
 
 			if (np->msi_flags & NV_MSI_X_ENABLED)