lttng-dev.lists.lttng.org archive mirror
 help / color / mirror / Atom feed
* [lttng-dev] [PATCH lttng-tools] Fix: test code assumes that child process is schedule to run before parent
@ 2021-04-09 14:41 Anders Wallin via lttng-dev
  2021-04-09 19:12 ` Mathieu Desnoyers via lttng-dev
  0 siblings, 1 reply; 17+ messages in thread
From: Anders Wallin via lttng-dev @ 2021-04-09 14:41 UTC (permalink / raw)
  To: lttng-dev

the following tests fails on arm64
- test_event_vpid_tracker ust 0 "${EVENT_NAME}"
- test_event_vpid_track_untrack ust 0 "${EVENT_NAME}"
- test_event_pid_tracker ust 0 "${EVENT_NAME}"
- test_event_pid_track_untrack ust 0 "${EVENT_NAME}"

Signed-off-by: Anders Wallin <wallinux@gmail.com>
---
 .../tools/tracker/test_event_tracker           | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/tests/regression/tools/tracker/test_event_tracker b/tests/regression/tools/tracker/test_event_tracker
index 711690af..7f7d68da 100755
--- a/tests/regression/tools/tracker/test_event_tracker
+++ b/tests/regression/tools/tracker/test_event_tracker
@@ -5,7 +5,7 @@
 #
 # SPDX-License-Identifier: GPL-2.0-only
 
-TEST_DESC="LTTng - Event traker test"
+TEST_DESC="LTTng - Event tracker test"
 
 CURDIR=$(dirname "$0")/
 TESTDIR="$CURDIR/../../.."
@@ -17,8 +17,8 @@ TESTAPP_KERNEL_BIN="$TESTAPP_PATH/$TESTAPP_KERNEL_NAME/$TESTAPP_KERNEL_NAME"
 SESSION_NAME="tracker"
 NR_ITER=100
 NUM_GLOBAL_TESTS=2
-NUM_UST_TESTS=283
-NUM_KERNEL_TESTS=462
+NUM_UST_TESTS=289
+NUM_KERNEL_TESTS=468
 NUM_TESTS=$((NUM_UST_TESTS+NUM_KERNEL_TESTS+NUM_GLOBAL_TESTS))
 
 NR_USEC_WAIT=0	#for UST gen events
@@ -130,6 +130,8 @@ function test_event_vpid_tracker()
 
 	prepare_"$domain"_app
 
+	lttng_untrack_"$domain"_ok "--all --vpid"
+
 	start_lttng_tracing_ok
 
 	if [ "$expect_event" -eq 1 ]; then
@@ -146,7 +148,7 @@ function test_event_vpid_tracker()
 	if [ "$expect_event" -eq 1 ]; then
 		validate_trace "$EVENT_NAME" "$trace_path"
 	else
-		validate_trace_empty "$trace_path"
+		validate_trace_session_"$domain"_empty "$trace_path"
 	fi
 
 	rm -rf "$trace_path"
@@ -173,6 +175,8 @@ function test_event_pid_tracker()
 
 	prepare_"$domain"_app
 
+	lttng_untrack_"$domain"_ok "--all --pid"
+
 	start_lttng_tracing_ok
 
 	if [ "$expect_event" -eq 1 ]; then
@@ -189,7 +193,7 @@ function test_event_pid_tracker()
 	if [ "$expect_event" -eq 1 ]; then
 		validate_trace "$EVENT_NAME" "$trace_path"
 	else
-		validate_trace_empty "$trace_path"
+	    	validate_trace_session_"$domain"_empty "$trace_path"
 	fi
 
 	rm -rf "$trace_path"
@@ -275,6 +279,8 @@ function test_event_vpid_track_untrack()
 
 	prepare_"$domain"_app
 
+	lttng_untrack_"$domain"_ok "--all --vpid"
+
 	start_lttng_tracing_ok
 
 	lttng_track_"$domain"_ok "--vpid ${CHILD_PID}"
@@ -315,6 +321,8 @@ function test_event_pid_track_untrack()
 
 	prepare_"$domain"_app
 
+	lttng_untrack_"$domain"_ok "--all --pid"
+
 	start_lttng_tracing_ok
 
 	lttng_track_"$domain"_ok "--pid ${CHILD_PID}"
-- 
2.31.1

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [lttng-dev] [PATCH lttng-tools] Fix: test code assumes that child process is schedule to run before parent
  2021-04-09 14:41 [lttng-dev] [PATCH lttng-tools] Fix: test code assumes that child process is schedule to run before parent Anders Wallin via lttng-dev
@ 2021-04-09 19:12 ` Mathieu Desnoyers via lttng-dev
  0 siblings, 0 replies; 17+ messages in thread
From: Mathieu Desnoyers via lttng-dev @ 2021-04-09 19:12 UTC (permalink / raw)
  To: Anders Wallin, Jeremie Galarneau; +Cc: lttng-dev

----- On Apr 9, 2021, at 10:41 AM, lttng-dev lttng-dev@lists.lttng.org wrote:

> the following tests fails on arm64
> - test_event_vpid_tracker ust 0 "${EVENT_NAME}"
> - test_event_vpid_track_untrack ust 0 "${EVENT_NAME}"
> - test_event_pid_tracker ust 0 "${EVENT_NAME}"
> - test_event_pid_track_untrack ust 0 "${EVENT_NAME}"

The approach looks good to me, thanks!

Acked-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

> 
> Signed-off-by: Anders Wallin <wallinux@gmail.com>
> ---
> .../tools/tracker/test_event_tracker           | 18 +++++++++++++-----
> 1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/regression/tools/tracker/test_event_tracker
> b/tests/regression/tools/tracker/test_event_tracker
> index 711690af..7f7d68da 100755
> --- a/tests/regression/tools/tracker/test_event_tracker
> +++ b/tests/regression/tools/tracker/test_event_tracker
> @@ -5,7 +5,7 @@
> #
> # SPDX-License-Identifier: GPL-2.0-only
> 
> -TEST_DESC="LTTng - Event traker test"
> +TEST_DESC="LTTng - Event tracker test"
> 
> CURDIR=$(dirname "$0")/
> TESTDIR="$CURDIR/../../.."
> @@ -17,8 +17,8 @@
> TESTAPP_KERNEL_BIN="$TESTAPP_PATH/$TESTAPP_KERNEL_NAME/$TESTAPP_KERNEL_NAME"
> SESSION_NAME="tracker"
> NR_ITER=100
> NUM_GLOBAL_TESTS=2
> -NUM_UST_TESTS=283
> -NUM_KERNEL_TESTS=462
> +NUM_UST_TESTS=289
> +NUM_KERNEL_TESTS=468
> NUM_TESTS=$((NUM_UST_TESTS+NUM_KERNEL_TESTS+NUM_GLOBAL_TESTS))
> 
> NR_USEC_WAIT=0	#for UST gen events
> @@ -130,6 +130,8 @@ function test_event_vpid_tracker()
> 
> 	prepare_"$domain"_app
> 
> +	lttng_untrack_"$domain"_ok "--all --vpid"
> +
> 	start_lttng_tracing_ok
> 
> 	if [ "$expect_event" -eq 1 ]; then
> @@ -146,7 +148,7 @@ function test_event_vpid_tracker()
> 	if [ "$expect_event" -eq 1 ]; then
> 		validate_trace "$EVENT_NAME" "$trace_path"
> 	else
> -		validate_trace_empty "$trace_path"
> +		validate_trace_session_"$domain"_empty "$trace_path"
> 	fi
> 
> 	rm -rf "$trace_path"
> @@ -173,6 +175,8 @@ function test_event_pid_tracker()
> 
> 	prepare_"$domain"_app
> 
> +	lttng_untrack_"$domain"_ok "--all --pid"
> +
> 	start_lttng_tracing_ok
> 
> 	if [ "$expect_event" -eq 1 ]; then
> @@ -189,7 +193,7 @@ function test_event_pid_tracker()
> 	if [ "$expect_event" -eq 1 ]; then
> 		validate_trace "$EVENT_NAME" "$trace_path"
> 	else
> -		validate_trace_empty "$trace_path"
> +	    	validate_trace_session_"$domain"_empty "$trace_path"
> 	fi
> 
> 	rm -rf "$trace_path"
> @@ -275,6 +279,8 @@ function test_event_vpid_track_untrack()
> 
> 	prepare_"$domain"_app
> 
> +	lttng_untrack_"$domain"_ok "--all --vpid"
> +
> 	start_lttng_tracing_ok
> 
> 	lttng_track_"$domain"_ok "--vpid ${CHILD_PID}"
> @@ -315,6 +321,8 @@ function test_event_pid_track_untrack()
> 
> 	prepare_"$domain"_app
> 
> +	lttng_untrack_"$domain"_ok "--all --pid"
> +
> 	start_lttng_tracing_ok
> 
> 	lttng_track_"$domain"_ok "--pid ${CHILD_PID}"
> --
> 2.31.1
> 
> _______________________________________________
> lttng-dev mailing list
> lttng-dev@lists.lttng.org
> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [lttng-dev] [PATCH lttng-tools] Fix: test code assumes that child process is schedule to run before parent
  2021-04-08 12:48     ` Mathieu Desnoyers via lttng-dev
@ 2021-04-08 15:47       ` Anders Wallin via lttng-dev
  0 siblings, 0 replies; 17+ messages in thread
From: Anders Wallin via lttng-dev @ 2021-04-08 15:47 UTC (permalink / raw)
  To: Mathieu Desnoyers; +Cc: lttng-dev


