Re: [PATCH 3/3] via-velocity: Fix races on shared interrupts

[Simon Kagstrom <simon.kagstrom@netinsight.net>]

Hi Laurent!

On Wed, 10 Feb 2010 09:41:59 -0800
Laurent Chavey <chavey@google.com> wrote:

> > -       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;
> 
> does the thread handling the interrupts check that an new
> interrupts was received while it was servicing a previous one ?
> wondering if there is a potential for an event that generates the interrupt
> to be missed.

I should say that this particular part of the patch was reworked in
version 2, which David took in here:
  
  http://git.kernel.org/?p=linux/kernel/git/davem/net-2.6.git;a=commitdiff;h=3f2e8d9f13246382fbda6f03178eef867a9bfbe2

Anyway, velocity_poll will try to empty all events within it's budget
from the device and is executing with the device interrupt turned off
(and now also with the local processor interrupts off). If something
would be posted when it's exiting, that's fine since it either

  1) Consumed it's entire budget, in which case it will stay in the
  polling mode anyway

or,

  2) Didn't consume the budget and will then turn on the interrupt
  again and get the new event promptly.


You are right about the code above though, that one is racy as David
explained here:

  http://permalink.gmane.org/gmane.linux.network/151578

// Simon