Re: sparc asyncd deadlock prone

Andrea Arcangeli (andrea@suse.de)
Sun, 17 Oct 1999 00:49:02 +0200 (CEST)

On Sat, 16 Oct 1999, Andrea Arcangeli wrote:

>If the cli() was necessary than the spin_*lock_irq() should be replaced
>with spin_*lock() and a cli() should be used after the
>interruptible_sleep_on() call.

I found another instance of the same bug in a driver. The __sti() is there
to allow the cli() to really grab the global irq lock. When
interruptible_sleep_on() returns irqs are still disabled (but only
locally...).

A better (but more intensive) fix would protect the variables with a
spinlock...

diff -urN 2.3.22/arch/sparc/mm/asyncd.c 2.3.22-deadlocks/arch/sparc/mm/asyncd.c
--- 2.3.22/arch/sparc/mm/asyncd.c Tue Sep 14 14:35:01 1999
+++ 2.3.22-deadlocks/arch/sparc/mm/asyncd.c Sat Oct 16 23:09:33 1999
@@ -259,10 +259,11 @@
save_flags(flags); cli();

while (!async_queue) {
- spin_lock_irq(&current->sigmask_lock);
+ spin_lock(&current->sigmask_lock);
flush_signals(current);
- spin_unlock_irq(&current->sigmask_lock);
+ spin_unlock(&current->sigmask_lock);
interruptible_sleep_on(&asyncd_wait);
+ __sti(); cli();
}

restore_flags(flags);
diff -urN 2.3.22/arch/sparc64/mm/asyncd.c 2.3.22-deadlocks/arch/sparc64/mm/asyncd.c
--- 2.3.22/arch/sparc64/mm/asyncd.c Tue Sep 14 14:33:10 1999
+++ 2.3.22-deadlocks/arch/sparc64/mm/asyncd.c Sat Oct 16 23:17:48 1999
@@ -262,10 +262,11 @@
save_flags(flags); cli();

while (!async_queue) {
- spin_lock_irq(&current->sigmask_lock);
+ spin_lock(&current->sigmask_lock);
flush_signals(current);
- spin_unlock_irq(&current->sigmask_lock);
+ spin_unlock(&current->sigmask_lock);
interruptible_sleep_on(&asyncd_wait);
+ __sti(); cli(); /* acquire gloabl_irq_lock */
}

restore_flags(flags);
diff -urN 2.3.22/drivers/ap1000/ddv.c 2.3.22-deadlocks/drivers/ap1000/ddv.c
--- 2.3.22/drivers/ap1000/ddv.c Tue Jul 13 02:00:56 1999
+++ 2.3.22-deadlocks/drivers/ap1000/ddv.c Sun Oct 17 00:43:11 1999
@@ -386,10 +386,11 @@
save_flags(flags); cli();

while (!rem_queue) {
- spin_lock_irq(&current->sigmask_lock);
+ spin_lock(&current->sigmask_lock);
flush_signals(current);
- spin_unlock_irq(&current->sigmask_lock);
+ spin_unlock(&current->sigmask_lock);
interruptible_sleep_on(&ddv_daemon_wait);
+ __sti(); cli();
}

rem = rem_queue;

Andrea

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/