Re: in_interrupt race

[george anzinger <george@mvista.com>]

Robert Love wrote:
> 
> On Sat, 2002-04-20 at 06:27, Paul Mackerras wrote:
> 
> > Thus if we have CONFIG_SMP and CONFIG_PREEMPT, there is a small but
> > non-zero probability that in_interrupt() will give the wrong answer if
> > it is called with preemption enabled.  If the process gets scheduled
> > from cpu A to cpu B between calling smp_processor_id() and evaluating
> > local_irq_count(cpu) or local_bh_count(), and cpu A then happens to be
> > in interrupt context at the point where the process resumes on cpu B,
> > then in_interrupt() will incorrectly return 1.
> 
> Looks like you are probably right ...
> 
> > One idea I had is to use a couple of bits in
> > current_thread_info()->flags to indicate whether local_irq_count and
> > local_bh_count are non-zero for the current cpu.  These bits could be
> > tested safely without having to disable preemption.

Preemption lock is implied by either of these being != 0, so this seems
consistant, but why not the whole counter?
> 
> For now we can just do this,
> 
> --- linux-2.5.8/include/asm-i386/hardirq.h      Sun Apr 14 15:18:55 2002
> +++ linux/include/asm-i386/hardirq.h    Mon Apr 22 14:56:29 2002
> @@ -21,8 +21,10 @@
>   * Are we in an interrupt context? Either doing bottom half
>   * or hardware interrupt processing?
>   */
> -#define in_interrupt() ({ int __cpu = smp_processor_id(); \
> -       (local_irq_count(__cpu) + local_bh_count(__cpu) != 0); })
> +#define in_interrupt() ({ int __cpu; preempt_disable(); \
> +       __cpu = smp_processor_id(); \
> +       (local_irq_count(__cpu) + local_bh_count(__cpu) != 0); \
> +       preempt_enable(); })
> 
>  #define in_irq() (local_irq_count(smp_processor_id()) != 0)
> 
> 
> Or perhaps leave the code as-is but make the rule preemption needs to be
> disabled before calling (either implicitly or explicitly).  I.e., via a
> call to preempt_disable or because interrupts are disabled, a lock is
> held, etc ...

Right, getting a consistant flag is not much use if it isn't used within
the same context.
> 
> > In fact almost all uses of local_irq_count() and local_bh_count() are
> > for the current cpu; the exceptions are the irqs_running() function
> > and some debug printks.  Maybe the irq and bh counters themselves
> > could be put into the thread_info struct, if irqs_running could be
> > implemented another way.
> 
> One thing Linus, DaveM, and I discussed a while back was actually
> getting rid of the irq and bh counts completely and folding them into
> preempt_count.  I am interested in this...

Yes.

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

-- 
George Anzinger   george@mvista.com
High-res-timers:  http://sourceforge.net/projects/high-res-timers/
Real time sched:  http://sourceforge.net/projects/rtsched/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml