[PATCH] Fix target_set tapset.

[PATCH] Fix target_set tapset.

[Przemyslaw Pawelczyk]

Add pid removal on exit syscall. Use dwarfless syscall probe aliases.
Correct formatting.
---
 tapset/target_set.stp |   28 ++++++++++++++++++----------
 1 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/tapset/target_set.stp b/tapset/target_set.stp
index c7878c5..e27c3c7 100644
--- a/tapset/target_set.stp
+++ b/tapset/target_set.stp
@@ -3,25 +3,33 @@ global _target_set # map: target-set-pid -> ancestor-pid
 
 function target_set_pid (pid)
 {
-  return ([pid] in _target_set)	 
+	return ([pid] in _target_set)
 }
 
 probe begin
 {
-  if (target()) _target_set [target()] = stp_pid()
+	if (target())
+		_target_set[target()] = stp_pid()
 }
 
-probe syscall.fork.return
+probe nd_syscall.fork.return
 {
-  pid=pid()
-  if (pid in _target_set) next
-  ppid=ppid()
-  if (ppid in _target_set) _target_set[pid]=ppid
+	pid = pid()
+	if (pid in _target_set)
+		next
+	ppid = ppid()
+	if (ppid in _target_set)
+		_target_set[pid] = ppid
+}
+
+probe nd_syscall.exit
+{
+	delete _target_set[pid()]
 }
 
 function target_set_report ()
 {
-  printf("target set:\n")
-  foreach (pid in _target_set+)
-    printf("%d begat %d\n", _target_set[pid], pid)
+	printf("target set:\n")
+	foreach (pid in _target_set+)
+		printf("%d begat %d\n", _target_set[pid], pid)
 }
-- 
1.5.6.5

Re: [PATCH] Fix target_set tapset.

[Josh Stone]

On 06/13/2009 04:10 PM, Przemyslaw Pawelczyk wrote:
> Add pid removal on exit syscall. Use dwarfless syscall probe aliases.
> Correct formatting.
> ---
[...]
> -probe syscall.fork.return
> +probe nd_syscall.fork.return

What do you think about preferring process.begin for utrace-enabled
kernels?  That should be lower overhead than a kprobe trap.

> +probe nd_syscall.exit
> +{
> +	delete _target_set[pid()]
>  }

The problem is that this makes target_set_report() useless after the
processes have exited (e.g. in an end probe).  I think we should just
trust that the target process won't beget more than MAXMAPENTRIES
children.  At the default 2048, that will probably be fine most of the time.

If you really want this, perhaps we could instead add the pid to a death
array, and then have a function to clear those out.  The clear could
either be explicitly called, or perhaps it would be an implicit call at
the end of target_set_report.  Then the calling script can do periodic
clear/reports if it knows there will be more than MAXMAPENTRIES children.


Josh

Re: [PATCH] Fix target_set tapset.

[Przemysław Pawełczyk]

On Mon, Jun 15, 2009 at 21:11, Josh Stone<jistone@redhat.com> wrote:
> On 06/13/2009 04:10 PM, Przemyslaw Pawelczyk wrote:
>> Add pid removal on exit syscall. Use dwarfless syscall probe aliases.
>> Correct formatting.
>> ---
> [...]
>> -probe syscall.fork.return
>> +probe nd_syscall.fork.return
>
> What do you think about preferring process.begin for utrace-enabled
> kernels?  That should be lower overhead than a kprobe trap.

This sounds good, however it leads to different path-execution on
various kernels and that is not good. IMHO better would be creating
another target_set-like tapset, but utrace-based only.

>> +probe nd_syscall.exit
>> +{
>> +     delete _target_set[pid()]
>>  }
>
> The problem is that this makes target_set_report() useless after the
> processes have exited (e.g. in an end probe).  I think we should just
> trust that the target process won't beget more than MAXMAPENTRIES
> children.  At the default 2048, that will probably be fine most of the time.

It should be useless after the precesses have exited, because then
target_set contains only target() or is even empty (depends on what we
mean by the processes), right?
If it is really target_set, then as name suggests it should be valid
all the time. Pid collisions are rare, but not impossible.

> If you really want this, perhaps we could instead add the pid to a death
> array, and then have a function to clear those out.  The clear could
> either be explicitly called, or perhaps it would be an implicit call at
> the end of target_set_report.  Then the calling script can do periodic
> clear/reports if it knows there will be more than MAXMAPENTRIES children.

Death pid array is other solution, but clearing routine definitely
shouldn't be located in target_set_report. Moreover target_set_report
shouldn't even print death pids, but new function could do this
(target_set_already_dead_report?). And if we're after this option,
then I can agree only if there is a separate clear function without
implicit calls in reporting functions with exception for functions
clearly pointing this out (target_set_already_dead_report_and_clear?).

> Josh

Regards.

-- 
Przemysław Pawełczyk

Re: [PATCH] Fix target_set tapset.

[Josh Stone]

On 06/16/2009 04:13 PM, PrzemysÂław PaweÂłczyk wrote:
> On Mon, Jun 15, 2009 at 21:11, Josh Stone<jistone@redhat.com> wrote:
>> On 06/13/2009 04:10 PM, Przemyslaw Pawelczyk wrote:
>>> Add pid removal on exit syscall. Use dwarfless syscall probe aliases.
>>> Correct formatting.
>>> ---
>> [...]
>>> -probe syscall.fork.return
>>> +probe nd_syscall.fork.return
>>
>> What do you think about preferring process.begin for utrace-enabled
>> kernels?  That should be lower overhead than a kprobe trap.
> 
> This sounds good, however it leads to different path-execution on
> various kernels and that is not good. IMHO better would be creating
> another target_set-like tapset, but utrace-based only.

Why is that not good?  As long as the semantics are the same, it should
be fine.  What problems do you foresee with using different paths?

I definitely don't want to split them, as that hurts portability and
confuses users.

> 
>>> +probe nd_syscall.exit
>>> +{
>>> +     delete _target_set[pid()]
>>>  }
>>
>> The problem is that this makes target_set_report() useless after the
>> processes have exited (e.g. in an end probe).  I think we should just
>> trust that the target process won't beget more than MAXMAPENTRIES
>> children.  At the default 2048, that will probably be fine most of the time.
> 
> It should be useless after the precesses have exited, because then
> target_set contains only target() or is even empty (depends on what we
> mean by the processes), right?
> If it is really target_set, then as name suggests it should be valid
> all the time. Pid collisions are rare, but not impossible.

Pid collisions are a valid point.  Remember too that we're storing the
ppid() as the array value.  If the parent dies before the child, and the
ppid is reused, then you could have a confusing ancestry.  There may
even be loops.

Anyway, my worry was that it may be seen as a regression from the old
code.  When I tested this patch, I used a script like:

  probe end { target_set_report() }

With the old code, I saw a list of "x begat y".  With your patch, I saw
nothing -- because you deleted the pids when they exited.  We can make
arguments that this may be more correct, as long as we're ok with the
changed semantics.

> 
>> If you really want this, perhaps we could instead add the pid to a death
>> array, and then have a function to clear those out.  The clear could
>> either be explicitly called, or perhaps it would be an implicit call at
>> the end of target_set_report.  Then the calling script can do periodic
>> clear/reports if it knows there will be more than MAXMAPENTRIES children.
> 
> Death pid array is other solution, but clearing routine definitely
> shouldn't be located in target_set_report. Moreover target_set_report
> shouldn't even print death pids, but new function could do this
> (target_set_already_dead_report?). And if we're after this option,
> then I can agree only if there is a separate clear function without
> implicit calls in reporting functions with exception for functions
> clearly pointing this out (target_set_already_dead_report_and_clear?).

Now I think you're just messing with me, but ok, I see that death arrays
are making this overly complex.  We should just decide whether the
records of dead pids should be kept around.

Frank, you wrote this tapset -- any opinion?

Josh

Re: [PATCH] Fix target_set tapset.

[Frank Ch. Eigler]

Josh Stone <jistone@redhat.com> writes:

> [...]
>> Death pid array is other solution, but clearing routine definitely
>> shouldn't be located in target_set_report. [...]
>
> Now I think you're just messing with me, but ok, I see that death arrays
> are making this overly complex.  We should just decide whether the
> records of dead pids should be kept around.

It can be useful to remember the dead ones sometimes, but perhaps not
for the purposes of this particular tapset, which is imagined to be
useful for controlling future probes.

If one wants the history too, a sibling tapset could be easily
authored, with a target_set_history_report() function triggering its
inclusion.  Or make it all moot with PR6525.

- FChE

Re: [PATCH] Fix target_set tapset.

[Przemysław Pawełczyk]

2009/6/17 Josh Stone <jistone@redhat.com>:
> On 06/16/2009 04:13 PM, Przemysław Pawełczyk wrote:
>> On Mon, Jun 15, 2009 at 21:11, Josh Stone<jistone@redhat.com> wrote:
>>> On 06/13/2009 04:10 PM, Przemyslaw Pawelczyk wrote:
>>>> Add pid removal on exit syscall. Use dwarfless syscall probe aliases.
>>>> Correct formatting.
>>>> ---
>>> [...]
>>>> -probe syscall.fork.return
>>>> +probe nd_syscall.fork.return
>>>
>>> What do you think about preferring process.begin for utrace-enabled
>>> kernels?  That should be lower overhead than a kprobe trap.
>>
>> This sounds good, however it leads to different path-execution on
>> various kernels and that is not good. IMHO better would be creating
>> another target_set-like tapset, but utrace-based only.
>
> Why is that not good?  As long as the semantics are the same, it should
> be fine.  What problems do you foresee with using different paths?

