[PATCH lttng-tools v3 2/3] Test the parsing of perf raw context

[PATCH lttng-tools v3 2/3] Test the parsing of perf raw context

[Julien Desfossez]

Only test the parsing of the new option in fast_regression since the
real integration test requires particular hardware and kernel
configuration which might not be available.

Signed-off-by: Julien Desfossez <jdesfossez@efficios.com>
---
 tests/fast_regression                |  1 +
 tests/regression/ust/test_event_perf | 47 ++++++++++++++++++++++++++----------
 2 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/tests/fast_regression b/tests/fast_regression
index 0c4f079..ab5521f 100644
--- a/tests/fast_regression
+++ b/tests/fast_regression
@@ -30,3 +30,4 @@ regression/ust/clock-override/test_clock_override
 regression/ust/rotation-destroy-flush/test_rotation_destroy_flush
 regression/ust/test_event_basic
 regression/ust/test_event_tracef
+regression/ust/test_event_perf
diff --git a/tests/regression/ust/test_event_perf b/tests/regression/ust/test_event_perf
index 7dc2168..133dafa 100755
--- a/tests/regression/ust/test_event_perf
+++ b/tests/regression/ust/test_event_perf
@@ -23,7 +23,7 @@ TESTDIR=$CURDIR/../..
 LTTNG_BIN="lttng"
 SESSION_NAME="perf_counters"
 EVENT_NAME="tp:tptest"
-NUM_TESTS=10
+NUM_TESTS=24
 NR_ITER=1
 NR_USEC_WAIT=1
 TESTAPP_PATH="$TESTDIR/utils/testapp"
@@ -42,14 +42,33 @@ function enable_ust_lttng_event_per_chan()
 	ok $? "Enable event $event_name for session $sess_name in channel $chan_name"
 }
 
-function add_ust_lttng_context()
+# Only test parsing of the enabling by raw ID
+function test_parsing_raw()
 {
-	sess_name="$1"
-	chan_name="$2"
-	type="$3"
+	TRACE_PATH=$(mktemp -d)
+	SESSION_NAME="ust_event_basic"
+	CHAN_NAME="mychan"
+
+	create_lttng_session_ok $SESSION_NAME $TRACE_PATH
+
+	enable_ust_lttng_channel_ok $SESSION_NAME $CHAN_NAME
+
+	enable_ust_lttng_event_per_chan $SESSION_NAME $EVENT_NAME $CHAN_NAME
 
-	$TESTDIR/../src/bin/lttng/$LTTNG_BIN add-context -s $sess_name -c $chan_name -t $type -u >/dev/null 2>&1
-	ok $? "Add context $type for session $sess_name in channel $chan_name"
+	add_context_ust_ok $SESSION_NAME $CHAN_NAME "perf:thread:raw:r0110:test"
+	add_context_ust_fail $SESSION_NAME $CHAN_NAME "perf:thread:raw:rZZZ:test"
+	add_context_ust_fail $SESSION_NAME $CHAN_NAME "perf:thread:raw:b0110:test"
+	add_context_ust_fail $SESSION_NAME $CHAN_NAME "perf:thread:raw:r0110:"
+	add_context_ust_fail $SESSION_NAME $CHAN_NAME "perf:thread:raw:r0110::"
+	add_context_ust_fail $SESSION_NAME $CHAN_NAME "perf:thread:raw:r:test"
+	add_context_ust_fail $SESSION_NAME $CHAN_NAME "perf:thread:raw:r::"
+	add_context_ust_fail $SESSION_NAME $CHAN_NAME "perf:thread:raw::"
+	add_context_ust_fail $SESSION_NAME $CHAN_NAME "perf:thread:raw"
+	add_context_ust_fail $SESSION_NAME $CHAN_NAME "perf:thread:raw:r0110:test:wrong"
+
+	destroy_lttng_session_ok $SESSION_NAME
+
+	rm -rf $TRACE_PATH
 }
 
 function test_event_basic()
@@ -64,7 +83,7 @@ function test_event_basic()
 
 	enable_ust_lttng_event_per_chan $SESSION_NAME $EVENT_NAME $CHAN_NAME
 
-	add_ust_lttng_context $SESSION_NAME $CHAN_NAME "perf:thread:page-fault"
+	add_context_ust_ok $SESSION_NAME $CHAN_NAME "perf:thread:page-fault"
 
 	start_lttng_tracing_ok
 
@@ -96,11 +115,13 @@ else
         isroot=0
 fi
 
