adding statements in alias definition as epilogue

adding statements in alias definition as epilogue

[Li Guanglei]

Currently the statement block that follows an alias definition is 
implicitly added as a prologue to any probe that refers to the alias. 
But it seems to me that only allowing adding those statements 
contained in an alias definition as a prologue makes me to lose some 
controls over them.

For example, I have a tapsets:

probe addevent.tskdispatch.cpuidle
         = kernel.inline("idle_balance")
{
         log_cpuidle_tracedata(HOOKID_TASK_CPUIDLE, backtrace)
}

What I want is to let user specify whether to log backtrace in his 
scripts, like the following:

stap -e "probe addevent.tskdispatch.cpuidle { backtrace=1 } -I 
LKET_TAPSETS

But unfortunately, it won't work because "backtrace=1" is added after 
"log_cpuidle_tracedata(HOOKID_TASK_CPUIDLE, backtrace)". So how about 
providing a way to specify whether the statements in a alias 
definition could be added as prologue or epilogue to any probe that 
refers to that alias?

Re: adding statements in alias definition as epilogue

[bibo,mao]

you can use parameter to implement this
probe start{
	backtrace = $1;
}

Li Guanglei wrote:
> Currently the statement block that follows an alias definition is 
> implicitly added as a prologue to any probe that refers to the alias. 
> But it seems to me that only allowing adding those statements contained 
> in an alias definition as a prologue makes me to lose some controls over 
> them.
> 
> For example, I have a tapsets:
> 
> probe addevent.tskdispatch.cpuidle
>         = kernel.inline("idle_balance")
> {
>         log_cpuidle_tracedata(HOOKID_TASK_CPUIDLE, backtrace)
> }
> 
> What I want is to let user specify whether to log backtrace in his 
> scripts, like the following:
> 
> stap -e "probe addevent.tskdispatch.cpuidle { backtrace=1 } -I LKET_TAPSETS
> 
> But unfortunately, it won't work because "backtrace=1" is added after 
> "log_cpuidle_tracedata(HOOKID_TASK_CPUIDLE, backtrace)". So how about 
> providing a way to specify whether the statements in a alias definition 
> could be added as prologue or epilogue to any probe that refers to that 
> alias?
> 

Re: adding statements in alias definition as epilogue

[Frank Ch. Eigler]

Li Guanglei <guanglei@cn.ibm.com> writes:

> [...] What I want is to let user specify whether to log backtrace in his
> scripts, like the following:
> stap -e "probe addevent.tskdispatch.cpuidle { backtrace=1 } [...]
> But unfortunately, it won't work because "backtrace=1" is added after
> "log_cpuidle_tracedata(HOOKID_TASK_CPUIDLE, backtrace)". [...]

You can have two alias definitions:

probe addevent.tskdispatch.cpuidle.backtrace = addevent.tskdispatch.cpuidle {
  backtrace = 1
}

probe addevent.tskdispatch.cpuidle = .... {
  if (backtrace) log_more ();
}


- FChE

Re: adding statements in alias definition as epilogue

[Guang Lei Li]

>> [...] What I want is to let user specify whether to log backtrace in 
his
>> scripts, like the following:
>> stap -e "probe addevent.tskdispatch.cpuidle { backtrace=1 } [...]
>> But unfortunately, it won't work because "backtrace=1" is added after
>> "log_cpuidle_tracedata(HOOKID_TASK_CPUIDLE, backtrace)". [...]

> You can have two alias definitions:

> probe addevent.tskdispatch.cpuidle.backtrace = 
addevent.tskdispatch.cpuidle {
>   backtrace = 1
> }

> probe addevent.tskdispatch.cpuidle = .... {
>   if (backtrace) log_more ();
> }
But this won't work either. Because addevent.tskdispatch.cpuidle.backtrace 
will
be expanded into:

probe ... {
    if (backtrace) log_more ();
    backtrace = 1
}

I tried the following patch:
root:/home/lgl/systemtap/src> diff -uprN elaborate.cxx.ori elaborate.cxx
--- elaborate.cxx.ori   2006-04-04 15:34:27.000000000 +0800
+++ elaborate.cxx       2006-04-04 15:34:53.000000000 +0800
@@ -334,15 +334,15 @@ alias_expansion_builder
     // there's concatenated code here and we only want one vardecl per
     // resulting variable.