Probing in user-space is not the same as probing in kernel-space. The
only problem I foresee are different results from similar kernels
depending on having (or not) utrace-patch.

> I definitely don't want to split them, as that hurts portability and
> confuses users.

True point.

>>>> +probe nd_syscall.exit
>>>> +{
>>>> +     delete _target_set[pid()]
>>>>  }
>>>
>>> The problem is that this makes target_set_report() useless after the
>>> processes have exited (e.g. in an end probe).  I think we should just
>>> trust that the target process won't beget more than MAXMAPENTRIES
>>> children.  At the default 2048, that will probably be fine most of the time.
>>
>> It should be useless after the precesses have exited, because then
>> target_set contains only target() or is even empty (depends on what we
>> mean by the processes), right?
>> If it is really target_set, then as name suggests it should be valid
>> all the time. Pid collisions are rare, but not impossible.
>
> Pid collisions are a valid point.  Remember too that we're storing the
> ppid() as the array value.  If the parent dies before the child, and the
> ppid is reused, then you could have a confusing ancestry.  There may
> even be loops.

You're right once again. Parent-child relation also should be fixed
during execution. You put me to shame, because I forgot about it...

> Anyway, my worry was that it may be seen as a regression from the old
> code.  When I tested this patch, I used a script like:
>
>  probe end { target_set_report() }
>
> With the old code, I saw a list of "x begat y".  With your patch, I saw
> nothing -- because you deleted the pids when they exited.  We can make
> arguments that this may be more correct, as long as we're ok with the
> changed semantics.

I see that I lost part of my previous mail (accidental delete?), where
I suggested introducing some global switch to define behavior -- old
(by default = 0) vs proper one (= 1). What you think about it?

>>> If you really want this, perhaps we could instead add the pid to a death
>>> array, and then have a function to clear those out.  The clear could
>>> either be explicitly called, or perhaps it would be an implicit call at
>>> the end of target_set_report.  Then the calling script can do periodic
>>> clear/reports if it knows there will be more than MAXMAPENTRIES children.
>>
>> Death pid array is other solution, but clearing routine definitely
>> shouldn't be located in target_set_report. Moreover target_set_report
>> shouldn't even print death pids, but new function could do this
>> (target_set_already_dead_report?). And if we're after this option,
>> then I can agree only if there is a separate clear function without
>> implicit calls in reporting functions with exception for functions
>> clearly pointing this out (target_set_already_dead_report_and_clear?).
>
> Now I think you're just messing with me, but ok, I see that death arrays
> are making this overly complex.  We should just decide whether the
> records of dead pids should be kept around.

I didn't want to sound rough and really sorry if it is how you read
it. I always strongly oppose to hidden yet not obvious duties of
functions.

> Frank, you wrote this tapset -- any opinion?

Indeed. Frank, please write what you think.

> Josh

Regards.

-- 
Przemysław Pawełczyk

Re: [PATCH] Fix target_set tapset.

[Josh Stone]

On 06/17/2009 12:05 PM, PrzemysÂław PaweÂłczyk wrote:
> 2009/6/17 Josh Stone <jistone@redhat.com>:
>> On 06/16/2009 04:13 PM, PrzemysÂław PaweÂłczyk wrote:
>>> On Mon, Jun 15, 2009 at 21:11, Josh Stone<jistone@redhat.com> wrote:
>>>> On 06/13/2009 04:10 PM, Przemyslaw Pawelczyk wrote:
>>>>> Add pid removal on exit syscall. Use dwarfless syscall probe aliases.
>>>>> Correct formatting.
>>>>> ---
>>>> [...]
>>>>> -probe syscall.fork.return
>>>>> +probe nd_syscall.fork.return
>>>>
>>>> What do you think about preferring process.begin for utrace-enabled
>>>> kernels?  That should be lower overhead than a kprobe trap.
>>>
>>> This sounds good, however it leads to different path-execution on
>>> various kernels and that is not good. IMHO better would be creating
>>> another target_set-like tapset, but utrace-based only.
>>
>> Why is that not good?  As long as the semantics are the same, it should
>> be fine.  What problems do you foresee with using different paths?
> 
> Probing in user-space is not the same as probing in kernel-space. The
> only problem I foresee are different results from similar kernels
> depending on having (or not) utrace-patch.

Well utrace is a kernel mechanism, and it shouldn't have too different
results, but that's ok.  We can use nd_syscall for now and perhaps
consider other enhancements later.

>> Pid collisions are a valid point.  Remember too that we're storing the
>> ppid() as the array value.  If the parent dies before the child, and the
>> ppid is reused, then you could have a confusing ancestry.  There may
>> even be loops.
> 
> You're right once again. Parent-child relation also should be fixed
> during execution. You put me to shame, because I forgot about it...

It's a tough call, because it's also not correct to say that the new
parent begat the child -- it's more like an adoption.

The more I look, the more I think the real value of this tapset is in
target_set_pid, and we shouldn't worry much about the intricacies of
target_set_report.

