> You missed the basic problem:
>
> CPU#1 CPU#2
>
> start_bh_atomic() start_bh_atomic()
> access bh count access bh count
> end_bh_atomic() end_bh_atomic()
Arggh, I confused global_bh_lock with global_bh_count!! Excuse me! I was
only reading the first characters: "atomic_inc(&global_bh_" in
start/stop_bh_atomic(). It's obvious that it couldn' t be global_bh_count
to be increased because otherwise start_bh_atomic() would istant deadlock
in sychronize_bh! Stupid me again...
Your point is clear to me now, thanks for your paticence...
--------------------------------------------------------------------------
But now I can' t see again why we doesn't wait_on_bh in synchronize_bh if
we are running in a irq handler...
get_active_bhs() and other bh functions seems just safe to me also without
holding the spinlock.
Here a new patch against 2.1.132 that should fix all races in
disable/enable_bh (it's very late again but I try anyway... ;). I checked
that the console race get fixed with it too (as with the atomic_t patch)
and this new one should give consistency also to bh_mask.
Index: linux/include/linux/interrupt.h
diff -u linux/include/linux/interrupt.h:1.1.1.3 linux/include/linux/interrupt.h:1.1.1.1.2.6
--- linux/include/linux/interrupt.h:1.1.1.3 Wed Dec 23 15:24:21 1998
+++ linux/include/linux/interrupt.h Mon Dec 28 03:26:27 1998
@@ -17,7 +17,8 @@
extern volatile unsigned char bh_running;
-extern atomic_t bh_mask_count[32];
+extern spinlock_t bh_lock;
+extern int bh_mask_count[32];
extern unsigned long bh_active;
extern unsigned long bh_mask;
extern void (*bh_base[32])(void);
Index: linux/kernel/softirq.c
diff -u linux/kernel/softirq.c:1.1.1.3 linux/kernel/softirq.c:1.1.1.1.2.5
--- linux/kernel/softirq.c:1.1.1.3 Wed Dec 23 15:25:17 1998
+++ linux/kernel/softirq.c Mon Dec 28 03:26:29 1998
@@ -20,7 +20,8 @@
/* intr_count died a painless death... -DaveM */
-atomic_t bh_mask_count[32];
+spinlock_t bh_lock = SPIN_LOCK_UNLOCKED;
+int bh_mask_count[32];
unsigned long bh_active = 0;
unsigned long bh_mask = 0;
void (*bh_base[32])(void);
Index: linux/include/asm-i386/softirq.h
diff -u linux/include/asm-i386/softirq.h:1.1.1.2 linux/include/asm-i386/softirq.h:1.1.1.1.2.5
--- linux/include/asm-i386/softirq.h:1.1.1.2 Wed Dec 23 15:22:50 1998
+++ linux/include/asm-i386/softirq.h Mon Dec 28 03:26:26 1998
@@ -12,7 +12,7 @@
extern inline void init_bh(int nr, void (*routine)(void))
{
bh_base[nr] = routine;
- atomic_set(&bh_mask_count[nr], 0);
+ bh_mask_count[nr] = 0;
bh_mask |= 1 << nr;
}
@@ -96,15 +96,23 @@
*/
extern inline void disable_bh(int nr)
{
+ unsigned long flags;
+
+ spin_lock_irqsave(&bh_lock, flags);
bh_mask &= ~(1 << nr);
- atomic_inc(&bh_mask_count[nr]);
+ bh_mask_count[nr]++;
+ spin_unlock_irqrestore(&bh_lock, flags);
synchronize_bh();
}
extern inline void enable_bh(int nr)
{
- if (atomic_dec_and_test(&bh_mask_count[nr]))
+ unsigned long flags;
+
+ spin_lock_irqsave(&bh_lock, flags);
+ if (!--bh_mask_count[nr])
bh_mask |= 1 << nr;
+ spin_unlock_irqrestore(&bh_lock, flags);
}
#endif /* __ASM_SOFTIRQ_H */
Here the irq.c patch that I return to don't understand why not to apply
it... ;)
Index: arch/i386/kernel/irq.c
===================================================================
RCS file: /var/cvs/linux/arch/i386/kernel/irq.c,v
retrieving revision 1.1.1.1.2.10
diff -u -r1.1.1.1.2.10 irq.c
--- irq.c 1998/12/23 00:29:43 1.1.1.1.2.10
+++ linux/arch/i386/kernel/irq.c 1998/12/27 16:30:20
@@ -21,6 +21,11 @@
* Copyright (C) 1998 Andrea Arcangeli
*/
+/*
+ * synchronize_bh can't synchronize _only_ if we are in a bh handler.
+ * Copyright (C) 1998 Andrea Arcangeli
+ */
+
#include <linux/ptrace.h>
#include <linux/errno.h>
#include <linux/kernel_stat.h>
@@ -449,7 +454,8 @@
*/
void synchronize_bh(void)
{
- if (atomic_read(&global_bh_count) && !in_interrupt())
+ if (atomic_read(&global_bh_count) &&
+ !local_bh_count[smp_processor_id()])
wait_on_bh();
}
Andrea Arcangeli
-
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/