-    for (unsigned i = 0; i < alias->body->statements.size(); ++i)
+    for (unsigned i = 0; i < use->body->statements.size(); ++i)
       {
-       statement *s = 
deep_copy_visitor::deep_copy(alias->body->statements[i]);
+       statement *s = 
deep_copy_visitor::deep_copy(use->body->statements[i]);
        n->body->statements.push_back(s);
       }

-    for (unsigned i = 0; i < use->body->statements.size(); ++i)
+    for (unsigned i = 0; i < alias->body->statements.size(); ++i)
       {
-       statement *s = 
deep_copy_visitor::deep_copy(use->body->statements[i]);
+       statement *s = 
deep_copy_visitor::deep_copy(alias->body->statements[i]);
        n->body->statements.push_back(s);
       }

And it can cause the statements in alias definitions to be put as 
epilogue.

So do we need the option to specify whether to put the statements in an 
alias definition as prologue or epilogue?
that is to say:

probe derived_probes := probe_alias {
   statements_in_derived_probes
}

probe probe_alias_definitions {
   statements_in_alias_definitions
}

and systemtap will generate:

probe derived_probes {
   statements_in_derived_probes
   statements_in_alias_definitions
}

And if you change "probe derived_probes := probe_alias" to "probe 
derived_probes = probe_alias", systemtap will generate:
probe derived_probes { 
   statements_in_alias_definitions
   statements_in_derived_probes
}

I am not sure if other people than me have this requirement too. But it 
seems to me that being able to put statements in an alias definitions as 
epilogue enables you to change the default behaviors define in original 
probe alias, which should be useful.


Li Guanglei

- Linux Performance, China Systems & Technology Lab
China Development Lab, Beijing Tel: 86-10-82782244 Ext.3516
Email: liguangl@cn.ibm.com, IMAP: guanglei@cn.ibm.com

Re: adding statements in alias definition as epilogue

[Frank Ch. Eigler]

Hi -

> > You can have two alias definitions: [...]
> But this won't work either.  [...]

Indeed not, you're right.  OK, how about forking it into independent twins?

probe addevent.tskdispatch.cpuidle = kernel.inline("idle_balance") {
 log_cpuidle_tracedata(HOOKID_TASK_CPUIDLE, 0)
}
probe addevent.tskdispatch.cpuidle.backtrace = kernel.inline("idle_balance") {
 log_cpuidle_tracedata(HOOKID_TASK_CPUIDLE, 1)
}


This assumes that per-probe backtrace configuration makes more sense
than, say, a single global variable.


> [...]  So do we need the option to specify whether to put the
> statements in an alias definition as prologue or epilogue?  [...]  I
> am not sure if other people than me have this requirement too. [...]

It still seems like a big step to introduce this inverted data/control
flow.  We would have to consider composing multiple levels of aliases,
to make sure a programmer and a user can reason easily about what
should happen.

- FChE

Re: adding statements in alias definition as epilogue

[Guang Lei Li]

> OK, how about forking it into independent twins?
> 
> probe addevent.tskdispatch.cpuidle = kernel.inline("idle_balance") {
>  log_cpuidle_tracedata(HOOKID_TASK_CPUIDLE, 0)
> }
> probe addevent.tskdispatch.cpuidle.backtrace = 
kernel.inline("idle_balance") {
>  log_cpuidle_tracedata(HOOKID_TASK_CPUIDLE, 1)
> }
> 
I ever thought abut doing like this. But the biggest problem of it is not 
only the redundant codes, but it will have trouble if you use wildcard(*) 
to specify a group of hooks, for example:

probe addevent.tskdispatching.* {}

will cause both addevent.tskdispatch.cpuidle.backtrace & 
addevent.tskdispatch.cpuidle to be triggered.

> This assumes that per-probe backtrace configuration makes more sense
> than, say, a single global variable.

'backtrace' used inside the probe definitions is a local variable, which 
is cheaper than using a global variable. And it also enables to turn 
on/off backtrace for each event separately instead of turn on/off all 
backtraces as a whole.

> 
> It still seems like a big step to introduce this inverted data/control
> flow.  We would have to consider composing multiple levels of aliases,
> to make sure a programmer and a user can reason easily about what
> should happen.
> 
> - FChE
Yes. Maybe some syntax for specifying different kinds of aliases need to 
be introduced.

Besides my specific need of alias definition as epilogue, I think it is 
also useful if you want a filter:

probe derived_probe := alias_with_filter_codes  {
   filter_on = 1
   scsi_lun = 2
   scsi_dev_major = 3
}

- Li Guanglei

Re: adding statements in alias definition as epilogue

[Frank Ch. Eigler]

Guang Lei Li <liguangl@cn.ibm.com> writes:

