[3550] in linux-scsi channel archive
Re: spinlocks in SCSI code
daemon@ATHENA.MIT.EDU (Doug Ledford)
Tue Mar 17 19:48:20 1998
Date: Tue, 17 Mar 1998 18:32:27 -0600
From: Doug Ledford <dledford@dialnet.net>
To: Kurt Garloff <garloff@kg1.ping.de>
CC: linux-scsi@vger.rutgers.edu, "Leonard N. Zubkoff" <lnz@dandelion.com>,
Gerard Roudier <groudier@club-internet.fr>
Kurt Garloff wrote:
>
> On Tue, Mar 17, 1998 at 01:29:50PM +0100, Kurt Garloff wrote:
> > Well, as a first test, I implemented this for the ncr53c8xx driver, as I'm
> > able to test it directly. Of course it works very well, as I have a UP
> > machine and the spin_lock_irqsave (); does exactly the same as save_flags();
> > cli();
> >
> > Too bad I don't have a SMP machine to test.
>
> Well, I can compile a SMP kernel for my UP machine.
> There's one situation, I didn't think of:
> The driver could try to lock the kernel twice, which will result in a
> deadlock. With the save_flags(); cli(); syntax, this was not detected, but
> with the spin_lock(); this will cause a hang.
>
> There are at least two places in the ncr53c8xx driver where this occurs:
> 1) ncr_timeout(): requeue_waiting_list(); is called which will call
> ncr_queue_command() which tries to get the spinlock.
> 2) ncr_complete() is called from within ncr_exception(), the interrupt
> handler, which holds the lock. ncr_complete() calls
> requeue_waiting_list().
>
> As a workaround, I added two spin_unlock() [without the _irqrestore] insns
> at the places where requeue_ ... is called. This is no clean solution, though.
Although I'm not the worlds foremost expert on spin locks, I would have to
guess that this would be, umm, problematic. You need to use the actual
spin_unlock_irqrestore() function instead so that the cli() doesn't get
released earlyier than planned. The problem I'm thinking of is this:
spin_lock_irqsave(); [ interrupts turned off ]
spin_unlock(); [ lock released, interrupts off ]
function();
spin_lock_irqsave(); [ lock restored, interrupts turned off again ]
...
spin_unlock_irqrestore(); [ lock released, interrupts *enabled* ]
return();
spin_lock(); [ lock restored, interrupts untouched and on ]
spin_unlock_irqrestore(); [ lock released, interrupts turned on,
but they were already on anyway ]
In short, I'm not sure, but I don't think the spin_lock_irqsave() and
associated functions are completely nestable. I would check to find out
before relying on these locks too much :)
--
Doug Ledford <dledford@dialnet.net>
Opinions expressed are my own, but
they should be everybody's.
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.rutgers.edu
