diff mbox

[05/14] net: axienet: Service completion interrupts ASAP

Message ID cafb13f2ebe4bb996bafb91e405d8609454bb920.1392220536.git.michal.simek@xilinx.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Michal Simek Feb. 12, 2014, 3:55 p.m. UTC 
From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

The packet completion interrupts for TX and RX should be serviced before
the packets are consumed. This ensures against the degenerate case when a
new completion interrupt is raised after the handler has exited but before
the interrupts are cleared. In this case its possible for the ISR to clear
an unhandled interrupt (leading to potential deadlock).

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Acked-by: Michal Simek <michal.simek@xilinx.com>
Tested-by: Jason Wu <huanyu@xilinx.com>
---

 drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

--
1.8.2.3

Comments

David Laight Feb. 12, 2014, 4:38 p.m. UTC | #1 
From: Michal Simek
> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> 
> The packet completion interrupts for TX and RX should be serviced before
> the packets are consumed. This ensures against the degenerate case when a
> new completion interrupt is raised after the handler has exited but before
> the interrupts are cleared. In this case its possible for the ISR to clear
> an unhandled interrupt (leading to potential deadlock).

I would clear the IRQ after processing the last packet, and then do a final
check for another packet.

That reduces the number of interrupts you take and then find there is
no work (because it was done on the previous interrupt).

There is a slight 'gotcha' in that the write to clear the IRQ can easily
get delayed enough the that cpu exits the ISR before the IRQ line
actually drops - leading the unwanted and unclaimed interrupts.
A posted write over PCIe could easily take long enough.

Maybe a hybrid scheme where the IRQ is cleared when the next entry
is still owned by the device would work.

	David



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

Patch

diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index 2e21ab2..3c24f97 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -809,6 +809,7 @@  static irqreturn_t axienet_tx_irq(int irq, void *_ndev)

 	status = axienet_dma_in32(lp, XAXIDMA_TX_SR_OFFSET);
 	if (status & (XAXIDMA_IRQ_IOC_MASK | XAXIDMA_IRQ_DELAY_MASK)) {
+		axienet_dma_out32(lp, XAXIDMA_TX_SR_OFFSET, status);
 		axienet_start_xmit_done(lp->ndev);
 		goto out;
 	}
@@ -832,9 +833,9 @@  static irqreturn_t axienet_tx_irq(int irq, void *_ndev)
 		axienet_dma_out32(lp, XAXIDMA_RX_CR_OFFSET, cr);

 		tasklet_schedule(&lp->dma_err_tasklet);
+		axienet_dma_out32(lp, XAXIDMA_TX_SR_OFFSET, status);
 	}
 out:
-	axienet_dma_out32(lp, XAXIDMA_TX_SR_OFFSET, status);
 	return IRQ_HANDLED;
 }

@@ -857,6 +858,7 @@  static irqreturn_t axienet_rx_irq(int irq, void *_ndev)

 	status = axienet_dma_in32(lp, XAXIDMA_RX_SR_OFFSET);
 	if (status & (XAXIDMA_IRQ_IOC_MASK | XAXIDMA_IRQ_DELAY_MASK)) {
+		axienet_dma_out32(lp, XAXIDMA_RX_SR_OFFSET, status);
 		axienet_recv(lp->ndev);
 		goto out;
 	}
@@ -880,9 +882,9 @@  static irqreturn_t axienet_rx_irq(int irq, void *_ndev)
 		axienet_dma_out32(lp, XAXIDMA_RX_CR_OFFSET, cr);

 		tasklet_schedule(&lp->dma_err_tasklet);
+		axienet_dma_out32(lp, XAXIDMA_RX_SR_OFFSET, status);
 	}
 out:
-	axienet_dma_out32(lp, XAXIDMA_RX_SR_OFFSET, status);
 	return IRQ_HANDLED;
 }