2007-03-21 David Smith <dsmith@redhat.com>

	PR 4146
	* tapsets.cxx (common_probe_entryfn_prologue): Added
	'interruptible' parameter.  If a probe is interruptible,
	interrupts are not disabled while the probe executes.  Preemption
	is disabled however.  Interruptible parameter defaults to false.
	(common_probe_entryfn_epilogue): Ditto.
	(be_derived_probe_group::emit_module_decl): Uses new
	'interruptible' parameter to mark begin/end probes as
	interruptible.
	(probe_derived_probe_group::emit_module): Initialize
	'actionremaining' with MAXACTION instead of initializing
	'actioncount' with 0.
	* translate.cxx (emit_common_header): Renamed 'actioncount' to
	'actionremaining'.  Turned logic around to initialize
	actionremaining to MAXACTION or MAXACTION_INTERRUPTIBLE then
	decrement it as actions occur.
	(translate_pass): Added MAXACTION_INTERRUPTIBLE initialization.
	* translate.h: Removed outdated comment portion.
	* stap.1.in: Documented MAXACTION_INTERRUPTIBLE.
	* NEWS: Added note about begin/end probes being run with
	interrupts enabled.

6 files changed, 69 insertions, 26 deletions

diff --git a/ChangeLog b/ChangeLog
index 1e277306..719dd386 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,27 @@
+2007-03-21  David Smith  <dsmith@redhat.com>
+
+	PR 4146
+	* tapsets.cxx (common_probe_entryfn_prologue): Added
+	'interruptible' parameter.  If a probe is interruptible,
+	interrupts are not disabled while the probe executes.  Preemption
+	is disabled however.  Interruptible parameter defaults to false.
+	(common_probe_entryfn_epilogue): Ditto.
+	(be_derived_probe_group::emit_module_decl): Uses new
+	'interruptible' parameter to mark begin/end probes as
+	interruptible.
+	(probe_derived_probe_group::emit_module): Initialize
+	'actionremaining' with MAXACTION instead of initializing
+	'actioncount' with 0.
+	* translate.cxx (emit_common_header): Renamed 'actioncount' to
+	'actionremaining'.  Turned logic around to initialize
+	actionremaining to MAXACTION or MAXACTION_INTERRUPTIBLE then
+	decrement it as actions occur.
+	(translate_pass): Added MAXACTION_INTERRUPTIBLE initialization.
+	* translate.h: Removed outdated comment portion.
+	* stap.1.in: Documented MAXACTION_INTERRUPTIBLE.
+	* NEWS: Added note about begin/end probes being run with
+	interrupts enabled.
+	
 2007-03-20  Frank Ch. Eigler  <fche@elastic.org>
 
 	PR 4224.
diff --git a/NEWS b/NEWS
index aa7feef4..a3d705d6 100644
--- a/NEWS
+++ b/NEWS
@@ -20,6 +20,9 @@
   address of the probe module, a broken-down memory consumption estimate, and
   the total number of probes.  This is meant as a debugging / auditing aid.
 
+- Begin/end probes are run with interrupts enabled (but with
+  preemption disabled).  This will allow begin/end probes to be
+  longer, to support generating longer reports.
 
 * What's new since version 0.5.10?
 
diff --git a/stap.1.in b/stap.1.in
index 02d6032f..787f8692 100644
--- a/stap.1.in
+++ b/stap.1.in
@@ -800,9 +800,15 @@ Maximum number of iterations to wait for locks on global variables
 before declaring possible deadlock and skipping the probe, default 1000.
 .TP
 MAXACTION
-Maximum number of statements to execute during any single probe hit,
+Maximum number of statements to execute during any single probe hit
+(with interrupts disabled),
 default 1000.
 .TP
+MAXACTION_INTERRUPTIBLE
+Maximum number of statements to execute during any single probe hit
+which is executed with interrupts enabled (such as begin/end probes),
+default (MAXACTION * 10).
+.TP
 MAXMAPENTRIES
 Maximum number of rows in any single global array, default 2048.
 .TP
