Fix common.probes.t_dtrace_contrib.tst_probestar_d timeout issue
ClosedPublic

Authored by lwhsu on Jul 20 2017, 2:50 PM.

Details

Summary

This test timeout on a quiet system. Because there is nobody triggers
syscall::*wait*:entry probe while test execution.

Add a simple program calls sigwaitinfo(2), it sleeps for 1 second at begining
because current dtest script doesn't support execute a program after invoking
dtrace script.

Test Plan

cd /usr/tests/cddl/usr.sbin/dtrace/common/probes && kyua debug t_dtrace_contrib:tst_probestar_d

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
lwhsu created this revision.Jul 20 2017, 2:50 PM
Herald added 1 blocking reviewer(s): gnn. · View Herald TranscriptJul 20 2017, 2:50 PM
Herald added a subscriber: imp. · View Herald Transcript
ngie added inline comments.Jul 20 2017, 3:06 PM
cddl/contrib/opensolaris/cmd/dtrace/test/tst/common/probes/tst.probestar.c
26–28 ↗(On Diff #30999)

style(9): sort headers.

31 ↗(On Diff #30999)

Use void.

37–38 ↗(On Diff #30999)

These should both be cast to void to quiet warnings from coverity.

44 ↗(On Diff #30999)

style(9): wrap the return value in parentheses.

lwhsu added inline comments.Jul 20 2017, 3:29 PM
cddl/contrib/opensolaris/cmd/dtrace/test/tst/common/probes/tst.probestar.c
31 ↗(On Diff #30999)

Do you mean main (void) or void main(int argc, char *argv[])?

I suppose former because we do return 0/1 in main.

markj added inline comments.Jul 20 2017, 5:49 PM
cddl/contrib/opensolaris/cmd/dtrace/test/tst/common/probes/tst.probestar.c
42 ↗(On Diff #30999)

The test harness will first start the program, and then invoke dtrace, passing the PID of the new process as an argument. The harness will also take care of killing the process after dtrace exits.

The sleep(1) will probably be enough time for dtrace to attach, but it's racy - what if the system is loaded and it takes more than a second to start dtrace? I think it would be more robust if the test program called sigtimedwait() once per second in an infinite loop.

ngie added inline comments.Jul 20 2017, 6:15 PM
cddl/contrib/opensolaris/cmd/dtrace/test/tst/common/probes/tst.probestar.c
31 ↗(On Diff #30999)

Yes, the arguments list for main should be void. This eliminates a -Wunused warning.

lwhsu updated this revision to Diff 31025.Jul 20 2017, 7:31 PM

mostly style fixes

lwhsu marked 6 inline comments as done.Jul 20 2017, 7:31 PM
lwhsu added inline comments.Jul 20 2017, 7:35 PM
cddl/contrib/opensolaris/cmd/dtrace/test/tst/common/probes/tst.probestar.c
42 ↗(On Diff #30999)

I was also thinking the same thing, but I wanted to test all the stars in the case (*wait*) so I chose sigwaitinfo(2) here. In my plan is fixing these timeout issues first to reduce test time, and modify dtest to enable executing a post-action while test. How do you think?

markj added inline comments.Jul 20 2017, 7:52 PM
cddl/contrib/opensolaris/cmd/dtrace/test/tst/common/probes/tst.probestar.c
42 ↗(On Diff #30999)

Well, you're still verifying that the last "*" matches the empty string.

I don't quite understand your proposal. What kind of post-action are you proposing for this case?

ngie added inline comments.Jul 21 2017, 2:41 AM
cddl/contrib/opensolaris/cmd/dtrace/test/tst/common/probes/tst.probestar.c
42 ↗(On Diff #30999)

I don't understand either comment to be frank:

  1. lwhsu: what do you mean by "executing a post-action while test"?
  2. markj: where is lwhsu still verifying that the last "*" matches the empty string?
37–38 ↗(On Diff #31025)

style(9): no spaces between (void) cast and sig....

lwhsu updated this revision to Diff 31053.Jul 21 2017, 12:50 PM
  • Change to call sigtimedwait(2)
  • style(9) fix
lwhsu updated this revision to Diff 31054.Jul 21 2017, 12:51 PM

Add missing Makefile

lwhsu added inline comments.Jul 21 2017, 12:55 PM
cddl/contrib/opensolaris/cmd/dtrace/test/tst/common/probes/tst.probestar.c
42 ↗(On Diff #30999)

@markj : verifying last * matches the empty string -- makes sense. I changed it to sigtimedwait(2) and simply ignore return value.
@ngie , @markj : I mean we can probably change dtest script, and let it supports doing a test like this:

  1. executing dtrace, keep the pid
  2. executing other program / script
  3. check the dtrace output and return value
  4. recycle (kill) the auxiliary script
ngie accepted this revision.Jul 21 2017, 1:39 PM

The change looks good to me at least. Thanks!

markj accepted this revision.Jul 21 2017, 5:08 PM
gnn accepted this revision.Jul 25 2017, 10:26 AM
This revision is now accepted and ready to land.Jul 25 2017, 10:26 AM
This revision was automatically updated to reflect the committed changes.