-skip $isroot "Root access is needed. Skipping UST perf tests." ${NUM_TESTS} ||
-{
-	start_lttng_sessiond
+start_lttng_sessiond
 
-	test_event_basic
+test_parsing_raw
 
-	stop_lttng_sessiond
+skip $isroot "Root access is needed. Skipping UST perf tests." 8 ||
+{
+	test_event_basic
 }
+
+stop_lttng_sessiond
-- 
1.9.1

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

[PATCH lttng-tools v3 3/3] Create a dedicated test suite for Perf

[Julien Desfossez]

Introduce the perf_regression test suite that must be run manually to
check if the support for the Perf-related features are available on the
current machine. This test cannot be run automatically since there are
some platforms where it can fail.

For now, the test only makes sure that we can trace events with perf
contexts enabled by raw ID in kernel and user-space. The test only works
if libpfm is installed on the system and fails if it is not installed.

Signed-off-by: Julien Desfossez <jdesfossez@efficios.com>
---
 .gitignore               |   1 +
 README.md                |   2 +
 configure.ac             |   8 +++
 tests/Makefile.am        |   8 +--
 tests/perf/Makefile.am   |   6 +++
 tests/perf/find_event.c  |  83 ++++++++++++++++++++++++++++++
 tests/perf/test_perf_raw | 129 +++++++++++++++++++++++++++++++++++++++++++++++
 tests/perf_regression    |   1 +
 8 files changed, 234 insertions(+), 4 deletions(-)
 create mode 100644 tests/perf/Makefile.am
 create mode 100644 tests/perf/find_event.c
 create mode 100755 tests/perf/test_perf_raw
 create mode 100644 tests/perf_regression

diff --git a/.gitignore b/.gitignore
index ac78292..c31dbc5 100644
--- a/.gitignore
+++ b/.gitignore
@@ -106,6 +106,7 @@ tests/regression/ust/python-logging/test_python_logging
 /tests/utils/testapp/gen-ust-tracef/gen-ust-tracef
 /tests/regression/tools/live/live_test
 /tests/unit/ini_config/ini_config
+/tests/perf/find_event
 
 # man pages
 /doc/man/*.1
diff --git a/README.md b/README.md
index 22de252..3156a15 100644
--- a/README.md
+++ b/README.md
@@ -57,6 +57,8 @@ The following items are _optional_ dependencies:
     pages with the `--help` option or with the `lttng help` command.
     Note that without `man`, you cannot get offline help with
     LTTng-tools commands, not even their usage.
+  - **`libpfm`**: needed to run the perf regression test suite.
+    - Debian/Ubuntu package: `libpfm4-dev`
 
 LTTng-tools supports both the [LTTng Linux Kernel tracer](https://lttng.org)
 and [LTTng user space tracer](https://lttng.org) released as part of the same
diff --git a/configure.ac b/configure.ac
index fb2b013..f96d25b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -467,6 +467,13 @@ AC_CHECK_LIB([c], [open_memstream],
 ]
 )
 
+# check for libpfm
+AC_CHECK_LIB([pfm], [pfm_initialize],
+	     [
+	      have_libpfm=yes
+	      ])
+AM_CONDITIONAL([LTTNG_TOOLS_BUILD_WITH_LIBPFM], [test "x$have_libpfm" = "xyes"])
+
 AC_ARG_ENABLE([git-version],
               [AC_HELP_STRING([--disable-git-version],
                               [Do not use the git version for the build])],
@@ -1008,6 +1015,7 @@ AC_CONFIG_FILES([
 	tests/stress/Makefile
 	tests/unit/Makefile
 	tests/unit/ini_config/Makefile
+	tests/perf/Makefile
 	tests/utils/Makefile
 	tests/utils/tap/Makefile
 	tests/utils/testapp/Makefile
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 3600e99..d460fb6 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -1,8 +1,8 @@
 SUBDIRS =
-DIST_SUBDIRS = utils regression unit stress destructive
+DIST_SUBDIRS = utils regression unit stress destructive perf
 
 if BUILD_TESTS
-SUBDIRS += . utils regression unit stress destructive
+SUBDIRS += . utils regression unit stress destructive perf
 if HAS_PGREP
 check-am:
 	$(top_srcdir)/tests/utils/warn_processes.sh $(PGREP)
@@ -14,8 +14,8 @@ else
 endif
 
 
-dist_noinst_SCRIPTS = run.sh fast_regression long_regression root_regression root_destructive_tests
-EXTRA_DIST = run.sh fast_regression long_regression root_regression README root_destructive_tests
+dist_noinst_SCRIPTS = run.sh fast_regression long_regression root_regression root_destructive_tests perf_regression
+EXTRA_DIST = run.sh fast_regression long_regression root_regression README root_destructive_tests perf_regression
 
 all-local:
 	@if [ x"$(srcdir)" != x"$(builddir)" ]; then \
diff --git a/tests/perf/Makefile.am b/tests/perf/Makefile.am
new file mode 100644
index 0000000..40bb754
--- /dev/null
+++ b/tests/perf/Makefile.am
@@ -0,0 +1,6 @@
+if LTTNG_TOOLS_BUILD_WITH_LIBPFM
+LIBS += -lpfm
+
+noinst_PROGRAMS = find_event
+find_event_SOURCES = find_event.c
+endif
diff --git a/tests/perf/find_event.c b/tests/perf/find_event.c
new file mode 100644
index 0000000..ae63800
--- /dev/null
+++ b/tests/perf/find_event.c
@@ -0,0 +1,83 @@
+/*
+ * Copyright (c)  2016 Julien Desfossez <jdesfossez@efficios.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * as published by the Free Software Foundation; only version 2
+ * of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include <stdio.h>
+#include <perfmon/pfmlib.h>
+#include <string.h>
+
+int main(int argc, char **argv)
+{
+	int ret, i;
+	unsigned int j;
+	pfm_pmu_info_t pinfo;
+
+	if (argc != 2) {
+		fprintf(stderr, "Usage: %s <pmu counter to find>\n"
+				"ex: %s UNHALTED_REFERENCE_CYCLES\n"
+				"Returns the first occurence it finds with "
+				"return code 0.\n"
+				"If not found returns 1, on error returns -1\n",
+				argv[0], argv[0]);
+		ret = -1;
+		goto end;
+	}
+
+	memset(&pinfo, 0, sizeof(pinfo));
+	pinfo.size = sizeof(pinfo);
+
+	ret = pfm_initialize();
+	if (ret != PFM_SUCCESS) {
+		fprintf(stderr, "Failed to initialise libpfm: %s",
+				pfm_strerror(ret));
+		ret = -1;
+		goto end;
+	}
+
+	pfm_for_all_pmus(j) {
+		ret = pfm_get_pmu_info(j, &pinfo);
+		if (ret != PFM_SUCCESS) {
+			continue;
+		}
+
+		for (i = pinfo.first_event; i != -1; i = pfm_get_event_next(i)) {
+			pfm_event_info_t info;
+
+			ret = pfm_get_event_info(i, PFM_OS_NONE, &info);
+			if (ret != PFM_SUCCESS) {
+				fprintf(stderr, "Cannot get event info: %s\n",
+						pfm_strerror(ret));
+				ret = -1;
+				goto end;
+			}
+
+			if (info.pmu != j)
+				continue;
+
+			if (strcmp(info.name, argv[1]) == 0) {
+				fprintf(stdout, "r%lx\n", info.code);
+				ret = 0;
+				goto end;
+			}
+		}
+	}
+
+	ret = 1;
+
+end:
+	return ret;
+}
diff --git a/tests/perf/test_perf_raw b/tests/perf/test_perf_raw
new file mode 100755
index 0000000..68b12d7
--- /dev/null
+++ b/tests/perf/test_perf_raw
@@ -0,0 +1,129 @@
+#!/bin/bash
+#
+# Copyright (C) - 2016 Julien Desfossez <jdesfossez@efficios.com>
+#
+# This program is free software; you can redistribute it and/or modify it
+# under the terms of the GNU General Public License, version 2 only, as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+# FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+# more details.
+#
+# You should have received a copy of the GNU General Public License along with
+# this program; if not, write to the Free Software Foundation, Inc., 51
+# Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+TEST_DESC="Perf counters"
+
+CURDIR=$(dirname $0)/
+TESTDIR=$CURDIR/..
+LTTNG_BIN="lttng"
+SESSION_NAME="perf_counters"
+NUM_TESTS=20
+NR_ITER=1
+NR_USEC_WAIT=1
+TESTAPP_PATH="$TESTDIR/utils/testapp"
+TESTAPP_NAME="gen-ust-events"
+TESTAPP_BIN="$TESTAPP_PATH/$TESTAPP_NAME/$TESTAPP_NAME"
+
+source $TESTDIR/utils/utils.sh
+
+function enable_ust_lttng_event_per_chan()
+{
+	sess_name="$1"
+	event_name="$2"
+	chan_name="$3"
+
+	$TESTDIR/../src/bin/lttng/$LTTNG_BIN enable-event "$event_name" -s $sess_name -c $chan_name -u >/dev/null 2>&1
+	ok $? "Enable event $event_name for session $sess_name in channel $chan_name"
+}
+
+function test_ust_raw()
+{
+	TRACE_PATH=$(mktemp -d)
+	SESSION_NAME="ust_perf"
+	CHAN_NAME="mychan"
+	EVENT_NAME="tp:tptest"
+	PMU="UNHALTED_REFERENCE_CYCLES"
+	PERFID=$($CURDIR/find_event $PMU)
+	test $? -eq "0"
+	ok $? "Find PMU $PMU"
+
+	create_lttng_session_ok $SESSION_NAME $TRACE_PATH
+
+	enable_ust_lttng_channel_ok $SESSION_NAME $CHAN_NAME
+
+	enable_ust_lttng_event_per_chan $SESSION_NAME $EVENT_NAME $CHAN_NAME
+
+	add_context_ust_ok $SESSION_NAME $CHAN_NAME "perf:thread:raw:${PERFID}:test"
+
+	start_lttng_tracing_ok
+
+	$TESTAPP_BIN $NR_ITER $NR_USEC_WAIT >/dev/null 2>&1
+
+	stop_lttng_tracing_ok
+
+	destroy_lttng_session_ok $SESSION_NAME
+
+	validate_trace "perf_thread_raw_${PERFID}_test" $TRACE_PATH
+
+	rm -rf $TRACE_PATH
+}
+
+function test_kernel_raw()
+{
+	TRACE_PATH=$(mktemp -d)
+	SESSION_NAME="kernel_perf"
+	CHAN_NAME="mychan"
+	EVENT_NAME="lttng_test_filter_event"
+	PMU="UNHALTED_REFERENCE_CYCLES"
+	PERFID=$($CURDIR/find_event $PMU)
+	test $? -eq "0"
+	ok $? "Find PMU $PMU"
+
+	create_lttng_session_ok $SESSION_NAME $TRACE_PATH
+
+	lttng_enable_kernel_channel_ok $SESSION_NAME $CHAN_NAME
+
+	enable_kernel_lttng_event_ok $SESSION_NAME $EVENT_NAME $CHAN_NAME
+
+	add_context_kernel_ok $SESSION_NAME $CHAN_NAME "perf:cpu:raw:${PERFID}:test"
+
+	start_lttng_tracing_ok
+
+	echo -n 10 > /proc/lttng-test-filter-event
+
+	stop_lttng_tracing_ok
+
+	destroy_lttng_session_ok $SESSION_NAME
+
+	validate_trace "perf_cpu_raw_${PERFID}_test" $TRACE_PATH
+
+	rm -rf $TRACE_PATH
+}
+
+if [ "$(id -u)" == "0" ]; then
+	isroot=1
+else
+	isroot=0
+fi
+
+# MUST set TESTDIR before calling those functions
+plan_tests $NUM_TESTS
+
+print_test_banner "$TEST_DESC"
+
+start_lttng_sessiond
+
+test_ust_raw
+
+skip $isroot "Root access is needed for kernel testing, skipping." 9 ||
+{
+	modprobe lttng-test
+	test_kernel_raw
+	rmmod lttng-test
+}
+
+stop_lttng_sessiond
diff --git a/tests/perf_regression b/tests/perf_regression
new file mode 100644
index 0000000..c4b9b86
--- /dev/null
+++ b/tests/perf_regression
@@ -0,0 +1 @@
+perf/test_perf_raw
-- 
1.9.1

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

Re: [PATCH lttng-tools v3 3/3] Create a dedicated test suite for Perf

[Julien Desfossez]

> diff --git a/tests/perf/find_event.c b/tests/perf/find_event.c
> new file mode 100644
> index 0000000..ae63800
> --- /dev/null
> +++ b/tests/perf/find_event.c
> @@ -0,0 +1,83 @@
> +/*
> + * Copyright (c)  2016 Julien Desfossez <jdesfossez@efficios.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * as published by the Free Software Foundation; only version 2
> + * of the License.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, write to the Free Software Foundation, Inc.,
> + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> + */
> +
> +#include <stdio.h>
> +#include <perfmon/pfmlib.h>
> +#include <string.h>
> +
> +int main(int argc, char **argv)
> +{
> +	int ret, i;
> +	unsigned int j;
> +	pfm_pmu_info_t pinfo;
> +
> +	if (argc != 2) {
> +		fprintf(stderr, "Usage: %s <pmu counter to find>\n"
> +				"ex: %s UNHALTED_REFERENCE_CYCLES\n"
> +				"Returns the first occurence it finds with "
> +				"return code 0.\n"
> +				"If not found returns 1, on error returns -1\n",
> +				argv[0], argv[0]);
> +		ret = -1;
> +		goto end;
> +	}
> +
> +	memset(&pinfo, 0, sizeof(pinfo));
> +	pinfo.size = sizeof(pinfo);
> +
> +	ret = pfm_initialize();
> +	if (ret != PFM_SUCCESS) {
> +		fprintf(stderr, "Failed to initialise libpfm: %s",
> +				pfm_strerror(ret));
> +		ret = -1;
> +		goto end;
> +	}
> +
> +	pfm_for_all_pmus(j) {
> +		ret = pfm_get_pmu_info(j, &pinfo);
> +		if (ret != PFM_SUCCESS) {
> +			continue;
> +		}
> +
> +		for (i = pinfo.first_event; i != -1; i = pfm_get_event_next(i)) {
> +			pfm_event_info_t info;
> +
> +			ret = pfm_get_event_info(i, PFM_OS_NONE, &info);
> +			if (ret != PFM_SUCCESS) {
> +				fprintf(stderr, "Cannot get event info: %s\n",
> +						pfm_strerror(ret));
> +				ret = -1;
> +				goto end;
> +			}
> +
> +			if (info.pmu != j)
> +				continue;
> +
> +			if (strcmp(info.name, argv[1]) == 0) {
> +				fprintf(stdout, "r%lx\n", info.code);
This should be:
fprintf(stdout, "r%" PRIx64 "\n", info.code);

I will wait for other comments if any before resubmitting a new version
(or you can directly apply the fix when you merge).

Thanks,

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

Re: [PATCH lttng-tools v3 3/3] Create a dedicated test suite for Perf

[Jérémie Galarneau]

The first two patches look good to me. See comments on this one below
(and your own reply).

On Tue, Jul 5, 2016 at 11:50 AM, Julien Desfossez
<jdesfossez@efficios.com> wrote:
> Introduce the perf_regression test suite that must be run manually to
> check if the support for the Perf-related features are available on the
> current machine. This test cannot be run automatically since there are
> some platforms where it can fail.

Is this still true given lttng-ust's new "read()" fallback?

>
> For now, the test only makes sure that we can trace events with perf
> contexts enabled by raw ID in kernel and user-space. The test only works
> if libpfm is installed on the system and fails if it is not installed.
>
> Signed-off-by: Julien Desfossez <jdesfossez@efficios.com>
> ---
>  .gitignore               |   1 +
>  README.md                |   2 +
>  configure.ac             |   8 +++
>  tests/Makefile.am        |   8 +--
>  tests/perf/Makefile.am   |   6 +++
>  tests/perf/find_event.c  |  83 ++++++++++++++++++++++++++++++
>  tests/perf/test_perf_raw | 129 +++++++++++++++++++++++++++++++++++++++++++++++
>  tests/perf_regression    |   1 +
>  8 files changed, 234 insertions(+), 4 deletions(-)
>  create mode 100644 tests/perf/Makefile.am
>  create mode 100644 tests/perf/find_event.c
>  create mode 100755 tests/perf/test_perf_raw
>  create mode 100644 tests/perf_regression
>
> diff --git a/.gitignore b/.gitignore
> index ac78292..c31dbc5 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -106,6 +106,7 @@ tests/regression/ust/python-logging/test_python_logging
>  /tests/utils/testapp/gen-ust-tracef/gen-ust-tracef
>  /tests/regression/tools/live/live_test
>  /tests/unit/ini_config/ini_config
> +/tests/perf/find_event
>
>  # man pages
>  /doc/man/*.1
> diff --git a/README.md b/README.md
> index 22de252..3156a15 100644
> --- a/README.md
> +++ b/README.md
> @@ -57,6 +57,8 @@ The following items are _optional_ dependencies:
>      pages with the `--help` option or with the `lttng help` command.
>      Note that without `man`, you cannot get offline help with
>      LTTng-tools commands, not even their usage.
> +  - **`libpfm`**: needed to run the perf regression test suite.
> +    - Debian/Ubuntu package: `libpfm4-dev`

Which version is required?

>
>  LTTng-tools supports both the [LTTng Linux Kernel tracer](https://lttng.org)
>  and [LTTng user space tracer](https://lttng.org) released as part of the same
> diff --git a/configure.ac b/configure.ac
> index fb2b013..f96d25b 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -467,6 +467,13 @@ AC_CHECK_LIB([c], [open_memstream],
>  ]
>  )
>
> +# check for libpfm
> +AC_CHECK_LIB([pfm], [pfm_initialize],
> +            [
> +             have_libpfm=yes
> +             ])
> +AM_CONDITIONAL([LTTNG_TOOLS_BUILD_WITH_LIBPFM], [test "x$have_libpfm" = "xyes"])
> +
>  AC_ARG_ENABLE([git-version],
>                [AC_HELP_STRING([--disable-git-version],
>                                [Do not use the git version for the build])],
> @@ -1008,6 +1015,7 @@ AC_CONFIG_FILES([
>         tests/stress/Makefile
>         tests/unit/Makefile
>         tests/unit/ini_config/Makefile
> +       tests/perf/Makefile

have_libpfm should be used to set an automake variable and enable the test
conditionally. Similar to what is done here:

https://github.com/lttng/lttng-tools/blob/master/tests/regression/Makefile.am#L44

>         tests/utils/Makefile
>         tests/utils/tap/Makefile
>         tests/utils/testapp/Makefile
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 3600e99..d460fb6 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -1,8 +1,8 @@
>  SUBDIRS =
> -DIST_SUBDIRS = utils regression unit stress destructive
> +DIST_SUBDIRS = utils regression unit stress destructive perf
>
>  if BUILD_TESTS
> -SUBDIRS += . utils regression unit stress destructive
> +SUBDIRS += . utils regression unit stress destructive perf
>  if HAS_PGREP
>  check-am:
>         $(top_srcdir)/tests/utils/warn_processes.sh $(PGREP)
> @@ -14,8 +14,8 @@ else
>  endif
>
>
> -dist_noinst_SCRIPTS = run.sh fast_regression long_regression root_regression root_destructive_tests
> -EXTRA_DIST = run.sh fast_regression long_regression root_regression README root_destructive_tests
> +dist_noinst_SCRIPTS = run.sh fast_regression long_regression root_regression root_destructive_tests perf_regression
> +EXTRA_DIST = run.sh fast_regression long_regression root_regression README root_destructive_tests perf_regression
>
>  all-local:
>         @if [ x"$(srcdir)" != x"$(builddir)" ]; then \
> diff --git a/tests/perf/Makefile.am b/tests/perf/Makefile.am
> new file mode 100644
> index 0000000..40bb754
> --- /dev/null
> +++ b/tests/perf/Makefile.am
> @@ -0,0 +1,6 @@
> +if LTTNG_TOOLS_BUILD_WITH_LIBPFM
> +LIBS += -lpfm
> +
> +noinst_PROGRAMS = find_event
> +find_event_SOURCES = find_event.c
> +endif
> diff --git a/tests/perf/find_event.c b/tests/perf/find_event.c
> new file mode 100644
> index 0000000..ae63800
> --- /dev/null
> +++ b/tests/perf/find_event.c
> @@ -0,0 +1,83 @@
> +/*
> + * Copyright (c)  2016 Julien Desfossez <jdesfossez@efficios.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * as published by the Free Software Foundation; only version 2
> + * of the License.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, write to the Free Software Foundation, Inc.,
> + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> + */
> +
> +#include <stdio.h>
> +#include <perfmon/pfmlib.h>
> +#include <string.h>
> +
> +int main(int argc, char **argv)
> +{
> +       int ret, i;
> +       unsigned int j;
> +       pfm_pmu_info_t pinfo;
> +
> +       if (argc != 2) {
> +               fprintf(stderr, "Usage: %s <pmu counter to find>\n"
> +                               "ex: %s UNHALTED_REFERENCE_CYCLES\n"
> +                               "Returns the first occurence it finds with "
> +                               "return code 0.\n"
> +                               "If not found returns 1, on error returns -1\n",
> +                               argv[0], argv[0]);
> +               ret = -1;
> +               goto end;
> +       }
> +
> +       memset(&pinfo, 0, sizeof(pinfo));
> +       pinfo.size = sizeof(pinfo);
> +
> +       ret = pfm_initialize();
> +       if (ret != PFM_SUCCESS) {
> +               fprintf(stderr, "Failed to initialise libpfm: %s",
> +                               pfm_strerror(ret));
> +               ret = -1;
> +               goto end;
> +       }
> +
> +       pfm_for_all_pmus(j) {
> +               ret = pfm_get_pmu_info(j, &pinfo);
> +               if (ret != PFM_SUCCESS) {
> +                       continue;
> +               }
> +
> +               for (i = pinfo.first_event; i != -1; i = pfm_get_event_next(i)) {
> +                       pfm_event_info_t info;
> +
> +                       ret = pfm_get_event_info(i, PFM_OS_NONE, &info);
> +                       if (ret != PFM_SUCCESS) {
> +                               fprintf(stderr, "Cannot get event info: %s\n",
> +                                               pfm_strerror(ret));
> +                               ret = -1;
> +                               goto end;
> +                       }
> +
> +                       if (info.pmu != j)
> +                               continue;

Missing braces on single-line conditional block.

> +
> +                       if (strcmp(info.name, argv[1]) == 0) {
> +                               fprintf(stdout, "r%lx\n", info.code);
> +                               ret = 0;
> +                               goto end;
> +                       }
> +               }
> +       }
> +
> +       ret = 1;
> +
> +end:
> +       return ret;
> +}
> diff --git a/tests/perf/test_perf_raw b/tests/perf/test_perf_raw
> new file mode 100755
> index 0000000..68b12d7
> --- /dev/null
> +++ b/tests/perf/test_perf_raw
> @@ -0,0 +1,129 @@
> +#!/bin/bash
> +#
> +# Copyright (C) - 2016 Julien Desfossez <jdesfossez@efficios.com>
> +#
> +# This program is free software; you can redistribute it and/or modify it
> +# under the terms of the GNU General Public License, version 2 only, as
> +# published by the Free Software Foundation.
> +#
> +# This program is distributed in the hope that it will be useful, but WITHOUT
> +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> +# FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> +# more details.
> +#
> +# You should have received a copy of the GNU General Public License along with
> +# this program; if not, write to the Free Software Foundation, Inc., 51
> +# Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> +
> +TEST_DESC="Perf counters"
> +
> +CURDIR=$(dirname $0)/
> +TESTDIR=$CURDIR/..
> +LTTNG_BIN="lttng"
> +SESSION_NAME="perf_counters"
> +NUM_TESTS=20
> +NR_ITER=1
> +NR_USEC_WAIT=1
> +TESTAPP_PATH="$TESTDIR/utils/testapp"
> +TESTAPP_NAME="gen-ust-events"
> +TESTAPP_BIN="$TESTAPP_PATH/$TESTAPP_NAME/$TESTAPP_NAME"
> +
> +source $TESTDIR/utils/utils.sh
> +
> +function enable_ust_lttng_event_per_chan()
> +{
> +       sess_name="$1"
> +       event_name="$2"
> +       chan_name="$3"
> +
> +       $TESTDIR/../src/bin/lttng/$LTTNG_BIN enable-event "$event_name" -s $sess_name -c $chan_name -u >/dev/null 2>&1
> +       ok $? "Enable event $event_name for session $sess_name in channel $chan_name"
> +}
> +
> +function test_ust_raw()
> +{
> +       TRACE_PATH=$(mktemp -d)
> +       SESSION_NAME="ust_perf"
> +       CHAN_NAME="mychan"
> +       EVENT_NAME="tp:tptest"
> +       PMU="UNHALTED_REFERENCE_CYCLES"
> +       PERFID=$($CURDIR/find_event $PMU)
> +       test $? -eq "0"
> +       ok $? "Find PMU $PMU"
> +
> +       create_lttng_session_ok $SESSION_NAME $TRACE_PATH
> +
> +       enable_ust_lttng_channel_ok $SESSION_NAME $CHAN_NAME
> +
> +       enable_ust_lttng_event_per_chan $SESSION_NAME $EVENT_NAME $CHAN_NAME
> +
> +       add_context_ust_ok $SESSION_NAME $CHAN_NAME "perf:thread:raw:${PERFID}:test"
> +
> +       start_lttng_tracing_ok
> +
> +       $TESTAPP_BIN $NR_ITER $NR_USEC_WAIT >/dev/null 2>&1
> +
> +       stop_lttng_tracing_ok
> +
> +       destroy_lttng_session_ok $SESSION_NAME
> +
> +       validate_trace "perf_thread_raw_${PERFID}_test" $TRACE_PATH
> +
> +       rm -rf $TRACE_PATH
> +}
> +
> +function test_kernel_raw()
> +{
> +       TRACE_PATH=$(mktemp -d)
> +       SESSION_NAME="kernel_perf"
> +       CHAN_NAME="mychan"
> +       EVENT_NAME="lttng_test_filter_event"
> +       PMU="UNHALTED_REFERENCE_CYCLES"
> +       PERFID=$($CURDIR/find_event $PMU)
> +       test $? -eq "0"
> +       ok $? "Find PMU $PMU"
> +
> +       create_lttng_session_ok $SESSION_NAME $TRACE_PATH
> +
> +       lttng_enable_kernel_channel_ok $SESSION_NAME $CHAN_NAME
> +
> +       enable_kernel_lttng_event_ok $SESSION_NAME $EVENT_NAME $CHAN_NAME
> +
> +       add_context_kernel_ok $SESSION_NAME $CHAN_NAME "perf:cpu:raw:${PERFID}:test"
> +
> +       start_lttng_tracing_ok
> +
> +       echo -n 10 > /proc/lttng-test-filter-event
> +
> +       stop_lttng_tracing_ok
> +
> +       destroy_lttng_session_ok $SESSION_NAME
> +
> +       validate_trace "perf_cpu_raw_${PERFID}_test" $TRACE_PATH
> +
> +       rm -rf $TRACE_PATH
> +}
> +
> +if [ "$(id -u)" == "0" ]; then
> +       isroot=1
> +else
> +       isroot=0
> +fi
> +
> +# MUST set TESTDIR before calling those functions
> +plan_tests $NUM_TESTS
> +
> +print_test_banner "$TEST_DESC"
> +
> +start_lttng_sessiond
> +
> +test_ust_raw
> +
> +skip $isroot "Root access is needed for kernel testing, skipping." 9 ||
> +{
> +       modprobe lttng-test
> +       test_kernel_raw
> +       rmmod lttng-test
> +}
> +
> +stop_lttng_sessiond
> diff --git a/tests/perf_regression b/tests/perf_regression
> new file mode 100644
> index 0000000..c4b9b86
> --- /dev/null
> +++ b/tests/perf_regression
> @@ -0,0 +1 @@
> +perf/test_perf_raw
> --
> 1.9.1
>



-- 
Jérémie Galarneau
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

Re: [PATCH lttng-tools v3 3/3] Create a dedicated test suite for Perf

[Julien Desfossez]

On 16-07-08 03:56 PM, Jérémie Galarneau wrote:
> The first two patches look good to me. See comments on this one below
> (and your own reply).
> 
> On Tue, Jul 5, 2016 at 11:50 AM, Julien Desfossez
> <jdesfossez@efficios.com> wrote:
>> Introduce the perf_regression test suite that must be run manually to
>> check if the support for the Perf-related features are available on the
>> current machine. This test cannot be run automatically since there are
>> some platforms where it can fail.
> 
> Is this still true given lttng-ust's new "read()" fallback?

Yes, the test tries to enable a raw context and it depends on the
hardware (which CPU, SoC, etc), so there are many places where it can
fail, including in VM in the CI.

>> For now, the test only makes sure that we can trace events with perf
>> contexts enabled by raw ID in kernel and user-space. The test only works
>> if libpfm is installed on the system and fails if it is not installed.
>>
>> Signed-off-by: Julien Desfossez <jdesfossez@efficios.com>
>> ---
>>  .gitignore               |   1 +
>>  README.md                |   2 +
>>  configure.ac             |   8 +++
>>  tests/Makefile.am        |   8 +--
>>  tests/perf/Makefile.am   |   6 +++
>>  tests/perf/find_event.c  |  83 ++++++++++++++++++++++++++++++
>>  tests/perf/test_perf_raw | 129 +++++++++++++++++++++++++++++++++++++++++++++++
>>  tests/perf_regression    |   1 +
>>  8 files changed, 234 insertions(+), 4 deletions(-)
>>  create mode 100644 tests/perf/Makefile.am
>>  create mode 100644 tests/perf/find_event.c
>>  create mode 100755 tests/perf/test_perf_raw
>>  create mode 100644 tests/perf_regression
>>
>> diff --git a/.gitignore b/.gitignore
>> index ac78292..c31dbc5 100644
>> --- a/.gitignore
>> +++ b/.gitignore
>> @@ -106,6 +106,7 @@ tests/regression/ust/python-logging/test_python_logging
>>  /tests/utils/testapp/gen-ust-tracef/gen-ust-tracef
>>  /tests/regression/tools/live/live_test
>>  /tests/unit/ini_config/ini_config
>> +/tests/perf/find_event
>>
>>  # man pages
>>  /doc/man/*.1
>> diff --git a/README.md b/README.md
>> index 22de252..3156a15 100644
>> --- a/README.md
>> +++ b/README.md
>> @@ -57,6 +57,8 @@ The following items are _optional_ dependencies:
>>      pages with the `--help` option or with the `lttng help` command.
>>      Note that without `man`, you cannot get offline help with
>>      LTTng-tools commands, not even their usage.
>> +  - **`libpfm`**: needed to run the perf regression test suite.
>> +    - Debian/Ubuntu package: `libpfm4-dev`
> 
> Which version is required?
> 
>>
>>  LTTng-tools supports both the [LTTng Linux Kernel tracer](https://lttng.org)
>>  and [LTTng user space tracer](https://lttng.org) released as part of the same
>> diff --git a/configure.ac b/configure.ac
>> index fb2b013..f96d25b 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -467,6 +467,13 @@ AC_CHECK_LIB([c], [open_memstream],
>>  ]
>>  )
>>
>> +# check for libpfm
>> +AC_CHECK_LIB([pfm], [pfm_initialize],
>> +            [
>> +             have_libpfm=yes
>> +             ])
>> +AM_CONDITIONAL([LTTNG_TOOLS_BUILD_WITH_LIBPFM], [test "x$have_libpfm" = "xyes"])
>> +
>>  AC_ARG_ENABLE([git-version],
>>                [AC_HELP_STRING([--disable-git-version],
>>                                [Do not use the git version for the build])],
>> @@ -1008,6 +1015,7 @@ AC_CONFIG_FILES([
>>         tests/stress/Makefile
>>         tests/unit/Makefile
>>         tests/unit/ini_config/Makefile
>> +       tests/perf/Makefile
> 
> have_libpfm should be used to set an automake variable and enable the test
> conditionally. Similar to what is done here:
> 
> https://github.com/lttng/lttng-tools/blob/master/tests/regression/Makefile.am#L44

Hum, the intent here is to make the tests fail if the dependency is not
available, but we do not want to enforce it as a dependency since most
deployments cannot run these tests. What we want here if for the test to
fail if a user tries to run it without the dependency installed, then
they will install the dependency and run it again. Having a transparent
way to disable these tests would render them useless because we will
think they passed.

There may be a better way of doing it, but we should avoid using magical
"skip" in this case, these tests should fail if the target architecture
does not support all the Perf-related features we support.

Thanks,

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

Re: [PATCH lttng-tools v3 3/3] Create a dedicated test suite for Perf

[Jérémie Galarneau]

On Fri, Jul 8, 2016 at 4:07 PM, Julien Desfossez
<jdesfossez@efficios.com> wrote:
> On 16-07-08 03:56 PM, Jérémie Galarneau wrote:
>> The first two patches look good to me. See comments on this one below
>> (and your own reply).
>>
>> On Tue, Jul 5, 2016 at 11:50 AM, Julien Desfossez
>> <jdesfossez@efficios.com> wrote:
>>> Introduce the perf_regression test suite that must be run manually to
>>> check if the support for the Perf-related features are available on the
>>> current machine. This test cannot be run automatically since there are
>>> some platforms where it can fail.
>>
>> Is this still true given lttng-ust's new "read()" fallback?
>
> Yes, the test tries to enable a raw context and it depends on the
> hardware (which CPU, SoC, etc), so there are many places where it can
> fail, including in VM in the CI.
>
>>> For now, the test only makes sure that we can trace events with perf
>>> contexts enabled by raw ID in kernel and user-space. The test only works
>>> if libpfm is installed on the system and fails if it is not installed.
>>>
>>> Signed-off-by: Julien Desfossez <jdesfossez@efficios.com>
>>> ---
>>>  .gitignore               |   1 +
>>>  README.md                |   2 +
>>>  configure.ac             |   8 +++
>>>  tests/Makefile.am        |   8 +--
>>>  tests/perf/Makefile.am   |   6 +++
>>>  tests/perf/find_event.c  |  83 ++++++++++++++++++++++++++++++
>>>  tests/perf/test_perf_raw | 129 +++++++++++++++++++++++++++++++++++++++++++++++
>>>  tests/perf_regression    |   1 +
>>>  8 files changed, 234 insertions(+), 4 deletions(-)
>>>  create mode 100644 tests/perf/Makefile.am
>>>  create mode 100644 tests/perf/find_event.c
>>>  create mode 100755 tests/perf/test_perf_raw
>>>  create mode 100644 tests/perf_regression
>>>
>>> diff --git a/.gitignore b/.gitignore
>>> index ac78292..c31dbc5 100644
>>> --- a/.gitignore
>>> +++ b/.gitignore
>>> @@ -106,6 +106,7 @@ tests/regression/ust/python-logging/test_python_logging
>>>  /tests/utils/testapp/gen-ust-tracef/gen-ust-tracef
>>>  /tests/regression/tools/live/live_test
>>>  /tests/unit/ini_config/ini_config
>>> +/tests/perf/find_event
>>>
>>>  # man pages
>>>  /doc/man/*.1
>>> diff --git a/README.md b/README.md
>>> index 22de252..3156a15 100644
>>> --- a/README.md
>>> +++ b/README.md
>>> @@ -57,6 +57,8 @@ The following items are _optional_ dependencies:
>>>      pages with the `--help` option or with the `lttng help` command.
>>>      Note that without `man`, you cannot get offline help with
>>>      LTTng-tools commands, not even their usage.
>>> +  - **`libpfm`**: needed to run the perf regression test suite.
>>> +    - Debian/Ubuntu package: `libpfm4-dev`
>>
>> Which version is required?
>>
>>>
>>>  LTTng-tools supports both the [LTTng Linux Kernel tracer](https://lttng.org)
>>>  and [LTTng user space tracer](https://lttng.org) released as part of the same
>>> diff --git a/configure.ac b/configure.ac
>>> index fb2b013..f96d25b 100644
>>> --- a/configure.ac
>>> +++ b/configure.ac
>>> @@ -467,6 +467,13 @@ AC_CHECK_LIB([c], [open_memstream],
>>>  ]
>>>  )
>>>
>>> +# check for libpfm
>>> +AC_CHECK_LIB([pfm], [pfm_initialize],
>>> +            [
>>> +             have_libpfm=yes
>>> +             ])
>>> +AM_CONDITIONAL([LTTNG_TOOLS_BUILD_WITH_LIBPFM], [test "x$have_libpfm" = "xyes"])
>>> +
>>>  AC_ARG_ENABLE([git-version],
>>>                [AC_HELP_STRING([--disable-git-version],
>>>                                [Do not use the git version for the build])],
>>> @@ -1008,6 +1015,7 @@ AC_CONFIG_FILES([
>>>         tests/stress/Makefile
>>>         tests/unit/Makefile
>>>         tests/unit/ini_config/Makefile
>>> +       tests/perf/Makefile
>>
>> have_libpfm should be used to set an automake variable and enable the test
>> conditionally. Similar to what is done here:
>>
>> https://github.com/lttng/lttng-tools/blob/master/tests/regression/Makefile.am#L44
>
> Hum, the intent here is to make the tests fail if the dependency is not
> available, but we do not want to enforce it as a dependency since most
> deployments cannot run these tests. What we want here if for the test to
> fail if a user tries to run it without the dependency installed, then
> they will install the dependency and run it again. Having a transparent
> way to disable these tests would render them useless because we will
> think they passed.

Agreed to not include them in the standard test suite. However, the
test should be skipped if the dependency is not found on the system,
along with a message explaining why.

Currently the test will fail with the following output if run without
libpfm4 installed on the system.

$ ./tests/perf/test_perf_raw
1..20
# Perf counters
ok 1 - Start session daemon
./tests/perf/test_perf_raw: line 50: ./tests/perf//find_event: No such
file or directory
not ok 2 - Find PMU UNHALTED_REFERENCE_CYCLES
#   Failed test 'Find PMU UNHALTED_REFERENCE_CYCLES'
#   in ./tests/perf/test_perf_raw:test_ust_raw() at line 52.
ok 3 - Create session ust_perf in /tmp/tmp.pbAELM2mp4
ok 4 - Enable channel mychan for session ust_perf
ok 5 - Enable event tp:tptest for session ust_perf in channel mychan
not ok 6 - Add context command for type: perf:thread:raw::test
#   Failed test 'Add context command for type: perf:thread:raw::test'
#   in ./tests/perf//../utils/utils.sh:add_context_lttng() at line 1208.
ok 7 - Start tracing for session
ok 8 - Stop lttng tracing for session
ok 9 - Destroy session ust_perf
not ok 10 - Validate trace for event perf_thread_raw__test
#   Failed test 'Validate trace for event perf_thread_raw__test'
#   in :() at line 170 fail ./tests/perf//../utils/tap/tap.sh.
# Found 0 occurences of perf_thread_raw__test
ok 11 # skip: Root access is needed for kernel testing, skipping.
ok 12 # skip: Root access is needed for kernel testing, skipping.
ok 13 # skip: Root access is needed for kernel testing, skipping.
ok 14 # skip: Root access is needed for kernel testing, skipping.
ok 15 # skip: Root access is needed for kernel testing, skipping.
ok 16 # skip: Root access is needed for kernel testing, skipping.
ok 17 # skip: Root access is needed for kernel testing, skipping.
ok 18 # skip: Root access is needed for kernel testing, skipping.
ok 19 # skip: Root access is needed for kernel testing, skipping.
# Killing lttng-sessiond and lt-lttng-sessiond pids: 27048 27049 27102 27111
ok 20 - Kill session daemon
# Looks like you failed 3 tests of 20.

This does't make it obvious why the test is failing.

>
> There may be a better way of doing it, but we should avoid using magical
> "skip" in this case, these tests should fail if the target architecture
> does not support all the Perf-related features we support.

I want to make it clear to the user that the test is failing because
of a missing dependency, and not because his HW doesn't have the
capabilities needed.

Skip with a clear error message seems appropriate here, especially
since the test will not be run automatically.

Btw, I went ahead and merged the first two patches. I'll merge an
updated version of this one separately.

Thanks!
Jérémie

>
> Thanks,
>
> Julien



-- 
Jérémie Galarneau
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

Re: [PATCH lttng-tools v3 3/3] Create a dedicated test suite for Perf

[Julien Desfossez]

> >> have_libpfm should be used to set an automake variable and enable the test
> >> conditionally. Similar to what is done here:
> >>
> >> https://github.com/lttng/lttng-tools/blob/master/tests/regression/Makefile.am#L44
> >
> > Hum, the intent here is to make the tests fail if the dependency is not
> > available, but we do not want to enforce it as a dependency since most
> > deployments cannot run these tests. What we want here if for the test to
> > fail if a user tries to run it without the dependency installed, then
> > they will install the dependency and run it again. Having a transparent
> > way to disable these tests would render them useless because we will
> > think they passed.
> 
> Agreed to not include them in the standard test suite. However, the
> test should be skipped if the dependency is not found on the system,
> along with a message explaining why.
> 
> Currently the test will fail with the following output if run without
> libpfm4 installed on the system.
> 
> $ ./tests/perf/test_perf_raw
> 1..20
> # Perf counters
> ok 1 - Start session daemon
> ./tests/perf/test_perf_raw: line 50: ./tests/perf//find_event: No such
> file or directory
> not ok 2 - Find PMU UNHALTED_REFERENCE_CYCLES
[...]
> 
> This does't make it obvious why the test is failing.
> 
> >
> > There may be a better way of doing it, but we should avoid using magical
> > "skip" in this case, these tests should fail if the target architecture
> > does not support all the Perf-related features we support.
> 
> I want to make it clear to the user that the test is failing because
> of a missing dependency, and not because his HW doesn't have the
> capabilities needed.
> 
> Skip with a clear error message seems appropriate here, especially
> since the test will not be run automatically.

How about having a first step in the test that checks if the dependency
is present and fails if not ? This would make it clear that the
dependency is needed to run the test, and it would not magically pass if
run in the background or in batch.

I am really worried that skipping here will confuse users, I see this
test suite as a way to guarantee that perf-related features all work on
their system if the test returns 0 at the end.

Thanks,

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

Re: [PATCH lttng-tools v3 3/3] Create a dedicated test suite for Perf

[Jérémie Galarneau]

On Fri, Jul 8, 2016 at 4:41 PM, Julien Desfossez
<jdesfossez@efficios.com> wrote:
>> >> have_libpfm should be used to set an automake variable and enable the test
>> >> conditionally. Similar to what is done here:
>> >>
>> >> https://github.com/lttng/lttng-tools/blob/master/tests/regression/Makefile.am#L44
>> >
>> > Hum, the intent here is to make the tests fail if the dependency is not
>> > available, but we do not want to enforce it as a dependency since most
>> > deployments cannot run these tests. What we want here if for the test to
>> > fail if a user tries to run it without the dependency installed, then
>> > they will install the dependency and run it again. Having a transparent
>> > way to disable these tests would render them useless because we will
>> > think they passed.
>>
>> Agreed to not include them in the standard test suite. However, the
>> test should be skipped if the dependency is not found on the system,
>> along with a message explaining why.
>>
>> Currently the test will fail with the following output if run without
>> libpfm4 installed on the system.
>>
>> $ ./tests/perf/test_perf_raw
>> 1..20
>> # Perf counters
>> ok 1 - Start session daemon
>> ./tests/perf/test_perf_raw: line 50: ./tests/perf//find_event: No such
>> file or directory
>> not ok 2 - Find PMU UNHALTED_REFERENCE_CYCLES
> [...]
>>
>> This does't make it obvious why the test is failing.
>>
>> >
>> > There may be a better way of doing it, but we should avoid using magical
>> > "skip" in this case, these tests should fail if the target architecture
>> > does not support all the Perf-related features we support.
>>
>> I want to make it clear to the user that the test is failing because
>> of a missing dependency, and not because his HW doesn't have the
>> capabilities needed.
>>
>> Skip with a clear error message seems appropriate here, especially
>> since the test will not be run automatically.
>
> How about having a first step in the test that checks if the dependency
> is present and fails if not ? This would make it clear that the
> dependency is needed to run the test, and it would not magically pass if
> run in the background or in batch.
>
> I am really worried that skipping here will confuse users, I see this
> test suite as a way to guarantee that perf-related features all work on
> their system if the test returns 0 at the end.

That's fair. Feel free to have a look at how we use autoconf to generate
the test scripts based on configure-time results in Babeltrace [1][2].

You should be able to export "LTTNG_TOOLS_BUILD_WITH_LIBPFM",
check its value in the script and bail out with an explanation.

[1] https://github.com/efficios/babeltrace/blob/master/tests/lib/test_ctf_writer_complete.in
[2] https://github.com/efficios/babeltrace/blob/master/configure.ac#L363

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



-- 
Jérémie Galarneau
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

Re: [PATCH lttng-tools v3 3/3] Create a dedicated test suite for Perf

[Jérémie Galarneau]

Have you had a chance to look into this since then?

Jérémie

On Fri, Jul 8, 2016 at 4:57 PM, Jérémie Galarneau
<jeremie.galarneau@efficios.com> wrote:
> On Fri, Jul 8, 2016 at 4:41 PM, Julien Desfossez
> <jdesfossez@efficios.com> wrote:
>>> >> have_libpfm should be used to set an automake variable and enable the test
>>> >> conditionally. Similar to what is done here:
>>> >>
>>> >> https://github.com/lttng/lttng-tools/blob/master/tests/regression/Makefile.am#L44
>>> >
>>> > Hum, the intent here is to make the tests fail if the dependency is not
>>> > available, but we do not want to enforce it as a dependency since most
>>> > deployments cannot run these tests. What we want here if for the test to
>>> > fail if a user tries to run it without the dependency installed, then
>>> > they will install the dependency and run it again. Having a transparent
>>> > way to disable these tests would render them useless because we will
>>> > think they passed.
>>>
>>> Agreed to not include them in the standard test suite. However, the
>>> test should be skipped if the dependency is not found on the system,
>>> along with a message explaining why.
>>>
>>> Currently the test will fail with the following output if run without
>>> libpfm4 installed on the system.
>>>
>>> $ ./tests/perf/test_perf_raw
>>> 1..20
>>> # Perf counters
>>> ok 1 - Start session daemon
>>> ./tests/perf/test_perf_raw: line 50: ./tests/perf//find_event: No such
>>> file or directory
>>> not ok 2 - Find PMU UNHALTED_REFERENCE_CYCLES
>> [...]
>>>
>>> This does't make it obvious why the test is failing.
>>>
>>> >
>>> > There may be a better way of doing it, but we should avoid using magical
>>> > "skip" in this case, these tests should fail if the target architecture
>>> > does not support all the Perf-related features we support.
>>>
>>> I want to make it clear to the user that the test is failing because
>>> of a missing dependency, and not because his HW doesn't have the
>>> capabilities needed.
>>>
>>> Skip with a clear error message seems appropriate here, especially
>>> since the test will not be run automatically.
>>
>> How about having a first step in the test that checks if the dependency
>> is present and fails if not ? This would make it clear that the
>> dependency is needed to run the test, and it would not magically pass if
>> run in the background or in batch.
>>
>> I am really worried that skipping here will confuse users, I see this
>> test suite as a way to guarantee that perf-related features all work on
>> their system if the test returns 0 at the end.
>
> That's fair. Feel free to have a look at how we use autoconf to generate
> the test scripts based on configure-time results in Babeltrace [1][2].
>
> You should be able to export "LTTNG_TOOLS_BUILD_WITH_LIBPFM",
> check its value in the script and bail out with an explanation.
>
> [1] https://github.com/efficios/babeltrace/blob/master/tests/lib/test_ctf_writer_complete.in
> [2] https://github.com/efficios/babeltrace/blob/master/configure.ac#L363
>
> Jeremie
>>
>> Thanks,
>>
>> Julien
>> _______________________________________________
>> lttng-dev mailing list
>> lttng-dev@lists.lttng.org
>> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
>
>
>
> --
> Jérémie Galarneau
> EfficiOS Inc.
> http://www.efficios.com



-- 
Jérémie Galarneau
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