Let -DINTERRUPTIBLE=0 mask interrupts in probes

Some time ago we loosened up the code for all probe types to allow
interrupts during the handler.  However, when probing something like
kernel.trace("*"), you get a mix of probes in and out of the interrupt
path, and it becomes much more common to have probes skipped due to
interrupt reentrancy.

The common_probe_entryfn_prologue and common_probe_entryfn_epilogue
functions had an interruptible flag, but this was no longer used
anywhere.  I removed this flag, but then reused the logic to check an
INTERRUPTIBLE macro instead.  Now users can use -DINTERRUPTIBLE=0 to
prevent interrupt reentrancy in their script, at the cost of a bit more
overhead to toggle the interrupt mask.

3 files changed, 41 insertions, 24 deletions

diff --git a/stap.1.in b/stap.1.in
index 3a0823e6..546bebb8 100644
--- a/stap.1.in
+++ b/stap.1.in
@@ -1055,6 +1055,17 @@ pool is large because individual uprobe objects are allocated for each
 process for each script-level probe.
 
 .PP
+With scripts that contain probes on any interrupt path, it is possible that
+those interrupts may occur in the middle of another probe handler.  The probe
+in the interrupt handler would be skipped in this case to avoid reentrance.
+To work around this issue, execute stap with the option
+.BR \-DINTERRUPTIBLE=0
+to mask interrupts throughout the probe handler.  This does add some extra
+overhead to the probes, but it may prevent reentrance for common problem
+cases.  However, probes in NMI handlers and in the callpath of the stap
+runtime may still be skipped due to reentrance.
+
+.PP
 Multiple scripts can write data into a relay buffer concurrently. A host
 script provides an interface for accessing its relay buffer to guest scripts.
 Then, the output of the guests are merged into the output of the host.