[-- Attachment #1.1: Type: text/plain, Size: 5311 bytes --]

Is it OK that validate_trace_session_ust_empty
and validate_trace_session_kernel_empty calls different functions?
Or is it cp/paste error?

function validate_trace_session_ust_empty()
{
validate_directory_empty "$1"/ust
}

function validate_trace_session_kernel_empty()
{
* validate_trace_empty "$1"/kernel*
}

Anders Wallin


On Thu, Apr 8, 2021 at 2:48 PM Mathieu Desnoyers <
mathieu.desnoyers@efficios.com> wrote:

>
>
> ----- On Apr 8, 2021, at 5:22 AM, Anders Wallin <wallinux@gmail.com>
> wrote:
>
> OK,
>
> I tried this and for
> - test_event_vpid_track_untrack ust 0 "${EVENT_NAME}"
> - test_event_pid_track_untrack ust 0 "${EVENT_NAME}"
> it works well.
>
> For
> - test_event_vpid_tracker ust 0 "${EVENT_NAME}"
> - test_event_pid_tracker ust 0 "${EVENT_NAME}"
> validate_trace_empty fails since there is no data at all in the trace_path
>
> This run is just calling copy of test_event_tracker running only
> "test_event_vpid_tracker ust 0 ..."
> 1..747
> # LTTng - Event tracker test
> ok 1 - Start session daemon
> # Test UST tracker
> ok 2 - Create session tracker in -o /tmp/tmp.lu5aeCJhQ3
> ok 3 - Enable ust event tp:tptest for session tracker
> ok 4 - Untrack command with opts: -u --all --vpid
> ok 5 - Start tracing for session
> ok 6 - Track command with opts: -u --vpid 11842
> ok 7 - Traced application stopped.
> ok 8 - Stop lttng tracing for session
> ok 9 - Destroy session tracker
>
>
> *not ok 10 - Failed to parse trace#   Failed test 'Failed to parse trace'*#
>   in ./regression/tools/tracker//../../../utils/tap/tap.sh:fail() at line
> 159.
> # Killing (signal SIGTERM) lttng-sessiond and lt-lttng-sessiond pids:
> 11808 11809 11830
> ok 11 - Wait after kill session daemon
> # Looks like you planned 747 tests but only ran 11.
> # Looks like you failed 1 test of 11.
>
> # find /tmp/tmp.lu5aeCJhQ3
> /tmp/tmp.lu5aeCJhQ3
> /tmp/tmp.lu5aeCJhQ3/ust
>
> Do you want me to add a special version of "validate_trace_empty" not
> returning
> "Failed to parse trace" for this? If so should it be added in
> "test_event_tracker" or in "utils.sh"
>
> Actually, this helper already exists in utils.sh:
> validate_trace_session_ust_empty (), which
> internally uses validate_directory_empty (). As you point out, we should
> use
> validate_trace_session_ust_empty for the validation of those test cases
> which populate no
> trace whatsoever.
>
> Thanks,
>
> Mathieu
>
>
> Anders Wallin
>
>
> On Wed, Apr 7, 2021 at 5:31 PM Mathieu Desnoyers <
> mathieu.desnoyers@efficios.com> wrote:
>
>> ----- On Apr 1, 2021, at 12:37 PM, lttng-dev lttng-dev@lists.lttng.org
>> wrote:
>>
>> > the following tests fails
>> > - test_event_vpid_tracker ust 0 "${EVENT_NAME}"
>> > - test_event_vpid_track_untrack ust 0 "${EVENT_NAME}"
>> > - test_event_pid_tracker ust 0 "${EVENT_NAME}"
>> > - test_event_pid_track_untrack ust 0 "${EVENT_NAME}"
>>
>> There is indeed an issue with these tests: there is a missing "untrack
>> all pids"
>> which should be done at the very beginning of each test to have the
>> expected
>> behavior. AFAIU this explains why there is a small window where traced
>> applications
>> can generate events when none are expected.
>>
>> Fixing this should solve the issues without requiring any addition to the
>> test
>> program.
>>
>> Thanks,
>>
>> Mathieu
>>
>> >
>> > Signed-off-by: Anders Wallin <wallinux@gmail.com>
>> > ---
>> > tests/regression/tools/tracker/test_event_tracker | 15 ++++++---------
>> > 1 file changed, 6 insertions(+), 9 deletions(-)
>> >
>> > diff --git a/tests/regression/tools/tracker/test_event_tracker
>> > b/tests/regression/tools/tracker/test_event_tracker
>> > index 711690af..78e9310b 100755
>> > --- a/tests/regression/tools/tracker/test_event_tracker
>> > +++ b/tests/regression/tools/tracker/test_event_tracker
>> > @@ -5,7 +5,7 @@
>> > #
>> > # SPDX-License-Identifier: GPL-2.0-only
>> >
>> > -TEST_DESC="LTTng - Event traker test"
>> > +TEST_DESC="LTTng - Event tracker test"
>> >
>> > CURDIR=$(dirname "$0")/
>> > TESTDIR="$CURDIR/../../.."
>> > @@ -30,27 +30,24 @@ SCRIPT_GROUPNAME="$(id -gn)"
>> >
>> > CHILD_PID=-1
>> > WAIT_PATH=
>> > -AFTER_FIRST_PATH=
>> > -BEFORE_LAST_PATH=
>> > +BEFORE_FIRST_PATH=
>> >
>> > source $TESTDIR/utils/utils.sh
>> >
>> > function prepare_ust_app
>> > {
>> > -     AFTER_FIRST_PATH=$(mktemp -u)
>> > -     BEFORE_LAST_PATH=$(mktemp -u)
>> > +     BEFORE_FIRST_PATH=$(mktemp -u)
>> >
>> > -     $TESTAPP_BIN -i $NR_ITER -w $NR_USEC_WAIT -a "$AFTER_FIRST_PATH"
>> -b
>> > "$BEFORE_LAST_PATH" &
>> > +     $TESTAPP_BIN -i $NR_ITER -w $NR_USEC_WAIT
>> --sync-before-first-event
>> > "$BEFORE_FIRST_PATH" &
>> >       CHILD_PID=$!
>> > }
>> >
>> > function trace_ust_app
>> > {
>> > -     touch "$BEFORE_LAST_PATH"
>> > +     touch "$BEFORE_FIRST_PATH"
>> >       wait
>> >       ok $? "Traced application stopped."
>> > -     rm "$BEFORE_LAST_PATH"
>> > -     rm "$AFTER_FIRST_PATH"
>> > +     rm "$BEFORE_FIRST_PATH"
>> > }
>> >
>> > function prepare_kernel_app
>> > --
>> > 2.31.1
>> >
>> > _______________________________________________
>> > lttng-dev mailing list
>> > lttng-dev@lists.lttng.org
>> > https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
>>
>> --
>> Mathieu Desnoyers
>> EfficiOS Inc.
>> http://www.efficios.com
>>
>
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com
>

[-- Attachment #1.2: Type: text/html, Size: 8410 bytes --]

[-- Attachment #2: Type: text/plain, Size: 156 bytes --]

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [lttng-dev] [PATCH lttng-tools] Fix: test code assumes that child process is schedule to run before parent
  2021-04-08  9:22   ` Anders Wallin via lttng-dev
@ 2021-04-08 12:48     ` Mathieu Desnoyers via lttng-dev
  2021-04-08 15:47       ` Anders Wallin via lttng-dev
  0 siblings, 1 reply; 17+ messages in thread
From: Mathieu Desnoyers via lttng-dev @ 2021-04-08 12:48 UTC (permalink / raw)
  To: Anders Wallin; +Cc: lttng-dev


[-- Attachment #1.1: Type: text/plain, Size: 4968 bytes --]

----- On Apr 8, 2021, at 5:22 AM, Anders Wallin <wallinux@gmail.com> wrote: 

> OK,

> I tried this and for
> - test_event_vpid_track_untrack ust 0 "${EVENT_NAME}"
> - test_event_pid_track_untrack ust 0 "${EVENT_NAME}"
> it works well.

> For
> - test_event_vpid_tracker ust 0 "${EVENT_NAME}"
> - test_event_pid_tracker ust 0 "${EVENT_NAME}"
> validate_trace_empty fails since there is no data at all in the trace_path

> This run is just calling copy of test_event_tracker running only
> "test_event_vpid_tracker ust 0 ..."
> 1..747
> # LTTng - Event tracker test
> ok 1 - Start session daemon
> # Test UST tracker
> ok 2 - Create session tracker in -o /tmp/tmp.lu5aeCJhQ3
> ok 3 - Enable ust event tp:tptest for session tracker
> ok 4 - Untrack command with opts: -u --all --vpid
> ok 5 - Start tracing for session
> ok 6 - Track command with opts: -u --vpid 11842
> ok 7 - Traced application stopped.
> ok 8 - Stop lttng tracing for session
> ok 9 - Destroy session tracker
> not ok 10 - Failed to parse trace
> # Failed test 'Failed to parse trace'
> # in ./regression/tools/tracker//../../../utils/tap/tap.sh:fail() at line 159.
> # Killing (signal SIGTERM) lttng-sessiond and lt-lttng-sessiond pids: 11808
> 11809 11830
> ok 11 - Wait after kill session daemon
> # Looks like you planned 747 tests but only ran 11.
> # Looks like you failed 1 test of 11.

> # find /tmp/tmp.lu5aeCJhQ3
> /tmp/tmp.lu5aeCJhQ3
> /tmp/tmp.lu5aeCJhQ3/ust

> Do you want me to add a special version of "validate_trace_empty" not returning
> "Failed to parse trace" for this? If so should it be added in
> "test_event_tracker" or in "utils.sh"

Actually, this helper already exists in utils.sh: validate_trace_session_ust_empty (), which 
internally uses validate_directory_empty (). As you point out, we should use 
validate_trace_session_ust_empty for the validation of those test cases which populate no 
trace whatsoever. 

Thanks, 

Mathieu 

> Anders Wallin

> On Wed, Apr 7, 2021 at 5:31 PM Mathieu Desnoyers < [
> mailto:mathieu.desnoyers@efficios.com | mathieu.desnoyers@efficios.com ] >
> wrote:

>> ----- On Apr 1, 2021, at 12:37 PM, lttng-dev [ mailto:lttng-dev@lists.lttng.org
>> | lttng-dev@lists.lttng.org ] wrote:

>> > the following tests fails
>> > - test_event_vpid_tracker ust 0 "${EVENT_NAME}"
>> > - test_event_vpid_track_untrack ust 0 "${EVENT_NAME}"
>> > - test_event_pid_tracker ust 0 "${EVENT_NAME}"
>> > - test_event_pid_track_untrack ust 0 "${EVENT_NAME}"

>> There is indeed an issue with these tests: there is a missing "untrack all pids"
>> which should be done at the very beginning of each test to have the expected
>> behavior. AFAIU this explains why there is a small window where traced
>> applications
>> can generate events when none are expected.

>> Fixing this should solve the issues without requiring any addition to the test
>> program.

>> Thanks,

>> Mathieu


>>> Signed-off-by: Anders Wallin < [ mailto:wallinux@gmail.com | wallinux@gmail.com
>> > ] >
>> > ---
>> > tests/regression/tools/tracker/test_event_tracker | 15 ++++++---------
>> > 1 file changed, 6 insertions(+), 9 deletions(-)

>> > diff --git a/tests/regression/tools/tracker/test_event_tracker
>> > b/tests/regression/tools/tracker/test_event_tracker
>> > index 711690af..78e9310b 100755
>> > --- a/tests/regression/tools/tracker/test_event_tracker
>> > +++ b/tests/regression/tools/tracker/test_event_tracker
>> > @@ -5,7 +5,7 @@
>> > #
>> > # SPDX-License-Identifier: GPL-2.0-only

>> > -TEST_DESC="LTTng - Event traker test"
>> > +TEST_DESC="LTTng - Event tracker test"

>> > CURDIR=$(dirname "$0")/
>> > TESTDIR="$CURDIR/../../.."
>> > @@ -30,27 +30,24 @@ SCRIPT_GROUPNAME="$(id -gn)"

>> > CHILD_PID=-1
>> > WAIT_PATH=
>> > -AFTER_FIRST_PATH=
>> > -BEFORE_LAST_PATH=
>> > +BEFORE_FIRST_PATH=

>> > source $TESTDIR/utils/utils.sh

>> > function prepare_ust_app
>> > {
>> > - AFTER_FIRST_PATH=$(mktemp -u)
>> > - BEFORE_LAST_PATH=$(mktemp -u)
>> > + BEFORE_FIRST_PATH=$(mktemp -u)

>> > - $TESTAPP_BIN -i $NR_ITER -w $NR_USEC_WAIT -a "$AFTER_FIRST_PATH" -b
>> > "$BEFORE_LAST_PATH" &
>> > + $TESTAPP_BIN -i $NR_ITER -w $NR_USEC_WAIT --sync-before-first-event
>> > "$BEFORE_FIRST_PATH" &
>> > CHILD_PID=$!
>> > }

>> > function trace_ust_app
>> > {
>> > - touch "$BEFORE_LAST_PATH"
>> > + touch "$BEFORE_FIRST_PATH"
>> > wait
>> > ok $? "Traced application stopped."
>> > - rm "$BEFORE_LAST_PATH"
>> > - rm "$AFTER_FIRST_PATH"
>> > + rm "$BEFORE_FIRST_PATH"
>> > }

>> > function prepare_kernel_app
>> > --
>> > 2.31.1

>> > _______________________________________________
>> > lttng-dev mailing list
>> > [ mailto:lttng-dev@lists.lttng.org | lttng-dev@lists.lttng.org ]
>>> [ https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev |
>> > https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev ]

>> --
>> Mathieu Desnoyers
>> EfficiOS Inc.
>> [ http://www.efficios.com/ | http://www.efficios.com ]

-- 
Mathieu Desnoyers 
EfficiOS Inc. 
http://www.efficios.com 

[-- Attachment #1.2: Type: text/html, Size: 7492 bytes --]

[-- Attachment #2: Type: text/plain, Size: 156 bytes --]

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [lttng-dev] [PATCH lttng-tools] Fix: test code assumes that child process is schedule to run before parent
  2021-04-07 15:31 ` Mathieu Desnoyers via lttng-dev
@ 2021-04-08  9:22   ` Anders Wallin via lttng-dev
  2021-04-08 12:48     ` Mathieu Desnoyers via lttng-dev
  0 siblings, 1 reply; 17+ messages in thread
From: Anders Wallin via lttng-dev @ 2021-04-08  9:22 UTC (permalink / raw)
  To: Mathieu Desnoyers; +Cc: lttng-dev


[-- Attachment #1.1: Type: text/plain, Size: 4222 bytes --]

OK,

I tried this and for
- test_event_vpid_track_untrack ust 0 "${EVENT_NAME}"
- test_event_pid_track_untrack ust 0 "${EVENT_NAME}"
it works well.

For
- test_event_vpid_tracker ust 0 "${EVENT_NAME}"
- test_event_pid_tracker ust 0 "${EVENT_NAME}"
validate_trace_empty fails since there is no data at all in the trace_path

This run is just calling copy of test_event_tracker running only
"test_event_vpid_tracker ust 0 ..."
1..747
# LTTng - Event tracker test
ok 1 - Start session daemon
# Test UST tracker
ok 2 - Create session tracker in -o /tmp/tmp.lu5aeCJhQ3
ok 3 - Enable ust event tp:tptest for session tracker
ok 4 - Untrack command with opts: -u --all --vpid
ok 5 - Start tracing for session
ok 6 - Track command with opts: -u --vpid 11842
ok 7 - Traced application stopped.
ok 8 - Stop lttng tracing for session
ok 9 - Destroy session tracker


*not ok 10 - Failed to parse trace#   Failed test 'Failed to parse trace'*#
  in ./regression/tools/tracker//../../../utils/tap/tap.sh:fail() at line
159.
# Killing (signal SIGTERM) lttng-sessiond and lt-lttng-sessiond pids: 11808
11809 11830
ok 11 - Wait after kill session daemon
# Looks like you planned 747 tests but only ran 11.
# Looks like you failed 1 test of 11.

# find /tmp/tmp.lu5aeCJhQ3
/tmp/tmp.lu5aeCJhQ3
/tmp/tmp.lu5aeCJhQ3/ust

Do you want me to add a special version of "validate_trace_empty" not
returning
"Failed to parse trace" for this? If so should it be added in
"test_event_tracker" or in "utils.sh"

Anders Wallin


On Wed, Apr 7, 2021 at 5:31 PM Mathieu Desnoyers <
mathieu.desnoyers@efficios.com> wrote:

> ----- On Apr 1, 2021, at 12:37 PM, lttng-dev lttng-dev@lists.lttng.org
> wrote:
>
> > the following tests fails
> > - test_event_vpid_tracker ust 0 "${EVENT_NAME}"
> > - test_event_vpid_track_untrack ust 0 "${EVENT_NAME}"
> > - test_event_pid_tracker ust 0 "${EVENT_NAME}"
> > - test_event_pid_track_untrack ust 0 "${EVENT_NAME}"
>
> There is indeed an issue with these tests: there is a missing "untrack all
> pids"
> which should be done at the very beginning of each test to have the
> expected
> behavior. AFAIU this explains why there is a small window where traced
> applications
> can generate events when none are expected.
>
> Fixing this should solve the issues without requiring any addition to the
> test
> program.
>
> Thanks,
>
> Mathieu
>
> >
> > Signed-off-by: Anders Wallin <wallinux@gmail.com>
> > ---
> > tests/regression/tools/tracker/test_event_tracker | 15 ++++++---------
> > 1 file changed, 6 insertions(+), 9 deletions(-)
> >
> > diff --git a/tests/regression/tools/tracker/test_event_tracker
> > b/tests/regression/tools/tracker/test_event_tracker
> > index 711690af..78e9310b 100755
> > --- a/tests/regression/tools/tracker/test_event_tracker
> > +++ b/tests/regression/tools/tracker/test_event_tracker
> > @@ -5,7 +5,7 @@
> > #
> > # SPDX-License-Identifier: GPL-2.0-only
> >
> > -TEST_DESC="LTTng - Event traker test"
> > +TEST_DESC="LTTng - Event tracker test"
> >
> > CURDIR=$(dirname "$0")/
> > TESTDIR="$CURDIR/../../.."
> > @@ -30,27 +30,24 @@ SCRIPT_GROUPNAME="$(id -gn)"
> >
> > CHILD_PID=-1
> > WAIT_PATH=
> > -AFTER_FIRST_PATH=
> > -BEFORE_LAST_PATH=
> > +BEFORE_FIRST_PATH=
> >
> > source $TESTDIR/utils/utils.sh
> >
> > function prepare_ust_app
> > {
> > -     AFTER_FIRST_PATH=$(mktemp -u)
> > -     BEFORE_LAST_PATH=$(mktemp -u)
> > +     BEFORE_FIRST_PATH=$(mktemp -u)
> >
> > -     $TESTAPP_BIN -i $NR_ITER -w $NR_USEC_WAIT -a "$AFTER_FIRST_PATH" -b
> > "$BEFORE_LAST_PATH" &
> > +     $TESTAPP_BIN -i $NR_ITER -w $NR_USEC_WAIT --sync-before-first-event
> > "$BEFORE_FIRST_PATH" &
> >       CHILD_PID=$!
> > }
> >
> > function trace_ust_app
> > {
> > -     touch "$BEFORE_LAST_PATH"
> > +     touch "$BEFORE_FIRST_PATH"
> >       wait
> >       ok $? "Traced application stopped."
> > -     rm "$BEFORE_LAST_PATH"
> > -     rm "$AFTER_FIRST_PATH"
> > +     rm "$BEFORE_FIRST_PATH"
> > }
> >
> > function prepare_kernel_app
> > --
> > 2.31.1
> >
> > _______________________________________________
> > lttng-dev mailing list
> > lttng-dev@lists.lttng.org
> > https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com
>

[-- Attachment #1.2: Type: text/html, Size: 5936 bytes --]

[-- Attachment #2: Type: text/plain, Size: 156 bytes --]

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [lttng-dev] [PATCH lttng-tools] Fix: test code assumes that child process is schedule to run before parent
  2021-04-01 16:37 Anders Wallin via lttng-dev
@ 2021-04-07 15:31 ` Mathieu Desnoyers via lttng-dev
  2021-04-08  9:22   ` Anders Wallin via lttng-dev
  0 siblings, 1 reply; 17+ messages in thread
From: Mathieu Desnoyers via lttng-dev @ 2021-04-07 15:31 UTC (permalink / raw)
  To: Anders Wallin; +Cc: lttng-dev

----- On Apr 1, 2021, at 12:37 PM, lttng-dev lttng-dev@lists.lttng.org wrote:

> the following tests fails
> - test_event_vpid_tracker ust 0 "${EVENT_NAME}"
> - test_event_vpid_track_untrack ust 0 "${EVENT_NAME}"
> - test_event_pid_tracker ust 0 "${EVENT_NAME}"
> - test_event_pid_track_untrack ust 0 "${EVENT_NAME}"

There is indeed an issue with these tests: there is a missing "untrack all pids"
which should be done at the very beginning of each test to have the expected
behavior. AFAIU this explains why there is a small window where traced applications
can generate events when none are expected.

Fixing this should solve the issues without requiring any addition to the test
program.

Thanks,

Mathieu

> 
> Signed-off-by: Anders Wallin <wallinux@gmail.com>
> ---
> tests/regression/tools/tracker/test_event_tracker | 15 ++++++---------
> 1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/tests/regression/tools/tracker/test_event_tracker
> b/tests/regression/tools/tracker/test_event_tracker
> index 711690af..78e9310b 100755
> --- a/tests/regression/tools/tracker/test_event_tracker
> +++ b/tests/regression/tools/tracker/test_event_tracker
> @@ -5,7 +5,7 @@
> #
> # SPDX-License-Identifier: GPL-2.0-only
> 
> -TEST_DESC="LTTng - Event traker test"
> +TEST_DESC="LTTng - Event tracker test"
> 
> CURDIR=$(dirname "$0")/
> TESTDIR="$CURDIR/../../.."
> @@ -30,27 +30,24 @@ SCRIPT_GROUPNAME="$(id -gn)"
> 
> CHILD_PID=-1
> WAIT_PATH=
> -AFTER_FIRST_PATH=
> -BEFORE_LAST_PATH=
> +BEFORE_FIRST_PATH=
> 
> source $TESTDIR/utils/utils.sh
> 
> function prepare_ust_app
> {
> -	AFTER_FIRST_PATH=$(mktemp -u)
> -	BEFORE_LAST_PATH=$(mktemp -u)
> +	BEFORE_FIRST_PATH=$(mktemp -u)
> 
> -	$TESTAPP_BIN -i $NR_ITER -w $NR_USEC_WAIT -a "$AFTER_FIRST_PATH" -b
> "$BEFORE_LAST_PATH" &
> +	$TESTAPP_BIN -i $NR_ITER -w $NR_USEC_WAIT --sync-before-first-event
> "$BEFORE_FIRST_PATH" &
> 	CHILD_PID=$!
> }
> 
> function trace_ust_app
> {
> -	touch "$BEFORE_LAST_PATH"
> +	touch "$BEFORE_FIRST_PATH"
> 	wait
> 	ok $? "Traced application stopped."
> -	rm "$BEFORE_LAST_PATH"
> -	rm "$AFTER_FIRST_PATH"
> +	rm "$BEFORE_FIRST_PATH"
> }
> 
> function prepare_kernel_app
> --
> 2.31.1
> 
> _______________________________________________
> lttng-dev mailing list
> lttng-dev@lists.lttng.org
> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [lttng-dev] [PATCH lttng-tools] Fix: test code assumes that child process is schedule to run before parent
@ 2021-04-01 16:37 Anders Wallin via lttng-dev
  2021-04-07 15:31 ` Mathieu Desnoyers via lttng-dev
  0 siblings, 1 reply; 17+ messages in thread
From: Anders Wallin via lttng-dev @ 2021-04-01 16:37 UTC (permalink / raw)
  To: lttng-dev

the following tests fails
- test_event_vpid_tracker ust 0 "${EVENT_NAME}"
- test_event_vpid_track_untrack ust 0 "${EVENT_NAME}"
- test_event_pid_tracker ust 0 "${EVENT_NAME}"
- test_event_pid_track_untrack ust 0 "${EVENT_NAME}"

Signed-off-by: Anders Wallin <wallinux@gmail.com>
---
 tests/regression/tools/tracker/test_event_tracker | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/tests/regression/tools/tracker/test_event_tracker b/tests/regression/tools/tracker/test_event_tracker
index 711690af..78e9310b 100755
--- a/tests/regression/tools/tracker/test_event_tracker
+++ b/tests/regression/tools/tracker/test_event_tracker
@@ -5,7 +5,7 @@
 #
 # SPDX-License-Identifier: GPL-2.0-only
 
-TEST_DESC="LTTng - Event traker test"
+TEST_DESC="LTTng - Event tracker test"
 
 CURDIR=$(dirname "$0")/
 TESTDIR="$CURDIR/../../.."
@@ -30,27 +30,24 @@ SCRIPT_GROUPNAME="$(id -gn)"
 
 CHILD_PID=-1
 WAIT_PATH=
-AFTER_FIRST_PATH=
-BEFORE_LAST_PATH=
+BEFORE_FIRST_PATH=
 
 source $TESTDIR/utils/utils.sh
 
 function prepare_ust_app
 {
-	AFTER_FIRST_PATH=$(mktemp -u)
-	BEFORE_LAST_PATH=$(mktemp -u)
+	BEFORE_FIRST_PATH=$(mktemp -u)
 
-	$TESTAPP_BIN -i $NR_ITER -w $NR_USEC_WAIT -a "$AFTER_FIRST_PATH" -b "$BEFORE_LAST_PATH" &
+	$TESTAPP_BIN -i $NR_ITER -w $NR_USEC_WAIT --sync-before-first-event "$BEFORE_FIRST_PATH" &
 	CHILD_PID=$!
 }
 
 function trace_ust_app
 {
-	touch "$BEFORE_LAST_PATH"
+	touch "$BEFORE_FIRST_PATH"
 	wait
 	ok $? "Traced application stopped."
-	rm "$BEFORE_LAST_PATH"
-	rm "$AFTER_FIRST_PATH"
+	rm "$BEFORE_FIRST_PATH"
 }
 
 function prepare_kernel_app
-- 
2.31.1

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [lttng-dev] [PATCH lttng-tools] Fix: test code assumes that child process is schedule to run before parent
  2021-04-01 16:19 ` Mathieu Desnoyers via lttng-dev
  2021-04-01 16:21   ` Mathieu Desnoyers via lttng-dev
@ 2021-04-01 16:33   ` Anders Wallin via lttng-dev
  1 sibling, 0 replies; 17+ messages in thread
From: Anders Wallin via lttng-dev @ 2021-04-01 16:33 UTC (permalink / raw)
  To: Mathieu Desnoyers; +Cc: lttng-dev, Jeremie Galarneau


[-- Attachment #1.1: Type: text/plain, Size: 2398 bytes --]

I agree, new version on the way w/o sleep!
Anders Wallin


On Thu, Apr 1, 2021 at 6:19 PM Mathieu Desnoyers <
mathieu.desnoyers@efficios.com> wrote:

> ----- On Mar 31, 2021, at 2:56 PM, lttng-dev lttng-dev@lists.lttng.org
> wrote:
>
> > the following tests fails on arm64
> > - test_event_vpid_tracker ust 0 "${EVENT_NAME}"
> > - test_event_vpid_track_untrack ust 0 "${EVENT_NAME}"
> > - test_event_pid_tracker ust 0 "${EVENT_NAME}"
> > - test_event_pid_track_untrack ust 0 "${EVENT_NAME}"
> >
> > Signed-off-by: Anders Wallin <wallinux@gmail.com>
> > ---
> > tests/regression/tools/tracker/test_event_tracker | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/tests/regression/tools/tracker/test_event_tracker
> > b/tests/regression/tools/tracker/test_event_tracker
> > index 711690af..649c7e61 100755
> > --- a/tests/regression/tools/tracker/test_event_tracker
> > +++ b/tests/regression/tools/tracker/test_event_tracker
> > @@ -5,7 +5,7 @@
> > #
> > # SPDX-License-Identifier: GPL-2.0-only
> >
> > -TEST_DESC="LTTng - Event traker test"
> > +TEST_DESC="LTTng - Event tracker test"
> >
> > CURDIR=$(dirname "$0")/
> > TESTDIR="$CURDIR/../../.."
> > @@ -42,6 +42,8 @@ function prepare_ust_app
> >
> >       $TESTAPP_BIN -i $NR_ITER -w $NR_USEC_WAIT -a "$AFTER_FIRST_PATH" -b
> >       "$BEFORE_LAST_PATH" &
> >       CHILD_PID=$!
> > +     # voluntary context switch to start $TESTAPP_BIN
> > +     sleep 0.1
>
> No, we have been bitten again and again by test issues hidden by sleeps in
> the
> test code. Using sleeps for synchronization is flaky.
>
> I don't know if we documented it, but we as maintainers are strongly
> against
> anything that looks like a delay-based approach to fixing a race in the
> tests.
> This typically just bury the race under the carpet and it shows up only in
> specific conditions on the CI workers.
>
> We need to add proper rendez-vous based synchronization to the test if some
> is missing.
>
> Adding Jérémie in CC.
>
> Thanks,
>
> Mathieu
>
>
> > }
> >
> > function trace_ust_app
> > --
> > 2.31.1
> >
> > _______________________________________________
> > lttng-dev mailing list
> > lttng-dev@lists.lttng.org
> > https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com
>

[-- Attachment #1.2: Type: text/html, Size: 3545 bytes --]

[-- Attachment #2: Type: text/plain, Size: 156 bytes --]

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [lttng-dev] [PATCH lttng-tools] Fix: test code assumes that child process is schedule to run before parent
  2021-04-01 16:19 ` Mathieu Desnoyers via lttng-dev
@ 2021-04-01 16:21   ` Mathieu Desnoyers via lttng-dev
  2021-04-01 16:33   ` Anders Wallin via lttng-dev
  1 sibling, 0 replies; 17+ messages in thread
From: Mathieu Desnoyers via lttng-dev @ 2021-04-01 16:21 UTC (permalink / raw)
  To: Mathieu Desnoyers; +Cc: Anders Wallin, lttng-dev, Jeremie Galarneau

----- On Apr 1, 2021, at 12:19 PM, lttng-dev lttng-dev@lists.lttng.org wrote:

> ----- On Mar 31, 2021, at 2:56 PM, lttng-dev lttng-dev@lists.lttng.org wrote:
> 
>> the following tests fails on arm64
>> - test_event_vpid_tracker ust 0 "${EVENT_NAME}"
>> - test_event_vpid_track_untrack ust 0 "${EVENT_NAME}"
>> - test_event_pid_tracker ust 0 "${EVENT_NAME}"
>> - test_event_pid_track_untrack ust 0 "${EVENT_NAME}"
>> 
>> Signed-off-by: Anders Wallin <wallinux@gmail.com>
>> ---
>> tests/regression/tools/tracker/test_event_tracker | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>> 
>> diff --git a/tests/regression/tools/tracker/test_event_tracker
>> b/tests/regression/tools/tracker/test_event_tracker
>> index 711690af..649c7e61 100755
>> --- a/tests/regression/tools/tracker/test_event_tracker
>> +++ b/tests/regression/tools/tracker/test_event_tracker
>> @@ -5,7 +5,7 @@
>> #
>> # SPDX-License-Identifier: GPL-2.0-only
>> 
>> -TEST_DESC="LTTng - Event traker test"
>> +TEST_DESC="LTTng - Event tracker test"
>> 
>> CURDIR=$(dirname "$0")/
>> TESTDIR="$CURDIR/../../.."
>> @@ -42,6 +42,8 @@ function prepare_ust_app
>> 
>> 	$TESTAPP_BIN -i $NR_ITER -w $NR_USEC_WAIT -a "$AFTER_FIRST_PATH" -b
>> 	"$BEFORE_LAST_PATH" &
>> 	CHILD_PID=$!
>> +	# voluntary context switch to start $TESTAPP_BIN
>> +	sleep 0.1
> 
> No, we have been bitten again and again by test issues hidden by sleeps in the
> test code. Using sleeps for synchronization is flaky.
> 
> I don't know if we documented it, but we as maintainers are strongly against
> anything that looks like a delay-based approach to fixing a race in the tests.
> This typically just bury the race under the carpet and it shows up only in
> specific conditions on the CI workers.
> 
> We need to add proper rendez-vous based synchronization to the test if some
> is missing.
> 
> Adding Jérémie in CC.

And then I read on the rest of the email thread... so it's being taken care of, good! :)

Thanks,

Mathieu

> 
> Thanks,
> 
> Mathieu
> 
> 
>> }
>> 
>> function trace_ust_app
>> --
>> 2.31.1
>> 
>> _______________________________________________
>> lttng-dev mailing list
>> lttng-dev@lists.lttng.org
>> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
> 
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com
> _______________________________________________
> lttng-dev mailing list
> lttng-dev@lists.lttng.org
> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [lttng-dev] [PATCH lttng-tools] Fix: test code assumes that child process is schedule to run before parent
  2021-03-31 18:56 Anders Wallin via lttng-dev
  2021-03-31 19:25 ` Jonathan Rajotte-Julien via lttng-dev
@ 2021-04-01 16:19 ` Mathieu Desnoyers via lttng-dev
  2021-04-01 16:21   ` Mathieu Desnoyers via lttng-dev
  2021-04-01 16:33   ` Anders Wallin via lttng-dev
  1 sibling, 2 replies; 17+ messages in thread
From: Mathieu Desnoyers via lttng-dev @ 2021-04-01 16:19 UTC (permalink / raw)
  To: Anders Wallin; +Cc: lttng-dev, Jeremie Galarneau

----- On Mar 31, 2021, at 2:56 PM, lttng-dev lttng-dev@lists.lttng.org wrote:

> the following tests fails on arm64
> - test_event_vpid_tracker ust 0 "${EVENT_NAME}"
> - test_event_vpid_track_untrack ust 0 "${EVENT_NAME}"
> - test_event_pid_tracker ust 0 "${EVENT_NAME}"
> - test_event_pid_track_untrack ust 0 "${EVENT_NAME}"
> 
> Signed-off-by: Anders Wallin <wallinux@gmail.com>
> ---
> tests/regression/tools/tracker/test_event_tracker | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/regression/tools/tracker/test_event_tracker
> b/tests/regression/tools/tracker/test_event_tracker
> index 711690af..649c7e61 100755
> --- a/tests/regression/tools/tracker/test_event_tracker
> +++ b/tests/regression/tools/tracker/test_event_tracker
> @@ -5,7 +5,7 @@
> #
> # SPDX-License-Identifier: GPL-2.0-only
> 
> -TEST_DESC="LTTng - Event traker test"
> +TEST_DESC="LTTng - Event tracker test"
> 
> CURDIR=$(dirname "$0")/
> TESTDIR="$CURDIR/../../.."
> @@ -42,6 +42,8 @@ function prepare_ust_app
> 
> 	$TESTAPP_BIN -i $NR_ITER -w $NR_USEC_WAIT -a "$AFTER_FIRST_PATH" -b
> 	"$BEFORE_LAST_PATH" &
> 	CHILD_PID=$!
> +	# voluntary context switch to start $TESTAPP_BIN
> +	sleep 0.1

No, we have been bitten again and again by test issues hidden by sleeps in the
test code. Using sleeps for synchronization is flaky.

I don't know if we documented it, but we as maintainers are strongly against
anything that looks like a delay-based approach to fixing a race in the tests.
This typically just bury the race under the carpet and it shows up only in
specific conditions on the CI workers.

We need to add proper rendez-vous based synchronization to the test if some
is missing.

Adding Jérémie in CC.

Thanks,

Mathieu


> }
> 
> function trace_ust_app
> --
> 2.31.1
> 
> _______________________________________________
> lttng-dev mailing list
> lttng-dev@lists.lttng.org
> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [lttng-dev] [PATCH lttng-tools] Fix: test code assumes that child process is schedule to run before parent
  2021-04-01 13:45         ` Jonathan Rajotte-Julien via lttng-dev
@ 2021-04-01 15:02           ` Anders Wallin via lttng-dev
  0 siblings, 0 replies; 17+ messages in thread
From: Anders Wallin via lttng-dev @ 2021-04-01 15:02 UTC (permalink / raw)
  To: Jonathan Rajotte-Julien; +Cc: lttng-dev


[-- Attachment #1.1: Type: text/plain, Size: 9268 bytes --]

Hi,

I think you understood what I meant. The issue I have is for this 4 test
cases in ./regression/tools/tracker/test_event_tracker
- test_event_vpid_tracker ust 0 "${EVENT_NAME}"
- test_event_vpid_track_untrack ust 0 "${EVENT_NAME}"
- test_event_pid_tracker ust 0 "${EVENT_NAME}"
- test_event_pid_track_untrack ust 0 "${EVENT_NAME}"

I have 2 patches ready, but I want to run some more tests first before
posting them

Anders Wallin


On Thu, Apr 1, 2021 at 3:45 PM Jonathan Rajotte-Julien <
jonathan.rajotte-julien@efficios.com> wrote:

> On Thu, Apr 01, 2021 at 03:21:10AM +0200, Anders Wallin wrote:
> > Hi Jonathan :)
> >
> > It's very unlikely that the race could occur, BUT it can happen.
> >
> > OK run
> > 1. test_event_tracker starts gen-ust-events
> > 2. test_event_tracker waits for gen-ust-events to create AFTER_FIRST_PATH
> > 3. gen-ust-event create first event and create AFTER_FIRST_PATH
> > 4. gen-ust-event continue and create 99 events
> > 5. gen-ust-event wait for  BEFORE_LAST_PATH
> > 6. test_event_track start collecting events (lttng start .....)
>
> > 7. test_event_tracker calls "lttng track ... -pid "Faulty PID"
> > 8. test_event_tracker touches BEFORE_LAST_PATH
> > 9. gen-ust-events creates the last event
> > 10. test_event finishes and since it tracks the faulty pid the result
> will
> > be 0 events == OK
>
> The sequence of event for the `test_event_tracker` function is (as of
> master):
>
> create
> enable event
> start
> track
>
> launch app
> wait for app return
> stop
> destroy
>
>
> The sequence you are describing here is:
>
> lauch app
> start
> track
> ...
>
> I'm pretty sure we are not talking about the same things. Please specify
> the
> test case and all functions involved and make sure to use the proper name
> for
> each of them.
>
> I suspect you are talking about test_event_pid_tracker. Still let's make
> sure of
> it. If it is, I do agree that it seems to have a window where we can gather
> event for.
>
> You might want to look if there is a real reason for this sequence instead
> of
> mimicking test_event_tracker
>
> Current code:
>  enable_"$domain"_lttng_event_ok $SESSION_NAME "$wildcard" "$channel"
>  prepare_"$domain"_app
>  start_lttng_tracing_ok
>
>  if [ "$expect_event" -eq 1 ]; then
>          lttng_track_"$domain"_ok "--vpid ${CHILD_PID}"
>  else
>          lttng_track_"$domain"_ok "--vpid $((CHILD_PID+1))"
>  fi
>  trace_"$domain"_app
>  stop_lttng_tracing_ok
>  destroy_lttng_session_ok $SESSION_NAME
>
>
> We might simply want to move the track command before the start
> considering have
> all the information to do so.
>
>  enable_"$domain"_lttng_event_ok $SESSION_NAME "$wildcard" "$channel"
>
>  prepare_"$domain"_app
>
>  if [ "$expect_event" -eq 1 ]; then
>          lttng_track_"$domain"_ok "--vpid ${CHILD_PID}"
>  else
>          lttng_track_"$domain"_ok "--vpid $((CHILD_PID+1))"
>  fi
>
>  start_lttng_tracing_ok
>  trace_"$domain"_app
>
>  stop_lttng_tracing_ok
>  destroy_lttng_session_ok $SESSION_NAME
>
> After testing this, seems like the validate_trace_empty does not handle the
> case were simply stream allocation did not take place since no application
> was
> 'valid' at the moment of the trace start.
>
> Well we got a good one here. We will wait for your updated patch and go
> from
> there.
>
> Cheers
>
> >
> > Faulty run
> > 1. test_event_tracker starts gen-ust-events
> > 2. test_event_tracker waits for gen-ust-events to create AFTER_FIRST_PATH
> > 3. gen-ust-event create first event and create AFTER_FIRST_PATH
> > 4. gen-ust-event gets rescheduled before it has created 99 events, e.g
> > after 9 events
> > 5. test_event_track start collecting events (lttng start .....)
> > 6. gen-ust-event is rescheduled and starts generating the remaining
> events.
> > 90 events
> > 7. lttng collects these 90 events since we have not setup "tracking" of
> PID
> > yet
> > 8. test_event_tracker calls "lttng track ... -pid "Faulty PID"
> > 9. test_event_tracker touches BEFORE_LAST_PATH
> > 10. gen-ust-events creates the last event
> > 11. test_event finishes and since it tracks the faulty pid the result
> > should  be 0 events, but we got 90 == FAULTY
> >
> > We can solve this by;
> > A: using NR_ITER=2
> > or
> > B: by adding a flag to gen-ust-events to wait before sending the first
> event
> > 1. test_event_tracker starts gen-ust-events
> > 2. test_event_tracker waits for gen-ust-events waits for
> BEFORE_FIRST_PATH
> > 3. test_event_track start collecting events (lttng start .....)
> > 4. test_event_tracker calls "lttng track ... -pid "Faulty PID"
> > 5. test_event_track creates BEFORE_FIRST_PATH
> > 6. gen_ust_event creates 100 events
> > 7. test_event_tracker waits for gen_event_ust to end
> > 8. test_event finishes and since it tracked the faulty pid the result
> will
> > be 0 events == OK
> >
> > This is in principle how gen-kernel-test-events works (but with different
> > arguments)
> > I would suggest to use B since that will be bulletproof
> >
> > Anders Wallin
> >
> >
> > On Wed, Mar 31, 2021 at 11:33 PM Jonathan Rajotte-Julien <
> > jonathan.rajotte-julien@efficios.com> wrote:
> >
> > > Hi,
> > >
> > > On Wed, Mar 31, 2021 at 11:09:42PM +0200, Anders Wallin wrote:
> > > > Hi Julian,
> > >
> > > You can use Jonathan. ;)
> > >
> > > >
> > > > Neither mine "sleep 0.1" or your version with "while [! -f
> ............"
> > > > are race condition free.
> > >
> > > I might be missing something here but as far as I understand the race
> you
> > > are
> > > trying to mitigate is that the test script execute/continue before the
> > > `backgrounded`
> > > command (background test app) had time to execute, right?
> > >
> > > If so at least waiting for the app to create a file is necessary. Now
> > > gen_kernel_test_events does not have this functionality. The
> > > PATH_WAIT_FILE is
> > > used to control when the testapp can continue. Hence the script still
> > > cannot
> > > know if the app have been scheduled.
> > >
> > > Now based on the test case you might need more synchronization for the
> test
> > > cases.
> > >
> > > Note that in the ust cases, the trace_ust_app uses `touch
> > > "$BEFORE_LAST_PATH"`
> > > that effectively unblock the app and allows it to perform the last
> > > tracepoint
> > > hit and the we `wait` on the background process.
> > >
> > > Note: some tests case are a bit clever and uses "trace_"$domain"_app"
> > > instead of
> > > calling trace_ust_app directly.
> > >
> > > For these tests case it seems that we are only expecting at least a
> single
> > > event
> > > matching the event name under test. Here the last tracepoint hit should
> > > satisfy
> > > this criteria.
> > >
> > > Am I missing a race?
> > >
> > > Cheers
> > >
> > >
> > > > I suggest that we add an option to gen-ust-events to wait before the
> > > first
> > > > event is generated.
> > > > gen_kernel_test_events already have this functionality to wait
> before the
> > > > first event.
> > > >
> > > > Something like this
> > > > static struct option long_options[] =
> > > > {
> > > > /* These options set a flag. */
> > > > {"iter", required_argument, 0, 'i'},
> > > > {"wait", required_argument, 0, 'w'},
> > > > {"sync-after-first-event", required_argument, 0, 'a'},
> > > > {"sync-before-last-event", required_argument, 0, 'b'},
> > > > {"sync-before-last-event-touch", required_argument, 0, 'c'},
> > > > {"sync-before-exit", required_argument, 0, 'd'},
> > > > {"sync-before-exit-touch", required_argument, 0, 'e'},
> > > > *+ {"sync-before-first-event", required_argument, 0, 'f'},*
> > > >
> > > > {0, 0, 0, 0}
> > > > };
> > > > ....
> > > >
> > > > I will create one or more patches for this tomorrow
> > > >
> > > > Anders Wallin
> > > >
> > > >
> > > > On Wed, Mar 31, 2021 at 9:25 PM Jonathan Rajotte-Julien <
> > > > jonathan.rajotte-julien@efficios.com> wrote:
> > > >
> > > > > > #
> > > > > > # SPDX-License-Identifier: GPL-2.0-only
> > > > > >
> > > > > > -TEST_DESC="LTTng - Event traker test"
> > > > > > +TEST_DESC="LTTng - Event tracker test"
> > > > > >
> > > > > > CURDIR=$(dirname "$0")/
> > > > > > TESTDIR="$CURDIR/../../.."
> > > > > > @@ -42,6 +42,8 @@ function prepare_ust_app
> > > > > >
> > > > > >       $TESTAPP_BIN -i $NR_ITER -w $NR_USEC_WAIT -a
> > > "$AFTER_FIRST_PATH" -b
> > > > > >       "$BEFORE_LAST_PATH" &
> > > > > >       CHILD_PID=$!
> > > > > > +     # voluntary context switch to start $TESTAPP_BIN
> > > > > > +     sleep 0.1
> > > > >
> > > > > A wait on the $AFTER_FIRST_PATH file would be probably more
> > > deterministic
> > > > > than a sleep here.
> > > > >
> > > > >   while [ ! -f "${AFTER_FIRST_PATH}" ]; do
> > > > >           sleep 0.1
> > > > >   done
> > > > >
> > > > > I would also expect something similar for the `prepare_kernel_app`
> > > > > function considering the same race is most probably present and
> simply
> > > not
> > > > > triggered by a chance of luck.
> > > > > Seems like gen-kernel-test-events does not expose the same sync
> > > > > capabilities here, please use gen-ust-events as an example of how
> it is
> > > > > done.
> > > > >
> > > > > Let us know how your testing goes.
> > > > >
> > > > > Thanks
> > > > >
> > >
> > > --
> > > Jonathan Rajotte-Julien
> > > EfficiOS
> > >
>
> --
> Jonathan Rajotte-Julien
> EfficiOS
>

[-- Attachment #1.2: Type: text/html, Size: 12595 bytes --]

[-- Attachment #2: Type: text/plain, Size: 156 bytes --]

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [lttng-dev] [PATCH lttng-tools] Fix: test code assumes that child process is schedule to run before parent
  2021-04-01  1:21       ` Anders Wallin via lttng-dev
@ 2021-04-01 13:45         ` Jonathan Rajotte-Julien via lttng-dev
  2021-04-01 15:02           ` Anders Wallin via lttng-dev
  0 siblings, 1 reply; 17+ messages in thread
From: Jonathan Rajotte-Julien via lttng-dev @ 2021-04-01 13:45 UTC (permalink / raw)
  To: Anders Wallin; +Cc: lttng-dev

On Thu, Apr 01, 2021 at 03:21:10AM +0200, Anders Wallin wrote:
> Hi Jonathan :)
> 
> It's very unlikely that the race could occur, BUT it can happen.
> 
> OK run
> 1. test_event_tracker starts gen-ust-events
> 2. test_event_tracker waits for gen-ust-events to create AFTER_FIRST_PATH
> 3. gen-ust-event create first event and create AFTER_FIRST_PATH
> 4. gen-ust-event continue and create 99 events
> 5. gen-ust-event wait for  BEFORE_LAST_PATH
> 6. test_event_track start collecting events (lttng start .....)

> 7. test_event_tracker calls "lttng track ... -pid "Faulty PID"
> 8. test_event_tracker touches BEFORE_LAST_PATH
> 9. gen-ust-events creates the last event
> 10. test_event finishes and since it tracks the faulty pid the result will
> be 0 events == OK

The sequence of event for the `test_event_tracker` function is (as of master):

create
enable event
start
track

launch app
wait for app return
stop
destroy


The sequence you are describing here is:

lauch app
start
track
...

I'm pretty sure we are not talking about the same things. Please specify the
test case and all functions involved and make sure to use the proper name for
each of them.

I suspect you are talking about test_event_pid_tracker. Still let's make sure of
it. If it is, I do agree that it seems to have a window where we can gather
event for.

You might want to look if there is a real reason for this sequence instead of
mimicking test_event_tracker

Current code:
 enable_"$domain"_lttng_event_ok $SESSION_NAME "$wildcard" "$channel"
 prepare_"$domain"_app
 start_lttng_tracing_ok

 if [ "$expect_event" -eq 1 ]; then
         lttng_track_"$domain"_ok "--vpid ${CHILD_PID}"
 else
         lttng_track_"$domain"_ok "--vpid $((CHILD_PID+1))"
 fi
 trace_"$domain"_app
 stop_lttng_tracing_ok
 destroy_lttng_session_ok $SESSION_NAME


We might simply want to move the track command before the start considering have
all the information to do so.

 enable_"$domain"_lttng_event_ok $SESSION_NAME "$wildcard" "$channel"

 prepare_"$domain"_app

 if [ "$expect_event" -eq 1 ]; then
         lttng_track_"$domain"_ok "--vpid ${CHILD_PID}"
 else
         lttng_track_"$domain"_ok "--vpid $((CHILD_PID+1))"
 fi

 start_lttng_tracing_ok
 trace_"$domain"_app

 stop_lttng_tracing_ok
 destroy_lttng_session_ok $SESSION_NAME

After testing this, seems like the validate_trace_empty does not handle the
case were simply stream allocation did not take place since no application was
'valid' at the moment of the trace start.

Well we got a good one here. We will wait for your updated patch and go from
there.

Cheers

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

-- 
Jonathan Rajotte-Julien
EfficiOS
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [lttng-dev] [PATCH lttng-tools] Fix: test code assumes that child process is schedule to run before parent
  2021-03-31 21:33     ` Jonathan Rajotte-Julien via lttng-dev
@ 2021-04-01  1:21       ` Anders Wallin via lttng-dev
  2021-04-01 13:45         ` Jonathan Rajotte-Julien via lttng-dev
  0 siblings, 1 reply; 17+ messages in thread
From: Anders Wallin via lttng-dev @ 2021-04-01  1:21 UTC (permalink / raw)
  To: Jonathan Rajotte-Julien; +Cc: lttng-dev


[-- Attachment #1.1: Type: text/plain, Size: 5911 bytes --]

Hi Jonathan :)

It's very unlikely that the race could occur, BUT it can happen.

OK run
1. test_event_tracker starts gen-ust-events
2. test_event_tracker waits for gen-ust-events to create AFTER_FIRST_PATH
3. gen-ust-event create first event and create AFTER_FIRST_PATH
4. gen-ust-event continue and create 99 events
5. gen-ust-event wait for  BEFORE_LAST_PATH
6. test_event_track start collecting events (lttng start .....)
7. test_event_tracker calls "lttng track ... -pid "Faulty PID"
8. test_event_tracker touches BEFORE_LAST_PATH
9. gen-ust-events creates the last event
10. test_event finishes and since it tracks the faulty pid the result will
be 0 events == OK

Faulty run
1. test_event_tracker starts gen-ust-events
2. test_event_tracker waits for gen-ust-events to create AFTER_FIRST_PATH
3. gen-ust-event create first event and create AFTER_FIRST_PATH
4. gen-ust-event gets rescheduled before it has created 99 events, e.g
after 9 events
5. test_event_track start collecting events (lttng start .....)
6. gen-ust-event is rescheduled and starts generating the remaining events.
90 events
7. lttng collects these 90 events since we have not setup "tracking" of PID
yet
8. test_event_tracker calls "lttng track ... -pid "Faulty PID"
9. test_event_tracker touches BEFORE_LAST_PATH
10. gen-ust-events creates the last event
11. test_event finishes and since it tracks the faulty pid the result
should  be 0 events, but we got 90 == FAULTY

We can solve this by;
A: using NR_ITER=2
or
B: by adding a flag to gen-ust-events to wait before sending the first event
1. test_event_tracker starts gen-ust-events
2. test_event_tracker waits for gen-ust-events waits for BEFORE_FIRST_PATH
3. test_event_track start collecting events (lttng start .....)
4. test_event_tracker calls "lttng track ... -pid "Faulty PID"
5. test_event_track creates BEFORE_FIRST_PATH
6. gen_ust_event creates 100 events
7. test_event_tracker waits for gen_event_ust to end
8. test_event finishes and since it tracked the faulty pid the result will
be 0 events == OK

This is in principle how gen-kernel-test-events works (but with different
arguments)
I would suggest to use B since that will be bulletproof

Anders Wallin


On Wed, Mar 31, 2021 at 11:33 PM Jonathan Rajotte-Julien <
jonathan.rajotte-julien@efficios.com> wrote:

> Hi,
>
> On Wed, Mar 31, 2021 at 11:09:42PM +0200, Anders Wallin wrote:
> > Hi Julian,
>
> You can use Jonathan. ;)
>
> >
> > Neither mine "sleep 0.1" or your version with "while [! -f ............"
> > are race condition free.
>
> I might be missing something here but as far as I understand the race you
> are
> trying to mitigate is that the test script execute/continue before the
> `backgrounded`
> command (background test app) had time to execute, right?
>
> If so at least waiting for the app to create a file is necessary. Now
> gen_kernel_test_events does not have this functionality. The
> PATH_WAIT_FILE is
> used to control when the testapp can continue. Hence the script still
> cannot
> know if the app have been scheduled.
>
> Now based on the test case you might need more synchronization for the test
> cases.
>
> Note that in the ust cases, the trace_ust_app uses `touch
> "$BEFORE_LAST_PATH"`
> that effectively unblock the app and allows it to perform the last
> tracepoint
> hit and the we `wait` on the background process.
>
> Note: some tests case are a bit clever and uses "trace_"$domain"_app"
> instead of
> calling trace_ust_app directly.
>
> For these tests case it seems that we are only expecting at least a single
> event
> matching the event name under test. Here the last tracepoint hit should
> satisfy
> this criteria.
>
> Am I missing a race?
>
> Cheers
>
>
> > I suggest that we add an option to gen-ust-events to wait before the
> first
> > event is generated.
> > gen_kernel_test_events already have this functionality to wait before the
> > first event.
> >
> > Something like this
> > static struct option long_options[] =
> > {
> > /* These options set a flag. */
> > {"iter", required_argument, 0, 'i'},
> > {"wait", required_argument, 0, 'w'},
> > {"sync-after-first-event", required_argument, 0, 'a'},
> > {"sync-before-last-event", required_argument, 0, 'b'},
> > {"sync-before-last-event-touch", required_argument, 0, 'c'},
> > {"sync-before-exit", required_argument, 0, 'd'},
> > {"sync-before-exit-touch", required_argument, 0, 'e'},
> > *+ {"sync-before-first-event", required_argument, 0, 'f'},*
> >
> > {0, 0, 0, 0}
> > };
> > ....
> >
> > I will create one or more patches for this tomorrow
> >
> > Anders Wallin
> >
> >
> > On Wed, Mar 31, 2021 at 9:25 PM Jonathan Rajotte-Julien <
> > jonathan.rajotte-julien@efficios.com> wrote:
> >
> > > > #
> > > > # SPDX-License-Identifier: GPL-2.0-only
> > > >
> > > > -TEST_DESC="LTTng - Event traker test"
> > > > +TEST_DESC="LTTng - Event tracker test"
> > > >
> > > > CURDIR=$(dirname "$0")/
> > > > TESTDIR="$CURDIR/../../.."
> > > > @@ -42,6 +42,8 @@ function prepare_ust_app
> > > >
> > > >       $TESTAPP_BIN -i $NR_ITER -w $NR_USEC_WAIT -a
> "$AFTER_FIRST_PATH" -b
> > > >       "$BEFORE_LAST_PATH" &
> > > >       CHILD_PID=$!
> > > > +     # voluntary context switch to start $TESTAPP_BIN
> > > > +     sleep 0.1
> > >
> > > A wait on the $AFTER_FIRST_PATH file would be probably more
> deterministic
> > > than a sleep here.
> > >
> > >   while [ ! -f "${AFTER_FIRST_PATH}" ]; do
> > >           sleep 0.1
> > >   done
> > >
> > > I would also expect something similar for the `prepare_kernel_app`
> > > function considering the same race is most probably present and simply
> not
> > > triggered by a chance of luck.
> > > Seems like gen-kernel-test-events does not expose the same sync
> > > capabilities here, please use gen-ust-events as an example of how it is
> > > done.
> > >
> > > Let us know how your testing goes.
> > >
> > > Thanks
> > >
>
> --
> Jonathan Rajotte-Julien
> EfficiOS
>

[-- Attachment #1.2: Type: text/html, Size: 7957 bytes --]

[-- Attachment #2: Type: text/plain, Size: 156 bytes --]

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [lttng-dev] [PATCH lttng-tools] Fix: test code assumes that child process is schedule to run before parent
  2021-03-31 21:09   ` Anders Wallin via lttng-dev
@ 2021-03-31 21:33     ` Jonathan Rajotte-Julien via lttng-dev
  2021-04-01  1:21       ` Anders Wallin via lttng-dev
  0 siblings, 1 reply; 17+ messages in thread
From: Jonathan Rajotte-Julien via lttng-dev @ 2021-03-31 21:33 UTC (permalink / raw)
  To: Anders Wallin; +Cc: lttng-dev

Hi,

On Wed, Mar 31, 2021 at 11:09:42PM +0200, Anders Wallin wrote:
> Hi Julian,

You can use Jonathan. ;)

> 
> Neither mine "sleep 0.1" or your version with "while [! -f ............"
> are race condition free.

I might be missing something here but as far as I understand the race you are
trying to mitigate is that the test script execute/continue before the `backgrounded`
command (background test app) had time to execute, right?

If so at least waiting for the app to create a file is necessary. Now
gen_kernel_test_events does not have this functionality. The PATH_WAIT_FILE is
used to control when the testapp can continue. Hence the script still cannot
know if the app have been scheduled.

Now based on the test case you might need more synchronization for the test
cases.

Note that in the ust cases, the trace_ust_app uses `touch "$BEFORE_LAST_PATH"`
that effectively unblock the app and allows it to perform the last tracepoint
hit and the we `wait` on the background process.

Note: some tests case are a bit clever and uses "trace_"$domain"_app" instead of
calling trace_ust_app directly.

For these tests case it seems that we are only expecting at least a single event
matching the event name under test. Here the last tracepoint hit should satisfy
this criteria.

Am I missing a race?

Cheers


> I suggest that we add an option to gen-ust-events to wait before the first
> event is generated.
> gen_kernel_test_events already have this functionality to wait before the
> first event.
> 
> Something like this
> static struct option long_options[] =
> {
> /* These options set a flag. */
> {"iter", required_argument, 0, 'i'},
> {"wait", required_argument, 0, 'w'},
> {"sync-after-first-event", required_argument, 0, 'a'},
> {"sync-before-last-event", required_argument, 0, 'b'},
> {"sync-before-last-event-touch", required_argument, 0, 'c'},
> {"sync-before-exit", required_argument, 0, 'd'},
> {"sync-before-exit-touch", required_argument, 0, 'e'},
> *+ {"sync-before-first-event", required_argument, 0, 'f'},*
> 
> {0, 0, 0, 0}
> };
> ....
> 
> I will create one or more patches for this tomorrow
> 
> Anders Wallin
> 
> 
> On Wed, Mar 31, 2021 at 9:25 PM Jonathan Rajotte-Julien <
> jonathan.rajotte-julien@efficios.com> wrote:
> 
> > > #
> > > # SPDX-License-Identifier: GPL-2.0-only
> > >
> > > -TEST_DESC="LTTng - Event traker test"
> > > +TEST_DESC="LTTng - Event tracker test"
> > >
> > > CURDIR=$(dirname "$0")/
> > > TESTDIR="$CURDIR/../../.."
> > > @@ -42,6 +42,8 @@ function prepare_ust_app
> > >
> > >       $TESTAPP_BIN -i $NR_ITER -w $NR_USEC_WAIT -a "$AFTER_FIRST_PATH" -b
> > >       "$BEFORE_LAST_PATH" &
> > >       CHILD_PID=$!
> > > +     # voluntary context switch to start $TESTAPP_BIN
> > > +     sleep 0.1
> >
> > A wait on the $AFTER_FIRST_PATH file would be probably more deterministic
> > than a sleep here.
> >
> >   while [ ! -f "${AFTER_FIRST_PATH}" ]; do
> >           sleep 0.1
> >   done
> >
> > I would also expect something similar for the `prepare_kernel_app`
> > function considering the same race is most probably present and simply not
> > triggered by a chance of luck.
> > Seems like gen-kernel-test-events does not expose the same sync
> > capabilities here, please use gen-ust-events as an example of how it is
> > done.
> >
> > Let us know how your testing goes.
> >
> > Thanks
> >

-- 
Jonathan Rajotte-Julien
EfficiOS
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [lttng-dev] [PATCH lttng-tools] Fix: test code assumes that child process is schedule to run before parent
  2021-03-31 19:25 ` Jonathan Rajotte-Julien via lttng-dev
@ 2021-03-31 21:09   ` Anders Wallin via lttng-dev
  2021-03-31 21:33     ` Jonathan Rajotte-Julien via lttng-dev
  0 siblings, 1 reply; 17+ messages in thread
From: Anders Wallin via lttng-dev @ 2021-03-31 21:09 UTC (permalink / raw)
  To: Jonathan Rajotte-Julien; +Cc: lttng-dev


[-- Attachment #1.1: Type: text/plain, Size: 2015 bytes --]

Hi Julian,

Neither mine "sleep 0.1" or your version with "while [! -f ............"
are race condition free.
I suggest that we add an option to gen-ust-events to wait before the first
event is generated.
gen_kernel_test_events already have this functionality to wait before the
first event.

Something like this
static struct option long_options[] =
{
/* These options set a flag. */
{"iter", required_argument, 0, 'i'},
{"wait", required_argument, 0, 'w'},
{"sync-after-first-event", required_argument, 0, 'a'},
{"sync-before-last-event", required_argument, 0, 'b'},
{"sync-before-last-event-touch", required_argument, 0, 'c'},
{"sync-before-exit", required_argument, 0, 'd'},
{"sync-before-exit-touch", required_argument, 0, 'e'},
*+ {"sync-before-first-event", required_argument, 0, 'f'},*

{0, 0, 0, 0}
};
....

I will create one or more patches for this tomorrow

Anders Wallin


On Wed, Mar 31, 2021 at 9:25 PM Jonathan Rajotte-Julien <
jonathan.rajotte-julien@efficios.com> wrote:

> > #
> > # SPDX-License-Identifier: GPL-2.0-only
> >
> > -TEST_DESC="LTTng - Event traker test"
> > +TEST_DESC="LTTng - Event tracker test"
> >
> > CURDIR=$(dirname "$0")/
> > TESTDIR="$CURDIR/../../.."
> > @@ -42,6 +42,8 @@ function prepare_ust_app
> >
> >       $TESTAPP_BIN -i $NR_ITER -w $NR_USEC_WAIT -a "$AFTER_FIRST_PATH" -b
> >       "$BEFORE_LAST_PATH" &
> >       CHILD_PID=$!
> > +     # voluntary context switch to start $TESTAPP_BIN
> > +     sleep 0.1
>
> A wait on the $AFTER_FIRST_PATH file would be probably more deterministic
> than a sleep here.
>
>   while [ ! -f "${AFTER_FIRST_PATH}" ]; do
>           sleep 0.1
>   done
>
> I would also expect something similar for the `prepare_kernel_app`
> function considering the same race is most probably present and simply not
> triggered by a chance of luck.
> Seems like gen-kernel-test-events does not expose the same sync
> capabilities here, please use gen-ust-events as an example of how it is
> done.
>
> Let us know how your testing goes.
>
> Thanks
>

[-- Attachment #1.2: Type: text/html, Size: 3125 bytes --]

[-- Attachment #2: Type: text/plain, Size: 156 bytes --]

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [lttng-dev] [PATCH lttng-tools] Fix: test code assumes that child process is schedule to run before parent
  2021-03-31 18:56 Anders Wallin via lttng-dev
@ 2021-03-31 19:25 ` Jonathan Rajotte-Julien via lttng-dev
  2021-03-31 21:09   ` Anders Wallin via lttng-dev
  2021-04-01 16:19 ` Mathieu Desnoyers via lttng-dev
  1 sibling, 1 reply; 17+ messages in thread
From: Jonathan Rajotte-Julien via lttng-dev @ 2021-03-31 19:25 UTC (permalink / raw)
  To: Anders Wallin; +Cc: lttng-dev

> #
> # SPDX-License-Identifier: GPL-2.0-only
> 
> -TEST_DESC="LTTng - Event traker test"
> +TEST_DESC="LTTng - Event tracker test"
> 
> CURDIR=$(dirname "$0")/
> TESTDIR="$CURDIR/../../.."
> @@ -42,6 +42,8 @@ function prepare_ust_app
> 
> 	$TESTAPP_BIN -i $NR_ITER -w $NR_USEC_WAIT -a "$AFTER_FIRST_PATH" -b
> 	"$BEFORE_LAST_PATH" &
> 	CHILD_PID=$!
> +	# voluntary context switch to start $TESTAPP_BIN
> +	sleep 0.1

A wait on the $AFTER_FIRST_PATH file would be probably more deterministic than a sleep here.

  while [ ! -f "${AFTER_FIRST_PATH}" ]; do
          sleep 0.1
  done

I would also expect something similar for the `prepare_kernel_app` function considering the same race is most probably present and simply not triggered by a chance of luck.
Seems like gen-kernel-test-events does not expose the same sync capabilities here, please use gen-ust-events as an example of how it is done.

Let us know how your testing goes.

Thanks
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [lttng-dev] [PATCH lttng-tools] Fix: test code assumes that child process is schedule to run before parent
@ 2021-03-31 18:56 Anders Wallin via lttng-dev
  2021-03-31 19:25 ` Jonathan Rajotte-Julien via lttng-dev
  2021-04-01 16:19 ` Mathieu Desnoyers via lttng-dev
  0 siblings, 2 replies; 17+ messages in thread
From: Anders Wallin via lttng-dev @ 2021-03-31 18:56 UTC (permalink / raw)
  To: lttng-dev

the following tests fails on arm64
- test_event_vpid_tracker ust 0 "${EVENT_NAME}"
- test_event_vpid_track_untrack ust 0 "${EVENT_NAME}"
- test_event_pid_tracker ust 0 "${EVENT_NAME}"
- test_event_pid_track_untrack ust 0 "${EVENT_NAME}"

Signed-off-by: Anders Wallin <wallinux@gmail.com>
---
 tests/regression/tools/tracker/test_event_tracker | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/regression/tools/tracker/test_event_tracker b/tests/regression/tools/tracker/test_event_tracker
index 711690af..649c7e61 100755
--- a/tests/regression/tools/tracker/test_event_tracker
+++ b/tests/regression/tools/tracker/test_event_tracker
@@ -5,7 +5,7 @@
 #
 # SPDX-License-Identifier: GPL-2.0-only
 
-TEST_DESC="LTTng - Event traker test"
+TEST_DESC="LTTng - Event tracker test"
 
 CURDIR=$(dirname "$0")/
 TESTDIR="$CURDIR/../../.."
@@ -42,6 +42,8 @@ function prepare_ust_app
 
 	$TESTAPP_BIN -i $NR_ITER -w $NR_USEC_WAIT -a "$AFTER_FIRST_PATH" -b "$BEFORE_LAST_PATH" &
 	CHILD_PID=$!
+	# voluntary context switch to start $TESTAPP_BIN
+	sleep 0.1
 }
 
 function trace_ust_app
-- 
2.31.1

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2021-04-09 19:12 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-09 14:41 [lttng-dev] [PATCH lttng-tools] Fix: test code assumes that child process is schedule to run before parent Anders Wallin via lttng-dev
2021-04-09 19:12 ` Mathieu Desnoyers via lttng-dev
  -- strict thread matches above, loose matches on Subject: below --
2021-04-01 16:37 Anders Wallin via lttng-dev
2021-04-07 15:31 ` Mathieu Desnoyers via lttng-dev
2021-04-08  9:22   ` Anders Wallin via lttng-dev
2021-04-08 12:48     ` Mathieu Desnoyers via lttng-dev
2021-04-08 15:47       ` Anders Wallin via lttng-dev
2021-03-31 18:56 Anders Wallin via lttng-dev
2021-03-31 19:25 ` Jonathan Rajotte-Julien via lttng-dev
2021-03-31 21:09   ` Anders Wallin via lttng-dev
2021-03-31 21:33     ` Jonathan Rajotte-Julien via lttng-dev
2021-04-01  1:21       ` Anders Wallin via lttng-dev
2021-04-01 13:45         ` Jonathan Rajotte-Julien via lttng-dev
2021-04-01 15:02           ` Anders Wallin via lttng-dev
2021-04-01 16:19 ` Mathieu Desnoyers via lttng-dev
2021-04-01 16:21   ` Mathieu Desnoyers via lttng-dev
2021-04-01 16:33   ` Anders Wallin via lttng-dev

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).