All of lore.kernel.org
 help / color / mirror / Atom feed
From: Robert Love <rml@tech9.net>
To: paulus@samba.org
Cc: linux-kernel@vger.kernel.org
Subject: Re: in_interrupt race
Date: 22 Apr 2002 15:02:53 -0400	[thread overview]
Message-ID: <1019502174.939.50.camel@phantasy> (raw)
In-Reply-To: <15553.17071.88897.914713@argo.ozlabs.ibm.com>

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.

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 ...

> 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...

	Robert Love


  reply	other threads:[~2002-04-22 19:02 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2002-04-20 10:27 in_interrupt race Paul Mackerras
2002-04-22 19:02 ` Robert Love [this message]
2002-04-22 21:39   ` george anzinger
2002-04-22 21:54     ` Robert Love
2002-04-22 23:06       ` Paul Mackerras
2002-04-22 23:15         ` Robert Love
2002-04-23  3:25         ` Rusty Russell
2002-04-23  8:31           ` Russell King
2002-04-24  4:43             ` Rusty Russell
2002-04-22 23:22     ` Paul Mackerras

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1019502174.939.50.camel@phantasy \
    --to=rml@tech9.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=paulus@samba.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.