Re: [BUG] flexcop lockdep

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 6 Apr 2007, Patrick Boettcher wrote:
> On Thu, 5 Apr 2007, Trent Piepho wrote:
> > On Thu, 5 Apr 2007, Patrick Boettcher wrote:
> > > On Thu, 5 Apr 2007, Borgi2008 wrote:
> > > > Am Mittwoch, den 04.04.2007, 23:29 +0300 schrieb Antti Seppälä:
> > > > >
> > > > > Actually, looking at the code I cannot figure out why there has to be a
> > > > > spinlock in the first place.
> > > > >
> > > > > The lock is only taken in the interrupt handler which already runs in
> > > > > atomic context so there is no use in making the handler protected by a
> > > > > spinlock. Am I missing something here?
> > > > >
> > > > > I think the spinlock is unnecessary and should be removed entirely.
> > >
> > > Even on SMP systems? ISRs are only atomic on one CPU.
> >
> > I thought ISRs were normally atomic even on SMP systems.  When an interrupt
> > occurs, that interrupt is disabled until the ISR returns.  Except in fast
> > handlers (which flexcop is not) all interrupts aren't disabled, and so an
> > ISR can be interrupted by a _different_ ISR, and a different ISR could be
> > running at the same time on another CPU.  But an ISR doesn't have to worry
> > about two copies of itself running at the same time.
> >
> > At least, that's how I think it works.
> >
> > I don't see how a spin lock that is only used from the ISR could be useful,
> > assuming the ISR is only called via an interrupt.  There is no reason you
> > couldn't call the isr function from some system call handler, but that
> > would be an unusual thing to do.
> >
> > Normally an ISR does need a spinlock, to access data shared by the
> > non-interrupt part of the driver.  I wonder if there is a bug in flexcop in
> > that nothing else uses irq_lock?
>
> You can only take a spinlock at IRQ-level and not at normal kernel level.

Sure you can.  There is a ton of code that does this.  It would be pretty
much impossible to create SMP safe ISRs if there was no locking mechanism
that worked across interrupt-level to kernel-level.

Look in Documentation/cli-sti-removal.txt:

        irq_handler (...)
        {
                unsigned long flags;
                ....
                spin_lock_irqsave(&driver_lock, flags);
                ....
                driver_data.finish = 1;
                driver_data.new_work = 0;
                ....
                spin_unlock_irqrestore(&driver_lock, flags);
                ....
        }
        ioctl_func (...)
        {
                ...
                spin_lock_irq(&driver_lock);
                ...
                driver_data.finish = 0;
                driver_data.new_work = 2;
                ...
                spin_unlock_irq(&driver_lock);
                ...
        }

_______________________________________________
linux-dvb mailing list
linux-dvb@xxxxxxxxxxx
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb


[Index of Archives]     [Linux Media]     [Video 4 Linux]     [Asterisk]     [Samba]     [Xorg]     [Xfree86]     [Linux USB]

  Powered by Linux