diff --git a/tapsets.cxx b/tapsets.cxx
index b748a487..ce9a0ca1 100644
--- a/tapsets.cxx
+++ b/tapsets.cxx
@@ -159,12 +159,12 @@ be_derived_probe::join_group (systemtap_session& s)
 void
 common_probe_entryfn_prologue (translator_output* o, string statestr,
                                string new_pp,
-			       bool overload_processing = true,
-                               bool interruptible = true)
+                               bool overload_processing = true)
 {
   o->newline() << "struct context* __restrict__ c;";
-  if (! interruptible)
-    o->newline() << "unsigned long flags;";
+  o->newline() << "#if !INTERRUPTIBLE";
+  o->newline() << "unsigned long flags;";
+  o->newline() << "#endif";
 
   if (overload_processing)
     o->newline() << "#if defined(STP_TIMING) || defined(STP_OVERLOAD)";
@@ -182,10 +182,11 @@ common_probe_entryfn_prologue (translator_output* o, string statestr,
   o->newline() << "static int _pfm_num_pmd_x;";
 #endif
 
-  if (! interruptible)
-    o->newline() << "local_irq_save (flags);";
-  else
-    o->newline() << "preempt_disable ();";
+  o->newline() << "#if INTERRUPTIBLE";
+  o->newline() << "preempt_disable ();";
+  o->newline() << "#else";
+  o->newline() << "local_irq_save (flags);";
+  o->newline() << "#endif";
 
   // Check for enough free enough stack space
   o->newline() << "if (unlikely ((((unsigned long) (& c)) & (THREAD_SIZE-1))"; // free space
@@ -233,10 +234,12 @@ common_probe_entryfn_prologue (translator_output* o, string statestr,
   o->newline() << "c->regparm = 0;";
   o->newline() << "c->marker_name = NULL;";
   o->newline() << "c->marker_format = NULL;";
-  if (! interruptible)
-    o->newline() << "c->actionremaining = MAXACTION;";
-  else
-    o->newline() << "c->actionremaining = MAXACTION_INTERRUPTIBLE;";
+
+  o->newline() << "#if INTERRUPTIBLE";
+  o->newline() << "c->actionremaining = MAXACTION_INTERRUPTIBLE;";
+  o->newline() << "#else";
+  o->newline() << "c->actionremaining = MAXACTION;";
+  o->newline() << "#endif";
   o->newline() << "#ifdef STP_TIMING";
   o->newline() << "c->statp = 0;";
   o->newline() << "#endif";
@@ -255,8 +258,7 @@ common_probe_entryfn_prologue (translator_output* o, string statestr,
 
 void
 common_probe_entryfn_epilogue (translator_output* o,
-			       bool overload_processing = true,
-                               bool interruptible = true)
+                               bool overload_processing = true)
 {
   if (overload_processing)
     o->newline() << "#if defined(STP_TIMING) || defined(STP_OVERLOAD)";
@@ -332,10 +334,11 @@ common_probe_entryfn_epilogue (translator_output* o,
   o->newline() << "_stp_exit ();";
   o->newline(-1) << "}";
 
-  if (! interruptible)
-    o->newline() << "local_irq_restore (flags);";
-  else
-    o->newline() << "preempt_enable_no_resched ();";
+  o->newline() << "#if INTERRUPTIBLE";
+  o->newline() << "preempt_enable_no_resched ();";
+  o->newline() << "#else";
+  o->newline() << "local_irq_restore (flags);";
+  o->newline() << "#endif";
 }
 
 
@@ -349,23 +352,23 @@ be_derived_probe_group::emit_module_decls (systemtap_session& s)
   s.op->newline() << "/* ---- begin/end probes ---- */";
   s.op->newline() << "static void enter_begin_probe (void (*fn)(struct context*), const char* pp) {";
   s.op->indent(1);
-  common_probe_entryfn_prologue (s.op, "STAP_SESSION_STARTING", "pp", false, true);
+  common_probe_entryfn_prologue (s.op, "STAP_SESSION_STARTING", "pp", false);
   s.op->newline() << "(*fn) (c);";
-  common_probe_entryfn_epilogue (s.op, false, true);
+  common_probe_entryfn_epilogue (s.op, false);
   s.op->newline(-1) << "}";
 
   s.op->newline() << "static void enter_end_probe (void (*fn)(struct context*), const char* pp) {";
   s.op->indent(1);
-  common_probe_entryfn_prologue (s.op, "STAP_SESSION_STOPPING", "pp", false, true);
+  common_probe_entryfn_prologue (s.op, "STAP_SESSION_STOPPING", "pp", false);
   s.op->newline() << "(*fn) (c);";
-  common_probe_entryfn_epilogue (s.op, false, true);
+  common_probe_entryfn_epilogue (s.op, false);
   s.op->newline(-1) << "}";
 
   s.op->newline() << "static void enter_error_probe (void (*fn)(struct context*), const char* pp) {";
   s.op->indent(1);
-  common_probe_entryfn_prologue (s.op, "STAP_SESSION_ERROR", "pp", false, true);
+  common_probe_entryfn_prologue (s.op, "STAP_SESSION_ERROR", "pp", false);
   s.op->newline() << "(*fn) (c);";
-  common_probe_entryfn_epilogue (s.op, false, true);
+  common_probe_entryfn_epilogue (s.op, false);
   s.op->newline(-1) << "}";
 
   s.op->newline() << "static struct stap_be_probe {";
diff --git a/translate.cxx b/translate.cxx
index b2ba5c72..17c37dc3 100644
--- a/translate.cxx
+++ b/translate.cxx
@@ -4908,6 +4908,9 @@ translate_pass (systemtap_session& s)
       s.op->newline() << "#ifndef MINSTACKSPACE";
       s.op->newline() << "#define MINSTACKSPACE 1024";
       s.op->newline() << "#endif";
+      s.op->newline() << "#ifndef INTERRUPTIBLE";
+      s.op->newline() << "#define INTERRUPTIBLE 1";
+      s.op->newline() << "#endif";
 
       // Overload processing
       s.op->newline() << "#ifndef STP_OVERLOAD_INTERVAL";