[lttng-dev] [PATCH lttng-tools v3 3/3] Create a dedicated test suite for Perf
Jérémie Galarneau
jeremie.galarneau at efficios.com
Fri Jul 8 20:28:15 UTC 2016
On Fri, Jul 8, 2016 at 4:07 PM, Julien Desfossez
<jdesfossez at 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 at 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 at 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
More information about the lttng-dev
mailing list