Messages in this thread | | | Date | Mon, 22 Mar 1999 23:12:14 -0800 (PST) | From | Linus Torvalds <> | Subject | Re: 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/
| |