[PATCH 0/9] atl1: add napi to Atheros L1 NIC

[PATCH 0/9] atl1: add napi to Atheros L1 NIC

[Tony Zelenoff]

Added NAPI interface to the driver of Atheros L1 NIC.
RX speed reported by iperf in TCP mode (with and without -N
switch) improved by ~7 Mbps. In UDP mode the speed up is
improved by ~4 Mbps.
Also hard RX/TX rings processing with memory allocation in
interrupt handlers is not very good idea. So, fixed this.

Note: thouse patches must be applied after patch, that fix
kernel panic in this driver.
Its subject is: "atl1: fix kernel panic in case of DMA errors"

Tony Zelenoff (9):
  atl1: handle rx in separate condition
  atl1: make driver napi compatible
  atl1: add napi process of tx interrupts
  atl1: use defined functions to disable irq
  atl1: make function to set imr of card
  atl1: add value to check ability of reenabling IRQs
  atl1: enable errors and link ints when rx/tx scheduled
  atl1: do not process interrupts in cycle in handler
  atl1: do not drop rx/tx interrupts before they are scheduled

 drivers/net/ethernet/atheros/atlx/atl1.c |  171 +++++++++++++++++++-----------
 drivers/net/ethernet/atheros/atlx/atl1.h |   17 +++-
 drivers/net/ethernet/atheros/atlx/atlx.c |   15 ++-
 3 files changed, 135 insertions(+), 68 deletions(-)

[PATCH 1/9] atl1: handle rx in separate condition

[Tony Zelenoff]

Remove rx from unlikely optimization in case of rx is very
likely thing for network card. This also reduce code a bit.

Signed-off-by: Tony Zelenoff <antonz@parallels.com>
---
 drivers/net/ethernet/atheros/atlx/atl1.c |   19 ++++++++++---------
 1 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/atheros/atlx/atl1.c b/drivers/net/ethernet/atheros/atlx/atl1.c
index c926857..6438239 100644
--- a/drivers/net/ethernet/atheros/atlx/atl1.c
+++ b/drivers/net/ethernet/atheros/atlx/atl1.c
@@ -2502,18 +2502,19 @@ static irqreturn_t atl1_intr(int irq, void *data)
 		if (status & ISR_CMB_TX)
 			atl1_intr_tx(adapter);
 
+		/* rx event */
+		if (status & ISR_CMB_RX)
+			alt1_intr_rx(adapter);
+
 		/* rx exception */
 		if (unlikely(status & (ISR_RXF_OV | ISR_RFD_UNRUN |
 			ISR_RRD_OV | ISR_HOST_RFD_UNRUN |
-			ISR_HOST_RRD_OV | ISR_CMB_RX))) {
-			if (status & (ISR_RXF_OV | ISR_RFD_UNRUN |
-				ISR_RRD_OV | ISR_HOST_RFD_UNRUN |
-				ISR_HOST_RRD_OV))
-				if (netif_msg_intr(adapter))
-					dev_printk(KERN_DEBUG,
-						&adapter->pdev->dev,
-						"rx exception, ISR = 0x%x\n",
-						status);
+			ISR_HOST_RRD_OV))) {
+			if (netif_msg_intr(adapter))
+				dev_printk(KERN_DEBUG,
+					&adapter->pdev->dev,
+					"rx exception, ISR = 0x%x\n",
+					status);
 			atl1_intr_rx(adapter);
 		}
 
-- 
1.7.1

[PATCH 2/9] atl1: make driver napi compatible

[Tony Zelenoff]

This is first step, here there is no fine interrupt
disabling which cause TX/ERR interrupts stalling when
RX scheduled ints processed.

Signed-off-by: Tony Zelenoff <antonz@parallels.com>
---
 drivers/net/ethernet/atheros/atlx/atl1.c |   46 ++++++++++++++++++++++++++----
 drivers/net/ethernet/atheros/atlx/atl1.h |    1 +
 2 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/atheros/atlx/atl1.c b/drivers/net/ethernet/atheros/atlx/atl1.c
index 6438239..d39c1b9 100644
--- a/drivers/net/ethernet/atheros/atlx/atl1.c
+++ b/drivers/net/ethernet/atheros/atlx/atl1.c
@@ -1917,7 +1917,7 @@ next:
 	return num_alloc;
 }
 