>> Anyway, my worry was that it may be seen as a regression from the old
>> code.  When I tested this patch, I used a script like:
>>
>>  probe end { target_set_report() }
>>
>> With the old code, I saw a list of "x begat y".  With your patch, I saw
>> nothing -- because you deleted the pids when they exited.  We can make
>> arguments that this may be more correct, as long as we're ok with the
>> changed semantics.
> 
> I see that I lost part of my previous mail (accidental delete?), where
> I suggested introducing some global switch to define behavior -- old
> (by default = 0) vs proper one (= 1). What you think about it?

Since target_set_pid is more useful with what you call the proper mode,
I'm starting to think we should just go that way and forget the old mode.

>> Now I think you're just messing with me, but ok, I see that death arrays
>> are making this overly complex.  We should just decide whether the
>> records of dead pids should be kept around.
> 
> I didn't want to sound rough and really sorry if it is how you read
> it. I always strongly oppose to hidden yet not obvious duties of
> functions.

I'm not hurt -- I meant that with a smile. :)

I'm going to rest my objection now and commit your patch.  We can create
Frank's suggested target_set_history_report() later if someone asks for it.

Josh

[PATCH 2/2] Add test for target_set tapset.

[Przemyslaw Pawelczyk]

---
 testsuite/systemtap.base/target_set.exp |   63 +++++++++++++++++++++++++++++++
 testsuite/systemtap.base/target_set.stp |   16 ++++++++
 2 files changed, 79 insertions(+), 0 deletions(-)
 create mode 100644 testsuite/systemtap.base/target_set.exp
 create mode 100644 testsuite/systemtap.base/target_set.stp

diff --git a/testsuite/systemtap.base/target_set.exp b/testsuite/systemtap.base/target_set.exp
new file mode 100644
index 0000000..1b80839
--- /dev/null
+++ b/testsuite/systemtap.base/target_set.exp
@@ -0,0 +1,63 @@
+# Test for target_set tapset
+
+set test target_set
+
+if {![installtest_p]} { untested $test; continue }
+
+set file $srcdir/$subdir/target_set.stp
+set time 1
+set stap_cmd  "( ( /bin/sleep $time ) && ( /bin/sleep $time ) )"
+set stap_cmd_child_level 3
+
+proc abort {} {
+	global test
+	fail $test
+	exit
+}
+
+proc expect_target_set generations {
+	expect {
+		"^target set:\r\n" { expect_target_set_pids $generations }
+		timeout { abort }
+	}
+}
+
+proc expect_target_set_pids generations {
+	global test
+	global stp_pid
+	for {set i 0} {$i < $generations} {incr i} {
+		expect {
+			-timeout 0
+			-re {^([0-9]+) begat ([0-9]+)\r\n} { set pid_array($expect_out(1,string)) $expect_out(2,string) }
+			timeout { abort }
+		}
+	}
+	set pid_it $stp_pid
+	while {[info exists pid_array($pid_it)]} {
+		if {[exec pgrep -P $pid_it] != $pid_array($pid_it)} {
+			abort
+		}
+		set pid_it $pid_array($pid_it)
+	}
+}
+
+spawn stap $file $time -c "$stap_cmd"
+
+expect {
+	-re {^(\d+)\r\n} { set stp_pid $expect_out(1,string) }
+	timeout { abort }
+}
+
+set timeout [expr $time + 1]
+
+expect_target_set_pids 1
+expect_target_set $stap_cmd_child_level
+expect_target_set $stap_cmd_child_level
+expect_target_set 0
+
+expect {
+	eof {}
+	timeout { abort }
+}
+
+pass $test
diff --git a/testsuite/systemtap.base/target_set.stp b/testsuite/systemtap.base/target_set.stp
new file mode 100644
index 0000000..c48040b
--- /dev/null
+++ b/testsuite/systemtap.base/target_set.stp
@@ -0,0 +1,16 @@
+probe begin
+{
+	println(stp_pid())
+	printf("%d begat %d\n", stp_pid(), target())
+}
+
+probe nd_syscall.nanosleep
+{
+	if (target_set_pid(pid()) && @cast(req_uaddr, "timespec", "<linux/time.h>")->tv_sec == $1)
+		target_set_report()
+}
+
+probe end
+{
+	target_set_report()
+}
-- 
1.5.6.5

[PATCH 1/2] Fix target_set tapset.

[Przemyslaw Pawelczyk]

Revise acquiring of pid and ppid in fork.return probe -- use returnval()
and pid() instead of pid() and ppid() respectively. Add pid removal on
exit syscall. Use dwarfless syscall probe aliases. Correct formatting.
---
 tapset/target_set.stp |   28 ++++++++++++++++++----------
 1 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/tapset/target_set.stp b/tapset/target_set.stp
