lkml.org 
[lkml]   [1999]   [Mar]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: 3 Ooopses in 2.2.3


On Tue, 23 Mar 1999, Andrea Arcangeli wrote:
>
> It's quite obvious that the bh handler must run only in not nested irqs.

"quite obvious" is not necessarily true, but it's a good rule that we have
tried to enforce.

> Currently do_bottom_half() tried to do that but without success:

What's the problem?

> static inline int hardirq_trylock(int cpu)
> {
> return !atomic_read(&global_irq_count) && !test_bit(0,&global_irq_lock);
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> }
>
> hardirq_trylock was avoiding the bh handler to run if there was an irq in
> progress, but the problem is that there could be a nested irq that started
> after the __sti() in the bh handling, and such nested irq could run
> do_bottom_half() itself without noticing that it's a nested irq.

Why do you think it wouldn't notice? If it is truly nested, then
global_irq_count is certainly not going to be zero.

You don't make much sense. First you say that the above doesn't catch
recursive interrupts, and then you say that it's too strict:

> And btw using atomic_read(&global_irq_count) was also too much
> aggressive, we could have used more simply local_irq_count[cpu] because
> a bh handler can always run in parallel with another unrelated irq in
> the other CPU (if the other irq would be started a bit after that would
> happen anyway).

I agree that we could use "local_irq_count[]" if we wanted to be more
permissive. I don't think we necessarily want to be that.

> This bug was quite subtle and I am not sure if it's really the cause of
> the crashes, but I suggest everybody who is having crashed/reboots to give
> a try it out this my patch against 2.2.3, maybe it will be enough ;).

Do you have any actual reason to believe that your patch actually makes
any difference at all? I still don't see it mattering.

Basically, the rules are:
- we never enable interrupts locally except when we have the proper
interrupt counts. So we cannot take nested software bottom half
interrupts either, because the bottom half code will notice that the
irq count is non-zero.
- When the bottom half handler is running, we _do_ have interrupts
enabled, and the hardware interrupt counters are zero, but at that
point we hold the softirq lock anyway, so we wouldn't try to re-enter
the bottom half handler.

So would you mind trying to explain why you think your patch matters?

There _is_ actually a bug, which is that the __sti() in do_bottom_half has
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.

Linus


-
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.227 / U:0.032 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site