netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Simon Kagstrom <simon.kagstrom@netinsight.net>
To: David Miller <davem@davemloft.net>, netdev@vger.kernel.org
Cc: davej@redhat.com, ben@decadent.org.uk
Subject: [PATCH v2 3/3] via-velocity: Fix races on shared interrupts
Date: Wed, 10 Feb 2010 10:38:25 +0100	[thread overview]
Message-ID: <20100210103825.6e9f6e09@marrow.netinsight.se> (raw)
In-Reply-To: <20100210103358.41a45851@marrow.netinsight.se>

This patch fixes two potential races in the velocity driver:

* Move the ACK and error handler to the interrupt handler. This fixes a
  potential race with shared interrupts when the other device interrupts
  before the NAPI poll handler has finished. As the velocity driver hasn't
  acked it's own interrupt, it will then steal the interrupt from the
  other device.

* Use spin_lock_irqsave in velocity_poll. In the current code, the
  interrupt handler will deadlock if e.g., the NAPI poll handler is
  executing when an interrupt (for another device) comes in since it
  tries to take the already held lock.

  Also unlock the spinlock only after enabling the interrupt in
  velocity_poll.

The error path is moved to the interrupt handler since this is where the
ISR is checked now.

Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
Signed-off-by: Anders Grafstrom <anders.grafstrom@netinsight.net>
---
ChangeLog:
	* (David Miller): Remove spin_trylock from the interrupt handler
	* (David Miller): Use spin_lock_irqsave in velocity_poll
	* Move spin_unlock_irqrestore below mac_enable_int in velocity_poll

 drivers/net/via-velocity.c |   21 ++++++++++-----------
 1 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/net/via-velocity.c b/drivers/net/via-velocity.c
index 5e213f7..18770ac 100644
--- a/drivers/net/via-velocity.c
+++ b/drivers/net/via-velocity.c
@@ -2148,16 +2148,9 @@ static int velocity_poll(struct napi_struct *napi, int budget)
 	struct velocity_info *vptr = container_of(napi,
 			struct velocity_info, napi);
 	unsigned int rx_done;
-	u32 isr_status;
-
-	spin_lock(&vptr->lock);
-	isr_status = mac_read_isr(vptr->mac_regs);
-
-	/* Ack the interrupt */
-	mac_write_isr(vptr->mac_regs, isr_status);
-	if (isr_status & (~(ISR_PRXI | ISR_PPRXI | ISR_PTXI | ISR_PPTXI)))
-		velocity_error(vptr, isr_status);
+	unsigned long flags;
 
+	spin_lock_irqsave(&vptr->lock, flags);
 	/*
 	 * Do rx and tx twice for performance (taken from the VIA
 	 * out-of-tree driver).
@@ -2167,13 +2160,12 @@ static int velocity_poll(struct napi_struct *napi, int budget)
 	rx_done += velocity_rx_srv(vptr, budget - rx_done);
 	velocity_tx_srv(vptr);
 
-	spin_unlock(&vptr->lock);
-
 	/* If budget not fully consumed, exit the polling mode */
 	if (rx_done < budget) {
 		napi_complete(napi);
 		mac_enable_int(vptr->mac_regs);
 	}
+	spin_unlock_irqrestore(&vptr->lock, flags);
 
 	return rx_done;
 }
@@ -2203,10 +2195,17 @@ static irqreturn_t velocity_intr(int irq, void *dev_instance)
 		return IRQ_NONE;
 	}
 
+	/* Ack the interrupt */
+	mac_write_isr(vptr->mac_regs, isr_status);
+
 	if (likely(napi_schedule_prep(&vptr->napi))) {
 		mac_disable_int(vptr->mac_regs);
 		__napi_schedule(&vptr->napi);
 	}
+
+	if (isr_status & (~(ISR_PRXI | ISR_PPRXI | ISR_PTXI | ISR_PPTXI)))
+		velocity_error(vptr, isr_status);
+
 	spin_unlock(&vptr->lock);
 
 	return IRQ_HANDLED;
-- 
1.6.0.4


  parent reply	other threads:[~2010-02-10  9:38 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-02-05 15:52 [PATCH 0/3]: via-velocity: Fixes for locking issues Simon Kagstrom
2010-02-05 15:54 ` [PATCH 1/3] via-velocity: Remove unused IRQ status parameter from rx_srv and tx_srv Simon Kagstrom
2010-02-05 15:55 ` [PATCH 2/3] via-velocity: Take spinlock on set coalesce Simon Kagstrom
2010-02-05 15:55 ` [PATCH 3/3] via-velocity: Fix races on shared interrupts Simon Kagstrom
2010-02-10 17:41   ` Laurent Chavey
2010-02-11  8:05     ` Simon Kagstrom
2010-02-10  0:31 ` [PATCH 0/3]: via-velocity: Fixes for locking issues David Miller
2010-02-10  9:33   ` Simon Kagstrom
2010-02-10  9:37     ` [PATCH v2 1/3] via-velocity: Remove unused IRQ status parameter from rx_srv and tx_srv Simon Kagstrom
2010-02-10  9:38     ` [PATCH v2 2/3] via-velocity: Take spinlock on set coalesce Simon Kagstrom
2010-02-10  9:38     ` Simon Kagstrom [this message]
2010-02-10 18:55     ` [PATCH 0/3]: via-velocity: Fixes for locking issues David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100210103825.6e9f6e09@marrow.netinsight.se \
    --to=simon.kagstrom@netinsight.net \
    --cc=ben@decadent.org.uk \
    --cc=davej@redhat.com \
    --cc=davem@davemloft.net \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).