index c7878c5..9a4bece 100644
--- a/tapset/target_set.stp
+++ b/tapset/target_set.stp
@@ -3,25 +3,33 @@ global _target_set # map: target-set-pid -> ancestor-pid
 
 function target_set_pid (pid)
 {
-  return ([pid] in _target_set)	 
+	return ([pid] in _target_set)
 }
 
 probe begin
 {
-  if (target()) _target_set [target()] = stp_pid()
+	if (target())
+		_target_set[target()] = stp_pid()
 }
 
-probe syscall.fork.return
+probe nd_syscall.fork.return
 {
-  pid=pid()
-  if (pid in _target_set) next
-  ppid=ppid()
-  if (ppid in _target_set) _target_set[pid]=ppid
+	pid = returnval()
+	if (pid in _target_set)
+		next
+	ppid = pid()
+	if (ppid in _target_set)
+		_target_set[pid] = ppid
+}
+
+probe nd_syscall.exit
+{
+	delete _target_set[pid()]
 }
 
 function target_set_report ()
 {
-  printf("target set:\n")
-  foreach (pid in _target_set+)
-    printf("%d begat %d\n", _target_set[pid], pid)
+	printf("target set:\n")
+	foreach (pid in _target_set+)
+		printf("%d begat %d\n", _target_set[pid], pid)
 }
-- 
1.5.6.5

Re: [PATCH 1/2] Fix target_set tapset.

[Josh Stone]

On 06/18/2009 03:27 PM, Przemyslaw Pawelczyk wrote:
> Revise acquiring of pid and ppid in fork.return probe -- use returnval()
> and pid() instead of pid() and ppid() respectively. Add pid removal on
> exit syscall. Use dwarfless syscall probe aliases. Correct formatting.
> ---

Applied, thanks!

Josh

Re: [PATCH 2/2] Add test for target_set tapset.

[Josh Stone]

On 06/18/2009 03:33 PM, Przemyslaw Pawelczyk wrote:
> +proc abort {} {
> +	global test
> +	fail $test
> +	exit
> +}

exit should not be used, because that will stop the entire testing
session.  We don't want failure here to prevent other tests from running.

> +	set pid_it $stp_pid
> +	while {[info exists pid_array($pid_it)]} {
> +		if {[exec pgrep -P $pid_it] != $pid_array($pid_it)} {
> +			abort
> +		}
> +		set pid_it $pid_array($pid_it)
> +	}

There's a race here that the sleep process might finish before pgrep
sees it.  Most of the time, one second will probably be plenty of time,
but on a slow and/or loaded system we could have false failures.

What if you just used an absurdly long timeout for sleep, and then "kill
-INT" it after you've verified the chain?

> +probe nd_syscall.nanosleep
> +{
> +	if (target_set_pid(pid()) && @cast(req_uaddr, "timespec", "<linux/time.h>")->tv_sec == $1)
> +		target_set_report()
> +}

Some systems have a 32-bit userspace with a 64-bit kernel, and in that
case you would need to catch nd_syscall.compat_nanosleep as well.


I feel like you're going through somewhat heroic efforts to validate
this in tcl, and you're not able to use any of the common infrastructure
we have for other tests.  Maybe it would easier to check results within
the script?  We couldn't check the report() that way, but
target_set_pid() is what we really care about anyway, right?

I'm imagining that in the nanosleep probe, you could recursively walk up
task_parent() until you hit stp_pid() or 1 (init).  Then as the
recursion unwinds, make sure that target_set_pid() matches.  You can use
system() to also launch a sleep that's outside of the target_set.  Does
that make sense?


Josh

Re: [PATCH 2/2] Add test for target_set tapset.

[Przemysław Pawełczyk]

On Fri, Jun 19, 2009 at 03:55, Josh Stone<jistone@redhat.com> wrote:
> On 06/18/2009 03:33 PM, Przemyslaw Pawelczyk wrote:
>> +proc abort {} {
>> +     global test
>> +     fail $test
>> +     exit
>> +}
>
> exit should not be used, because that will stop the entire testing
> session.  We don't want failure here to prevent other tests from running.

At the time I was unaware of this.

>> +     set pid_it $stp_pid
>> +     while {[info exists pid_array($pid_it)]} {
>> +             if {[exec pgrep -P $pid_it] != $pid_array($pid_it)} {
>> +                     abort
>> +             }
>> +             set pid_it $pid_array($pid_it)
>> +     }
>
> There's a race here that the sleep process might finish before pgrep
> sees it.  Most of the time, one second will probably be plenty of time,
> but on a slow and/or loaded system we could have false failures.
>
> What if you just used an absurdly long timeout for sleep, and then "kill
> -INT" it after you've verified the chain?

Great idea. Thanks.

>> +probe nd_syscall.nanosleep
>> +{
>> +     if (target_set_pid(pid()) && @cast(req_uaddr, "timespec", "<linux/time.h>")->tv_sec == $1)
>> +             target_set_report()
>> +}
>
> Some systems have a 32-bit userspace with a 64-bit kernel, and in that
> case you would need to catch nd_syscall.compat_nanosleep as well.

