This is the mail archive of the systemtap@sourceware.org mailing list for the systemtap project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]