diff --git a/tapsets.cxx b/tapsets.cxx
index 78d47dc7..0dddc33a 100644
--- a/tapsets.cxx
+++ b/tapsets.cxx
@@ -129,10 +129,12 @@ be_derived_probe::join_group (systemtap_session& s)
 // ------------------------------------------------------------------------
 void
 common_probe_entryfn_prologue (translator_output* o, string statestr,
-			       bool overload_processing = true)
+			       bool overload_processing = true,
+			       bool interruptible = false)
 {
   o->newline() << "struct context* __restrict__ c;";
-  o->newline() << "unsigned long flags;";
+  if (! interruptible)
+    o->newline() << "unsigned long flags;";
 
   if (overload_processing)
     o->newline() << "#if defined(STP_TIMING) || defined(STP_OVERLOAD)";
@@ -150,7 +152,10 @@ common_probe_entryfn_prologue (translator_output* o, string statestr,
   o->newline() << "static int _pfm_num_pmd_x;";
 #endif
 
-  o->newline() << "local_irq_save (flags);";
+  if (! interruptible)
+    o->newline() << "local_irq_save (flags);";
+  else
+    o->newline() << "preempt_disable ();";
 
   // Check for enough free enough stack space
   o->newline() << "if (unlikely ((((unsigned long) (& c)) & (THREAD_SIZE-1))"; // free space
@@ -185,7 +190,10 @@ common_probe_entryfn_prologue (translator_output* o, string statestr,
   o->newline() << "c->regs = 0;";
   o->newline() << "c->pi = 0;";
   o->newline() << "c->probe_point = 0;";
-  o->newline() << "c->actioncount = 0;";
+  if (! interruptible)
+    o->newline() << "c->actionremaining = MAXACTION;";
+  else
+    o->newline() << "c->actionremaining = MAXACTION_INTERRUPTIBLE;";
   o->newline() << "#ifdef STP_TIMING";
   o->newline() << "c->statp = 0;";
   o->newline() << "#endif";
@@ -194,7 +202,8 @@ common_probe_entryfn_prologue (translator_output* o, string statestr,
 
 void
 common_probe_entryfn_epilogue (translator_output* o,
-			       bool overload_processing = true)
+			       bool overload_processing = true,
+			       bool interruptible = false)
 {
   if (overload_processing)
     o->newline() << "#if defined(STP_TIMING) || defined(STP_OVERLOAD)";
@@ -262,7 +271,10 @@ common_probe_entryfn_epilogue (translator_output* o,
   o->newline(-1) << "probe_epilogue:"; // context is free
   o->indent(1);
 
-  o->newline() << "local_irq_restore (flags);";
+  if (! interruptible)
+    o->newline() << "local_irq_restore (flags);";
+  else
+    o->newline() << "preempt_enable_no_resched ();";
 }
 
 
@@ -276,17 +288,17 @@ be_derived_probe_group::emit_module_decls (systemtap_session& s)
   s.op->newline() << "/* ---- begin/end probes ---- */";
   s.op->newline() << "void enter_begin_probe (void (*fn)(struct context*)) {";
   s.op->indent(1);
-  common_probe_entryfn_prologue (s.op, "STAP_SESSION_STARTING", false);
+  common_probe_entryfn_prologue (s.op, "STAP_SESSION_STARTING", false, true);
   s.op->newline() << "c->probe_point = \"begin\";";
   s.op->newline() << "(*fn) (c);";
-  common_probe_entryfn_epilogue (s.op, false);
+  common_probe_entryfn_epilogue (s.op, false, true);
   s.op->newline(-1) << "}";
   s.op->newline() << "void enter_end_probe (void (*fn)(struct context*)) {";
   s.op->indent(1);
-  common_probe_entryfn_prologue (s.op, "STAP_SESSION_STOPPING", false);
+  common_probe_entryfn_prologue (s.op, "STAP_SESSION_STOPPING", false, true);
   s.op->newline() << "c->probe_point = \"end\";";
   s.op->newline() << "(*fn) (c);";
-  common_probe_entryfn_epilogue (s.op, false);
+  common_probe_entryfn_epilogue (s.op, false, true);
   s.op->newline(-1) << "}";
 }
 
@@ -4130,7 +4142,7 @@ profile_derived_probe_group::emit_module_decls (systemtap_session& s)
         {
           // Some lightweight inter-probe context resetting 
           // XXX: not quite right: MAXERRORS not respected
-          s.op->newline() << "c->actioncount = 0;";
+          s.op->newline() << "c->actionremaining = MAXACTION;";
         }
       s.op->newline() << "if (c->last_error == NULL) " << probes[i]->name << " (c);";
     }
diff --git a/translate.cxx b/translate.cxx
index d3f9467a..e656c5ee 100644
--- a/translate.cxx
+++ b/translate.cxx
@@ -808,7 +808,7 @@ c_unparser::emit_common_header ()
   o->newline() << "struct context {";
   o->newline(1) << "atomic_t busy;";
   o->newline() << "const char *probe_point;";
-  o->newline() << "unsigned actioncount;";
+  o->newline() << "int actionremaining;";
   o->newline() << "unsigned nesting;";
   o->newline() << "const char *last_error;";
   // NB: last_error is used as a health flag within a probe.
@@ -2045,9 +2045,9 @@ c_unparser::visit_statement (statement *s, unsigned actions)
 
   if (actions > 0)
     {
-      o->newline() << "c->actioncount += " << actions << ";";
+      o->newline() << "c->actionremaining -= " << actions << ";";
       // XXX: This check is inserted too frequently.
-      o->newline() << "if (unlikely (c->actioncount > MAXACTION)) {";
+      o->newline() << "if (unlikely (c->actionremaining <= 0)) {";
       o->newline(1) << "c->last_error = \"MAXACTION exceeded\";";
       o->newline() << "goto " << outlabel << ";";
       o->newline(-1) << "}";
@@ -2189,9 +2189,9 @@ c_unparser::visit_for_loop (for_loop *s)
   // condition
   o->newline(-1) << toplabel << ":";
 
-  // Emit an explicit actioncount increment here to cover the act of
-  // iteration.  Equivalently, it can stand for the evaluation of the
-  // condition expression.
+  // Emit an explicit action here to cover the act of iteration.
+  // Equivalently, it can stand for the evaluation of the condition
+  // expression.
   o->indent(1);
   visit_statement (0, 1);
 
@@ -2413,8 +2413,8 @@ c_unparser::visit_foreach_loop (foreach_loop *s)
       // condition
       o->newline(-1) << toplabel << ":";
 
-      // Emit an explicit actioncount increment here to cover the act of
-      // iteration.  Equivalently, it can stand for the evaluation of the
+      // Emit an explicit action here to cover the act of iteration.
+      // Equivalently, it can stand for the evaluation of the
       // condition expression.
       o->indent(1);
       visit_statement (0, 1);
@@ -4072,6 +4072,9 @@ translate_pass (systemtap_session& s)
       s.op->newline() << "#ifndef MAXACTION";
       s.op->newline() << "#define MAXACTION 1000";
       s.op->newline() << "#endif";
+      s.op->newline() << "#ifndef MAXACTION_INTERRUPTIBLE";
+      s.op->newline() << "#define MAXACTION_INTERRUPTIBLE (MAXACTION * 10)";
+      s.op->newline() << "#endif";
       s.op->newline() << "#ifndef MAXTRYLOCK";
       s.op->newline() << "#define MAXTRYLOCK MAXACTION";
       s.op->newline() << "#endif";
diff --git a/translate.h b/translate.h
index ab0ad98b..eae7534f 100644
--- a/translate.h
+++ b/translate.h
@@ -58,12 +58,7 @@ struct unparser
   // struct context {
   //   unsigned errorcount;
   //   unsigned busy;
-  //   unsigned actioncount;
-  //   unsigned nesting;
-  //   union {
-  //     struct { .... } probe_NUM_locals;
-  //     struct { .... } function_NAME_locals;
-  //   } locals [MAXNESTING];
+  //   ...
   // } context [MAXCONCURRENCY];
 
   virtual void emit_global (vardecl* v) = 0;