From patchwork Fri Feb 5 15:55:45 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Kagstrom X-Patchwork-Id: 44651 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 91788B7CC1 for ; Sat, 6 Feb 2010 02:55:58 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754218Ab0BEPzx (ORCPT ); Fri, 5 Feb 2010 10:55:53 -0500 Received: from ernst.netinsight.se ([194.16.221.21]:34084 "HELO ernst.netinsight.se" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1751252Ab0BEPzw (ORCPT ); Fri, 5 Feb 2010 10:55:52 -0500 Received: from marrow.netinsight.se (unverified [10.100.3.78]) by ernst.netinsight.se (EMWAC SMTPRS 0.83) with SMTP id ; Fri, 05 Feb 2010 16:55:45 +0100 Date: Fri, 5 Feb 2010 16:55:45 +0100 From: Simon Kagstrom To: netdev@vger.kernel.org, davem@davemloft.net Cc: davej@redhat.com, ben@decadent.org.uk Subject: [PATCH 3/3] via-velocity: Fix races on shared interrupts Message-ID: <20100205165545.28832c53@marrow.netinsight.se> In-Reply-To: <20100205165253.3f316b98@marrow.netinsight.se> References: <20100205165253.3f316b98@marrow.netinsight.se> X-Mailer: Claws Mail 3.7.4 (GTK+ 2.16.1; i486-pc-linux-gnu) Mime-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org 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_trylock in the interrupt handler. To avoid having the interrupt off for long periods of time, velocity_poll uses non-irqsave spinlocks. 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. Signed-off-by: Simon Kagstrom Signed-off-by: Anders Grafstrom --- drivers/net/via-velocity.c | 26 +++++++++++++++++--------- 1 files changed, 17 insertions(+), 9 deletions(-) diff --git a/drivers/net/via-velocity.c b/drivers/net/via-velocity.c index 5e213f7..6882e7c 100644 --- a/drivers/net/via-velocity.c +++ b/drivers/net/via-velocity.c @@ -2148,16 +2148,8 @@ 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); - /* * Do rx and tx twice for performance (taken from the VIA * out-of-tree driver). @@ -2194,7 +2186,16 @@ static irqreturn_t velocity_intr(int irq, void *dev_instance) struct velocity_info *vptr = netdev_priv(dev); u32 isr_status; - spin_lock(&vptr->lock); + /* Check if the lock is taken, and if so ignore the interrupt. This + * can happen with shared interrupts, where the other device can + * interrupt during velocity_poll (where the lock is held). + * + * With spinlock debugging active on a uniprocessor, this will give + * a warning which can safely be ignored. + */ + if (!spin_trylock(&vptr->lock)) + return IRQ_NONE; + isr_status = mac_read_isr(vptr->mac_regs); /* Not us ? */ @@ -2203,10 +2204,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;