[DTrace-devel] [PATCH 4/5] test, usdt: test multiple simultaneous tracers of multiple processes

[Nick Alcock]

On 13 May 2023, Kris Van Hees verbalised:

> How do you know that test PASS actually indicates that the condition you aim
> to test was exercised and found to work?  As I see it, you do not distinguish
> between the probe firings, so it is perfectly possible that you get the first
> five from the first test and dtrace, and the second five from the second test
> and dtrace, meaning that they were consecutive and this no simultaneous tracing
> ever took place.

True. I guess I was hoping we'd be lucky? :) (actually I assumed that
dtrace startup would surely not take longer than five seconds so we'd
get at least some overlap.)

> I modified the test to pass i into the go() probe call (probe go(int i)) and
> then add arg0 at the end of the printf, and I actually got exactly the
> situation I mention above (and a few times not all probes were reported, which
> means that the initial sleep(3) wasn't long enough).

Five seconds not long enough?! How slow *is* dtrace startup on that
system :/ the longer we make these delays the slower make check gets,
and it's too slow already.

I tried to fix this with raise() but alas that only lets you signal the
current process, and if the process is sitting waiting in pause() it's
not likely to become current, and I don't see anything we can trace on
to get it to be current (tick certainly won't do). I can't do a
system(SIGUSR2) or whatever either because system() gets processed by
userspace at consume time, i.e. even in BEGIN it would get processed
*after* everything ran, not before. (Maybe system() et al should trigger
an immediate consume regardless of switchrate, so you can use it in
BEGIN in the obvious way. Honestly I thought it did...)

As you reported, I'm seeing the probe firing in each dtrace... too
often: i.e., it claims ten firings from each...

... oh crap it's reporting is-enabled probes being hit as the
corresponding real probe firing isn't it! WRONG-O. Fixing, will be in
the next is-enabled round. Our existing tests need souping up if they
didn't spot *that*. (I suppose adding this test constitutes such souping
up, but it could do with a dedicated test too. I'll add one.)

> This test needs more work to ensure it tests what it should in a predictable
> manner, and in a way that the results verification (possibly using a .r.p
> script) verifies that simultaneous tracing took place.
>
> (Disclaimer: I tested this on a tree that has all pending patches, i.e. yours,
> Eugene's, and mine) so it is possible that your testing appeared to work OK
> because it predates some other bugfixes.)

I can believe that. It does still seem to work for me, though (and it
doesn't fail 100% of the time, which it did before my fix in the
previous patch).

-- 
NULL && (void)