Skip to contentSkip to conversation

s2io: don't run MSI handlers if device is offline.

PATCH In review Linas Vepstas 3 participants 4 messages May 22, 2007 – May 25, 2007

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

Linas Vepstas

author
#0 lore raw

PATCH 1/1 s2io: don't run MSI handlers if device is offline.

Don't run any of the MSI handlers if the channel is off; also don't gather device statatistics. Also, netif_wake not needed, per suggestions from
Sivakumar Subramani <[email protected]>.

Signed-off-by Linas Vepstas <[email protected]>
Cc Ramkrishna Vepa <[email protected]>
Cc Sivakumar Subramani <[email protected]>
Cc Sreenivasa Honnur <[email protected]>
Cc Rastapur Santosh <[email protected]>
Cc Wen Xiong <[email protected]>

----

drivers/net/s2io.c +12 -1
index e46e164..871c37c 100644
@@ -4202,6 +4202,9 @@ static irqreturn_t s2io_msi_handle(int i
 	struct mac_info *mac_control;
 	struct config_param *config;
 
+	if (pci_channel_offline(sp->pdev))
+		return IRQ_NONE;
+
 	atomic_inc(&sp->isr_cnt);
 	mac_control = &sp->mac_control;
 	config = &sp->config;

@@ -4232,6 +4235,9 @@ static irqreturn_t s2io_msix_ring_handle
 	struct ring_info *ring = (struct ring_info *)dev_id;
 	struct s2io_nic *sp = ring->nic;
 
+	if (pci_channel_offline(sp->pdev))
+		return IRQ_NONE;
+
 	atomic_inc(&sp->isr_cnt);
 
 	rx_intr_handler(ring);

@@ -4246,6 +4252,9 @@ static irqreturn_t s2io_msix_fifo_handle
 	struct fifo_info *fifo = (struct fifo_info *)dev_id;
 	struct s2io_nic *sp = fifo->nic;
 
+	if (pci_channel_offline(sp->pdev))
+		return IRQ_NONE;
+
 	atomic_inc(&sp->isr_cnt);
 	tx_intr_handler(fifo);
 	atomic_dec(&sp->isr_cnt);

@@ -4428,6 +4437,9 @@ static void s2io_updt_stats(struct s2io_
 	u64 val64;
 	int cnt = 0;
 
+	if (pci_channel_offline(sp->pdev))
+		return;
+
 	if (atomic_read(&sp->card_state) == CARD_UP) {
 		/* Apprx 30us on a 133 MHz bus */
 		val64 = SET_UPDT_CLICKS(10) |

@@ -8122,5 +8134,4 @@ static void s2io_io_resume(struct pci_de
 	}
 
 	netif_device_attach(netdev);
-	netif_wake_queue(netdev);
 }
#1 lore raw

Fix looks good. No comments.
~Siva

-----Original Message-----
From: Linas Vepstas [mailto:[email protected]]
Sent: Wednesday, May 23, 2007 4:20 AM
To: Jeff Garzik; Andrew Morton
Cc: [email protected]; [email protected]; Ramkrishna Vepa; Sivakumar Subramani; Sreenivasa Honnur; Rastapur Santosh; Wen Xiong
Subject: [PATCH] s2io: don't run MSI handlers if device is offline.

Don't run any of the MSI handlers if the channel is off; also don't gather device statatistics. Also, netif_wake not needed, per suggestions from Sivakumar Subramani <[email protected]>.

Signed-off-by Linas Vepstas <[email protected]>
Cc Ramkrishna Vepa <[email protected]>
Cc Sivakumar Subramani <[email protected]>
Cc Sreenivasa Honnur <[email protected]>
Cc Rastapur Santosh <[email protected]>
Cc Wen Xiong <[email protected]>

----

drivers/net/s2io.c index renamed from drivers/net/s2io.c +0 -0

e46e164..871c37c 100644

drivers/net/s2io.c +12 -1
@@ -4202,6 +4202,9 @@ static irqreturn_t s2io_msi_handle(int i
 	struct mac_info *mac_control;
 	struct config_param *config;
 
+	if (pci_channel_offline(sp->pdev))
+		return IRQ_NONE;
+
 	atomic_inc(&sp->isr_cnt);
 	mac_control = &sp->mac_control;
 	config = &sp->config;

@@ -4232,6 +4235,9 @@ static irqreturn_t s2io_msix_ring_handle
 	struct ring_info *ring = (struct ring_info *)dev_id;
 	struct s2io_nic *sp = ring->nic;
 
+	if (pci_channel_offline(sp->pdev))
+		return IRQ_NONE;
+
 	atomic_inc(&sp->isr_cnt);
 
 	rx_intr_handler(ring);

@@ -4246,6 +4252,9 @@ static irqreturn_t s2io_msix_fifo_handle
 	struct fifo_info *fifo = (struct fifo_info *)dev_id;
 	struct s2io_nic *sp = fifo->nic;
 
+	if (pci_channel_offline(sp->pdev))
+		return IRQ_NONE;
+
 	atomic_inc(&sp->isr_cnt);
 	tx_intr_handler(fifo);
 	atomic_dec(&sp->isr_cnt);

@@ -4428,6 +4437,9 @@ static void s2io_updt_stats(struct s2io_
 	u64 val64;
 	int cnt = 0;
 
+	if (pci_channel_offline(sp->pdev))
+		return;
+
 	if (atomic_read(&sp->card_state) == CARD_UP) {
 		/* Apprx 30us on a 133 MHz bus */
 		val64 = SET_UPDT_CLICKS(10) |

@@ -8122,5 +8134,4 @@ static void s2io_io_resume(struct pci_de
 	}
 
 	netif_device_attach(netdev);
-	netif_wake_queue(netdev);
 }
#2 lore raw
In reply to Linas Vepstas
Quoted 10 lines · Linas Vepstas

Don't run any of the MSI handlers if the channel is off;
also don't gather device statatistics. Also, netif_wake not needed, per suggestions from Sivakumar Subramani <[email protected]>.

Signed-off-by Linas Vepstas <[email protected]>
Cc Ramkrishna Vepa <[email protected]>
Cc Sivakumar Subramani <[email protected]>
Cc Sreenivasa Honnur <[email protected]>
Cc Rastapur Santosh <[email protected]>
Cc Wen Xiong <[email protected]>

Why are the interrupt handlers being called at all, then?

This seems to be papering over another bug.

author
#3 lore raw
Quoted 9 lines · Jeff Garzik
Quoted 5 lines · Linas Vepstas

Don't run any of the MSI handlers if the channel is off; also don't gather device statatistics. Also, netif_wake not needed, per suggestions from
Sivakumar Subramani <[email protected]>.

Why are the interrupt handlers being called at all, then?

This seems to be papering over another bug.

Ahh ... are you suggesting that the arch should be performing this check in driver-independent code, and not delivering the MSI if the pci channel is deemed offline? Yes, right, good point.

So far, I'd hadn't really given this much thought, and had been letting the dev drivers perform this check, but, yes, it is probably better to have one common check performed in the generic arch-specific code, and the interrupt dropped, if the device is offlined.

I'll look into this.

--linas