[DTrace-devel] [PATCH 3/3] Verify the activity state before calling each clause

[Kris Van Hees]

On Wed, Sep 30, 2020 at 02:29:15PM -0700, Eugene Loh wrote:
> On 09/30/2020 10:33 AM, Kris Van Hees wrote:
> 
> > On Wed, Sep 30, 2020 at 09:47:54AM -0700, Eugene Loh wrote:
> >> Reviewed-by: Eugene Loh <eugene.loh at oracle.com>
> >> with the usual minor comment/question:
> >>
> >> On 09/29/2020 04:14 PM, Kris Van Hees wrote:
> >>> When a clause contains an exit() action, processing of all other
> >>> clauses (even for the same probe firing) must not be executed
> >>> anymore.  The activity state check therefore needs to not just happen
> >>> in the trampoline, but also before every clause within the program.
> >> Arguably, the activity check is no longer needed in the trampoline at
> >> all, right?  (Leaving it in is a minor implementation choice.  It saves
> >> the execution of a couple of instructions if probes fire at the end of a
> >> run.)  Maybe change the last sentence to:
> >>
> >> "The activity state check therefore needs to happen before every clause
> >> within the program."
> > Actually, it isn't left in the trampoline jsut to save a few instructions
> > from being executed since that gain is minimal).  It is there because not
> > having it could result in rather unpredictable behaviour in which clauses
> > end up being executed for a probe if the state is set to ACTIVE *after* a
> > probe fires but before we've iterated over all its clauses.  If a given
> > probe has n clauses, you could end up with clauses [0 .. i] being skipped
> > while clauses [i .. n] get executed.  Given the way clauses are executed
> > in a strict first to last order and script writers depend on that), having
> > only some of the clauses be executed in the beginning can lead to strange
> > results.
> 
> I'm confused.  If the activity state is not right (whether in the 
> trampoline prologue or before any clause), we jump to lbl_exit, which 
> comes after all the clauses.  If, when the first clause is about to be 
> executed, the activity state is not correct, then none of the clauses 
> will be executed, regardless of whether the activity state is checked in 
> the trampoline prologue or not.

Ah yes, I see what you mean.  I was still thinking in terms of the legacy
implementation where we keep looping through the ECBs and check the state
for each.  In the new implementation I am checking before each clause (which
is equivalent to checking for each ECB) but indeed jumping to the overall
exit of the trampoline rather than continuing with the next clause.

Too many different mechanisms to keep track of in my brain :)

So yes, the check at the beginning of the trampoline is indeed not strictly
necessary but it is good to have anyway to make the overhead of probes being
attached while we haven't started tracing yet minimal.

> > We have a similar situation when probing ends, where only [0 .. i] might be
> > executed, and not [i .. n] but that is specifically implied by the semantics
> > of the exit() action.
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel