diff mbox series

[v2,01/10] perf tools: Integrating the CoreSight decoding library

Message ID 1516039994-314-2-git-send-email-mathieu.poirier@linaro.org
State New
Headers show
Series perf tools: Add support for CoreSight trace decoding | expand

Commit Message

Mathieu Poirier Jan. 15, 2018, 6:13 p.m. UTC 
The Open CoreSight Decoding Library (openCSD) is a free and open
library to decode traces collected by the CoreSight hardware
infrastructure.

This patch adds the required mechanic to recognise the presence
of the openCSD library on a system and set up miscellaneous flags
to be used in the compilation of the trace decoding feature.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>

---
 tools/build/Makefile.feature          |  6 ++++--
 tools/build/feature/Makefile          |  7 ++++++-
 tools/build/feature/test-all.c        |  5 +++++
 tools/build/feature/test-libopencsd.c |  8 ++++++++
 tools/perf/Makefile.config            | 25 +++++++++++++++++++++++++
 tools/perf/Makefile.perf              |  2 ++
 6 files changed, 50 insertions(+), 3 deletions(-)
 create mode 100644 tools/build/feature/test-libopencsd.c

-- 
2.7.4

Comments

Jiri Olsa Jan. 16, 2018, 12:14 p.m. UTC | #1 
On Mon, Jan 15, 2018 at 11:13:05AM -0700, Mathieu Poirier wrote:
> The Open CoreSight Decoding Library (openCSD) is a free and open

> library to decode traces collected by the CoreSight hardware

> infrastructure.

> 

> This patch adds the required mechanic to recognise the presence

> of the openCSD library on a system and set up miscellaneous flags

> to be used in the compilation of the trace decoding feature.

> 

> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>

> ---

>  tools/build/Makefile.feature          |  6 ++++--

>  tools/build/feature/Makefile          |  7 ++++++-

>  tools/build/feature/test-all.c        |  5 +++++

>  tools/build/feature/test-libopencsd.c |  8 ++++++++

>  tools/perf/Makefile.config            | 25 +++++++++++++++++++++++++

>  tools/perf/Makefile.perf              |  2 ++

>  6 files changed, 50 insertions(+), 3 deletions(-)

>  create mode 100644 tools/build/feature/test-libopencsd.c

> 

> diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature

> index e52fcefee379..dc3d97b248ab 100644

> --- a/tools/build/Makefile.feature

> +++ b/tools/build/Makefile.feature

> @@ -66,7 +66,8 @@ FEATURE_TESTS_BASIC :=                  \

>          bpf                             \

>          sched_getcpu			\

>          sdt				\

> -        setns

> +        setns				\

> +	libopencsd


please keep the space/tabs context, here and other places in this patch

thanks,
jirka
Jiri Olsa Jan. 16, 2018, 12:15 p.m. UTC | #2 
On Mon, Jan 15, 2018 at 11:13:05AM -0700, Mathieu Poirier wrote:
> The Open CoreSight Decoding Library (openCSD) is a free and open

> library to decode traces collected by the CoreSight hardware

> infrastructure.

> 

> This patch adds the required mechanic to recognise the presence

> of the openCSD library on a system and set up miscellaneous flags

> to be used in the compilation of the trace decoding feature.

> 

> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>

> ---

>  tools/build/Makefile.feature          |  6 ++++--

>  tools/build/feature/Makefile          |  7 ++++++-

>  tools/build/feature/test-all.c        |  5 +++++

>  tools/build/feature/test-libopencsd.c |  8 ++++++++

>  tools/perf/Makefile.config            | 25 +++++++++++++++++++++++++

>  tools/perf/Makefile.perf              |  2 ++

>  6 files changed, 50 insertions(+), 3 deletions(-)

>  create mode 100644 tools/build/feature/test-libopencsd.c

> 

> diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature

> index e52fcefee379..dc3d97b248ab 100644

> --- a/tools/build/Makefile.feature

> +++ b/tools/build/Makefile.feature

> @@ -66,7 +66,8 @@ FEATURE_TESTS_BASIC :=                  \

>          bpf                             \

>          sched_getcpu			\

>          sdt				\

> -        setns

> +        setns				\

> +	libopencsd

>  

>  # FEATURE_TESTS_BASIC + FEATURE_TESTS_EXTRA is the complete list

>  # of all feature tests

> @@ -108,7 +109,8 @@ FEATURE_DISPLAY ?=              \

>           zlib                   \

>           lzma                   \

>           get_cpuid              \

> -         bpf

> +         bpf			\

> +	 libopencsd


we put in this list only generic libraries, this one seems arch
specific please put it into FEATURE_TESTS_EXTRA list

the state is displayed by running make 'VF=1', with this patch:
  https://marc.info/?l=linux-kernel&m=151549001914932&w=2

Arnaldo, could you please check on that one ^^^?

thanks,
jirka
Mathieu Poirier Jan. 16, 2018, 6:04 p.m. UTC | #3 
On Tue, Jan 16, 2018 at 01:15:00PM +0100, Jiri Olsa wrote:
> On Mon, Jan 15, 2018 at 11:13:05AM -0700, Mathieu Poirier wrote:

> > The Open CoreSight Decoding Library (openCSD) is a free and open