Ok.

> I feel like you're going through somewhat heroic efforts to validate
> this in tcl, and you're not able to use any of the common infrastructure
> we have for other tests.  Maybe it would easier to check results within
> the script?  We couldn't check the report() that way, but
> target_set_pid() is what we really care about anyway, right?
>
> I'm imagining that in the nanosleep probe, you could recursively walk up
> task_parent() until you hit stp_pid() or 1 (init).  Then as the
> recursion unwinds, make sure that target_set_pid() matches.  You can use
> system() to also launch a sleep that's outside of the target_set.  Does
> that make sense?

IMO tested systemtap script should do as little as possible and the
testing is duty of the tester, here: expect/tcl script. Easiness of
implementing tester is secondary thing.

I'll send v2 of the patch right away.

> Josh

Regards.

-- 
Przemysław Pawełczyk

[PATCH v2] Add test for target_set tapset.

[Przemyslaw Pawelczyk]

---
 testsuite/systemtap.base/target_set.exp |   67 +++++++++++++++++++++++++++++++
 testsuite/systemtap.base/target_set.stp |   22 ++++++++++
 2 files changed, 89 insertions(+), 0 deletions(-)
 create mode 100644 testsuite/systemtap.base/target_set.exp
 create mode 100644 testsuite/systemtap.base/target_set.stp

diff --git a/testsuite/systemtap.base/target_set.exp b/testsuite/systemtap.base/target_set.exp
new file mode 100644
index 0000000..1971ab6
--- /dev/null
+++ b/testsuite/systemtap.base/target_set.exp
@@ -0,0 +1,67 @@
+# Test for target_set tapset
+
+set test target_set
+
+if {![installtest_p]} { untested $test; continue }
+
+set file $srcdir/$subdir/target_set.stp
+set time 12345
+set stap_cmd  "( ( /bin/sleep $time ) ; ( /bin/sleep $time ) ; ( /bin/sleep $time ) )"
+set stap_cmd_sleep_count 3
+set stap_cmd_child_level 3
+
+proc failtest {} {
+	global test
+	fail $test
+}
+
+proc expect_target_set_string {} {
+	expect {
+		"^target set:\r\n" { }
+		timeout { failtest; return -code return }
+	}
+}
+
+proc expect_target_set_pids generations {
+	global test
+	global stp_pid
+	for {set i 0} {$i < $generations} {incr i} {
+		expect {
+			-timeout 0
+			-re {^([0-9]+) begat ([0-9]+)\r\n} { set pid_array($expect_out(1,string)) $expect_out(2,string) }
+			timeout { failtest; return -code return }
+		}
+	}
+	set pid_it $stp_pid
+	while {[info exists pid_array($pid_it)]} {
+		if {[exec pgrep -P $pid_it] != $pid_array($pid_it)} {
+			failtest; return -code return
+		}
+		set pid_it $pid_array($pid_it)
+	}
+	if {$generations > 1} {
+		exec kill -INT $pid_it
+	}
+}
+
+spawn stap $file $time -c "$stap_cmd"
+
+expect {
+	-re {^(\d+)\r\n} { set stp_pid $expect_out(1,string) }
+	timeout { failtest; return }
+}
+
+expect_target_set_pids 1
+for {set i 0} {$i < $stap_cmd_sleep_count} {incr i} {
+	expect_target_set_string
+	expect_target_set_pids $stap_cmd_child_level
+}
+expect_target_set_string
+expect_target_set_pids 0
+
+expect {
+	eof {}
+	timeout { failtest; return }
+}
+
+pass $test
diff --git a/testsuite/systemtap.base/target_set.stp b/testsuite/systemtap.base/target_set.stp
new file mode 100644
index 0000000..fa31cfa
--- /dev/null
+++ b/testsuite/systemtap.base/target_set.stp
@@ -0,0 +1,22 @@
+probe begin
+{
+	stp_pid = stp_pid()
+	printf("%d\n%d begat %d\n", stp_pid, stp_pid, target())
+}
+
+probe nd_syscall.compat_nanosleep
+{
+	if (target_set_pid(pid()) && @cast(req_uaddr, "timespec", "<linux/time.h>")->tv_sec == $1)
+		target_set_report()
+}
+
+probe nd_syscall.nanosleep
+{
+	if (target_set_pid(pid()) && @cast(req_uaddr, "timespec", "<linux/time.h>")->tv_sec == $1)
+		target_set_report()
+}
+
+probe end
+{
+	target_set_report()
+}
-- 
1.5.6.5

[PATCH v2.5][DRAFT] Add test for target_set tapset.

[Przemyslaw Pawelczyk]

