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

[Julien Desfossez <jdesfossez@efficios.com>]

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