> > library to decode traces collected by the CoreSight hardware

> > infrastructure.

> > 

> > This patch adds the required mechanic to recognise the presence

> > of the openCSD library on a system and set up miscellaneous flags

> > to be used in the compilation of the trace decoding feature.

> > 

> > Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>

> > ---

> >  tools/build/Makefile.feature          |  6 ++++--

> >  tools/build/feature/Makefile          |  7 ++++++-

> >  tools/build/feature/test-all.c        |  5 +++++

> >  tools/build/feature/test-libopencsd.c |  8 ++++++++

> >  tools/perf/Makefile.config            | 25 +++++++++++++++++++++++++

> >  tools/perf/Makefile.perf              |  2 ++

> >  6 files changed, 50 insertions(+), 3 deletions(-)

> >  create mode 100644 tools/build/feature/test-libopencsd.c

> > 

> > diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature

> > index e52fcefee379..dc3d97b248ab 100644

> > --- a/tools/build/Makefile.feature

> > +++ b/tools/build/Makefile.feature

> > @@ -66,7 +66,8 @@ FEATURE_TESTS_BASIC :=                  \

> >          bpf                             \

> >          sched_getcpu			\

> >          sdt				\

> > -        setns

> > +        setns				\

> > +	libopencsd

> >  

> >  # FEATURE_TESTS_BASIC + FEATURE_TESTS_EXTRA is the complete list

> >  # of all feature tests

> > @@ -108,7 +109,8 @@ FEATURE_DISPLAY ?=              \

> >           zlib                   \

> >           lzma                   \

> >           get_cpuid              \

> > -         bpf

> > +         bpf			\

> > +	 libopencsd

> 

> we put in this list only generic libraries, this one seems arch

> specific please put it into FEATURE_TESTS_EXTRA list


Very well - thanks for taking the time to review my work.
Mathieu

> 

> the state is displayed by running make 'VF=1', with this patch:

>   https://marc.info/?l=linux-kernel&m=151549001914932&w=2

> 

> Arnaldo, could you please check on that one ^^^?

> 

> thanks,

> jirka
Mathieu Poirier Jan. 16, 2018, 8:30 p.m. UTC | #4 
On 16 January 2018 at 05:15, Jiri Olsa <jolsa@redhat.com> wrote:
> On Mon, Jan 15, 2018 at 11:13:05AM -0700, Mathieu Poirier wrote:

>> The Open CoreSight Decoding Library (openCSD) is a free and open

>> library to decode traces collected by the CoreSight hardware

>> infrastructure.

>>

>> This patch adds the required mechanic to recognise the presence

>> of the openCSD library on a system and set up miscellaneous flags

>> to be used in the compilation of the trace decoding feature.

>>

>> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>

>> ---

>>  tools/build/Makefile.feature          |  6 ++++--

>>  tools/build/feature/Makefile          |  7 ++++++-

>>  tools/build/feature/test-all.c        |  5 +++++

>>  tools/build/feature/test-libopencsd.c |  8 ++++++++

>>  tools/perf/Makefile.config            | 25 +++++++++++++++++++++++++

>>  tools/perf/Makefile.perf              |  2 ++

>>  6 files changed, 50 insertions(+), 3 deletions(-)

>>  create mode 100644 tools/build/feature/test-libopencsd.c

>>

>> diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature

>> index e52fcefee379..dc3d97b248ab 100644

>> --- a/tools/build/Makefile.feature

>> +++ b/tools/build/Makefile.feature

>> @@ -66,7 +66,8 @@ FEATURE_TESTS_BASIC :=                  \

>>          bpf                             \

>>          sched_getcpu                 \

>>          sdt                          \

>> -        setns

>> +        setns                                \

>> +     libopencsd

>>

>>  # FEATURE_TESTS_BASIC + FEATURE_TESTS_EXTRA is the complete list

>>  # of all feature tests

>> @@ -108,7 +109,8 @@ FEATURE_DISPLAY ?=              \

>>           zlib                   \

>>           lzma                   \

>>           get_cpuid              \

>> -         bpf

>> +         bpf                 \

>> +      libopencsd

>

> we put in this list only generic libraries, this one seems arch

> specific please put it into FEATURE_TESTS_EXTRA list


Humm... Jiri,

I was thinking that libopencsd should fall in the same category as
libunwind-arm and libunwind-aarch64.  Both are not architecture
specific and used to process traces acquired on ARM platforms.  As
such I suggest to keep libopencsd as part of FEATURE_TESTS_BASIC and
remove it from under FEATURE_DISPLAY - how does that sound to you?

>

> the state is displayed by running make 'VF=1', with this patch:

>   https://marc.info/?l=linux-kernel&m=151549001914932&w=2

>

> Arnaldo, could you please check on that one ^^^?

>

> thanks,

> jirka
Jiri Olsa Jan. 17, 2018, 8:06 a.m. UTC | #5 
On Tue, Jan 16, 2018 at 01:30:33PM -0700, Mathieu Poirier wrote:
> On 16 January 2018 at 05:15, Jiri Olsa <jolsa@redhat.com> wrote:

> > On Mon, Jan 15, 2018 at 11:13:05AM -0700, Mathieu Poirier wrote:

> >> The Open CoreSight Decoding Library (openCSD) is a free and open

> >> library to decode traces collected by the CoreSight hardware

> >> infrastructure.

> >>

> >> This patch adds the required mechanic to recognise the presence

> >> of the openCSD library on a system and set up miscellaneous flags

> >> to be used in the compilation of the trace decoding feature.

> >>

> >> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>

> >> ---

> >>  tools/build/Makefile.feature          |  6 ++++--

> >>  tools/build/feature/Makefile          |  7 ++++++-

> >>  tools/build/feature/test-all.c        |  5 +++++

> >>  tools/build/feature/test-libopencsd.c |  8 ++++++++

> >>  tools/perf/Makefile.config            | 25 +++++++++++++++++++++++++

> >>  tools/perf/Makefile.perf              |  2 ++

> >>  6 files changed, 50 insertions(+), 3 deletions(-)

> >>  create mode 100644 tools/build/feature/test-libopencsd.c

> >>

> >> diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature

> >> index e52fcefee379..dc3d97b248ab 100644

> >> --- a/tools/build/Makefile.feature

> >> +++ b/tools/build/Makefile.feature

> >> @@ -66,7 +66,8 @@ FEATURE_TESTS_BASIC :=                  \

> >>          bpf                             \

> >>          sched_getcpu                 \

> >>          sdt                          \

> >> -        setns

> >> +        setns                                \

> >> +     libopencsd

> >>

> >>  # FEATURE_TESTS_BASIC + FEATURE_TESTS_EXTRA is the complete list

> >>  # of all feature tests

> >> @@ -108,7 +109,8 @@ FEATURE_DISPLAY ?=              \

> >>           zlib                   \

> >>           lzma                   \

> >>           get_cpuid              \

> >> -         bpf

> >> +         bpf                 \

> >> +      libopencsd

> >

> > we put in this list only generic libraries, this one seems arch

> > specific please put it into FEATURE_TESTS_EXTRA list

> 

> Humm... Jiri,

> 

> I was thinking that libopencsd should fall in the same category as

> libunwind-arm and libunwind-aarch64.  Both are not architecture

> specific and used to process traces acquired on ARM platforms.  As

> such I suggest to keep libopencsd as part of FEATURE_TESTS_BASIC and

> remove it from under FEATURE_DISPLAY - how does that sound to you?

> 


ok, that sounds good

jirka
Arnaldo Carvalho de Melo Jan. 18, 2018, 1:41 p.m. UTC | #6 
Em Wed, Jan 17, 2018 at 09:06:40AM +0100, Jiri Olsa escreveu:
> On Tue, Jan 16, 2018 at 01:30:33PM -0700, Mathieu Poirier wrote:

> > On 16 January 2018 at 05:15, Jiri Olsa <jolsa@redhat.com> wrote:

> > > On Mon, Jan 15, 2018 at 11:13:05AM -0700, Mathieu Poirier wrote:

> > >> +++ b/tools/build/Makefile.feature

> > >> @@ -66,7 +66,8 @@ FEATURE_TESTS_BASIC :=                  \

> > >>          bpf                             \

> > >>          sched_getcpu                 \

> > >>          sdt                          \

> > >> -        setns

> > >> +        setns                                \

> > >> +     libopencsd

> > >>

> > >>  # FEATURE_TESTS_BASIC + FEATURE_TESTS_EXTRA is the complete list

> > >>  # of all feature tests

> > >> @@ -108,7 +109,8 @@ FEATURE_DISPLAY ?=              \

> > >>           zlib                   \

> > >>           lzma                   \

> > >>           get_cpuid              \

> > >> -         bpf

> > >> +         bpf                 \

> > >> +      libopencsd


> > > we put in this list only generic libraries, this one seems arch

> > > specific please put it into FEATURE_TESTS_EXTRA list


> > I was thinking that libopencsd should fall in the same category as

> > libunwind-arm and libunwind-aarch64.  Both are not architecture

> > specific and used to process traces acquired on ARM platforms.  As

> > such I suggest to keep libopencsd as part of FEATURE_TESTS_BASIC and

> > remove it from under FEATURE_DISPLAY - how does that sound to you?

 
> ok, that sounds good


Hi Jiri,

	Shouldn't libopencsd be treated like libbabeltrace was before
the required version was widely available in distros?

	I.e. these csets should have the rationale for that:

Enabling it once it became widely available:

   24787afbcd01 ("perf tools: Enable LIBBABELTRACE by default")

Disabling it because we would need to get things from tarballs/git
repos, build it in our machines, as requested by Ingo:

  6ab2b762befd ("perf build: Disable libbabeltrace check by default")

- Arnaldo
Jiri Olsa Jan. 18, 2018, 1:59 p.m. UTC | #7 
On Thu, Jan 18, 2018 at 10:41:39AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Wed, Jan 17, 2018 at 09:06:40AM +0100, Jiri Olsa escreveu:

> > On Tue, Jan 16, 2018 at 01:30:33PM -0700, Mathieu Poirier wrote:

> > > On 16 January 2018 at 05:15, Jiri Olsa <jolsa@redhat.com> wrote:

> > > > On Mon, Jan 15, 2018 at 11:13:05AM -0700, Mathieu Poirier wrote:

> > > >> +++ b/tools/build/Makefile.feature

> > > >> @@ -66,7 +66,8 @@ FEATURE_TESTS_BASIC :=                  \

> > > >>          bpf                             \

> > > >>          sched_getcpu                 \

> > > >>          sdt                          \

> > > >> -        setns

> > > >> +        setns                                \

> > > >> +     libopencsd

> > > >>

> > > >>  # FEATURE_TESTS_BASIC + FEATURE_TESTS_EXTRA is the complete list

> > > >>  # of all feature tests

> > > >> @@ -108,7 +109,8 @@ FEATURE_DISPLAY ?=              \

> > > >>           zlib                   \

> > > >>           lzma                   \

> > > >>           get_cpuid              \

> > > >> -         bpf

> > > >> +         bpf                 \

> > > >> +      libopencsd

> 

> > > > we put in this list only generic libraries, this one seems arch

> > > > specific please put it into FEATURE_TESTS_EXTRA list

> 

> > > I was thinking that libopencsd should fall in the same category as

> > > libunwind-arm and libunwind-aarch64.  Both are not architecture

> > > specific and used to process traces acquired on ARM platforms.  As

> > > such I suggest to keep libopencsd as part of FEATURE_TESTS_BASIC and

> > > remove it from under FEATURE_DISPLAY - how does that sound to you?

>  

> > ok, that sounds good

> 

> Hi Jiri,

> 

> 	Shouldn't libopencsd be treated like libbabeltrace was before

> the required version was widely available in distros?

> 

> 	I.e. these csets should have the rationale for that:

> 

> Enabling it once it became widely available:

> 

>    24787afbcd01 ("perf tools: Enable LIBBABELTRACE by default")

> 

> Disabling it because we would need to get things from tarballs/git

> repos, build it in our machines, as requested by Ingo:

> 

>   6ab2b762befd ("perf build: Disable libbabeltrace check by default")


I think at that time we did not have a way to hide the check,
now we have FEATURE_DISPLAY seprated so we can still check
for it, but users won't be bothered with [ FAIL ] output

jirka
Arnaldo Carvalho de Melo Jan. 18, 2018, 2:14 p.m. UTC | #8 
Em Thu, Jan 18, 2018 at 02:59:48PM +0100, Jiri Olsa escreveu:
> On Thu, Jan 18, 2018 at 10:41:39AM -0300, Arnaldo Carvalho de Melo wrote:

> > 	Shouldn't libopencsd be treated like libbabeltrace was before

> > the required version was widely available in distros?


> > 	I.e. these csets should have the rationale for that:


> > Enabling it once it became widely available:


> >    24787afbcd01 ("perf tools: Enable LIBBABELTRACE by default")


> > Disabling it because we would need to get things from tarballs/git

> > repos, build it in our machines, as requested by Ingo:


> >   6ab2b762befd ("perf build: Disable libbabeltrace check by default")

> 

> I think at that time we did not have a way to hide the check,

> now we have FEATURE_DISPLAY seprated so we can still check

> for it, but users won't be bothered with [ FAIL ] output


Ok, users won't be bothered with the fail output, but we tried hard to
get the build fast by having it only test for things that are widely
available, right? I.e. if we know something is not widely available then
we better not try to build with it and get faster builds, wasn't that
part of the rationale in the babeltrace case?

If one has to build from sources some library, then its not a problem to
have in the make command line a LIBOPENCSD=1 switch?

- Arnaldo
Jiri Olsa Jan. 18, 2018, 2:27 p.m. UTC | #9 
On Thu, Jan 18, 2018 at 11:14:23AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Jan 18, 2018 at 02:59:48PM +0100, Jiri Olsa escreveu:

> > On Thu, Jan 18, 2018 at 10:41:39AM -0300, Arnaldo Carvalho de Melo wrote:

> > > 	Shouldn't libopencsd be treated like libbabeltrace was before

> > > the required version was widely available in distros?

> 

> > > 	I.e. these csets should have the rationale for that:

> 

> > > Enabling it once it became widely available:

> 

> > >    24787afbcd01 ("perf tools: Enable LIBBABELTRACE by default")

> 

> > > Disabling it because we would need to get things from tarballs/git

> > > repos, build it in our machines, as requested by Ingo:

> 

> > >   6ab2b762befd ("perf build: Disable libbabeltrace check by default")

> > 

> > I think at that time we did not have a way to hide the check,

> > now we have FEATURE_DISPLAY seprated so we can still check

> > for it, but users won't be bothered with [ FAIL ] output

> 

> Ok, users won't be bothered with the fail output, but we tried hard to

> get the build fast by having it only test for things that are widely

> available, right? I.e. if we know something is not widely available then

> we better not try to build with it and get faster builds, wasn't that

> part of the rationale in the babeltrace case?

> 

> If one has to build from sources some library, then its not a problem to

> have in the make command line a LIBOPENCSD=1 switch?


right, we can do it like that

jirka
Arnaldo Carvalho de Melo Jan. 19, 2018, 2:58 p.m. UTC | #10 
Em Thu, Jan 18, 2018 at 03:27:43PM +0100, Jiri Olsa escreveu:
> On Thu, Jan 18, 2018 at 11:14:23AM -0300, Arnaldo Carvalho de Melo wrote:

> > Em Thu, Jan 18, 2018 at 02:59:48PM +0100, Jiri Olsa escreveu:

> > > On Thu, Jan 18, 2018 at 10:41:39AM -0300, Arnaldo Carvalho de Melo wrote:

> > > > 	Shouldn't libopencsd be treated like libbabeltrace was before

> > > > the required version was widely available in distros?

> > 

> > > > 	I.e. these csets should have the rationale for that:

> > 

> > > > Enabling it once it became widely available:

> > 

> > > >    24787afbcd01 ("perf tools: Enable LIBBABELTRACE by default")

> > 

> > > > Disabling it because we would need to get things from tarballs/git

> > > > repos, build it in our machines, as requested by Ingo:

> > 

> > > >   6ab2b762befd ("perf build: Disable libbabeltrace check by default")

> > > 

> > > I think at that time we did not have a way to hide the check,

> > > now we have FEATURE_DISPLAY seprated so we can still check

> > > for it, but users won't be bothered with [ FAIL ] output

> > 

> > Ok, users won't be bothered with the fail output, but we tried hard to

> > get the build fast by having it only test for things that are widely

> > available, right? I.e. if we know something is not widely available then

> > we better not try to build with it and get faster builds, wasn't that

> > part of the rationale in the babeltrace case?

> > 

> > If one has to build from sources some library, then its not a problem to

> > have in the make command line a LIBOPENCSD=1 switch?

> 

> right, we can do it like that


So I'm applying v2 and we can go on from there, to make progress, ok?
I'm adding your Acked-by to all but the build ones, ok?

- Arnaldo
Jiri Olsa Jan. 19, 2018, 3:12 p.m. UTC | #11 
On Fri, Jan 19, 2018 at 11:58:19AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Jan 18, 2018 at 03:27:43PM +0100, Jiri Olsa escreveu:

> > On Thu, Jan 18, 2018 at 11:14:23AM -0300, Arnaldo Carvalho de Melo wrote:

> > > Em Thu, Jan 18, 2018 at 02:59:48PM +0100, Jiri Olsa escreveu:

> > > > On Thu, Jan 18, 2018 at 10:41:39AM -0300, Arnaldo Carvalho de Melo wrote:

> > > > > 	Shouldn't libopencsd be treated like libbabeltrace was before

> > > > > the required version was widely available in distros?

> > > 

> > > > > 	I.e. these csets should have the rationale for that:

> > > 

> > > > > Enabling it once it became widely available:

> > > 

> > > > >    24787afbcd01 ("perf tools: Enable LIBBABELTRACE by default")

> > > 

> > > > > Disabling it because we would need to get things from tarballs/git

> > > > > repos, build it in our machines, as requested by Ingo:

> > > 

> > > > >   6ab2b762befd ("perf build: Disable libbabeltrace check by default")

> > > > 

> > > > I think at that time we did not have a way to hide the check,

> > > > now we have FEATURE_DISPLAY seprated so we can still check

> > > > for it, but users won't be bothered with [ FAIL ] output

> > > 

> > > Ok, users won't be bothered with the fail output, but we tried hard to

> > > get the build fast by having it only test for things that are widely

> > > available, right? I.e. if we know something is not widely available then

> > > we better not try to build with it and get faster builds, wasn't that

> > > part of the rationale in the babeltrace case?

> > > 

> > > If one has to build from sources some library, then its not a problem to

> > > have in the make command line a LIBOPENCSD=1 switch?

> > 

> > right, we can do it like that

> 

> So I'm applying v2 and we can go on from there, to make progress, ok?

> I'm adding your Acked-by to all but the build ones, ok?


I think v3 was in better shape.. wrt tabs and overall display

jirka
Mathieu Poirier Jan. 19, 2018, 3:24 p.m. UTC | #12 
On 19 January 2018 at 08:12, Jiri Olsa <jolsa@redhat.com> wrote:
> On Fri, Jan 19, 2018 at 11:58:19AM -0300, Arnaldo Carvalho de Melo wrote:

>> Em Thu, Jan 18, 2018 at 03:27:43PM +0100, Jiri Olsa escreveu:

>> > On Thu, Jan 18, 2018 at 11:14:23AM -0300, Arnaldo Carvalho de Melo wrote:

>> > > Em Thu, Jan 18, 2018 at 02:59:48PM +0100, Jiri Olsa escreveu:

>> > > > On Thu, Jan 18, 2018 at 10:41:39AM -0300, Arnaldo Carvalho de Melo wrote:

>> > > > >       Shouldn't libopencsd be treated like libbabeltrace was before

>> > > > > the required version was widely available in distros?

>> > >

>> > > > >       I.e. these csets should have the rationale for that:

>> > >

>> > > > > Enabling it once it became widely available:

>> > >

>> > > > >    24787afbcd01 ("perf tools: Enable LIBBABELTRACE by default")

>> > >

>> > > > > Disabling it because we would need to get things from tarballs/git

>> > > > > repos, build it in our machines, as requested by Ingo:

>> > >

>> > > > >   6ab2b762befd ("perf build: Disable libbabeltrace check by default")

>> > > >

>> > > > I think at that time we did not have a way to hide the check,

>> > > > now we have FEATURE_DISPLAY seprated so we can still check

>> > > > for it, but users won't be bothered with [ FAIL ] output

>> > >

>> > > Ok, users won't be bothered with the fail output, but we tried hard to

>> > > get the build fast by having it only test for things that are widely

>> > > available, right? I.e. if we know something is not widely available then

>> > > we better not try to build with it and get faster builds, wasn't that

>> > > part of the rationale in the babeltrace case?

>> > >

>> > > If one has to build from sources some library, then its not a problem to

>> > > have in the make command line a LIBOPENCSD=1 switch?

>> >

>> > right, we can do it like that

>>

>> So I'm applying v2 and we can go on from there, to make progress, ok?

>> I'm adding your Acked-by to all but the build ones, ok?

>

> I think v3 was in better shape.. wrt tabs and overall display

>

> jirka


Jiri is correct - V3 should be considered.
Arnaldo Carvalho de Melo Jan. 19, 2018, 3:55 p.m. UTC | #13 
Em Fri, Jan 19, 2018 at 08:24:56AM -0700, Mathieu Poirier escreveu:
> On 19 January 2018 at 08:12, Jiri Olsa <jolsa@redhat.com> wrote:

> > On Fri, Jan 19, 2018 at 11:58:19AM -0300, Arnaldo Carvalho de Melo wrote:

> >> Em Thu, Jan 18, 2018 at 03:27:43PM +0100, Jiri Olsa escreveu:

> >> > On Thu, Jan 18, 2018 at 11:14:23AM -0300, Arnaldo Carvalho de Melo wrote:

> >> > > Em Thu, Jan 18, 2018 at 02:59:48PM +0100, Jiri Olsa escreveu:

> >> > > > On Thu, Jan 18, 2018 at 10:41:39AM -0300, Arnaldo Carvalho de Melo wrote:

> >> > > > >       Shouldn't libopencsd be treated like libbabeltrace was before

> >> > > > > the required version was widely available in distros?

> >> > >

> >> > > > >       I.e. these csets should have the rationale for that:

> >> > >

> >> > > > > Enabling it once it became widely available:

> >> > >

> >> > > > >    24787afbcd01 ("perf tools: Enable LIBBABELTRACE by default")

> >> > >

> >> > > > > Disabling it because we would need to get things from tarballs/git

> >> > > > > repos, build it in our machines, as requested by Ingo:

> >> > >

> >> > > > >   6ab2b762befd ("perf build: Disable libbabeltrace check by default")

> >> > > >

> >> > > > I think at that time we did not have a way to hide the check,

> >> > > > now we have FEATURE_DISPLAY seprated so we can still check

> >> > > > for it, but users won't be bothered with [ FAIL ] output

> >> > >

> >> > > Ok, users won't be bothered with the fail output, but we tried hard to

> >> > > get the build fast by having it only test for things that are widely

> >> > > available, right? I.e. if we know something is not widely available then

> >> > > we better not try to build with it and get faster builds, wasn't that

> >> > > part of the rationale in the babeltrace case?

> >> > >

> >> > > If one has to build from sources some library, then its not a problem to

> >> > > have in the make command line a LIBOPENCSD=1 switch?

> >> >

> >> > right, we can do it like that

> >>

> >> So I'm applying v2 and we can go on from there, to make progress, ok?

> >> I'm adding your Acked-by to all but the build ones, ok?

> >

> > I think v3 was in better shape.. wrt tabs and overall display

> >

> > jirka

> 

> Jiri is correct - V3 should be considered.


So, please take a look at my perf/core branch, hopefully my mistake was
just on the message saying I would apply v2, check that v3 was what I
applied.

- Arnaldo
Mathieu Poirier Jan. 19, 2018, 5:28 p.m. UTC | #14 
On 19 January 2018 at 08:55, Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> Em Fri, Jan 19, 2018 at 08:24:56AM -0700, Mathieu Poirier escreveu:

>> On 19 January 2018 at 08:12, Jiri Olsa <jolsa@redhat.com> wrote:

>> > On Fri, Jan 19, 2018 at 11:58:19AM -0300, Arnaldo Carvalho de Melo wrote:

>> >> Em Thu, Jan 18, 2018 at 03:27:43PM +0100, Jiri Olsa escreveu:

>> >> > On Thu, Jan 18, 2018 at 11:14:23AM -0300, Arnaldo Carvalho de Melo wrote:

>> >> > > Em Thu, Jan 18, 2018 at 02:59:48PM +0100, Jiri Olsa escreveu:

>> >> > > > On Thu, Jan 18, 2018 at 10:41:39AM -0300, Arnaldo Carvalho de Melo wrote:

>> >> > > > >       Shouldn't libopencsd be treated like libbabeltrace was before

>> >> > > > > the required version was widely available in distros?

>> >> > >

>> >> > > > >       I.e. these csets should have the rationale for that:

>> >> > >

>> >> > > > > Enabling it once it became widely available:

>> >> > >

>> >> > > > >    24787afbcd01 ("perf tools: Enable LIBBABELTRACE by default")

>> >> > >

>> >> > > > > Disabling it because we would need to get things from tarballs/git

>> >> > > > > repos, build it in our machines, as requested by Ingo:

>> >> > >

>> >> > > > >   6ab2b762befd ("perf build: Disable libbabeltrace check by default")

>> >> > > >

>> >> > > > I think at that time we did not have a way to hide the check,

>> >> > > > now we have FEATURE_DISPLAY seprated so we can still check

>> >> > > > for it, but users won't be bothered with [ FAIL ] output

>> >> > >

>> >> > > Ok, users won't be bothered with the fail output, but we tried hard to

>> >> > > get the build fast by having it only test for things that are widely

>> >> > > available, right? I.e. if we know something is not widely available then

>> >> > > we better not try to build with it and get faster builds, wasn't that

>> >> > > part of the rationale in the babeltrace case?

>> >> > >

>> >> > > If one has to build from sources some library, then its not a problem to

>> >> > > have in the make command line a LIBOPENCSD=1 switch?

>> >> >

>> >> > right, we can do it like that

>> >>

>> >> So I'm applying v2 and we can go on from there, to make progress, ok?

>> >> I'm adding your Acked-by to all but the build ones, ok?

>> >

>> > I think v3 was in better shape.. wrt tabs and overall display

>> >

>> > jirka

>>

>> Jiri is correct - V3 should be considered.

>

> So, please take a look at my perf/core branch, hopefully my mistake was

> just on the message saying I would apply v2, check that v3 was what I

> applied.


The correct version was applied - thanks.

Mathieu

>

> - Arnaldo
Jiri Olsa Jan. 19, 2018, 6:46 p.m. UTC | #15 
On Fri, Jan 19, 2018 at 10:28:24AM -0700, Mathieu Poirier wrote:
> On 19 January 2018 at 08:55, Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> > Em Fri, Jan 19, 2018 at 08:24:56AM -0700, Mathieu Poirier escreveu:

> >> On 19 January 2018 at 08:12, Jiri Olsa <jolsa@redhat.com> wrote:

> >> > On Fri, Jan 19, 2018 at 11:58:19AM -0300, Arnaldo Carvalho de Melo wrote:

> >> >> Em Thu, Jan 18, 2018 at 03:27:43PM +0100, Jiri Olsa escreveu:

> >> >> > On Thu, Jan 18, 2018 at 11:14:23AM -0300, Arnaldo Carvalho de Melo wrote:

> >> >> > > Em Thu, Jan 18, 2018 at 02:59:48PM +0100, Jiri Olsa escreveu:

> >> >> > > > On Thu, Jan 18, 2018 at 10:41:39AM -0300, Arnaldo Carvalho de Melo wrote:

> >> >> > > > >       Shouldn't libopencsd be treated like libbabeltrace was before

> >> >> > > > > the required version was widely available in distros?

> >> >> > >

> >> >> > > > >       I.e. these csets should have the rationale for that:

> >> >> > >

> >> >> > > > > Enabling it once it became widely available:

> >> >> > >

> >> >> > > > >    24787afbcd01 ("perf tools: Enable LIBBABELTRACE by default")

> >> >> > >

> >> >> > > > > Disabling it because we would need to get things from tarballs/git

> >> >> > > > > repos, build it in our machines, as requested by Ingo:

> >> >> > >

> >> >> > > > >   6ab2b762befd ("perf build: Disable libbabeltrace check by default")

> >> >> > > >

> >> >> > > > I think at that time we did not have a way to hide the check,

> >> >> > > > now we have FEATURE_DISPLAY seprated so we can still check

> >> >> > > > for it, but users won't be bothered with [ FAIL ] output

> >> >> > >

> >> >> > > Ok, users won't be bothered with the fail output, but we tried hard to

> >> >> > > get the build fast by having it only test for things that are widely

> >> >> > > available, right? I.e. if we know something is not widely available then

> >> >> > > we better not try to build with it and get faster builds, wasn't that

> >> >> > > part of the rationale in the babeltrace case?

> >> >> > >

> >> >> > > If one has to build from sources some library, then its not a problem to

> >> >> > > have in the make command line a LIBOPENCSD=1 switch?

> >> >> >

> >> >> > right, we can do it like that

> >> >>

> >> >> So I'm applying v2 and we can go on from there, to make progress, ok?

> >> >> I'm adding your Acked-by to all but the build ones, ok?

> >> >

> >> > I think v3 was in better shape.. wrt tabs and overall display

> >> >

> >> > jirka

> >>

> >> Jiri is correct - V3 should be considered.

> >

> > So, please take a look at my perf/core branch, hopefully my mistake was

> > just on the message saying I would apply v2, check that v3 was what I

> > applied.

> 

> The correct version was applied - thanks.


yep, looks good

jirka
diff mbox series

Patch

diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
index e52fcefee379..dc3d97b248ab 100644
--- a/tools/build/Makefile.feature
+++ b/tools/build/Makefile.feature
@@ -66,7 +66,8 @@  FEATURE_TESTS_BASIC :=                  \
         bpf                             \
         sched_getcpu			\
         sdt				\
-        setns
+        setns				\
+	libopencsd
 
 # FEATURE_TESTS_BASIC + FEATURE_TESTS_EXTRA is the complete list
 # of all feature tests
@@ -108,7 +109,8 @@  FEATURE_DISPLAY ?=              \
          zlib                   \
          lzma                   \
          get_cpuid              \