Requires: Support OR (using ||) in preprocessor's conditions.
---
 testsuite/systemtap.base/target_set.exp |   67 +++++++++++++++++++++++++++++++
 testsuite/systemtap.base/target_set.stp |   24 +++++++++++
 2 files changed, 91 insertions(+), 0 deletions(-)
 create mode 100644 testsuite/systemtap.base/target_set.exp
 create mode 100644 testsuite/systemtap.base/target_set.stp

diff --git a/testsuite/systemtap.base/target_set.exp b/testsuite/systemtap.base/target_set.exp
new file mode 100644
index 0000000..998fd4a
--- /dev/null
+++ b/testsuite/systemtap.base/target_set.exp
@@ -0,0 +1,67 @@
+# Test for target_set tapset
+
+set test target_set
+
+if {![installtest_p]} { untested $test; continue }
+
+set file $srcdir/$subdir/target_set.stp
+set time 12345
+set stap_cmd  "( ( /bin/sleep $time ) ; ( /bin/sleep $time ) ; ( /bin/sleep $time ) )"
+set stap_cmd_sleep_count 3
+set stap_cmd_child_level 3
+
+proc failtest {} {
+	global test
+	fail $test
+}
+
+proc expect_target_set_string {} {
+	expect {
+		"^target set:\r\n" { }
+		timeout { failtest; return -code return }
+	}
+}
+
+proc expect_target_set_pids generations {
+	global test
+	global stp_pid
+	for {set i 0} {$i < $generations} {incr i} {
+		expect {
+			-re {^([0-9]+) begat ([0-9]+)\r\n} { set pid_array($expect_out(1,string)) $expect_out(2,string) }
+			timeout { failtest; return -code return }
+		}
+	}
+	set pid_it $stp_pid
+	while {[info exists pid_array($pid_it)]} {
+		if {[exec pgrep -P $pid_it] != $pid_array($pid_it)} {
+			failtest; return -code return
+		}
+		set pid_it $pid_array($pid_it)
+	}
+	if {$generations > 1} {
+		exec kill -INT $pid_it
+	}
+}
+
+spawn stap $file $time -c "$stap_cmd"
+
+expect {
+	-re {^(\d+)\r\n} { set stp_pid $expect_out(1,string) }
+	timeout { failtest; return }
+}
+
+expect_target_set_pids 1
+for {set i 0} {$i < $stap_cmd_sleep_count} {incr i} {
+	expect_target_set_string
+	expect_target_set_pids $stap_cmd_child_level
+}
+expect_target_set_string
+expect_target_set_pids 0
+
+expect {
+	eof {}
+	timeout { failtest; return }
+}
+
+wait
+pass $test
diff --git a/testsuite/systemtap.base/target_set.stp b/testsuite/systemtap.base/target_set.stp
new file mode 100644
index 0000000..4d87114
--- /dev/null
+++ b/testsuite/systemtap.base/target_set.stp
@@ -0,0 +1,24 @@
+probe begin
+{
+	stp_pid = stp_pid()
+	printf("%d\n%d begat %d\n", stp_pid, stp_pid, target())
+}
+
+probe nd_syscall.nanosleep
+{
+	if (target_set_pid(pid()) && @cast(req_uaddr, "timespec", "kernel<linux/time.h>")->tv_sec == $1)
+		target_set_report()
+}
+
+%( arch == "x86_64" || arch == "ppc64" || arch == "ia64" %?
+probe nd_syscall.compat_nanosleep
+{
+	if (target_set_pid(pid()) && @cast(req_uaddr, "compat_timespec", "kernel<linux/compat.h>")->tv_sec == $1)
+		target_set_report()
+}
+%)
+
+probe end
+{
+	target_set_report()
+}
-- 
1.5.6.5

Re: [PATCH v2] Add test for target_set tapset.

[Przemysław Pawełczyk]