-static void atl1_intr_rx(struct atl1_adapter *adapter)
+static int atl1_intr_rx(struct atl1_adapter *adapter, int budget)
 {
 	int i, count;
 	u16 length;
@@ -1933,7 +1933,7 @@ static void atl1_intr_rx(struct atl1_adapter *adapter)
 
 	rrd_next_to_clean = atomic_read(&rrd_ring->next_to_clean);
 
-	while (1) {
+	while (count <= budget) {
 		rrd = ATL1_RRD_DESC(rrd_ring, rrd_next_to_clean);
 		i = 1;
 		if (likely(rrd->xsz.valid)) {	/* packet valid */
@@ -2032,7 +2032,7 @@ rrd_ok:
 
 			__vlan_hwaccel_put_tag(skb, vlan_tag);
 		}
-		netif_rx(skb);
+		netif_receive_skb(skb);
 
 		/* let protocol layer free skb */
 		buffer_info->skb = NULL;
@@ -2065,6 +2065,8 @@ rrd_ok:
 		iowrite32(value, adapter->hw.hw_addr + REG_MAILBOX);
 		spin_unlock(&adapter->mb_lock);
 	}
+
+	return count;
 }
 
 static void atl1_intr_tx(struct atl1_adapter *adapter)
@@ -2439,6 +2441,33 @@ static netdev_tx_t atl1_xmit_frame(struct sk_buff *skb,
 	return NETDEV_TX_OK;
 }
 
+static int atl1_rx_clean(struct napi_struct *napi, int budget)
+{
+	struct atl1_adapter *adapter = container_of(napi, struct atl1_adapter, napi);
+	int work_done = atl1_intr_rx(adapter, budget);
+
+	/* Let's come again to process some more packets */
+	if (work_done >= budget)
+		return work_done;
+
+	napi_complete(napi);
+	/* re-enable Interrupt */
+	iowrite32(ISR_DIS_SMB | ISR_DIS_DMA, adapter->hw.hw_addr + REG_ISR);
+	return work_done;
+}
+
+static inline int atl1_sched_rx(struct atl1_adapter* adapter)
+{
+	if (likely(napi_schedule_prep(&adapter->napi))) {
+		__napi_schedule(&adapter->napi);
+		return 1;
+	}
+
+	dev_printk(KERN_ERR, &adapter->pdev->dev,
+		   "rx: INTs must be disabled!");
+	return 0;
+}
+
 /*
  * atl1_intr - Interrupt Handler
  * @irq: interrupt number
@@ -2503,8 +2532,9 @@ static irqreturn_t atl1_intr(int irq, void *data)
 			atl1_intr_tx(adapter);
 
 		/* rx event */
-		if (status & ISR_CMB_RX)
-			alt1_intr_rx(adapter);
+		if (status & ISR_CMB_RX && atl1_sched_rx(adapter))
+			/* Go away with INTs disabled */
+			return IRQ_HANDLED;
 
 		/* rx exception */
 		if (unlikely(status & (ISR_RXF_OV | ISR_RFD_UNRUN |
@@ -2515,7 +2545,8 @@ static irqreturn_t atl1_intr(int irq, void *data)
 					&adapter->pdev->dev,
 					"rx exception, ISR = 0x%x\n",
 					status);
-			atl1_intr_rx(adapter);
+			if (atl1_sched_rx(adapter))
+				return IRQ_HANDLED;
 		}
 
 		if (--max_ints < 0)
@@ -2600,6 +2631,7 @@ static s32 atl1_up(struct atl1_adapter *adapter)
 	if (unlikely(err))
 		goto err_up;
 
+	napi_enable(&adapter->napi);
 	atlx_irq_enable(adapter);
 	atl1_check_link(adapter);
 	netif_start_queue(netdev);
@@ -2616,6 +2648,7 @@ static void atl1_down(struct atl1_adapter *adapter)
 {
 	struct net_device *netdev = adapter->netdev;
 
+	napi_disable(&adapter->napi);
 	netif_stop_queue(netdev);
 	del_timer_sync(&adapter->phy_config_timer);
 	adapter->phy_timer_pending = false;
@@ -2972,6 +3005,7 @@ static int __devinit atl1_probe(struct pci_dev *pdev,
 
 	netdev->netdev_ops = &atl1_netdev_ops;
 	netdev->watchdog_timeo = 5 * HZ;
+	netif_napi_add(netdev, &adapter->napi, atl1_rx_clean, 64);
 
 	netdev->ethtool_ops = &atl1_ethtool_ops;
 	adapter->bd_number = cards_found;
diff --git a/drivers/net/ethernet/atheros/atlx/atl1.h b/drivers/net/ethernet/atheros/atlx/atl1.h
index e04bf4d..b55084c 100644
--- a/drivers/net/ethernet/atheros/atlx/atl1.h
+++ b/drivers/net/ethernet/atheros/atlx/atl1.h
@@ -758,6 +758,7 @@ struct atl1_adapter {
 	u16 link_speed;
 	u16 link_duplex;
 	spinlock_t lock;
+	struct napi_struct napi;
 	struct work_struct reset_dev_task;
 	struct work_struct link_chg_task;
 
-- 
1.7.1

Re: [PATCH 2/9] atl1: make driver napi compatible

[Ben Hutchings]

On Fri, 2012-04-13 at 20:09 +0400, Tony Zelenoff wrote:
> This is first step, here there is no fine interrupt
> disabling which cause TX/ERR interrupts stalling when
> RX scheduled ints processed.
> 
> Signed-off-by: Tony Zelenoff <antonz@parallels.com>
> ---
>  drivers/net/ethernet/atheros/atlx/atl1.c |   46 ++++++++++++++++++++++++++----
>  drivers/net/ethernet/atheros/atlx/atl1.h |    1 +
>  2 files changed, 41 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ethernet/atheros/atlx/atl1.c b/drivers/net/ethernet/atheros/atlx/atl1.c
> index 6438239..d39c1b9 100644
> --- a/drivers/net/ethernet/atheros/atlx/atl1.c
> +++ b/drivers/net/ethernet/atheros/atlx/atl1.c
> @@ -1917,7 +1917,7 @@ next:
>  	return num_alloc;
>  }
>  
> -static void atl1_intr_rx(struct atl1_adapter *adapter)
> +static int atl1_intr_rx(struct atl1_adapter *adapter, int budget)
>  {
>  	int i, count;
>  	u16 length;
> @@ -1933,7 +1933,7 @@ static void atl1_intr_rx(struct atl1_adapter *adapter)
>  
>  	rrd_next_to_clean = atomic_read(&rrd_ring->next_to_clean);
>  
> -	while (1) {
> +	while (count <= budget) {
[...]

Off by one; the test should be count < budget.  Otherwise you can exit
with count == budget + 1 and net_rx_action() will WARN you about that.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH 2/9] atl1: make driver napi compatible

[David Miller]

From: Ben Hutchings <bhutchings@solarflare.com>
Date: Fri, 13 Apr 2012 18:15:15 +0100

> On Fri, 2012-04-13 at 20:09 +0400, Tony Zelenoff wrote:
>> This is first step, here there is no fine interrupt
>> disabling which cause TX/ERR interrupts stalling when
>> RX scheduled ints processed.
>> 
>> Signed-off-by: Tony Zelenoff <antonz@parallels.com>
>> ---
>>  drivers/net/ethernet/atheros/atlx/atl1.c |   46 ++++++++++++++++++++++++++----
>>  drivers/net/ethernet/atheros/atlx/atl1.h |    1 +
>>  2 files changed, 41 insertions(+), 6 deletions(-)
>> 
>> diff --git a/drivers/net/ethernet/atheros/atlx/atl1.c b/drivers/net/ethernet/atheros/atlx/atl1.c
>> index 6438239..d39c1b9 100644
>> --- a/drivers/net/ethernet/atheros/atlx/atl1.c
>> +++ b/drivers/net/ethernet/atheros/atlx/atl1.c
>> @@ -1917,7 +1917,7 @@ next:
>>  	return num_alloc;
>>  }
>>  
>> -static void atl1_intr_rx(struct atl1_adapter *adapter)
>> +static int atl1_intr_rx(struct atl1_adapter *adapter, int budget)
>>  {
>>  	int i, count;
>>  	u16 length;
>> @@ -1933,7 +1933,7 @@ static void atl1_intr_rx(struct atl1_adapter *adapter)
>>  
>>  	rrd_next_to_clean = atomic_read(&rrd_ring->next_to_clean);
>>  
>> -	while (1) {
>> +	while (count <= budget) {
> [...]
> 
> Off by one; the test should be count < budget.  Otherwise you can exit
> with count == budget + 1 and net_rx_action() will WARN you about that.

Right, this needs to be fixed up.

[PATCH 3/9] atl1: add napi process of tx interrupts

[Tony Zelenoff]

Make the tx ints processing same as rx ones via napi.
The idea got from e1000. The interrupt disabling is
still not fine grained.

Signed-off-by: Tony Zelenoff <antonz@parallels.com>
---
 drivers/net/ethernet/atheros/atlx/atl1.c |   27 ++++++++++++++++-----------
 1 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/atheros/atlx/atl1.c b/drivers/net/ethernet/atheros/atlx/atl1.c
index d39c1b9..a8935df 100644
--- a/drivers/net/ethernet/atheros/atlx/atl1.c
+++ b/drivers/net/ethernet/atheros/atlx/atl1.c
@@ -2069,12 +2069,13 @@ rrd_ok:
 	return count;
 }
 
-static void atl1_intr_tx(struct atl1_adapter *adapter)
+static int atl1_intr_tx(struct atl1_adapter *adapter)
 {
 	struct atl1_tpd_ring *tpd_ring = &adapter->tpd_ring;
 	struct atl1_buffer *buffer_info;
 	u16 sw_tpd_next_to_clean;
 	u16 cmb_tpd_next_to_clean;
+	int count = 0;
 
 	sw_tpd_next_to_clean = atomic_read(&tpd_ring->next_to_clean);
 	cmb_tpd_next_to_clean = le16_to_cpu(adapter->cmb.cmb->tpd_cons_idx);
@@ -2094,12 +2095,16 @@ static void atl1_intr_tx(struct atl1_adapter *adapter)
 
 		if (++sw_tpd_next_to_clean == tpd_ring->count)
 			sw_tpd_next_to_clean = 0;
+
+		count++;
 	}
 	atomic_set(&tpd_ring->next_to_clean, sw_tpd_next_to_clean);
 
 	if (netif_queue_stopped(adapter->netdev) &&
 	    netif_carrier_ok(adapter->netdev))
 		netif_wake_queue(adapter->netdev);
+
+	return count;
 }
 
 static u16 atl1_tpd_avail(struct atl1_tpd_ring *tpd_ring)
@@ -2441,11 +2446,14 @@ static netdev_tx_t atl1_xmit_frame(struct sk_buff *skb,
 	return NETDEV_TX_OK;
 }
 
-static int atl1_rx_clean(struct napi_struct *napi, int budget)
+static int atl1_rings_clean(struct napi_struct *napi, int budget)
 {
 	struct atl1_adapter *adapter = container_of(napi, struct atl1_adapter, napi);
 	int work_done = atl1_intr_rx(adapter, budget);
 
+	if (atl1_intr_tx(adapter))
+		work_done = budget;
+
 	/* Let's come again to process some more packets */
 	if (work_done >= budget)
 		return work_done;
@@ -2456,7 +2464,7 @@ static int atl1_rx_clean(struct napi_struct *napi, int budget)
 	return work_done;
 }
 
-static inline int atl1_sched_rx(struct atl1_adapter* adapter)
+static inline int atl1_sched_rings_clean(struct atl1_adapter* adapter)
 {
 	if (likely(napi_schedule_prep(&adapter->napi))) {
 		__napi_schedule(&adapter->napi);
@@ -2527,12 +2535,9 @@ static irqreturn_t atl1_intr(int irq, void *data)
 			atl1_check_for_link(adapter);
 		}
 
-		/* transmit event */
-		if (status & ISR_CMB_TX)
-			atl1_intr_tx(adapter);
-
-		/* rx event */
-		if (status & ISR_CMB_RX && atl1_sched_rx(adapter))
+		/* transmit or receive event */
+		if (status & (ISR_CMB_TX | ISR_CMB_RX) &&
+		    atl1_sched_rings_clean(adapter))
 			/* Go away with INTs disabled */
 			return IRQ_HANDLED;
 
@@ -2545,7 +2550,7 @@ static irqreturn_t atl1_intr(int irq, void *data)
 					&adapter->pdev->dev,
 					"rx exception, ISR = 0x%x\n",
 					status);
-			if (atl1_sched_rx(adapter))
+			if (atl1_sched_rings_clean(adapter))
 				return IRQ_HANDLED;
 		}
 
@@ -3005,7 +3010,7 @@ static int __devinit atl1_probe(struct pci_dev *pdev,
 
 	netdev->netdev_ops = &atl1_netdev_ops;
 	netdev->watchdog_timeo = 5 * HZ;
-	netif_napi_add(netdev, &adapter->napi, atl1_rx_clean, 64);
+	netif_napi_add(netdev, &adapter->napi, atl1_rings_clean, 64);
 
 	netdev->ethtool_ops = &atl1_ethtool_ops;
 	adapter->bd_number = cards_found;
-- 
1.7.1

[PATCH 4/9] atl1: use defined functions to disable irq

[Tony Zelenoff]

Looks like direct writes to IMR register is not good idea,
because there are exist functions to make this work.

Signed-off-by: Tony Zelenoff <antonz@parallels.com>
---
 drivers/net/ethernet/atheros/atlx/atl1.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/atheros/atlx/atl1.c b/drivers/net/ethernet/atheros/atlx/atl1.c
index a8935df..4fcdf57 100644
--- a/drivers/net/ethernet/atheros/atlx/atl1.c
+++ b/drivers/net/ethernet/atheros/atlx/atl1.c
@@ -266,7 +266,7 @@ static s32 atl1_reset_hw(struct atl1_hw *hw)
 	 * interrupts & Clear any pending interrupt events
 	 */
 	/*
-	 * iowrite32(0, hw->hw_addr + REG_IMR);
+	 * atlx_irq_disable(adapter);
 	 * iowrite32(0xffffffff, hw->hw_addr + REG_ISR);
 	 */
 
@@ -2512,7 +2512,7 @@ static irqreturn_t atl1_intr(int irq, void *data)
 				dev_printk(KERN_DEBUG, &adapter->pdev->dev,
 					"pcie phy link down %x\n", status);
 			if (netif_running(adapter->netdev)) {	/* reset MAC */
-				iowrite32(0, adapter->hw.hw_addr + REG_IMR);
+				atlx_irq_disable(adapter);
 				schedule_work(&adapter->reset_dev_task);
 				return IRQ_HANDLED;
 			}
@@ -2524,7 +2524,7 @@ static irqreturn_t atl1_intr(int irq, void *data)
 				dev_printk(KERN_DEBUG, &adapter->pdev->dev,
 					"pcie DMA r/w error (status = 0x%x)\n",
 					status);
-			iowrite32(0, adapter->hw.hw_addr + REG_IMR);
+			atlx_irq_disable(adapter);
 			schedule_work(&adapter->reset_dev_task);
 			return IRQ_HANDLED;
 		}
-- 
1.7.1

[PATCH 5/9] atl1: make function to set imr of card

[Tony Zelenoff]

This function should be used later to set/remove proper
bits in imr to disable only rx ints.

Signed-off-by: Tony Zelenoff <antonz@parallels.com>
---
 drivers/net/ethernet/atheros/atlx/atlx.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/atheros/atlx/atlx.c b/drivers/net/ethernet/atheros/atlx/atlx.c
index c9e9dc5..6c4a706 100644
--- a/drivers/net/ethernet/atheros/atlx/atlx.c
+++ b/drivers/net/ethernet/atheros/atlx/atlx.c
@@ -155,14 +155,20 @@ static void atlx_set_multi(struct net_device *netdev)
 	}
 }
 
+static inline void atlx_imr_set(struct atlx_adapter *adapter,
+				unsigned int imr)
+{
+	iowrite32(imr, adapter->hw.hw_addr + REG_IMR);
+	ioread32(adapter->hw.hw_addr + REG_IMR);
+}
+
 /*
  * atlx_irq_enable - Enable default interrupt generation settings
  * @adapter: board private structure
  */
 static void atlx_irq_enable(struct atlx_adapter *adapter)
 {
-	iowrite32(IMR_NORMAL_MASK, adapter->hw.hw_addr + REG_IMR);
-	ioread32(adapter->hw.hw_addr + REG_IMR);
+	atlx_imr_set(adapter, IMR_NORMAL_MASK);
 }
 
 /*
@@ -171,8 +177,7 @@ static void atlx_irq_enable(struct atlx_adapter *adapter)
  */
 static void atlx_irq_disable(struct atlx_adapter *adapter)
 {
-	iowrite32(0, adapter->hw.hw_addr + REG_IMR);
-	ioread32(adapter->hw.hw_addr + REG_IMR);
+	atlx_imr_set(adapter, 0);
 	synchronize_irq(adapter->pdev->irq);
 }
 
-- 
1.7.1

[PATCH 6/9] atl1: add value to check ability of reenabling IRQs

[Tony Zelenoff]

Unfortunately it is not clear from code is usage of
IMR register possible or not. So, to prevent possible
side-effects of reading this register i prefer store
interrupts enable flag separately.

Signed-off-by: Tony Zelenoff <antonz@parallels.com>
---
 drivers/net/ethernet/atheros/atlx/atl1.h |    6 ++++++
 drivers/net/ethernet/atheros/atlx/atlx.c |    2 ++
 2 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/atheros/atlx/atl1.h b/drivers/net/ethernet/atheros/atlx/atl1.h
index b55084c..5209875 100644
--- a/drivers/net/ethernet/atheros/atlx/atl1.h
+++ b/drivers/net/ethernet/atheros/atlx/atl1.h
@@ -782,6 +782,12 @@ struct atl1_adapter {
 	u16 ict;		/* interrupt clear timer (2us resolution */
 	struct mii_if_info mii;	/* MII interface info */
 
+	/*
+	 * Use this value to check is napi handler allowed to
+	 * enable ints or not
+	 */
+	bool int_enabled;
+
 	u32 bd_number;		/* board number */
 	bool pci_using_64;
 	struct atl1_hw hw;
diff --git a/drivers/net/ethernet/atheros/atlx/atlx.c b/drivers/net/ethernet/atheros/atlx/atlx.c
index 6c4a706..b4f3aa4 100644
--- a/drivers/net/ethernet/atheros/atlx/atlx.c
+++ b/drivers/net/ethernet/atheros/atlx/atlx.c
@@ -169,6 +169,7 @@ static inline void atlx_imr_set(struct atlx_adapter *adapter,
 static void atlx_irq_enable(struct atlx_adapter *adapter)
 {
 	atlx_imr_set(adapter, IMR_NORMAL_MASK);
+	adapter->int_enabled = true;
 }
 
 /*
@@ -177,6 +178,7 @@ static void atlx_irq_enable(struct atlx_adapter *adapter)
  */
 static void atlx_irq_disable(struct atlx_adapter *adapter)
 {
+	adapter->int_enabled = false;
 	atlx_imr_set(adapter, 0);
 	synchronize_irq(adapter->pdev->irq);
 }
-- 
1.7.1

[PATCH 7/9] atl1: enable errors and link ints when rx/tx scheduled

[Tony Zelenoff]

Signed-off-by: Tony Zelenoff <antonz@parallels.com>
---
 drivers/net/ethernet/atheros/atlx/atl1.c |   32 ++++++++++++++++++++---------
 drivers/net/ethernet/atheros/atlx/atl1.h |   10 ++++++--
 2 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/atheros/atlx/atl1.c b/drivers/net/ethernet/atheros/atlx/atl1.c
index 4fcdf57..5781597 100644
--- a/drivers/net/ethernet/atheros/atlx/atl1.c
+++ b/drivers/net/ethernet/atheros/atlx/atl1.c
@@ -2460,20 +2460,33 @@ static int atl1_rings_clean(struct napi_struct *napi, int budget)
 
 	napi_complete(napi);
 	/* re-enable Interrupt */
-	iowrite32(ISR_DIS_SMB | ISR_DIS_DMA, adapter->hw.hw_addr + REG_ISR);
+	if (likely(adapter->int_enabled))
+		atlx_imr_set(adapter, IMR_NORMAL_MASK);
 	return work_done;
 }
 
 static inline int atl1_sched_rings_clean(struct atl1_adapter* adapter)
 {
-	if (likely(napi_schedule_prep(&adapter->napi))) {
-		__napi_schedule(&adapter->napi);
+	if (!napi_schedule_prep(&adapter->napi))
+		/* It is possible in case even the RX/TX ints are disabled via IMR
+		 * register the ISR bits are set anyway (but do not produce IRQ).
+		 * To handle such situation the napi functions used to check is
+		 * something scheduled or not.
+		 */
+		return 0;
+
+	__napi_schedule(&adapter->napi);
+
+	/*
+	 * Disable RX/TX ints via IMR register if it is
+	 * allowed. NAPI handler must reenable them in same
+	 * way.
+	 */
+	if (!adapter->int_enabled)
 		return 1;
-	}
 
-	dev_printk(KERN_ERR, &adapter->pdev->dev,
-		   "rx: INTs must be disabled!");
-	return 0;
+	atlx_imr_set(adapter, IMR_NORXTX_MASK);
+	return 1;
 }
 
 /*
@@ -2538,8 +2551,7 @@ static irqreturn_t atl1_intr(int irq, void *data)
 		/* transmit or receive event */
 		if (status & (ISR_CMB_TX | ISR_CMB_RX) &&
 		    atl1_sched_rings_clean(adapter))
-			/* Go away with INTs disabled */
-			return IRQ_HANDLED;
+			break;
 
 		/* rx exception */
 		if (unlikely(status & (ISR_RXF_OV | ISR_RFD_UNRUN |
@@ -2551,7 +2563,7 @@ static irqreturn_t atl1_intr(int irq, void *data)
 					"rx exception, ISR = 0x%x\n",
 					status);
 			if (atl1_sched_rings_clean(adapter))
-				return IRQ_HANDLED;
+				break;
 		}
 
 		if (--max_ints < 0)
diff --git a/drivers/net/ethernet/atheros/atlx/atl1.h b/drivers/net/ethernet/atheros/atlx/atl1.h
index 5209875..3bf79a5 100644
--- a/drivers/net/ethernet/atheros/atlx/atl1.h
+++ b/drivers/net/ethernet/atheros/atlx/atl1.h
@@ -275,13 +275,17 @@ static u32 atl1_check_link(struct atl1_adapter *adapter);
 #define ISR_DIS_SMB				0x20000000
 #define ISR_DIS_DMA				0x40000000
 
-/* Normal Interrupt mask  */
-#define IMR_NORMAL_MASK	(\
+/* Normal Interrupt mask without RX/TX enabled */
+#define IMR_NORXTX_MASK	(\
 	ISR_SMB		|\
 	ISR_GPHY	|\
 	ISR_PHY_LINKDOWN|\
 	ISR_DMAR_TO_RST	|\
-	ISR_DMAW_TO_RST	|\
+	ISR_DMAW_TO_RST)
+
+/* Normal Interrupt mask  */
+#define IMR_NORMAL_MASK	(\
+	IMR_NORXTX_MASK	|\
 	ISR_CMB_TX	|\
 	ISR_CMB_RX)
 
-- 
1.7.1

[PATCH 8/9] atl1: do not process interrupts in cycle in handler

[Tony Zelenoff]

As the rx/tx handled inside napi handler, the cycle is
not needed now, because only the rx/tx need such kind of
processing.

Signed-off-by: Tony Zelenoff <antonz@parallels.com>
---
 drivers/net/ethernet/atheros/atlx/atl1.c |  101 ++++++++++++++----------------
 1 files changed, 46 insertions(+), 55 deletions(-)

diff --git a/drivers/net/ethernet/atheros/atlx/atl1.c b/drivers/net/ethernet/atheros/atlx/atl1.c
index 5781597..3c57a30 100644
--- a/drivers/net/ethernet/atheros/atlx/atl1.c
+++ b/drivers/net/ethernet/atheros/atlx/atl1.c
@@ -2499,77 +2499,68 @@ static irqreturn_t atl1_intr(int irq, void *data)
 {
 	struct atl1_adapter *adapter = netdev_priv(data);
 	u32 status;
-	int max_ints = 10;
 
 	status = adapter->cmb.cmb->int_stats;
 	if (!status)
 		return IRQ_NONE;
 
-	do {
-		/* clear CMB interrupt status at once */
-		adapter->cmb.cmb->int_stats = 0;
-
-		if (status & ISR_GPHY)	/* clear phy status */
-			atlx_clear_phy_int(adapter);
+	/* clear CMB interrupt status at once */
+	adapter->cmb.cmb->int_stats = 0;
 
-		/* clear ISR status, and Enable CMB DMA/Disable Interrupt */
-		iowrite32(status | ISR_DIS_INT, adapter->hw.hw_addr + REG_ISR);
+	if (status & ISR_GPHY)	/* clear phy status */
+		atlx_clear_phy_int(adapter);
 
-		/* check if SMB intr */
-		if (status & ISR_SMB)
-			atl1_inc_smb(adapter);
+	/* clear ISR status, and Enable CMB DMA/Disable Interrupt */
+	iowrite32(status | ISR_DIS_INT, adapter->hw.hw_addr + REG_ISR);
 
-		/* check if PCIE PHY Link down */
-		if (status & ISR_PHY_LINKDOWN) {
-			if (netif_msg_intr(adapter))
-				dev_printk(KERN_DEBUG, &adapter->pdev->dev,
-					"pcie phy link down %x\n", status);
-			if (netif_running(adapter->netdev)) {	/* reset MAC */
-				atlx_irq_disable(adapter);
-				schedule_work(&adapter->reset_dev_task);
-				return IRQ_HANDLED;
-			}
-		}
+	/* check if SMB intr */
+	if (status & ISR_SMB)
+		atl1_inc_smb(adapter);
 
-		/* check if DMA read/write error ? */
-		if (status & (ISR_DMAR_TO_RST | ISR_DMAW_TO_RST)) {
-			if (netif_msg_intr(adapter))
-				dev_printk(KERN_DEBUG, &adapter->pdev->dev,
-					"pcie DMA r/w error (status = 0x%x)\n",
-					status);
+	/* check if PCIE PHY Link down */
+	if (status & ISR_PHY_LINKDOWN) {
+		if (netif_msg_intr(adapter))
+			dev_printk(KERN_DEBUG, &adapter->pdev->dev,
+				"pcie phy link down %x\n", status);
+		if (netif_running(adapter->netdev)) {	/* reset MAC */
 			atlx_irq_disable(adapter);
 			schedule_work(&adapter->reset_dev_task);
 			return IRQ_HANDLED;
 		}
+	}
 
-		/* link event */
-		if (status & ISR_GPHY) {
-			adapter->soft_stats.tx_carrier_errors++;
-			atl1_check_for_link(adapter);
-		}
-
-		/* transmit or receive event */
-		if (status & (ISR_CMB_TX | ISR_CMB_RX) &&
-		    atl1_sched_rings_clean(adapter))
-			break;
-
-		/* rx exception */
-		if (unlikely(status & (ISR_RXF_OV | ISR_RFD_UNRUN |
-			ISR_RRD_OV | ISR_HOST_RFD_UNRUN |
-			ISR_HOST_RRD_OV))) {
-			if (netif_msg_intr(adapter))
-				dev_printk(KERN_DEBUG,
-					&adapter->pdev->dev,
-					"rx exception, ISR = 0x%x\n",
-					status);
-			if (atl1_sched_rings_clean(adapter))
-				break;
-		}
-
-		if (--max_ints < 0)
-			break;
-
-	} while ((status = adapter->cmb.cmb->int_stats));
+	/* check if DMA read/write error ? */
+	if (status & (ISR_DMAR_TO_RST | ISR_DMAW_TO_RST)) {
+		if (netif_msg_intr(adapter))
+			dev_printk(KERN_DEBUG, &adapter->pdev->dev,
+				"pcie DMA r/w error (status = 0x%x)\n",
+				status);
+		atlx_irq_disable(adapter);
+		schedule_work(&adapter->reset_dev_task);
+		return IRQ_HANDLED;
+	}
+
+	/* link event */
+	if (status & ISR_GPHY) {
+		adapter->soft_stats.tx_carrier_errors++;
+		atl1_check_for_link(adapter);
+	}
+
+	/* transmit or receive event */
+	if (status & (ISR_CMB_TX | ISR_CMB_RX))
+	    atl1_sched_rings_clean(adapter);
+
+	/* rx exception */
+	if (unlikely(status & (ISR_RXF_OV | ISR_RFD_UNRUN |
+		ISR_RRD_OV | ISR_HOST_RFD_UNRUN |
+		ISR_HOST_RRD_OV))) {
+		if (netif_msg_intr(adapter))
+			dev_printk(KERN_DEBUG,
+				&adapter->pdev->dev,
+				"rx exception, ISR = 0x%x\n",
+				status);
+		atl1_sched_rings_clean(adapter);
+	}
 
 	/* re-enable Interrupt */
 	iowrite32(ISR_DIS_SMB | ISR_DIS_DMA, adapter->hw.hw_addr + REG_ISR);
-- 
1.7.1

[PATCH 9/9] atl1: do not drop rx/tx interrupts before they are scheduled

[Tony Zelenoff]

To prevent interrupts lost they should be dropped only if
they are scheduled via napi interfaces. In other case, there is
exists situation when napi handler process TX interrupt, stay in
RX processing and in that moment any other interrupt received.
Then before this patch TX bit in ISR will be cleaned, napi
schedule will not occur in case of currently processing event and
TX interrupt definitely will be lost.

Signed-off-by: Tony Zelenoff <antonz@parallels.com>
---
 drivers/net/ethernet/atheros/atlx/atl1.c |   14 ++++++++++----
 1 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/atheros/atlx/atl1.c b/drivers/net/ethernet/atheros/atlx/atl1.c
index 3c57a30..a0b87b1 100644
--- a/drivers/net/ethernet/atheros/atlx/atl1.c
+++ b/drivers/net/ethernet/atheros/atlx/atl1.c
@@ -2504,8 +2504,12 @@ static irqreturn_t atl1_intr(int irq, void *data)
 	if (!status)
 		return IRQ_NONE;
 
-	/* clear CMB interrupt status at once */
-	adapter->cmb.cmb->int_stats = 0;
+	/* clear CMB interrupt status at once,
+	 * but leave rx/tx interrupt status in case it should be dropped
+	 * only if rx/tx processing queued. In other case interrupt
+	 * can be lost.
+	 */
+	adapter->cmb.cmb->int_stats = status & (ISR_CMB_TX | ISR_CMB_RX);
 
 	if (status & ISR_GPHY)	/* clear phy status */
 		atlx_clear_phy_int(adapter);
@@ -2547,8 +2551,10 @@ static irqreturn_t atl1_intr(int irq, void *data)
 	}
 
 	/* transmit or receive event */
-	if (status & (ISR_CMB_TX | ISR_CMB_RX))
-	    atl1_sched_rings_clean(adapter);
+	if (status & (ISR_CMB_TX | ISR_CMB_RX) &&
+	    atl1_sched_rings_clean(adapter))
+		adapter->cmb.cmb->int_stats = adapter->cmb.cmb->int_stats &
+					      ~(ISR_CMB_TX | ISR_CMB_RX);
 
 	/* rx exception */
 	if (unlikely(status & (ISR_RXF_OV | ISR_RFD_UNRUN |
-- 
1.7.1

Re: [PATCH 0/9] atl1: add napi to Atheros L1 NIC

[David Miller]

From: Tony Zelenoff <antonz@parallels.com>
Date: Fri, 13 Apr 2012 20:09:45 +0400

> Added NAPI interface to the driver of Atheros L1 NIC.
> RX speed reported by iperf in TCP mode (with and without -N
> switch) improved by ~7 Mbps. In UDP mode the speed up is
> improved by ~4 Mbps.
> Also hard RX/TX rings processing with memory allocation in
> interrupt handlers is not very good idea. So, fixed this.
> 
> Note: thouse patches must be applied after patch, that fix
> kernel panic in this driver.
> Its subject is: "atl1: fix kernel panic in case of DMA errors"

Unfortunately this is bullshit.  These patches actually depend upon
that bug fix not being applied.  Look at the patches, they reference
and use the reset workqueue the bug fix removed.

Anyways, I sorted through all of these merge hassles, and fixed up the
off-by-one bug Ben spotted in patch #2 because I was tired of waiting
and didn't want these patches to just keep rotting in patchwork.

Re: [PATCH 0/9] atl1: add napi to Atheros L1 NIC

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Sun, 15 Apr 2012 13:03:46 -0400 (EDT)

> From: Tony Zelenoff <antonz@parallels.com>
> Date: Fri, 13 Apr 2012 20:09:45 +0400
> 
>> Added NAPI interface to the driver of Atheros L1 NIC.
>> RX speed reported by iperf in TCP mode (with and without -N
>> switch) improved by ~7 Mbps. In UDP mode the speed up is
>> improved by ~4 Mbps.
>> Also hard RX/TX rings processing with memory allocation in
>> interrupt handlers is not very good idea. So, fixed this.
>> 
>> Note: thouse patches must be applied after patch, that fix
>> kernel panic in this driver.
>> Its subject is: "atl1: fix kernel panic in case of DMA errors"
> 
> Unfortunately this is bullshit.  These patches actually depend upon
> that bug fix not being applied.  Look at the patches, they reference
> and use the reset workqueue the bug fix removed.

My bad, I thought I had merged the net tree into net-next before
processing your patches but I didn't.

I just fixed this all up in the net-next tree, please double
check my work.

Thank you.

Re: [PATCH 0/9] atl1: add napi to Atheros L1 NIC

[Tony Zelenoff]

4/15/12 9:28 PM, David Miller написал:
> From: David Miller<davem@davemloft.net>
> Date: Sun, 15 Apr 2012 13:03:46 -0400 (EDT)
>
>> From: Tony Zelenoff<antonz@parallels.com>
>> Date: Fri, 13 Apr 2012 20:09:45 +0400
>>
>>> Added NAPI interface to the driver of Atheros L1 NIC.
>>> RX speed reported by iperf in TCP mode (with and without -N
>>> switch) improved by ~7 Mbps. In UDP mode the speed up is
>>> improved by ~4 Mbps.
>>> Also hard RX/TX rings processing with memory allocation in
>>> interrupt handlers is not very good idea. So, fixed this.
>>>
>>> Note: thouse patches must be applied after patch, that fix
>>> kernel panic in this driver.
>>> Its subject is: "atl1: fix kernel panic in case of DMA errors"
>>
>> Unfortunately this is bullshit.  These patches actually depend upon
>> that bug fix not being applied.  Look at the patches, they reference
>> and use the reset workqueue the bug fix removed.
Sorry, for long time to reply, unfortunately i had no internet access at 
this weekend.

> My bad, I thought I had merged the net tree into net-next before
> processing your patches but I didn't.
>
> I just fixed this all up in the net-next tree, please double
> check my work.
In atl1.h there is tx_timeout_task member of atl1_adapter remained. It 
was removed from this structure when kernel panic was fixed.
The patch will be soon.

Also, i'm interested in what to do with Chris Nook address in contacts 
list. Reason - mailer respond that this address is not valid anymore. 
Should i remove him from contacts or leave?

Mailer response:

   csnook@redhat.com
     SMTP error from remote mail server after RCPT TO:<csnook@redhat.com>:
     host mx1.redhat.com [209.132.183.28]: 550 5.2.1 
<csnook@redhat.com>... Mailbox disabled for this recipient

Re: [PATCH 0/9] atl1: add napi to Atheros L1 NIC

[David Miller]

From: Tony Zelenoff <antonz@parallels.com>
Date: Mon, 16 Apr 2012 11:22:43 +0400

> Also, i'm interested in what to do with Chris Nook address in contacts
> list. Reason - mailer respond that this address is not valid
> anymore. Should i remove him from contacts or leave?
> 
> Mailer response:
> 
>   csnook@redhat.com
>     SMTP error from remote mail server after RCPT TO:<csnook@redhat.com>:
>     host mx1.redhat.com [209.132.183.28]: 550 5.2.1
>     <csnook@redhat.com>... Mailbox disabled for this recipient

I think you should remove it.

Re: [PATCH 0/9] atl1: add napi to Atheros L1 NIC

[Chris Snook]

On Mon, Apr 16, 2012 at 12:22 AM, Tony Zelenoff <antonz@parallels.com> wrote:
> Also, i'm interested in what to do with Chris Nook address in contacts list.
> Reason - mailer respond that this address is not valid anymore. Should i
> remove him from contacts or leave?
>
> Mailer response:
>
>  csnook@redhat.com
>    SMTP error from remote mail server after RCPT TO:<csnook@redhat.com>:
>    host mx1.redhat.com [209.132.183.28]: 550 5.2.1 <csnook@redhat.com>...
> Mailbox disabled for this recipient

MAINTAINERS is your friend :)

Re: [PATCH 0/9] atl1: add napi to Atheros L1 NIC

[Tony Zelenoff]

4/17/12 12:51 AM, Chris Snook написал:
> On Mon, Apr 16, 2012 at 12:22 AM, Tony Zelenoff<antonz@parallels.com>  wrote:
>> Also, i'm interested in what to do with Chris Nook address in contacts list.
>> Reason - mailer respond that this address is not valid anymore. Should i
>> remove him from contacts or leave?
>>
>> Mailer response:
>>
>>   csnook@redhat.com
>>     SMTP error from remote mail server after RCPT TO:<csnook@redhat.com>:
>>     host mx1.redhat.com [209.132.183.28]: 550 5.2.1<csnook@redhat.com>...
>> Mailbox disabled for this recipient
>
> MAINTAINERS is your friend :)
Oh, nice) May be change email address in contacts list?