<div dir="ltr"><div dir="ltr">Hi Jonathan :)<br><br>It's very unlikely that the race could occur, BUT it can happen.<br><div dir="ltr"><br></div><div>OK run<br></div></div><div dir="ltr"><div dir="ltr">1. test_event_tracker starts gen-ust-events</div><div>2. test_event_tracker waits for gen-ust-events to create AFTER_FIRST_PATH</div><div>3. gen-ust-event create first event and create AFTER_FIRST_PATH</div><div>4. gen-ust-event continue and create 99 events</div><div>5. gen-ust-event wait for  BEFORE_LAST_PATH</div><div>6. test_event_track start collecting events (lttng start .....)</div><div>7. test_event_tracker calls "lttng track ... -pid "Faulty PID"<br></div><div>8. test_event_tracker touches BEFORE_LAST_PATH</div><div>9. gen-ust-events creates the last event</div><div>10. test_event finishes and since it tracks the faulty pid the result will be 0 events == OK</div><div><br></div><div></div></div><div>Faulty run</div><div dir="ltr">1. test_event_tracker starts gen-ust-events<br></div><div>2. test_event_tracker waits for gen-ust-events to create AFTER_FIRST_PATH</div><div>3. gen-ust-event create first event and create AFTER_FIRST_PATH</div><div>4. gen-ust-event gets rescheduled before it has created 99 events, e.g after 9 events</div><div>5. test_event_track start collecting events (lttng start .....)</div><div>6. gen-ust-event is rescheduled and starts generating the remaining events. 90 events<br>7. lttng collects these 90 events since we have not setup "tracking" of PID yet</div><div>8. test_event_tracker calls "lttng track ... -pid "Faulty PID"</div><div>9. test_event_tracker touches BEFORE_LAST_PATH</div><div>10. gen-ust-events creates the last event</div><div>11. test_event finishes and since it tracks the faulty pid the result should  be 0 events, but we got 90 == FAULTY</div><div><br></div><div>We can solve this by;</div><div>A: using NR_ITER=2</div><div>or</div><div>B: by adding a flag to gen-ust-events to wait before sending the first event<br></div><div><div dir="ltr">1. test_event_tracker starts gen-ust-events</div><div>2. test_event_tracker waits for gen-ust-events waits for BEFORE_FIRST_PATH</div><div>3. test_event_track start collecting events (lttng start .....)<br></div><div>4. test_event_tracker calls "lttng track ... -pid "Faulty PID"<br></div><div>5. test_event_track creates BEFORE_FIRST_PATH</div><div></div><div>6. gen_ust_event creates 100 events</div><div>7. test_event_tracker waits for gen_event_ust to end</div><div>8. test_event finishes and since it tracked the faulty pid the result will be 0 events == OK</div><div></div></div><div dir="ltr"><br></div><div>This is in principle how gen-kernel-test-events works (but with different arguments)</div><div>I would suggest to use B since that will be bulletproof</div><div dir="ltr"><br>Anders Wallin<br></div><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Mar 31, 2021 at 11:33 PM Jonathan Rajotte-Julien <<a href="mailto:jonathan.rajotte-julien@efficios.com">jonathan.rajotte-julien@efficios.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi,<br>
<br>
On Wed, Mar 31, 2021 at 11:09:42PM +0200, Anders Wallin wrote:<br>
> Hi Julian,<br>
<br>
You can use Jonathan. ;)<br>
<br>
> <br>
> Neither mine "sleep 0.1" or your version with "while [! -f ............"<br>
> are race condition free.<br>
<br>
I might be missing something here but as far as I understand the race you are<br>
trying to mitigate is that the test script execute/continue before the `backgrounded`<br>
command (background test app) had time to execute, right?<br>
<br>
If so at least waiting for the app to create a file is necessary. Now<br>
gen_kernel_test_events does not have this functionality. The PATH_WAIT_FILE is<br>
used to control when the testapp can continue. Hence the script still cannot<br>
know if the app have been scheduled.<br>
<br>
Now based on the test case you might need more synchronization for the test<br>
cases.<br>
<br>
Note that in the ust cases, the trace_ust_app uses `touch "$BEFORE_LAST_PATH"`<br>
that effectively unblock the app and allows it to perform the last tracepoint<br>
hit and the we `wait` on the background process.<br>
<br>
Note: some tests case are a bit clever and uses "trace_"$domain"_app" instead of<br>
calling trace_ust_app directly.<br>
<br>
For these tests case it seems that we are only expecting at least a single event<br>
matching the event name under test. Here the last tracepoint hit should satisfy<br>
this criteria.<br>
<br>
Am I missing a race?<br>
<br>
Cheers<br>
<br>
<br>
> I suggest that we add an option to gen-ust-events to wait before the first<br>
> event is generated.<br>
> gen_kernel_test_events already have this functionality to wait before the<br>
> first event.<br>
> <br>
> Something like this<br>
> static struct option long_options[] =<br>
> {<br>
> /* These options set a flag. */<br>
> {"iter", required_argument, 0, 'i'},<br>
> {"wait", required_argument, 0, 'w'},<br>
> {"sync-after-first-event", required_argument, 0, 'a'},<br>
> {"sync-before-last-event", required_argument, 0, 'b'},<br>
> {"sync-before-last-event-touch", required_argument, 0, 'c'},<br>
> {"sync-before-exit", required_argument, 0, 'd'},<br>
> {"sync-before-exit-touch", required_argument, 0, 'e'},<br>
> *+ {"sync-before-first-event", required_argument, 0, 'f'},*<br>
> <br>
> {0, 0, 0, 0}<br>
> };<br>
> ....<br>
> <br>
> I will create one or more patches for this tomorrow<br>
> <br>
> Anders Wallin<br>
> <br>
> <br>
> On Wed, Mar 31, 2021 at 9:25 PM Jonathan Rajotte-Julien <<br>
> <a href="mailto:jonathan.rajotte-julien@efficios.com" target="_blank">jonathan.rajotte-julien@efficios.com</a>> wrote:<br>
> <br>
> > > #<br>
> > > # SPDX-License-Identifier: GPL-2.0-only<br>
> > ><br>
> > > -TEST_DESC="LTTng - Event traker test"<br>
> > > +TEST_DESC="LTTng - Event tracker test"<br>
> > ><br>
> > > CURDIR=$(dirname "$0")/<br>
> > > TESTDIR="$CURDIR/../../.."<br>
> > > @@ -42,6 +42,8 @@ function prepare_ust_app<br>
> > ><br>
> > >       $TESTAPP_BIN -i $NR_ITER -w $NR_USEC_WAIT -a "$AFTER_FIRST_PATH" -b<br>
> > >       "$BEFORE_LAST_PATH" &<br>
> > >       CHILD_PID=$!<br>
> > > +     # voluntary context switch to start $TESTAPP_BIN<br>
> > > +     sleep 0.1<br>
> ><br>
> > A wait on the $AFTER_FIRST_PATH file would be probably more deterministic<br>
> > than a sleep here.<br>
> ><br>
> >   while [ ! -f "${AFTER_FIRST_PATH}" ]; do<br>
> >           sleep 0.1<br>
> >   done<br>
> ><br>
> > I would also expect something similar for the `prepare_kernel_app`<br>
> > function considering the same race is most probably present and simply not<br>
> > triggered by a chance of luck.<br>
> > Seems like gen-kernel-test-events does not expose the same sync<br>
> > capabilities here, please use gen-ust-events as an example of how it is<br>
> > done.<br>
> ><br>
> > Let us know how your testing goes.<br>
> ><br>
> > Thanks<br>
> ><br>
<br>
-- <br>
Jonathan Rajotte-Julien<br>
EfficiOS<br>
</blockquote></div></div>