lkml.org 
[lkml]   [1999]   [Mar]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: 3 Ooopses in 2.2.3
On Mon, 22 Mar 1999, Linus Torvalds wrote:

>There _is_ actually a bug, which is that the __sti() in do_bottom_half has

Infact it's the bug that I was talking about and that I fixed in an
alternate way.

>to be paired with a __cli(), because otherwise the above rules are not
>always satisfied, but that's fixed in the current pre-kernels already.

OK. My whole point against _only_ pairing with the __cli() to fix the bug,
is that when do_bottom_half() is recalled by entry.S and schedule(), we
don't need to __cli() at all. And I also see hardirq_trylock too much
aggressive (should be checking only local_irq_count and not
global_irq_count). My previous patch fixed the same bug without adding
overhead to do_bottom_half() and also removing the check for
global_irq_count in hardirq_trylock. Probably it wasn't very elegant
because I added a nested_level information that we could obtain also using
the proper irq-bh-locking and checking local_irq_count in hardirq_trylock.

Note that I agree that putting a __cli() before releasing the bh lock,
will fix the bug, but I don't agree that it's a smart enough fix. At least
it should be a __save_flags(); __sti(); ... __restore_flags(); sequence.

So here it is a new _minimal_ patch that won't add the nested_level
information, but that will avoid to exit from do_bottom_half with irq
disabled, and will allow a bh to run even if there are irqs running on the
other CPU (as my previous patch just did in an alternate manner that was
more obviously right according to me ;):

Index: include/asm-i386/hardirq.h
===================================================================
RCS file: /var/cvs/linux/include/asm-i386/hardirq.h,v
retrieving revision 1.1.2.1
diff -u -r1.1.2.1 hardirq.h
--- hardirq.h 1999/01/18 01:33:32 1.1.2.1
+++ linux/include/asm-i386/hardirq.h 1999/03/23 12:35:55
@@ -53,7 +53,7 @@

static inline int hardirq_trylock(int cpu)
{
- return !atomic_read(&global_irq_count) && !test_bit(0,&global_irq_lock);
+ return !local_irq_count[cpu] && !test_bit(0,&global_irq_lock);
}

#define hardirq_endlock(cpu) do { } while (0)
Index: kernel/softirq.c
===================================================================
RCS file: /var/cvs/linux/kernel/softirq.c,v
retrieving revision 1.1.2.2
diff -u -r1.1.2.2 softirq.c
--- softirq.c 1999/02/02 16:41:42 1.1.2.2
+++ linux/kernel/softirq.c 1999/03/23 13:43:12
@@ -53,8 +53,11 @@

if (softirq_trylock(cpu)) {
if (hardirq_trylock(cpu)) {
+ unsigned long flags;
+ __save_flags(flags);
__sti();
run_bottom_halves();
+ __restore_flags(flags);
hardirq_endlock(cpu);
}
softirq_endlock(cpu);

Maybe I am simply a bit too much paranoid in not taking the cpu with irq
disabled ;)... Comments?

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/

\
 
 \ /
  Last update: 2005-03-22 13:50    [W:0.452 / U:0.396 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site