* [PATCH lttng-tools v3 2/3] Test the parsing of perf raw context [not found] <1467733848-18128-1-git-send-email-jdesfossez@efficios.com> @ 2016-07-05 15:50 ` Julien Desfossez 2016-07-05 15:50 ` [PATCH lttng-tools v3 3/3] Create a dedicated test suite for Perf Julien Desfossez [not found] ` <1467733848-18128-3-git-send-email-jdesfossez@efficios.com> 2 siblings, 0 replies; 9+ messages in thread From: Julien Desfossez @ 2016-07-05 15:50 UTC (permalink / raw) To: jgalar; +Cc: lttng-dev, 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH lttng-tools v3 3/3] Create a dedicated test suite for Perf [not found] <1467733848-18128-1-git-send-email-jdesfossez@efficios.com> 2016-07-05 15:50 ` [PATCH lttng-tools v3 2/3] Test the parsing of perf raw context Julien Desfossez @ 2016-07-05 15:50 ` Julien Desfossez [not found] ` <1467733848-18128-3-git-send-email-jdesfossez@efficios.com> 2 siblings, 0 replies; 9+ messages in thread From: Julien Desfossez @ 2016-07-05 15:50 UTC (permalink / raw) To: jgalar; +Cc: lttng-dev, 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
[parent not found: <1467733848-18128-3-git-send-email-jdesfossez@efficios.com>]
* Re: [PATCH lttng-tools v3 3/3] Create a dedicated test suite for Perf [not found] ` <1467733848-18128-3-git-send-email-jdesfossez@efficios.com> @ 2016-07-06 16:06 ` Julien Desfossez 2016-07-08 19:56 ` Jérémie Galarneau [not found] ` <CA+jJMxv06zCw2wRaG+d5prs9BRJ6TG65k-9g4e6zKZQP+SLihQ@mail.gmail.com> 2 siblings, 0 replies; 9+ messages in thread From: Julien Desfossez @ 2016-07-06 16:06 UTC (permalink / raw) To: jgalar; +Cc: lttng-dev > 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH lttng-tools v3 3/3] Create a dedicated test suite for Perf [not found] ` <1467733848-18128-3-git-send-email-jdesfossez@efficios.com> 2016-07-06 16:06 ` Julien Desfossez @ 2016-07-08 19:56 ` Jérémie Galarneau [not found] ` <CA+jJMxv06zCw2wRaG+d5prs9BRJ6TG65k-9g4e6zKZQP+SLihQ@mail.gmail.com> 2 siblings, 0 replies; 9+ messages in thread From: Jérémie Galarneau @ 2016-07-08 19:56 UTC (permalink / raw) To: Julien Desfossez; +Cc: lttng-dev, Jeremie 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <CA+jJMxv06zCw2wRaG+d5prs9BRJ6TG65k-9g4e6zKZQP+SLihQ@mail.gmail.com>]
* Re: [PATCH lttng-tools v3 3/3] Create a dedicated test suite for Perf [not found] ` <CA+jJMxv06zCw2wRaG+d5prs9BRJ6TG65k-9g4e6zKZQP+SLihQ@mail.gmail.com> @ 2016-07-08 20:07 ` Julien Desfossez [not found] ` <5780080D.4090902@efficios.com> 1 sibling, 0 replies; 9+ messages in thread From: Julien Desfossez @ 2016-07-08 20:07 UTC (permalink / raw) To: Jérémie Galarneau; +Cc: lttng-dev 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <5780080D.4090902@efficios.com>]
* Re: [PATCH lttng-tools v3 3/3] Create a dedicated test suite for Perf [not found] ` <5780080D.4090902@efficios.com> @ 2016-07-08 20:28 ` Jérémie Galarneau [not found] ` <CA+jJMxsyOzNFPyST3i17BbWuAuBhd_ao-DYY2qLeRaiBaScEHw@mail.gmail.com> 1 sibling, 0 replies; 9+ messages in thread From: Jérémie Galarneau @ 2016-07-08 20:28 UTC (permalink / raw) To: Julien Desfossez; +Cc: lttng-dev 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <CA+jJMxsyOzNFPyST3i17BbWuAuBhd_ao-DYY2qLeRaiBaScEHw@mail.gmail.com>]
* Re: [PATCH lttng-tools v3 3/3] Create a dedicated test suite for Perf [not found] ` <CA+jJMxsyOzNFPyST3i17BbWuAuBhd_ao-DYY2qLeRaiBaScEHw@mail.gmail.com> @ 2016-07-08 20:41 ` Julien Desfossez [not found] ` <20160708204158.GA3372@sinkpad.home.lan> 1 sibling, 0 replies; 9+ messages in thread From: Julien Desfossez @ 2016-07-08 20:41 UTC (permalink / raw) To: Jérémie Galarneau; +Cc: lttng-dev > >> 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <20160708204158.GA3372@sinkpad.home.lan>]
* Re: [PATCH lttng-tools v3 3/3] Create a dedicated test suite for Perf [not found] ` <20160708204158.GA3372@sinkpad.home.lan> @ 2016-07-08 20:57 ` Jérémie Galarneau [not found] ` <CA+jJMxtSQ_SU4tR8-WoZjcZsSv_-TiTWk-+0pfMmKkXbrNy=gQ@mail.gmail.com> 1 sibling, 0 replies; 9+ messages in thread From: Jérémie Galarneau @ 2016-07-08 20:57 UTC (permalink / raw) To: Julien Desfossez; +Cc: lttng-dev 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <CA+jJMxtSQ_SU4tR8-WoZjcZsSv_-TiTWk-+0pfMmKkXbrNy=gQ@mail.gmail.com>]
* Re: [PATCH lttng-tools v3 3/3] Create a dedicated test suite for Perf [not found] ` <CA+jJMxtSQ_SU4tR8-WoZjcZsSv_-TiTWk-+0pfMmKkXbrNy=gQ@mail.gmail.com> @ 2016-09-12 19:10 ` Jérémie Galarneau 0 siblings, 0 replies; 9+ messages in thread From: Jérémie Galarneau @ 2016-09-12 19:10 UTC (permalink / raw) To: Julien Desfossez; +Cc: lttng-dev 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-09-12 19:11 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <1467733848-18128-1-git-send-email-jdesfossez@efficios.com>
2016-07-05 15:50 ` [PATCH lttng-tools v3 2/3] Test the parsing of perf raw context Julien Desfossez
2016-07-05 15:50 ` [PATCH lttng-tools v3 3/3] Create a dedicated test suite for Perf Julien Desfossez
[not found] ` <1467733848-18128-3-git-send-email-jdesfossez@efficios.com>
2016-07-06 16:06 ` Julien Desfossez
2016-07-08 19:56 ` Jérémie Galarneau
[not found] ` <CA+jJMxv06zCw2wRaG+d5prs9BRJ6TG65k-9g4e6zKZQP+SLihQ@mail.gmail.com>
2016-07-08 20:07 ` Julien Desfossez
[not found] ` <5780080D.4090902@efficios.com>
2016-07-08 20:28 ` Jérémie Galarneau
[not found] ` <CA+jJMxsyOzNFPyST3i17BbWuAuBhd_ao-DYY2qLeRaiBaScEHw@mail.gmail.com>
2016-07-08 20:41 ` Julien Desfossez
[not found] ` <20160708204158.GA3372@sinkpad.home.lan>
2016-07-08 20:57 ` Jérémie Galarneau
[not found] ` <CA+jJMxtSQ_SU4tR8-WoZjcZsSv_-TiTWk-+0pfMmKkXbrNy=gQ@mail.gmail.com>
2016-09-12 19:10 ` Jérémie Galarneau
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.