> [...]
> probe addevent.tskdispatching.* {}
>
> will cause both addevent.tskdispatch.cpuidle.backtrace & 
> addevent.tskdispatch.cpuidle to be triggered.

Actually, it doesn't: the wildcard only consumes one component of the
probe point name.


> > This assumes that per-probe backtrace configuration makes more sense
> > than, say, a single global variable.
> 
> 'backtrace' used inside the probe definitions is a local variable, which 
> is cheaper than using a global variable.

Only slightly (one extra shared lock).

> And it also enables to turn on/off backtrace for each event
> separately instead of turn on/off all backtraces as a whole.

I see that, but is this capability important & useful?  Plus, apropos
an earlier message, how does the concept of high-performance binary
tracing mesh at all with slow and bulky backtrace generation?


> [...]  Besides my specific need of alias definition as epilogue, I
> think it is also useful if you want a filter: [...]
>
> probe derived_probe := alias_with_filter_codes  {
>    filter_on = 1
>    scsi_lun = 2
>    scsi_dev_major = 3
> }

This style of usage could be accommodated with explicit code
(analogous to "if (target() != pid()) next") instead of alias epilogues.


- FChE

Re: adding statements in alias definition as epilogue

[Guang Lei Li]

systemtap-owner@sourceware.org wrote on 2006-04-05 20:03:23:

> Guang Lei Li <liguangl@cn.ibm.com> writes:
> 
> > [...]
> > probe addevent.tskdispatching.* {}
> >
> > will cause both addevent.tskdispatch.cpuidle.backtrace & 
> > addevent.tskdispatch.cpuidle to be triggered.
> 
> Actually, it doesn't: the wildcard only consumes one component of the
> probe point name.
I tried, and found:
probe addevent.tskdispatching.* = probe addevent.tskdispatching.cpuidle
but addevent.tskdispatching.cpuidle.backtrace is removed away from the 
generated probes.

> 
> > And it also enables to turn on/off backtrace for each event
> > separately instead of turn on/off all backtraces as a whole.
> 
> I see that, but is this capability important & useful?  Plus, apropos
> an earlier message, how does the concept of high-performance binary
> tracing mesh at all with slow and bulky backtrace generation?
> 
Because current backtrace printing is costly, so I want to let the user to 
only turn on backtrace for those events necessary.
But you are right, backtrace printing is too slow, we need a faster way of 
getting the backtrace

> 
> > [...]  Besides my specific need of alias definition as epilogue, I
> > think it is also useful if you want a filter: [...]
> >
> > probe derived_probe := alias_with_filter_codes  {
> >    filter_on = 1
> >    scsi_lun = 2
> >    scsi_dev_major = 3
> > }
> 
> This style of usage could be accommodated with explicit code
> (analogous to "if (target() != pid()) next") instead of alias epilogues.
It is only suitable for filtering by pid, which provides a way of passing 
in the target pid by "stap -c cmd" or "stap -x targetid". But how can you 
pass in filter_on, scsi_lun & scsi_dev_major? The newly arguments support 
in stap could make it, but it seems not a good solution in this example.

> 
> 
> - FChE

Re: adding statements in alias definition as epilogue

[Frank Ch. Eigler]

Guang Lei Li <liguangl@cn.ibm.com> writes:

> [...] But you are right, backtrace printing is too slow, we need a
> faster way of getting the backtrace

Unfortunately, even just gathering the PC address history requires
searching through up to several kilobytes of stack frames - that must
be at least 40K cycles all by itself.  My guess is that converting the
result to symbol+offset strings is almost as much work again (though
could perhaps be deferred).

> > > probe derived_probe := alias_with_filter_codes  {
> > >    filter_on = 1
> > >    scsi_lun = 2
> > >    scsi_dev_major = 3
> > > }
> > 
> > This style of usage could be accommodated with explicit code
> > (analogous to "if (target() != pid()) next") instead of alias epilogues.

> [...]  But how can you pass in filter_on, scsi_lun & scsi_dev_major?

Like this:

probe derived_probe:= base_alias_without_filter_codes_without_tracing {
   if (! (scsi_lun == 2 && scsi_dev_major == 3)) next
   base_trace_function (...)
}

But you got me thinking about the alias epilogue option.  It would
solve this problem in a nicer way.  Anyone with arguments pro/con?


> The newly arguments support in stap could make it, but it seems not
> a good solution in this example.

Whether the numbers are hardcoded in script or pasted from the command
line does not appear to make any difference.


- FChE