On Fri, Jun 19, 2009 at 22:59, Przemyslaw Pawelczyk
<przemyslaw@pawelczyk.it> wrote:
> +proc expect_target_set_pids generations {
> +       global test
> +       global stp_pid
> +       for {set i 0} {$i < $generations} {incr i} {
> +               expect {
> +                       -timeout 0
> +                       -re {^([0-9]+) begat ([0-9]+)\r\n} { set pid_array($expect_out(1,string)) $expect_out(2,string) }
> +                       timeout { failtest; return -code return }
> +               }
> +       }

This zero timeout is way to optimistic and leads to occasional
failures as Josh pointed out on #systemtap.

> +probe nd_syscall.compat_nanosleep
> +{
> +       if (target_set_pid(pid()) && @cast(req_uaddr, "timespec", "<linux/time.h>")->tv_sec == $1)
> +               target_set_report()
> +}

This is completely wrong and shouldn't see the light of day. I'm really ashamed.
Thanks Josh for noticing this and the problem of compat_* functions in general.
Still I don't see any solution for CONFIG_COMPAT=n on 64-bit architectures.

I have already sent next draft version of the patch. Any comments and
suggestions are welcomed, but please send them as reply to that patch
(not this mail). Thank you.

Regards.

-- 
Przemysław Pawełczyk

[PATCH v3] Add test for target_set tapset.

[Przemyslaw Pawelczyk]

---
 testsuite/systemtap.base/target_set.exp |   67 +++++++++++++++++++++++++++++++
 testsuite/systemtap.base/target_set.stp |   16 +++++++
 2 files changed, 83 insertions(+), 0 deletions(-)
 create mode 100644 testsuite/systemtap.base/target_set.exp
 create mode 100644 testsuite/systemtap.base/target_set.stp

diff --git a/testsuite/systemtap.base/target_set.exp b/testsuite/systemtap.base/target_set.exp
new file mode 100644
index 0000000..998fd4a
--- /dev/null
+++ b/testsuite/systemtap.base/target_set.exp
@@ -0,0 +1,67 @@
+# Test for target_set tapset
+
+set test target_set
+
+if {![installtest_p]} { untested $test; continue }
+
+set file $srcdir/$subdir/target_set.stp
+set time 12345
+set stap_cmd  "( ( /bin/sleep $time ) ; ( /bin/sleep $time ) ; ( /bin/sleep $time ) )"
+set stap_cmd_sleep_count 3
+set stap_cmd_child_level 3
+
+proc failtest {} {
+	global test
+	fail $test
+}
+
+proc expect_target_set_string {} {
+	expect {
+		"^target set:\r\n" { }
+		timeout { failtest; return -code return }
+	}
+}
+
+proc expect_target_set_pids generations {
+	global test
+	global stp_pid
+	for {set i 0} {$i < $generations} {incr i} {
+		expect {
+			-re {^([0-9]+) begat ([0-9]+)\r\n} { set pid_array($expect_out(1,string)) $expect_out(2,string) }
+			timeout { failtest; return -code return }
+		}
+	}
+	set pid_it $stp_pid
+	while {[info exists pid_array($pid_it)]} {
+		if {[exec pgrep -P $pid_it] != $pid_array($pid_it)} {
+			failtest; return -code return
+		}
+		set pid_it $pid_array($pid_it)
+	}
+	if {$generations > 1} {
+		exec kill -INT $pid_it
+	}
+}
+
+spawn stap $file $time -c "$stap_cmd"
+
+expect {
+	-re {^(\d+)\r\n} { set stp_pid $expect_out(1,string) }
+	timeout { failtest; return }
+}
+
+expect_target_set_pids 1
+for {set i 0} {$i < $stap_cmd_sleep_count} {incr i} {
+	expect_target_set_string
+	expect_target_set_pids $stap_cmd_child_level
+}
+expect_target_set_string
+expect_target_set_pids 0
+
+expect {
+	eof {}
+	timeout { failtest; return }
+}
+
+wait
+pass $test
diff --git a/testsuite/systemtap.base/target_set.stp b/testsuite/systemtap.base/target_set.stp
new file mode 100644
index 0000000..002ba89
--- /dev/null
+++ b/testsuite/systemtap.base/target_set.stp
@@ -0,0 +1,16 @@
+probe begin
+{
+	stp_pid = stp_pid()
+	printf("%d\n%d begat %d\n", stp_pid, stp_pid, target())
+}
+
+probe syscall.nanosleep, syscall.compat_nanosleep ?
+{
+	if (target_set_pid(pid()) && $rqtp->tv_sec == $1)
+		target_set_report()
+}
+
+probe end
+{
+	target_set_report()
+}
-- 
1.5.6.5

Re: [PATCH v3] Add test for target_set tapset.

[Josh Stone]

Applied, thanks!

Josh

how to get one process's resource usage by systemtap

[tgh]

Thank you
I mean , if I want to get the resource usage information for each process
per minute, such as, cpu usage, memory usage, I/O usage,  
How can I get these information with systemtap, 
Are There some example scripts for use,  
could you help me 

Thank you




-----邮件原件-----
发件人: Josh Stone [mailto:jistone@redhat.com] 
发送时间: 2009年6月19日 10:13
收件人: tgh
主题: Re: how to get one process's resource usage by systemtap

On 06/18/2009 06:40 PM, tgh wrote:
> Hi
> 	I use systemtap, and want to get a given process's memory usage, cpu
> usage, IO usage, etc., each time interval, and I do not know how to
achieve
> it,
> 	I have used systemtap to get the resource usage about the whole
> system on linux 2.6.19, but I hear that in linux2.6.27 , it can get each
> process's resouce usage, but I don't know how to get it, 
> 	Could you help me, give me some example, or where can I get relative
> example
> 
> Thank you in advance

That's a good question.  I'm not sure what you're referring to in 2.6.27
-- can you give more details about what you heard or read?  It would
also help me to know a bit more about what you are trying to achieve.

Please consider re-sending your question to systemtap@sourceware.org,
so other experts can also help, and other users can see the answers.

Thanks,

Josh