S2io: Check for CARD_DOWN before handling traffic
Subsystems (2)
PATCH 4/5 S2io: Check for CARD_DOWN before handling traffic
- Added check to return from the traffic handling function, if the card status is DOWN.
---
drivers/net/s2io.c +18 -6
@@ -2927,6 +2927,11 @@ static int s2io_poll(struct net_device *int i;atomic_inc (&nic->isr_cnt); +if (unlikely (atomic_read (&nic->card_state) ==CARD_DOWN )) { +atomic_dec (&nic->isr_cnt); +return IRQ_NONE ; + } + mac_control = &nic->mac_control; config = &nic->config;
@@ -3062,12 +3067,6 @@ static void rx_intr_handler(struct ring_struct RxD3 * rxdp3;spin_lock (&nic->rx_lock); -if (atomic_read (&nic->card_state) ==CARD_DOWN ) { -DBG_PRINT (INTR_DBG ,"%s: %s going down for reset\n" , - __FUNCTION__, dev->name); -spin_unlock (&nic->rx_lock); -return ; - } get_info = ring_data->rx_curr_get_info; get_block = get_info.block_index;
@@ -4404,6 +4403,10 @@ static irqreturn_t s2io_msix_ring_handlestruct s2io_nic *sp = ring->nic;atomic_inc (&sp->isr_cnt); +if (unlikely (atomic_read (&sp->card_state) ==CARD_DOWN )){ +atomic_dec (&sp->isr_cnt); +return IRQ_HANDLED ; + }rx_intr_handler (ring);s2io_chk_rx_buffers (sp, ring->ring_no);
@@ -4418,6 +4421,10 @@ static irqreturn_t s2io_msix_fifo_handlestruct s2io_nic *sp = fifo->nic;atomic_inc (&sp->isr_cnt); +if (unlikely (atomic_read (&sp->card_state) ==CARD_DOWN )){ +atomic_dec (&sp->isr_cnt); +return IRQ_HANDLED ; + }tx_intr_handler (fifo);atomic_dec (&sp->isr_cnt);return IRQ_HANDLED ;
@@ -4896,6 +4903,11 @@ static irqreturn_t s2io_isr(int irq, voireturn IRQ_NONE ;atomic_inc (&sp->isr_cnt); +if (unlikely (atomic_read (&sp->card_state) ==CARD_DOWN )) { +atomic_dec (&sp->isr_cnt); +return IRQ_NONE ; + } + mac_control = &sp->mac_control; config = &sp->config;
Quoted 12 lines · Ramkrishna Vepa
- Added check to return from the traffic handling function, if the card status is DOWN.
Signed-off-by Sivakumar Subramani <[email protected]>Signed-off-by Santosh Rastapur <[email protected]>Signed-off-by Ramkrishna Vepa <[email protected]>---
diff -Nurppatch3/drivers/net/s2io.cpatch4/drivers/net/s2io.cdrivers/net/s2io.c +0
-0@@ -2927,6 +2927,11 @@ static int s2io_poll(struct net_device *int i;
drivers/net/s2io.c static int s2io_poll(struct net_device * · line 2937
Quoted 10 lines
drivers/net/s2io.c +5
-0atomic_inc (&nic->isr_cnt); +if (unlikely (atomic_read (&nic->card_state) ==CARD_DOWN )) { +atomic_dec (&nic->isr_cnt); +return IRQ_NONE ; + } + mac_control = &nic->mac_control; config = &nic->config;
invalid return value, for this function
Overall, this looks quite racy -- why does the card state differ from net_device state in the first place?
Jeff,
Quoted 1 line
Quoted 1 line
- Added check to return from the traffic handling function, if the
card
Quoted 4 lines
status
Quoted 3 lines
is DOWN.
Signed-off-by Sivakumar Subramani
Quoted 5 lines
Quoted 5 lines
Signed-off-by Santosh Rastapur <[email protected]>Signed-off-by Ramkrishna Vepa <[email protected]>---
diff -Nurppatch3/drivers/net/s2io.cpatch4/drivers/net/s2io.c
---patch3/drivers/net/s2io.c2007-08-15 08:57:32.000000000
-0700
Quoted 1 line
Quoted 1 line
+++
patch4/drivers/net/s2io.c2007-08-15 08:42:14.000000000
-0700
Quoted 15 lines
drivers/net/s2io.c static int s2io_poll(struct net_device * · line 2937
Quoted 12 lines
drivers/net/s2io.c +5
-0@@ -2927,6 +2927,11 @@ static int s2io_poll(struct net_device *int i;atomic_inc (&nic->isr_cnt); +if (unlikely (atomic_read (&nic->card_state) ==CARD_DOWN )) { +atomic_dec (&nic->isr_cnt); +return IRQ_NONE ; + } + mac_control = &nic->mac_control; config = &nic->config;invalid return value, for this function
Quoted 2 lines
Overall, this looks quite racy -- why does the card state differ from
net_devicestate in the first place?
[Ram] Agreed, it is racy. Will use test_and_set_bit(), set_bit() and clear_bit().
The card reset could be initiated due to an internal error detected in one of the blocks in the chip, while interrupts are still occurring. There is a net_device state (LINK_STATE_START) set at open and reset at close but none for temporary disabled/enabled states. Did you want us to use this enum along with the CARRIER states?
Ram