-         bpf
+         bpf			\
+	 libopencsd
 
 # Set FEATURE_CHECK_(C|LD)FLAGS-all for all FEATURE_TESTS features.
 # If in the future we need per-feature checks/flags for features not
diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
index cff38f342283..2b108a6cfac3 100644
--- a/tools/build/feature/Makefile
+++ b/tools/build/feature/Makefile
@@ -52,7 +52,8 @@  FILES=                                          \
          test-cxx.bin                           \
          test-jvmti.bin				\
          test-sched_getcpu.bin			\
-         test-setns.bin
+         test-setns.bin				\
+	 test-libopencsd.bin
 
 FILES := $(addprefix $(OUTPUT),$(FILES))
 
@@ -104,6 +105,10 @@  $(OUTPUT)test-sched_getcpu.bin:
 $(OUTPUT)test-setns.bin:
 	$(BUILD)
 
+$(OUTPUT)test-libopencsd.bin:
+	$(BUILD) # -lopencsd_c_api -lopencsd provided by
+		 # $(FEATURE_CHECK_LDFLAGS-libopencsd)
+
 DWARFLIBS := -ldw
 ifeq ($(findstring -static,${LDFLAGS}),-static)
 DWARFLIBS += -lelf -lebl -lz -llzma -lbz2
diff --git a/tools/build/feature/test-all.c b/tools/build/feature/test-all.c
index 6fdf83263ab7..38ab0fd7a74c 100644
--- a/tools/build/feature/test-all.c
+++ b/tools/build/feature/test-all.c
@@ -162,6 +162,10 @@ 
 # include "test-setns.c"
 #undef main
 
+#define main main_test_setns
+# include "test-libopencsd.c"
+#undef main
+
 int main(int argc, char *argv[])
 {
 	main_test_libpython();
@@ -199,6 +203,7 @@  int main(int argc, char *argv[])
 	main_test_sched_getcpu();
 	main_test_sdt();
 	main_test_setns();
+	main_test_libopencsd();
 
 	return 0;
 }
diff --git a/tools/build/feature/test-libopencsd.c b/tools/build/feature/test-libopencsd.c
new file mode 100644
index 000000000000..cd4fb99eb9d3
--- /dev/null
+++ b/tools/build/feature/test-libopencsd.c
@@ -0,0 +1,8 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+#include <opencsd/c_api/opencsd_c_api.h>
+
+int main(void)
+{
+	(void)ocsd_get_version();
+	return 0;
+}
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index 12dec6ea5ed2..15ef1e55c7d0 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -105,6 +105,16 @@  FEATURE_CHECK_LDFLAGS-libunwind = $(LIBUNWIND_LDFLAGS) $(LIBUNWIND_LIBS)
 FEATURE_CHECK_CFLAGS-libunwind-debug-frame = $(LIBUNWIND_CFLAGS)
 FEATURE_CHECK_LDFLAGS-libunwind-debug-frame = $(LIBUNWIND_LDFLAGS) $(LIBUNWIND_LIBS)
 
+ifdef CSINCLUDES
+  LIBOPENCSD_CFLAGS := -I$(CSINCLUDES)
+endif
+OPENCSDLIBS := -lopencsd_c_api -lopencsd
+ifdef CSLIBS
+  LIBOPENCSD_LDFLAGS := -L$(CSLIBS)
+endif
+FEATURE_CHECK_CFLAGS-libopencsd := $(LIBOPENCSD_CFLAGS)
+FEATURE_CHECK_LDFLAGS-libopencsd := $(LIBOPENCSD_LDFLAGS) $(OPENCSDLIBS)
+
 ifeq ($(NO_PERF_REGS),0)
   CFLAGS += -DHAVE_PERF_REGS_SUPPORT
 endif
@@ -353,6 +363,21 @@  ifeq ($(feature-setns), 1)
   $(call detected,CONFIG_SETNS)
 endif
 
+ifndef NO_CORESIGHT
+  ifeq ($(feature-libopencsd), 1)
+    CFLAGS += -DHAVE_CSTRACE_SUPPORT $(LIBOPENCSD_CFLAGS)
+    LDFLAGS += $(LIBOPENCSD_LDFLAGS)
+    EXTLIBS += $(OPENCSDLIBS)
+    $(call detected,CONFIG_LIBOPENCSD)
+    ifdef CSTRACE_RAW
+      CFLAGS += -DCS_DEBUG_RAW
+      ifeq (${CSTRACE_RAW}, packed)
+        CFLAGS += -DCS_RAW_PACKED
+      endif
+    endif
+  endif
+endif
+
 ifndef NO_LIBELF
   CFLAGS += -DHAVE_LIBELF_SUPPORT
   EXTLIBS += -lelf
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 9fdefd748e2e..92dfa9580eab 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -98,6 +98,8 @@  include ../scripts/utilities.mak
 # When selected, pass LLVM_CONFIG=/path/to/llvm-config to `make' if
 # llvm-config is not in $PATH.
 
+# Define NO_CORESIGHT if you do not want support for CoreSight trace decoding.
+
 # As per kernel Makefile, avoid funny character set dependencies
 unexport LC_ALL
 LC_COLLATE=C