Message ID | 1513356299-26274-1-git-send-email-mathieu.poirier@linaro.org |
---|---|
Headers | show |
Series | perf tools: Add support for CoreSight trace decoding | expand |
Hi Mathieu, On Fri, Dec 15, 2017 at 09:44:49AM -0700, Mathieu Poirier wrote: > This patchset adds support for per-thread CoreSight trace decoding from the > "perf report" interface. It is largely modelled on what has been done for > intelPT traces and currently targets the ETMv4 architecture. Support for > cpu-wide scenarios and ETMv3/PTMv1.1 will follow shortly. > > The trace decoding support is done using the Open CoreSight Decoding > Library (openCSD), a stand alone open source project available here [1]. > Integration of the openCSD library with the perf tools follow what has > been done for other support libraries. If the library has been installed > on a system the build scripts will include support for CoreSight trace > decoding: > > ... zlib: [ on ] > ... lzma: [ OFF ] > ... get_cpuid: [ on ] > ... bpf: [ on ] > ... libopencsd: [ on ] <------ > > Instructions on how to build and install the openCSD library are provided > in the HOWTO.md of the project repository. We elected to keep the decoder > library independent of the kernel tree as it is also used outside of the > perf toolset and various non-linux projects. > > The work applies cleanly to [2] and proper functionning of the feature > depends on this patch [3]. With latest perf code, it reports another error when analyse perf data: "0x3e0 [0x50]: failed to process type: 1". After roughly analysis, I found this is caused by one dummy event (in the binary from offset 0xf8 to offset 0x178). Because this event type is not set for 'PERF_SAMPLE_TIME', so the function perf_evsel__parse_sample_timestamp() checks the event has not set 'PERF_SAMPLE_TIME' then directly bail out with error. 000000f0: 0800 0000 0000 0000 0100 0000 7000 0000 ............p... 00000100: 0900 0000 0000 0000 0100 0000 0000 0000 ................ 00000110: 0300 0100 0000 0000 0400 0000 0000 0000 ................ 00000120: 6133 8401 0000 0000 0000 0000 0000 0000 a3.............. 00000130: 0000 0000 0000 0000 0000 0000 0000 0000 ................ 00000140: 0000 0000 0000 0000 0000 0000 0000 0000 ................ 00000150: 0000 0000 0000 0000 0000 0000 0000 0000 ................ 00000160: 0000 0000 0000 0000 7000 0000 0000 0000 ........p....... 00000170: 0800 0000 0000 0000 4600 0000 0000 6802 ........F.....h. You could check the perf binary from [1]. Please note, this perf data I capatured from kernel 4.14-rc6, so is it might be compatible issue between 4.14-rc6 and 4.15? [1] http://people.linaro.org/~leo.yan/binaries/perf_4.15_r4/perf.data Thanks, Leo Yan > Review and comments would be greatly appreciated. > > Regards, > Mathieu > > [1]. https://github.com/Linaro/OpenCSD > [2]. git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git perf/core > [3]. https://lkml.org/lkml/2017/12/14/612 > > Mathieu Poirier (8): > perf tools: Integrating the CoreSight decoding library > perf tools: Add initial entry point for decoder CoreSight traces > perf tools: Add decoder mechanic to support dumping trace data > perf tools: Add support for decoding CoreSight trace data > perf tools: Add functionality to communicate with the openCSD decoder > pert tools: Add queue management functionality > perf tools: Add full support for CoreSight trace decoding > perf tools: Add mechanic to synthesise CoreSight trace packets > > Tor Jeremiassen (2): > perf tools: Add processing of coresight metadata > MAINTAINERS: Adding entry for CoreSight trace decoding > > MAINTAINERS | 3 +- > tools/build/Makefile.feature | 6 +- > tools/build/feature/Makefile | 6 +- > tools/build/feature/test-all.c | 5 + > tools/build/feature/test-libopencsd.c | 8 + > tools/perf/Makefile.config | 13 + > tools/perf/util/Build | 6 + > tools/perf/util/auxtrace.c | 2 + > tools/perf/util/cs-etm-decoder/Build | 1 + > tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 513 ++++++++++++ > tools/perf/util/cs-etm-decoder/cs-etm-decoder.h | 105 +++ > tools/perf/util/cs-etm.c | 1023 +++++++++++++++++++++++ > tools/perf/util/cs-etm.h | 18 + > 13 files changed, 1705 insertions(+), 4 deletions(-) > create mode 100644 tools/build/feature/test-libopencsd.c > create mode 100644 tools/perf/util/cs-etm-decoder/Build > create mode 100644 tools/perf/util/cs-etm-decoder/cs-etm-decoder.c > create mode 100644 tools/perf/util/cs-etm-decoder/cs-etm-decoder.h > create mode 100644 tools/perf/util/cs-etm.c > > -- > 2.7.4 >
Good day Leo, On 29 December 2017 at 17:51, Leo Yan <leo.yan@linaro.org> wrote: > Hi Mathieu, > > On Fri, Dec 15, 2017 at 09:44:49AM -0700, Mathieu Poirier wrote: >> This patchset adds support for per-thread CoreSight trace decoding from the >> "perf report" interface. It is largely modelled on what has been done for >> intelPT traces and currently targets the ETMv4 architecture. Support for >> cpu-wide scenarios and ETMv3/PTMv1.1 will follow shortly. >> >> The trace decoding support is done using the Open CoreSight Decoding >> Library (openCSD), a stand alone open source project available here [1]. >> Integration of the openCSD library with the perf tools follow what has >> been done for other support libraries. If the library has been installed >> on a system the build scripts will include support for CoreSight trace >> decoding: >> >> ... zlib: [ on ] >> ... lzma: [ OFF ] >> ... get_cpuid: [ on ] >> ... bpf: [ on ] >> ... libopencsd: [ on ] <------ >> >> Instructions on how to build and install the openCSD library are provided >> in the HOWTO.md of the project repository. We elected to keep the decoder >> library independent of the kernel tree as it is also used outside of the >> perf toolset and various non-linux projects. >> >> The work applies cleanly to [2] and proper functionning of the feature >> depends on this patch [3]. > > With latest perf code, it reports another error when analyse perf > data: "0x3e0 [0x50]: failed to process type: 1". > > After roughly analysis, I found this is caused by one dummy event (in > the binary from offset 0xf8 to offset 0x178). Because this event type > is not set for 'PERF_SAMPLE_TIME', so the function > perf_evsel__parse_sample_timestamp() checks the event has not set > 'PERF_SAMPLE_TIME' then directly bail out with error. This patch should fix the problem: https://patchwork.kernel.org/patch/10121515/ > > 000000f0: 0800 0000 0000 0000 0100 0000 7000 0000 ............p... > 00000100: 0900 0000 0000 0000 0100 0000 0000 0000 ................ > 00000110: 0300 0100 0000 0000 0400 0000 0000 0000 ................ > 00000120: 6133 8401 0000 0000 0000 0000 0000 0000 a3.............. > 00000130: 0000 0000 0000 0000 0000 0000 0000 0000 ................ > 00000140: 0000 0000 0000 0000 0000 0000 0000 0000 ................ > 00000150: 0000 0000 0000 0000 0000 0000 0000 0000 ................ > 00000160: 0000 0000 0000 0000 7000 0000 0000 0000 ........p....... > 00000170: 0800 0000 0000 0000 4600 0000 0000 6802 ........F.....h. > > You could check the perf binary from [1]. Please note, this perf data > I capatured from kernel 4.14-rc6, so is it might be compatible issue > between 4.14-rc6 and 4.15? > > [1] http://people.linaro.org/~leo.yan/binaries/perf_4.15_r4/perf.data > > Thanks, > Leo Yan > >> Review and comments would be greatly appreciated. >> >> Regards, >> Mathieu >> >> [1]. https://github.com/Linaro/OpenCSD >> [2]. git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git perf/core >> [3]. https://lkml.org/lkml/2017/12/14/612 >> >> Mathieu Poirier (8): >> perf tools: Integrating the CoreSight decoding library >> perf tools: Add initial entry point for decoder CoreSight traces >> perf tools: Add decoder mechanic to support dumping trace data >> perf tools: Add support for decoding CoreSight trace data >> perf tools: Add functionality to communicate with the openCSD decoder >> pert tools: Add queue management functionality >> perf tools: Add full support for CoreSight trace decoding >> perf tools: Add mechanic to synthesise CoreSight trace packets >> >> Tor Jeremiassen (2): >> perf tools: Add processing of coresight metadata >> MAINTAINERS: Adding entry for CoreSight trace decoding >> >> MAINTAINERS | 3 +- >> tools/build/Makefile.feature | 6 +- >> tools/build/feature/Makefile | 6 +- >> tools/build/feature/test-all.c | 5 + >> tools/build/feature/test-libopencsd.c | 8 + >> tools/perf/Makefile.config | 13 + >> tools/perf/util/Build | 6 + >> tools/perf/util/auxtrace.c | 2 + >> tools/perf/util/cs-etm-decoder/Build | 1 + >> tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 513 ++++++++++++ >> tools/perf/util/cs-etm-decoder/cs-etm-decoder.h | 105 +++ >> tools/perf/util/cs-etm.c | 1023 +++++++++++++++++++++++ >> tools/perf/util/cs-etm.h | 18 + >> 13 files changed, 1705 insertions(+), 4 deletions(-) >> create mode 100644 tools/build/feature/test-libopencsd.c >> create mode 100644 tools/perf/util/cs-etm-decoder/Build >> create mode 100644 tools/perf/util/cs-etm-decoder/cs-etm-decoder.c >> create mode 100644 tools/perf/util/cs-etm-decoder/cs-etm-decoder.h >> create mode 100644 tools/perf/util/cs-etm.c >> >> -- >> 2.7.4 >>
On Fri, 15 Dec 2017 09:44:49 -0700 Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > This patchset adds support for per-thread CoreSight trace decoding from the > "perf report" interface. It is largely modelled on what has been done for > intelPT traces and currently targets the ETMv4 architecture. Support for > cpu-wide scenarios and ETMv3/PTMv1.1 will follow shortly. > > The trace decoding support is done using the Open CoreSight Decoding > Library (openCSD), a stand alone open source project available here [1]. > Integration of the openCSD library with the perf tools follow what has > been done for other support libraries. If the library has been installed > on a system the build scripts will include support for CoreSight trace > decoding: > > ... zlib: [ on ] > ... lzma: [ OFF ] > ... get_cpuid: [ on ] > ... bpf: [ on ] > ... libopencsd: [ on ] <------ > > Instructions on how to build and install the openCSD library are provided > in the HOWTO.md of the project repository. Usually when a perf builder sees something they need "on," they - or, at least I - start querying the host's package manager for something that provides it (e.g., apt search/install libopencsd), but since no distro provides libopencsd, this is bad because it misleads the user. > We elected to keep the decoder > library independent of the kernel tree as it is also used outside of the > perf toolset and various non-linux projects. Where? Not that that won't mean it can't be included in the kernel source tree anyway: Doing so would enable support without burdening the linux perf user with adding any external custom library dependencies. Keeping the library external will also inevitably introduce more source level synchronization problems because the perf sources being built may not be compatible with their version of the library, whether due to new features like new trace hardware support, or API changes. As Mark Brown (cc'd) mentioned on the Coresight mailing list, this may be able to be done the same way the dtc is incorporated into the kernel, where only its relevant sources are included and updated as needed: see linux/scripts/dtc/update-dtc-source.sh. Hopefully the upstream maintainers (acme, etc.) can chime in with their opinion on how to include the library, if at all? It's essentially the same type of code as the Intel-PT decoder code, currently found under tools/perf/util/intel-pt-decoder, except we're trying to keep some sense of connection to the parent project ("Intel (R) Processor Trace Decoder Library" in Intel's case [1]) alive. Thanks, Kim [1] https://github.com/01org/processor-trace
On Wed, Jan 10, 2018 at 06:08:21PM -0600, Kim Phillips wrote: > Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > > Instructions on how to build and install the openCSD library are provided > > in the HOWTO.md of the project repository. > Usually when a perf builder sees something they need "on," they - or, > at least I - start querying the host's package manager for something > that provides it (e.g., apt search/install libopencsd), but since no > distro provides libopencsd, this is bad because it misleads the user. It's on the radar to push this at distros fairly soon. Part of the discussion was wanting to get things to the point where the tools using the library were far enough along that we could be reasonably sure that there weren't any problems that were going to require ABI breaks to fix before pushing the library at distros since ABI churn isn't nice for packagers to deal with. There's also a bit of a chicken and egg problem in that it's a lot easier to get distros to package libraries that have users available (some are not really bothered about this of course but it still helps). > Keeping the library external will also inevitably introduce more > source level synchronization problems because the perf sources being > built may not be compatible with their version of the library, whether > due to new features like new trace hardware support, or API changes. Perf users installing from source rather than from a package (who do tend to the more technical side even for kernel developers) already have to cope with potentially installing at least dwarf, gtk2, libaudit, libbfd, libelf, libnuma, libperl, libpython, libslang, libcrypto, libunwind, libdw-dwarf-unwind, zlib, lzma, bpf and OpenJDK depending on which features they want. I'm not sure that adding one more library is going to be the end of the world here, especially once the packaging starts to filter through distros. Until that happens at least people are no worse off for not having the feature. > As Mark Brown (cc'd) mentioned on the Coresight mailing list, this may > be able to be done the same way the dtc is incorporated into the > kernel, where only its relevant sources are included and updated as > needed: see linux/scripts/dtc/update-dtc-source.sh. Bear in mind that we need dtc for essentially all kernel development on ARM and when it was introduced it was a new requirement for existing systems, it's a bit of a different case here where it's an optional feature in an optional tool.
On 11 January 2018 at 05:23, Mark Brown <broonie@kernel.org> wrote: > On Wed, Jan 10, 2018 at 06:08:21PM -0600, Kim Phillips wrote: >> Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > >> > Instructions on how to build and install the openCSD library are provided >> > in the HOWTO.md of the project repository. > >> Usually when a perf builder sees something they need "on," they - or, >> at least I - start querying the host's package manager for something >> that provides it (e.g., apt search/install libopencsd), but since no >> distro provides libopencsd, this is bad because it misleads the user. > > It's on the radar to push this at distros fairly soon. Part of the > discussion was wanting to get things to the point where the tools using > the library were far enough along that we could be reasonably sure that > there weren't any problems that were going to require ABI breaks to fix > before pushing the library at distros since ABI churn isn't nice for > packagers to deal with. There's also a bit of a chicken and egg problem > in that it's a lot easier to get distros to package libraries that have > users available (some are not really bothered about this of course but > it still helps). Moreover including in the kernel tree every library that can potentially be used by the perf tools simply doesn't scale. The perf tools project has come up with a very cleaver way to deal with external dependencies and I don't see why the OpenCSD library should be different. > >> Keeping the library external will also inevitably introduce more >> source level synchronization problems because the perf sources being >> built may not be compatible with their version of the library, whether >> due to new features like new trace hardware support, or API changes. > > Perf users installing from source rather than from a package (who do > tend to the more technical side even for kernel developers) already have > to cope with potentially installing at least dwarf, gtk2, libaudit, > libbfd, libelf, libnuma, libperl, libpython, libslang, libcrypto, > libunwind, libdw-dwarf-unwind, zlib, lzma, bpf and OpenJDK depending on > which features they want. I'm not sure that adding one more library is > going to be the end of the world here, especially once the packaging > starts to filter through distros. Until that happens at least people > are no worse off for not having the feature. I completely agree. Just like any other package, people that want the very latest code need to install from source. > >> As Mark Brown (cc'd) mentioned on the Coresight mailing list, this may >> be able to be done the same way the dtc is incorporated into the >> kernel, where only its relevant sources are included and updated as >> needed: see linux/scripts/dtc/update-dtc-source.sh. > > Bear in mind that we need dtc for essentially all kernel development on > ARM and when it was introduced it was a new requirement for existing > systems, it's a bit of a different case here where it's an optional > feature in an optional tool.
On Thu, 11 Jan 2018 08:45:21 -0700 Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > On 11 January 2018 at 05:23, Mark Brown <broonie@kernel.org> wrote: > > On Wed, Jan 10, 2018 at 06:08:21PM -0600, Kim Phillips wrote: > >> Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > > > >> > Instructions on how to build and install the openCSD library are provided > >> > in the HOWTO.md of the project repository. > > > >> Usually when a perf builder sees something they need "on," they - or, > >> at least I - start querying the host's package manager for something > >> that provides it (e.g., apt search/install libopencsd), but since no > >> distro provides libopencsd, this is bad because it misleads the user. > > > > It's on the radar to push this at distros fairly soon. Adding packages to distros takes years, this patchset is being submitted for inclusion *now*. So until then, it would greatly facilitate users if the relevant libopencsd source files were self-contained within perf from the get go. > > Part of the > > discussion was wanting to get things to the point where the tools using > > the library were far enough along that we could be reasonably sure that Curious, what other tools are there? > > there weren't any problems that were going to require ABI breaks to fix > > before pushing the library at distros since ABI churn isn't nice for > > packagers to deal with. Why make perf the guinea pig? Whatever, this doesn't preclude adding the code into the tree; it can be removed years from now when libopencsd becomes ubiquitous among distros. > > There's also a bit of a chicken and egg problem > > in that it's a lot easier to get distros to package libraries that have > > users available (some are not really bothered about this of course but > > it still helps). > > Moreover including in the kernel tree every library that can > potentially be used by the perf tools simply doesn't scale. This is a trace decoder library we're talking about: there are no others in perf's system features autodetection list. And why wouldn't adding such libraries scale? > The perf > tools project has come up with a very cleaver way to deal with > external dependencies and I don't see why the OpenCSD library should > be different. Again, the opencsd library is a decoder library: this patchseries adds it as a package dependency (when it isn't even a package in any distro), and it's different in that it's the first decoder library to be submitted as an external dependency (i.e., not fully built-in, like Intel's, or even the Arm SPE's pending submission). > >> Keeping the library external will also inevitably introduce more > >> source level synchronization problems because the perf sources being > >> built may not be compatible with their version of the library, whether > >> due to new features like new trace hardware support, or API changes. > > > > Perf users installing from source rather than from a package (who do > > tend to the more technical side even for kernel developers) already have > > to cope with potentially installing at least dwarf, gtk2, libaudit, > > libbfd, libelf, libnuma, libperl, libpython, libslang, libcrypto, > > libunwind, libdw-dwarf-unwind, zlib, lzma, bpf and OpenJDK depending on > > which features they want. I'm not sure that adding one more library is > > going to be the end of the world here, especially once the packaging > > starts to filter through distros. Until that happens at least people > > are no worse off for not having the feature. > > I completely agree. Just like any other package, people that want the > very latest code need to install from source. A fully-integrated solution would work better for people, e.g., how are people supposed to know what 'latest' is when there are separate, unsynchronized git repos? > >> As Mark Brown (cc'd) mentioned on the Coresight mailing list, this may > >> be able to be done the same way the dtc is incorporated into the > >> kernel, where only its relevant sources are included and updated as > >> needed: see linux/scripts/dtc/update-dtc-source.sh. > > > > Bear in mind that we need dtc for essentially all kernel development on > > ARM and when it was introduced it was a new requirement for existing > > systems, it's a bit of a different case here where it's an optional > > feature in an optional tool. That argument applies to Intel-PT, yet its decoder is self-contained within perf: all non-x86 perf binaries are capable of decoding PT. We'd want that for Arm Coresight where perf gets statically built to run on much more constrained systems like Android. Or are you referring to the higher level linux/scripts/ location of the dtc? That's not my point: the libopencsd sources can live under somewhere like linux/tools/. Kim
On 11 January 2018 at 10:28, Kim Phillips <kim.phillips@arm.com> wrote: > On Thu, 11 Jan 2018 08:45:21 -0700 > Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > >> On 11 January 2018 at 05:23, Mark Brown <broonie@kernel.org> wrote: >> > On Wed, Jan 10, 2018 at 06:08:21PM -0600, Kim Phillips wrote: >> >> Mathieu Poirier <mathieu.poirier@linaro.org> wrote: >> > >> >> > Instructions on how to build and install the openCSD library are provided >> >> > in the HOWTO.md of the project repository. >> > >> >> Usually when a perf builder sees something they need "on," they - or, >> >> at least I - start querying the host's package manager for something >> >> that provides it (e.g., apt search/install libopencsd), but since no >> >> distro provides libopencsd, this is bad because it misleads the user. >> > >> > It's on the radar to push this at distros fairly soon. > > Adding packages to distros takes years, this patchset is being > submitted for inclusion *now*. So until then, it would greatly > facilitate users if the relevant libopencsd source files were > self-contained within perf from the get go. I do not agree with you on the front that it takes years. On the flip side it would take a significant amount of time and effort to refactor the openCSD library so that it can be added to the kernel tree. This patchset is available now with a solution that follows what has already been done for dozens of other external library. There is no point in delaying the inclusion of the functionality when an end-to-end solution exists. > >> > Part of the >> > discussion was wanting to get things to the point where the tools using >> > the library were far enough along that we could be reasonably sure that > > Curious, what other tools are there? Ask around at ARM. > >> > there weren't any problems that were going to require ABI breaks to fix >> > before pushing the library at distros since ABI churn isn't nice for >> > packagers to deal with. > > Why make perf the guinea pig? Whatever, this doesn't preclude > adding the code into the tree; it can be removed years from now when > libopencsd becomes ubiquitous among distros. The same can be said about proceeding the other way around - the openCSD library can be added to the kernel tree later if it is deemed necessary. Until then I really don't see why we'd prevent people from accessing the functionality. > >> > There's also a bit of a chicken and egg problem >> > in that it's a lot easier to get distros to package libraries that have >> > users available (some are not really bothered about this of course but >> > it still helps). >> >> Moreover including in the kernel tree every library that can >> potentially be used by the perf tools simply doesn't scale. > > This is a trace decoder library we're talking about: there are no > others in perf's system features autodetection list. And why wouldn't > adding such libraries scale? I don't see why a decoder library and say, libelf, need to be treated differently. > >> The perf >> tools project has come up with a very cleaver way to deal with >> external dependencies and I don't see why the OpenCSD library should >> be different. > > Again, the opencsd library is a decoder library: this patchseries adds > it as a package dependency (when it isn't even a package in any > distro), and it's different in that it's the first decoder library to > be submitted as an external dependency (i.e., not fully built-in, like > Intel's, or even the Arm SPE's pending submission). I don't see why we absolutely need to do exactly the same as Intel. The library is public and this patchset neatly integrates it with the perf tools. > >> >> Keeping the library external will also inevitably introduce more >> >> source level synchronization problems because the perf sources being >> >> built may not be compatible with their version of the library, whether >> >> due to new features like new trace hardware support, or API changes. >> > >> > Perf users installing from source rather than from a package (who do >> > tend to the more technical side even for kernel developers) already have >> > to cope with potentially installing at least dwarf, gtk2, libaudit, >> > libbfd, libelf, libnuma, libperl, libpython, libslang, libcrypto, >> > libunwind, libdw-dwarf-unwind, zlib, lzma, bpf and OpenJDK depending on >> > which features they want. I'm not sure that adding one more library is >> > going to be the end of the world here, especially once the packaging >> > starts to filter through distros. Until that happens at least people >> > are no worse off for not having the feature. >> >> I completely agree. Just like any other package, people that want the >> very latest code need to install from source. > > A fully-integrated solution would work better for people, e.g., how are > people supposed to know what 'latest' is when there are separate, > unsynchronized git repos? The same applies to any of the other libraries perf is working with. > >> >> As Mark Brown (cc'd) mentioned on the Coresight mailing list, this may >> >> be able to be done the same way the dtc is incorporated into the >> >> kernel, where only its relevant sources are included and updated as >> >> needed: see linux/scripts/dtc/update-dtc-source.sh. >> > >> > Bear in mind that we need dtc for essentially all kernel development on >> > ARM and when it was introduced it was a new requirement for existing >> > systems, it's a bit of a different case here where it's an optional >> > feature in an optional tool. > > That argument applies to Intel-PT, yet its decoder is self-contained > within perf: all non-x86 perf binaries are capable of decoding PT. > We'd want that for Arm Coresight where perf gets statically built to > run on much more constrained systems like Android. Traces can't be decoded properly without the support of external libraries, whether we are talking about PT or CS. > > Or are you referring to the higher level linux/scripts/ location of the > dtc? That's not my point: the libopencsd sources can live under > somewhere like linux/tools/. > > Kim
On Thu, 11 Jan 2018 14:11:00 -0700 Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > On 11 January 2018 at 10:28, Kim Phillips <kim.phillips@arm.com> wrote: > > On Thu, 11 Jan 2018 08:45:21 -0700 > > Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > > > >> On 11 January 2018 at 05:23, Mark Brown <broonie@kernel.org> wrote: > >> > On Wed, Jan 10, 2018 at 06:08:21PM -0600, Kim Phillips wrote: > >> >> Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > >> > > >> >> > Instructions on how to build and install the openCSD library are provided > >> >> > in the HOWTO.md of the project repository. > >> > > >> >> Usually when a perf builder sees something they need "on," they - or, > >> >> at least I - start querying the host's package manager for something > >> >> that provides it (e.g., apt search/install libopencsd), but since no > >> >> distro provides libopencsd, this is bad because it misleads the user. > >> > > >> > It's on the radar to push this at distros fairly soon. > > > > Adding packages to distros takes years, this patchset is being > > submitted for inclusion *now*. So until then, it would greatly > > facilitate users if the relevant libopencsd source files were > > self-contained within perf from the get go. > > I do not agree with you on the front that it takes years. On the flip > side it would take a significant amount of time and effort to refactor > the openCSD library so that it can be added to the kernel tree. This The dtc wasn't refactored before it was added to the kernel tree. > patchset is available now with a solution that follows what has > already been done for dozens of other external library. There is no > point in delaying the inclusion of the functionality when an > end-to-end solution exists. See above: I'm not necessarily suggesting the code get refactored. > >> > Part of the > >> > discussion was wanting to get things to the point where the tools using > >> > the library were far enough along that we could be reasonably sure that > > > > Curious, what other tools are there? > > Ask around at ARM. I'm asking the person that claimed it. > >> > there weren't any problems that were going to require ABI breaks to fix > >> > before pushing the library at distros since ABI churn isn't nice for > >> > packagers to deal with. > > > > Why make perf the guinea pig? Whatever, this doesn't preclude > > adding the code into the tree; it can be removed years from now when > > libopencsd becomes ubiquitous among distros. > > The same can be said about proceeding the other way around - the > openCSD library can be added to the kernel tree later if it is deemed > necessary. Until then I really don't see why we'd prevent people from > accessing the functionality. Again, I'm not suggesting the code be refactored... > >> > There's also a bit of a chicken and egg problem > >> > in that it's a lot easier to get distros to package libraries that have > >> > users available (some are not really bothered about this of course but > >> > it still helps). > >> > >> Moreover including in the kernel tree every library that can > >> potentially be used by the perf tools simply doesn't scale. > > > > This is a trace decoder library we're talking about: there are no > > others in perf's system features autodetection list. And why wouldn't > > adding such libraries scale? > > I don't see why a decoder library and say, libelf, need to be treated > differently. libelf is a mature library based on an industry-wide standard, not to mention already packaged by most (all?) distros. > >> The perf > >> tools project has come up with a very cleaver way to deal with > >> external dependencies and I don't see why the OpenCSD library should > >> be different. > > > > Again, the opencsd library is a decoder library: this patchseries adds > > it as a package dependency (when it isn't even a package in any > > distro), and it's different in that it's the first decoder library to > > be submitted as an external dependency (i.e., not fully built-in, like > > Intel's, or even the Arm SPE's pending submission). > > I don't see why we absolutely need to do exactly the same as Intel. > The library is public and this patchset neatly integrates it with the > perf tools. We don't, but it'd be more efficient, upstream-acceptance-wise, but as you brought up above, we wouldn't be able to since we'd have to rewrite libopencsd to conform to upstream codingstyle, etc., so I'm suggesting we might look at a better enablement strategy like how the dtc works. It'd be nice if the upstream maintainers would comment on what would be acceptable instead of us going back and forth between each other. > >> >> Keeping the library external will also inevitably introduce more > >> >> source level synchronization problems because the perf sources being > >> >> built may not be compatible with their version of the library, whether > >> >> due to new features like new trace hardware support, or API changes. > >> > > >> > Perf users installing from source rather than from a package (who do > >> > tend to the more technical side even for kernel developers) already have > >> > to cope with potentially installing at least dwarf, gtk2, libaudit, > >> > libbfd, libelf, libnuma, libperl, libpython, libslang, libcrypto, > >> > libunwind, libdw-dwarf-unwind, zlib, lzma, bpf and OpenJDK depending on > >> > which features they want. I'm not sure that adding one more library is > >> > going to be the end of the world here, especially once the packaging > >> > starts to filter through distros. Until that happens at least people > >> > are no worse off for not having the feature. > >> > >> I completely agree. Just like any other package, people that want the > >> very latest code need to install from source. > > > > A fully-integrated solution would work better for people, e.g., how are > > people supposed to know what 'latest' is when there are separate, > > unsynchronized git repos? > > The same applies to any of the other libraries perf is working with. The packaged libraries? They are stable: they don't come in the form of cloning a git repo and building from scratch. The decoder libraries? They are self-contained within perf. > >> >> As Mark Brown (cc'd) mentioned on the Coresight mailing list, this may > >> >> be able to be done the same way the dtc is incorporated into the > >> >> kernel, where only its relevant sources are included and updated as > >> >> needed: see linux/scripts/dtc/update-dtc-source.sh. > >> > > >> > Bear in mind that we need dtc for essentially all kernel development on > >> > ARM and when it was introduced it was a new requirement for existing > >> > systems, it's a bit of a different case here where it's an optional > >> > feature in an optional tool. > > > > That argument applies to Intel-PT, yet its decoder is self-contained > > within perf: all non-x86 perf binaries are capable of decoding PT. > > We'd want that for Arm Coresight where perf gets statically built to > > run on much more constrained systems like Android. > > Traces can't be decoded properly without the support of external > libraries, whether we are talking about PT or CS. Not true; perf has PT decoding self-contained. Thanks, Kim
On 11 January 2018 at 14:49, Kim Phillips <kim.phillips@arm.com> wrote: > On Thu, 11 Jan 2018 14:11:00 -0700 > Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > >> On 11 January 2018 at 10:28, Kim Phillips <kim.phillips@arm.com> wrote: >> > On Thu, 11 Jan 2018 08:45:21 -0700 >> > Mathieu Poirier <mathieu.poirier@linaro.org> wrote: >> > >> >> On 11 January 2018 at 05:23, Mark Brown <broonie@kernel.org> wrote: >> >> > On Wed, Jan 10, 2018 at 06:08:21PM -0600, Kim Phillips wrote: >> >> >> Mathieu Poirier <mathieu.poirier@linaro.org> wrote: >> >> > >> >> >> > Instructions on how to build and install the openCSD library are provided >> >> >> > in the HOWTO.md of the project repository. >> >> > >> >> >> Usually when a perf builder sees something they need "on," they - or, >> >> >> at least I - start querying the host's package manager for something >> >> >> that provides it (e.g., apt search/install libopencsd), but since no >> >> >> distro provides libopencsd, this is bad because it misleads the user. >> >> > >> >> > It's on the radar to push this at distros fairly soon. >> > >> > Adding packages to distros takes years, this patchset is being >> > submitted for inclusion *now*. So until then, it would greatly >> > facilitate users if the relevant libopencsd source files were >> > self-contained within perf from the get go. >> >> I do not agree with you on the front that it takes years. On the flip >> side it would take a significant amount of time and effort to refactor >> the openCSD library so that it can be added to the kernel tree. This > > The dtc wasn't refactored before it was added to the kernel tree. > >> patchset is available now with a solution that follows what has >> already been done for dozens of other external library. There is no >> point in delaying the inclusion of the functionality when an >> end-to-end solution exists. > > See above: I'm not necessarily suggesting the code get refactored. > >> >> > Part of the >> >> > discussion was wanting to get things to the point where the tools using >> >> > the library were far enough along that we could be reasonably sure that >> > >> > Curious, what other tools are there? >> >> Ask around at ARM. > > I'm asking the person that claimed it. > >> >> > there weren't any problems that were going to require ABI breaks to fix >> >> > before pushing the library at distros since ABI churn isn't nice for >> >> > packagers to deal with. >> > >> > Why make perf the guinea pig? Whatever, this doesn't preclude >> > adding the code into the tree; it can be removed years from now when >> > libopencsd becomes ubiquitous among distros. >> >> The same can be said about proceeding the other way around - the >> openCSD library can be added to the kernel tree later if it is deemed >> necessary. Until then I really don't see why we'd prevent people from >> accessing the functionality. > > Again, I'm not suggesting the code be refactored... > >> >> > There's also a bit of a chicken and egg problem >> >> > in that it's a lot easier to get distros to package libraries that have >> >> > users available (some are not really bothered about this of course but >> >> > it still helps). >> >> >> >> Moreover including in the kernel tree every library that can >> >> potentially be used by the perf tools simply doesn't scale. >> > >> > This is a trace decoder library we're talking about: there are no >> > others in perf's system features autodetection list. And why wouldn't >> > adding such libraries scale? >> >> I don't see why a decoder library and say, libelf, need to be treated >> differently. > > libelf is a mature library based on an industry-wide standard, not to > mention already packaged by most (all?) distros. > >> >> The perf >> >> tools project has come up with a very cleaver way to deal with >> >> external dependencies and I don't see why the OpenCSD library should >> >> be different. >> > >> > Again, the opencsd library is a decoder library: this patchseries adds >> > it as a package dependency (when it isn't even a package in any >> > distro), and it's different in that it's the first decoder library to >> > be submitted as an external dependency (i.e., not fully built-in, like >> > Intel's, or even the Arm SPE's pending submission). >> >> I don't see why we absolutely need to do exactly the same as Intel. >> The library is public and this patchset neatly integrates it with the >> perf tools. > > We don't, but it'd be more efficient, upstream-acceptance-wise, but as > you brought up above, we wouldn't be able to since we'd have to rewrite > libopencsd to conform to upstream codingstyle, etc., so I'm suggesting > we might look at a better enablement strategy like how the dtc works. > > It'd be nice if the upstream maintainers would comment on what would be > acceptable instead of us going back and forth between each other. Agreed. > >> >> >> Keeping the library external will also inevitably introduce more >> >> >> source level synchronization problems because the perf sources being >> >> >> built may not be compatible with their version of the library, whether >> >> >> due to new features like new trace hardware support, or API changes. >> >> > >> >> > Perf users installing from source rather than from a package (who do >> >> > tend to the more technical side even for kernel developers) already have >> >> > to cope with potentially installing at least dwarf, gtk2, libaudit, >> >> > libbfd, libelf, libnuma, libperl, libpython, libslang, libcrypto, >> >> > libunwind, libdw-dwarf-unwind, zlib, lzma, bpf and OpenJDK depending on >> >> > which features they want. I'm not sure that adding one more library is >> >> > going to be the end of the world here, especially once the packaging >> >> > starts to filter through distros. Until that happens at least people >> >> > are no worse off for not having the feature. >> >> >> >> I completely agree. Just like any other package, people that want the >> >> very latest code need to install from source. >> > >> > A fully-integrated solution would work better for people, e.g., how are >> > people supposed to know what 'latest' is when there are separate, >> > unsynchronized git repos? >> >> The same applies to any of the other libraries perf is working with. > > The packaged libraries? They are stable: they don't come in the form > of cloning a git repo and building from scratch. > > The decoder libraries? They are self-contained within perf. > >> >> >> As Mark Brown (cc'd) mentioned on the Coresight mailing list, this may >> >> >> be able to be done the same way the dtc is incorporated into the >> >> >> kernel, where only its relevant sources are included and updated as >> >> >> needed: see linux/scripts/dtc/update-dtc-source.sh. >> >> > >> >> > Bear in mind that we need dtc for essentially all kernel development on >> >> > ARM and when it was introduced it was a new requirement for existing >> >> > systems, it's a bit of a different case here where it's an optional >> >> > feature in an optional tool. >> > >> > That argument applies to Intel-PT, yet its decoder is self-contained >> > within perf: all non-x86 perf binaries are capable of decoding PT. >> > We'd want that for Arm Coresight where perf gets statically built to >> > run on much more constrained systems like Android. >> >> Traces can't be decoded properly without the support of external >> libraries, whether we are talking about PT or CS. > > Not true; perf has PT decoding self-contained. > > Thanks, > > Kim
Hi, On 11 January 2018 at 22:18, Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > On 11 January 2018 at 14:49, Kim Phillips <kim.phillips@arm.com> wrote: >> On Thu, 11 Jan 2018 14:11:00 -0700 >> Mathieu Poirier <mathieu.poirier@linaro.org> wrote: >> >>> On 11 January 2018 at 10:28, Kim Phillips <kim.phillips@arm.com> wrote: >>> > On Thu, 11 Jan 2018 08:45:21 -0700 >>> > Mathieu Poirier <mathieu.poirier@linaro.org> wrote: >>> > >>> >> On 11 January 2018 at 05:23, Mark Brown <broonie@kernel.org> wrote: >>> >> > On Wed, Jan 10, 2018 at 06:08:21PM -0600, Kim Phillips wrote: >>> >> >> Mathieu Poirier <mathieu.poirier@linaro.org> wrote: >>> >> > >>> >> >> > Instructions on how to build and install the openCSD library are provided >>> >> >> > in the HOWTO.md of the project repository. >>> >> > >>> >> >> Usually when a perf builder sees something they need "on," they - or, >>> >> >> at least I - start querying the host's package manager for something >>> >> >> that provides it (e.g., apt search/install libopencsd), but since no >>> >> >> distro provides libopencsd, this is bad because it misleads the user. >>> >> > >>> >> > It's on the radar to push this at distros fairly soon. >>> > >>> > Adding packages to distros takes years, this patchset is being >>> > submitted for inclusion *now*. So until then, it would greatly >>> > facilitate users if the relevant libopencsd source files were >>> > self-contained within perf from the get go. >>> >>> I do not agree with you on the front that it takes years. On the flip >>> side it would take a significant amount of time and effort to refactor >>> the openCSD library so that it can be added to the kernel tree. This >> >> The dtc wasn't refactored before it was added to the kernel tree. The openCSD library consists of c. 140 C++ source and header files, plus a few headers to form the libraries C-API, formatted in what is an accepted standard within ARM - indent of 4 spaces and no tabs. Now, afaik, this is unlikely to be acceptable to the kernel tree. The decision to use C++ was made at the start of this project - the objective was to produce a standalone, open source, reference trace decoder library for CoreSight, to be used by any tooling that wanted it, both open source and proprietary, for linux and windows OS, on x86 and native ARM, plus baremetal on ARM. The majority of stakeholders involved at the start of the project expressed a preference for C++ - in part as a significant amount of the code donated to the project was already C++. The C-API was agreed as an addition to enable integration with projects that preferred / required that - perf being one example. Unlike intel PT, that has a single protocol, tightly bound to the core it is run on, (similar to SPE in this respect), the library supports multiple trace protocols, which are configured and controlled independently of the cores that generate them. The library supports all extant protocols - ETMv3 & 4, PTM and STM, plus the ability to de-multiplex individual trace streams from the CS trace frames, decoding these into a common generic standard, removing the need for tooling to aware of the underlying protocol. Furthermore, given that CoreSight hardware can support proprietary trace protocols (e.g. DSPs) being added to and captured in the standard trace streams, the library supports facilities to plug in additional external decoders.The library also provides ancilliary tooling support - e.g. string representations of protocol packets. The library currently supports ETMv4.1 -> we have a requirement to support v4.3 once these cores begin to appear, and it is inevitable that the trace protocols will develop as the ARM architecture develops. Adding the decoder library to the kernel tree will result in code churn that is unnecessary for the kernel tree as the library is developed, especially if functionality is added that is not used by A class cores (see comment below re data trace). >> >>> patchset is available now with a solution that follows what has >>> already been done for dozens of other external library. There is no >>> point in delaying the inclusion of the functionality when an >>> end-to-end solution exists. >> >> See above: I'm not necessarily suggesting the code get refactored. >> >>> >> > Part of the >>> >> > discussion was wanting to get things to the point where the tools using >>> >> > the library were far enough along that we could be reasonably sure that >>> > >>> > Curious, what other tools are there? >>> >>> Ask around at ARM. >> >> I'm asking the person that claimed it. >> The intent is to use openCSD as a decoder within existing ARM tools - though the library will need to be extended to decode Data trace before this is possible. (ARM A class processors are instruction trace only. ) Additionally the library test program itself is in fact a useful tool for investigating trace capture / corruption / setup errors in hardware. It is capable of listing trace packets from snapshots captured by DS-5 or the open source CoreSight Access Library. >>> >> > there weren't any problems that were going to require ABI breaks to fix >>> >> > before pushing the library at distros since ABI churn isn't nice for >>> >> > packagers to deal with. >>> > >>> > Why make perf the guinea pig? Whatever, this doesn't preclude >>> > adding the code into the tree; it can be removed years from now when >>> > libopencsd becomes ubiquitous among distros. >>> >>> The same can be said about proceeding the other way around - the >>> openCSD library can be added to the kernel tree later if it is deemed >>> necessary. Until then I really don't see why we'd prevent people from >>> accessing the functionality. >> >> Again, I'm not suggesting the code be refactored... >> >>> >> > There's also a bit of a chicken and egg problem >>> >> > in that it's a lot easier to get distros to package libraries that have >>> >> > users available (some are not really bothered about this of course but >>> >> > it still helps). >>> >> >>> >> Moreover including in the kernel tree every library that can >>> >> potentially be used by the perf tools simply doesn't scale. >>> > >>> > This is a trace decoder library we're talking about: there are no >>> > others in perf's system features autodetection list. And why wouldn't >>> > adding such libraries scale? >>> >>> I don't see why a decoder library and say, libelf, need to be treated >>> differently. >> >> libelf is a mature library based on an industry-wide standard, not to >> mention already packaged by most (all?) distros. >> We re-factored the library name and the layout of the C-API necessary header files in the latest release (v0.8) to allow for installation into a linux system and the build changes made to perf. Now, while it is true that we need to be careful that users are aware they need to use the latest release, in general if the library is not present on a system; if is not part of the distribution, it is very easy to find using a simple search. Frankly whenever I appear to be missing a library in linux the first thing I do is a web search, to figure out what package I am missing (since it is not always obvious to me what libraries lie in which packages). Google 'libopencsd' and you see the github HOWTO.md for OpenCSD very quickly. I think users interested in using the library will have the know-how to find an install it if the perf build process marks it as missing. The HOWTO will be maintained as part of the library to explain the installation and integration with perf. Given all the above, I cannot see any engineering benefit from adding the source to the kernel tree. We have a stable and maintainable solution with the library added to the perf build as an external dependency. Regards Mike >>> >> The perf >>> >> tools project has come up with a very cleaver way to deal with >>> >> external dependencies and I don't see why the OpenCSD library should >>> >> be different. >>> > >>> > Again, the opencsd library is a decoder library: this patchseries adds >>> > it as a package dependency (when it isn't even a package in any >>> > distro), and it's different in that it's the first decoder library to >>> > be submitted as an external dependency (i.e., not fully built-in, like >>> > Intel's, or even the Arm SPE's pending submission). >>> >>> I don't see why we absolutely need to do exactly the same as Intel. >>> The library is public and this patchset neatly integrates it with the >>> perf tools. >> >> We don't, but it'd be more efficient, upstream-acceptance-wise, but as >> you brought up above, we wouldn't be able to since we'd have to rewrite >> libopencsd to conform to upstream codingstyle, etc., so I'm suggesting >> we might look at a better enablement strategy like how the dtc works. >> >> It'd be nice if the upstream maintainers would comment on what would be >> acceptable instead of us going back and forth between each other. > > Agreed. > >> >>> >> >> Keeping the library external will also inevitably introduce more >>> >> >> source level synchronization problems because the perf sources being >>> >> >> built may not be compatible with their version of the library, whether >>> >> >> due to new features like new trace hardware support, or API changes. >>> >> > >>> >> > Perf users installing from source rather than from a package (who do >>> >> > tend to the more technical side even for kernel developers) already have >>> >> > to cope with potentially installing at least dwarf, gtk2, libaudit, >>> >> > libbfd, libelf, libnuma, libperl, libpython, libslang, libcrypto, >>> >> > libunwind, libdw-dwarf-unwind, zlib, lzma, bpf and OpenJDK depending on >>> >> > which features they want. I'm not sure that adding one more library is >>> >> > going to be the end of the world here, especially once the packaging >>> >> > starts to filter through distros. Until that happens at least people >>> >> > are no worse off for not having the feature. >>> >> >>> >> I completely agree. Just like any other package, people that want the >>> >> very latest code need to install from source. >>> > >>> > A fully-integrated solution would work better for people, e.g., how are >>> > people supposed to know what 'latest' is when there are separate, >>> > unsynchronized git repos? >>> >>> The same applies to any of the other libraries perf is working with. >> >> The packaged libraries? They are stable: they don't come in the form >> of cloning a git repo and building from scratch. >> >> The decoder libraries? They are self-contained within perf. >> >>> >> >> As Mark Brown (cc'd) mentioned on the Coresight mailing list, this may >>> >> >> be able to be done the same way the dtc is incorporated into the >>> >> >> kernel, where only its relevant sources are included and updated as >>> >> >> needed: see linux/scripts/dtc/update-dtc-source.sh. >>> >> > >>> >> > Bear in mind that we need dtc for essentially all kernel development on >>> >> > ARM and when it was introduced it was a new requirement for existing >>> >> > systems, it's a bit of a different case here where it's an optional >>> >> > feature in an optional tool. >>> > >>> > That argument applies to Intel-PT, yet its decoder is self-contained >>> > within perf: all non-x86 perf binaries are capable of decoding PT. >>> > We'd want that for Arm Coresight where perf gets statically built to >>> > run on much more constrained systems like Android. >>> >>> Traces can't be decoded properly without the support of external >>> libraries, whether we are talking about PT or CS. >> >> Not true; perf has PT decoding self-contained. >> >> Thanks, >> >> Kim > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- Mike Leach Principal Engineer, ARM Ltd. Blackburn Design Centre. UK
On Tue, 16 Jan 2018 12:35:26 +0000 Mike Leach <mike.leach@linaro.org> wrote: > Hi, > > On 11 January 2018 at 22:18, Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > > On 11 January 2018 at 14:49, Kim Phillips <kim.phillips@arm.com> wrote: > >> On Thu, 11 Jan 2018 14:11:00 -0700 > >> Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > >> > >>> On 11 January 2018 at 10:28, Kim Phillips <kim.phillips@arm.com> wrote: > >>> > On Thu, 11 Jan 2018 08:45:21 -0700 > >>> > Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > >>> > > >>> >> On 11 January 2018 at 05:23, Mark Brown <broonie@kernel.org> wrote: > >>> >> > On Wed, Jan 10, 2018 at 06:08:21PM -0600, Kim Phillips wrote: > >>> >> >> Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > >>> >> > > >>> >> >> > Instructions on how to build and install the openCSD library are provided > >>> >> >> > in the HOWTO.md of the project repository. > >>> >> > > >>> >> >> Usually when a perf builder sees something they need "on," they - or, > >>> >> >> at least I - start querying the host's package manager for something > >>> >> >> that provides it (e.g., apt search/install libopencsd), but since no > >>> >> >> distro provides libopencsd, this is bad because it misleads the user. > >>> >> > > >>> >> > It's on the radar to push this at distros fairly soon. > >>> > > >>> > Adding packages to distros takes years, this patchset is being > >>> > submitted for inclusion *now*. So until then, it would greatly > >>> > facilitate users if the relevant libopencsd source files were > >>> > self-contained within perf from the get go. > >>> > >>> I do not agree with you on the front that it takes years. On the flip > >>> side it would take a significant amount of time and effort to refactor > >>> the openCSD library so that it can be added to the kernel tree. This > >> > >> The dtc wasn't refactored before it was added to the kernel tree. > > The openCSD library consists of c. 140 C++ source and header files, > plus a few headers to form the libraries C-API, formatted in what is > an accepted standard within ARM - indent of 4 spaces and no tabs. > Now, afaik, this is unlikely to be acceptable to the kernel tree. like I said, the refactoring (restyling?) shouldn't have to be necessary, as was with the dtc. Plus, only the necessary files would be imported by the update script: a figure an order of magnitude less than 140. > The decision to use C++ was made at the start of this project - the I don't think that matters either. > Unlike intel PT, that has a single protocol, tightly bound to the core > it is run on, (similar to SPE in this respect), the library supports > multiple trace protocols, which are configured and controlled > independently of the cores that generate them. The library supports > all extant protocols - ETMv3 & 4, PTM and STM, plus the ability to > de-multiplex individual trace streams from the CS trace frames, > decoding these into a common generic standard, removing the need for > tooling to aware of the underlying protocol. Furthermore, given that So if we limited the first submission to just the relevant ETMv4 files imported, we'd start off in the same place as Intel PT: a single protocol. Not that it's a crime to extend it later to newer versions of the ETM protocol, and possibly backport the older stuff at a later date. The object here is to enable the current-generation ETM user wrt perf decoding as seamlessly and as easily as possible. > CoreSight hardware can support proprietary trace protocols (e.g. DSPs) Likewise, we're not concerned about these cases in the first submission of the ETM decoder patch to perf. > The library currently supports ETMv4.1 -> we have a requirement to > support v4.3 once these cores begin to appear, and it is inevitable > that the trace protocols will develop as the ARM architecture > develops. Adding the decoder library to the kernel tree will result in > code churn that is unnecessary for the kernel tree as the library is > developed, especially if functionality is added that is not used by A > class cores (see comment below re data trace). Generally, if new hardware needs to be supported, the kernel tree will accept patches to support it. During development, the developers are free to maintain public git repos with their work-in-progress, as usual, for those seeking early access to the new feature. > >>> patchset is available now with a solution that follows what has > >>> already been done for dozens of other external library. There is no > >>> point in delaying the inclusion of the functionality when an > >>> end-to-end solution exists. > >> > >> See above: I'm not necessarily suggesting the code get refactored. > >> > >>> >> > Part of the > >>> >> > discussion was wanting to get things to the point where the tools using > >>> >> > the library were far enough along that we could be reasonably sure that > >>> > > >>> > Curious, what other tools are there? > >>> > >>> Ask around at ARM. > >> > >> I'm asking the person that claimed it. > > The intent is to use openCSD as a decoder within existing ARM tools - > though the library will need to be extended to decode Data trace > before this is possible. (ARM A class processors are instruction trace > only. ) So there are no other users in the linux ecosystem, meaning nothing is gained by packaging it in distros, and all the more reason to enable it by directly importing it into perf, so users get all the benefit without the wait and without the hassle of having to manually install the library and worry about compatibility between perf. > Additionally the library test program itself is in fact a useful tool > for investigating trace capture / corruption / setup errors in > hardware. It is capable of listing trace packets from snapshots > captured by DS-5 or the open source CoreSight Access Library. Like I said above, the libopencsd files perf doesn't need don't need to be imported by the update script. > >>> >> > there weren't any problems that were going to require ABI breaks to fix > >>> >> > before pushing the library at distros since ABI churn isn't nice for > >>> >> > packagers to deal with. > >>> > > >>> > Why make perf the guinea pig? Whatever, this doesn't preclude > >>> > adding the code into the tree; it can be removed years from now when > >>> > libopencsd becomes ubiquitous among distros. > >>> > >>> The same can be said about proceeding the other way around - the > >>> openCSD library can be added to the kernel tree later if it is deemed > >>> necessary. Until then I really don't see why we'd prevent people from > >>> accessing the functionality. > >> > >> Again, I'm not suggesting the code be refactored... > >> > >>> >> > There's also a bit of a chicken and egg problem > >>> >> > in that it's a lot easier to get distros to package libraries that have > >>> >> > users available (some are not really bothered about this of course but > >>> >> > it still helps). > >>> >> > >>> >> Moreover including in the kernel tree every library that can > >>> >> potentially be used by the perf tools simply doesn't scale. > >>> > > >>> > This is a trace decoder library we're talking about: there are no > >>> > others in perf's system features autodetection list. And why wouldn't > >>> > adding such libraries scale? > >>> > >>> I don't see why a decoder library and say, libelf, need to be treated > >>> differently. > >> > >> libelf is a mature library based on an industry-wide standard, not to > >> mention already packaged by most (all?) distros. > > We re-factored the library name and the layout of the C-API necessary > header files in the latest release (v0.8) to allow for installation > into a linux system and the build changes made to perf. > Now, while it is true that we need to be careful that users are aware > they need to use the latest release, in general if the library is not > present on a system; if is not part of the distribution, it is very > easy to find using a simple search. The way it's being done in this patchseries, users are going to query their package managers for libopencsd and not find it: very misleading. To avoid having to manually install the library and inevitably have it go out of sync with perf, it'd behoove us to include the sources directly as of now. > Frankly whenever I appear to be > missing a library in linux the first thing I do is a web search, to > figure out what package I am missing (since it is not always obvious > to me what libraries lie in which packages). > > Google 'libopencsd' and you see the github HOWTO.md for OpenCSD very quickly. > > I think users interested in using the library will have the know-how > to find an install it if the perf build process marks it as missing. > The HOWTO will be maintained as part of the library to explain the > installation and integration with perf. We can avoid all of this. > Given all the above, I cannot see any engineering benefit from adding > the source to the kernel tree. We have a stable and maintainable > solution with the library added to the perf build as an external > dependency. That's not true: the perf and libopencsd sources can get out of sync, whereas if the perf-dependent files of libopencsd were maintained along with the perf source itself, that would not be possible. Kim
On Tue, Jan 16, 2018 at 11:01:02AM -0600, Kim Phillips wrote: > On Tue, 16 Jan 2018 12:35:26 +0000 > Mike Leach <mike.leach@linaro.org> wrote: > > > Hi, > > > > On 11 January 2018 at 22:18, Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > > > On 11 January 2018 at 14:49, Kim Phillips <kim.phillips@arm.com> wrote: > > >> On Thu, 11 Jan 2018 14:11:00 -0700 > > >> Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > > >> > > >>> On 11 January 2018 at 10:28, Kim Phillips <kim.phillips@arm.com> wrote: > > >>> > On Thu, 11 Jan 2018 08:45:21 -0700 > > >>> > Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > > >>> > > > >>> >> On 11 January 2018 at 05:23, Mark Brown <broonie@kernel.org> wrote: > > >>> >> > On Wed, Jan 10, 2018 at 06:08:21PM -0600, Kim Phillips wrote: > > >>> >> >> Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > > >>> >> > > > >>> >> >> > Instructions on how to build and install the openCSD library are provided > > >>> >> >> > in the HOWTO.md of the project repository. > > >>> >> > > > >>> >> >> Usually when a perf builder sees something they need "on," they - or, > > >>> >> >> at least I - start querying the host's package manager for something > > >>> >> >> that provides it (e.g., apt search/install libopencsd), but since no > > >>> >> >> distro provides libopencsd, this is bad because it misleads the user. > > >>> >> > > > >>> >> > It's on the radar to push this at distros fairly soon. > > >>> > > > >>> > Adding packages to distros takes years, this patchset is being > > >>> > submitted for inclusion *now*. So until then, it would greatly > > >>> > facilitate users if the relevant libopencsd source files were > > >>> > self-contained within perf from the get go. > > >>> > > >>> I do not agree with you on the front that it takes years. On the flip > > >>> side it would take a significant amount of time and effort to refactor > > >>> the openCSD library so that it can be added to the kernel tree. This > > >> > > >> The dtc wasn't refactored before it was added to the kernel tree. > > > > The openCSD library consists of c. 140 C++ source and header files, > > plus a few headers to form the libraries C-API, formatted in what is > > an accepted standard within ARM - indent of 4 spaces and no tabs. > > Now, afaik, this is unlikely to be acceptable to the kernel tree. > > like I said, the refactoring (restyling?) shouldn't have to be > necessary, as was with the dtc. Plus, only the necessary files would > be imported by the update script: a figure an order of magnitude less > than 140. Again, we don't need to do all that if we keep the library separate. > > > The decision to use C++ was made at the start of this project - the > > I don't think that matters either. > > > Unlike intel PT, that has a single protocol, tightly bound to the core > > it is run on, (similar to SPE in this respect), the library supports > > multiple trace protocols, which are configured and controlled > > independently of the cores that generate them. The library supports > > all extant protocols - ETMv3 & 4, PTM and STM, plus the ability to > > de-multiplex individual trace streams from the CS trace frames, > > decoding these into a common generic standard, removing the need for > > tooling to aware of the underlying protocol. Furthermore, given that > > So if we limited the first submission to just the relevant ETMv4 files > imported, we'd start off in the same place as Intel PT: a single > protocol. Not that it's a crime to extend it later to newer versions > of the ETM protocol, and possibly backport the older stuff at a later > date. The object here is to enable the current-generation ETM user wrt > perf decoding as seamlessly and as easily as possible. > > > CoreSight hardware can support proprietary trace protocols (e.g. DSPs) > > Likewise, we're not concerned about these cases in the first submission > of the ETM decoder patch to perf. It would be extremely time consuming to start splitting the library by functionality and then, incrementally start (re)adding the features. I rather spend that time working on new functionality and making the CoreSight subsystem better. > > > The library currently supports ETMv4.1 -> we have a requirement to > > support v4.3 once these cores begin to appear, and it is inevitable > > that the trace protocols will develop as the ARM architecture > > develops. Adding the decoder library to the kernel tree will result in > > code churn that is unnecessary for the kernel tree as the library is > > developed, especially if functionality is added that is not used by A > > class cores (see comment below re data trace). > > Generally, if new hardware needs to be supported, the kernel tree will > accept patches to support it. > > During development, the developers are free to maintain public git > repos with their work-in-progress, as usual, for those seeking early > access to the new feature. > > > >>> patchset is available now with a solution that follows what has > > >>> already been done for dozens of other external library. There is no > > >>> point in delaying the inclusion of the functionality when an > > >>> end-to-end solution exists. > > >> > > >> See above: I'm not necessarily suggesting the code get refactored. > > >> > > >>> >> > Part of the > > >>> >> > discussion was wanting to get things to the point where the tools using > > >>> >> > the library were far enough along that we could be reasonably sure that > > >>> > > > >>> > Curious, what other tools are there? > > >>> > > >>> Ask around at ARM. > > >> > > >> I'm asking the person that claimed it. > > > > The intent is to use openCSD as a decoder within existing ARM tools - > > though the library will need to be extended to decode Data trace > > before this is possible. (ARM A class processors are instruction trace > > only. ) > > So there are no other users in the linux ecosystem, meaning nothing > is gained by packaging it in distros, and all the more reason to enable > it by directly importing it into perf, so users get all the benefit > without the wait and without the hassle of having to manually install > the library and worry about compatibility between perf. Once again I think you're blowing the down side out of proportion. Like Mark Brown said we have to start somewhere. Once we have this in work on proper packaging will start immediately. > > > Additionally the library test program itself is in fact a useful tool > > for investigating trace capture / corruption / setup errors in > > hardware. It is capable of listing trace packets from snapshots > > captured by DS-5 or the open source CoreSight Access Library. > > Like I said above, the libopencsd files perf doesn't need don't need to > be imported by the update script. > > > >>> >> > there weren't any problems that were going to require ABI breaks to fix > > >>> >> > before pushing the library at distros since ABI churn isn't nice for > > >>> >> > packagers to deal with. > > >>> > > > >>> > Why make perf the guinea pig? Whatever, this doesn't preclude > > >>> > adding the code into the tree; it can be removed years from now when > > >>> > libopencsd becomes ubiquitous among distros. > > >>> > > >>> The same can be said about proceeding the other way around - the > > >>> openCSD library can be added to the kernel tree later if it is deemed > > >>> necessary. Until then I really don't see why we'd prevent people from > > >>> accessing the functionality. > > >> > > >> Again, I'm not suggesting the code be refactored... > > >> > > >>> >> > There's also a bit of a chicken and egg problem > > >>> >> > in that it's a lot easier to get distros to package libraries that have > > >>> >> > users available (some are not really bothered about this of course but > > >>> >> > it still helps). > > >>> >> > > >>> >> Moreover including in the kernel tree every library that can > > >>> >> potentially be used by the perf tools simply doesn't scale. > > >>> > > > >>> > This is a trace decoder library we're talking about: there are no > > >>> > others in perf's system features autodetection list. And why wouldn't > > >>> > adding such libraries scale? > > >>> > > >>> I don't see why a decoder library and say, libelf, need to be treated > > >>> differently. > > >> > > >> libelf is a mature library based on an industry-wide standard, not to > > >> mention already packaged by most (all?) distros. > > > > We re-factored the library name and the layout of the C-API necessary > > header files in the latest release (v0.8) to allow for installation > > into a linux system and the build changes made to perf. > > Now, while it is true that we need to be careful that users are aware > > they need to use the latest release, in general if the library is not > > present on a system; if is not part of the distribution, it is very > > easy to find using a simple search. > > The way it's being done in this patchseries, users are going to query > their package managers for libopencsd and not find it: very > misleading. To avoid having to manually install the library and > inevitably have it go out of sync with perf, it'd behoove us to > include the sources directly as of now. There's nothing going out of sync - until we have proper packaging people simply need to recompile their perf utility. As pointed out before that wouldn't be hard to do for people using hardware assisted tracing. > > > Frankly whenever I appear to be > > missing a library in linux the first thing I do is a web search, to > > figure out what package I am missing (since it is not always obvious > > to me what libraries lie in which packages). > > > > Google 'libopencsd' and you see the github HOWTO.md for OpenCSD very quickly. > > > > I think users interested in using the library will have the know-how > > to find an install it if the perf build process marks it as missing. > > The HOWTO will be maintained as part of the library to explain the > > installation and integration with perf. > > We can avoid all of this. > > > Given all the above, I cannot see any engineering benefit from adding > > the source to the kernel tree. We have a stable and maintainable > > solution with the library added to the perf build as an external > > dependency. > > That's not true: the perf and libopencsd sources can get out of sync, > whereas if the perf-dependent files of libopencsd were maintained along > with the perf source itself, that would not be possible. Not true: kernel subsystem get out of sync regularly, this wouldn't be any different. > > Kim
On Tue, 16 Jan 2018 10:58:06 -0700 Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > On Tue, Jan 16, 2018 at 11:01:02AM -0600, Kim Phillips wrote: > > On Tue, 16 Jan 2018 12:35:26 +0000 > > Mike Leach <mike.leach@linaro.org> wrote: > > > > > Hi, > > > > > > On 11 January 2018 at 22:18, Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > > > > On 11 January 2018 at 14:49, Kim Phillips <kim.phillips@arm.com> wrote: > > > >> On Thu, 11 Jan 2018 14:11:00 -0700 > > > >> Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > > > >> > > > >>> On 11 January 2018 at 10:28, Kim Phillips <kim.phillips@arm.com> wrote: > > > >>> > On Thu, 11 Jan 2018 08:45:21 -0700 > > > >>> > Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > > > >>> > > > > >>> >> On 11 January 2018 at 05:23, Mark Brown <broonie@kernel.org> wrote: > > > >>> >> > On Wed, Jan 10, 2018 at 06:08:21PM -0600, Kim Phillips wrote: > > > >>> >> >> Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > > > >>> >> > > > > >>> >> >> > Instructions on how to build and install the openCSD library are provided > > > >>> >> >> > in the HOWTO.md of the project repository. > > > >>> >> > > > > >>> >> >> Usually when a perf builder sees something they need "on," they - or, > > > >>> >> >> at least I - start querying the host's package manager for something > > > >>> >> >> that provides it (e.g., apt search/install libopencsd), but since no > > > >>> >> >> distro provides libopencsd, this is bad because it misleads the user. > > > >>> >> > > > > >>> >> > It's on the radar to push this at distros fairly soon. > > > >>> > > > > >>> > Adding packages to distros takes years, this patchset is being > > > >>> > submitted for inclusion *now*. So until then, it would greatly > > > >>> > facilitate users if the relevant libopencsd source files were > > > >>> > self-contained within perf from the get go. > > > >>> > > > >>> I do not agree with you on the front that it takes years. On the flip > > > >>> side it would take a significant amount of time and effort to refactor > > > >>> the openCSD library so that it can be added to the kernel tree. This > > > >> > > > >> The dtc wasn't refactored before it was added to the kernel tree. > > > > > > The openCSD library consists of c. 140 C++ source and header files, > > > plus a few headers to form the libraries C-API, formatted in what is > > > an accepted standard within ARM - indent of 4 spaces and no tabs. > > > Now, afaik, this is unlikely to be acceptable to the kernel tree. > > > > like I said, the refactoring (restyling?) shouldn't have to be > > necessary, as was with the dtc. Plus, only the necessary files would > > be imported by the update script: a figure an order of magnitude less > > than 140. > > Again, we don't need to do all that if we keep the library separate. Do all what? I'm saying no refactoring/restyling should be necessary, even only for the files relevant to perf. If you're talking about writing the update script, the dtc example can be used as a base, and I'd rather we do it up front so our users don't have to deal with manually installing libopencsd and not knowing when to update it. > > > The decision to use C++ was made at the start of this project - the > > > > I don't think that matters either. > > > > > Unlike intel PT, that has a single protocol, tightly bound to the core > > > it is run on, (similar to SPE in this respect), the library supports > > > multiple trace protocols, which are configured and controlled > > > independently of the cores that generate them. The library supports > > > all extant protocols - ETMv3 & 4, PTM and STM, plus the ability to > > > de-multiplex individual trace streams from the CS trace frames, > > > decoding these into a common generic standard, removing the need for > > > tooling to aware of the underlying protocol. Furthermore, given that > > > > So if we limited the first submission to just the relevant ETMv4 files > > imported, we'd start off in the same place as Intel PT: a single > > protocol. Not that it's a crime to extend it later to newer versions > > of the ETM protocol, and possibly backport the older stuff at a later > > date. The object here is to enable the current-generation ETM user wrt > > perf decoding as seamlessly and as easily as possible. > > > > > CoreSight hardware can support proprietary trace protocols (e.g. DSPs) > > > > Likewise, we're not concerned about these cases in the first submission > > of the ETM decoder patch to perf. > > It would be extremely time consuming to start splitting the library by > functionality and then, incrementally start (re)adding the features. I rather > spend that time working on new functionality and making the CoreSight subsystem > better. The library is already organized by ETM h/w version for the most part, so we'd add the ETMv4 files to the update script first, then the others - older or newer - as they get successfully tested, and at the tester's leisure: all this requires is a dtc update script and initial test on ETMv4 data, and avoids users from having to deal with misleading behaviour when building the coresight subsytem. > > > The library currently supports ETMv4.1 -> we have a requirement to > > > support v4.3 once these cores begin to appear, and it is inevitable > > > that the trace protocols will develop as the ARM architecture > > > develops. Adding the decoder library to the kernel tree will result in > > > code churn that is unnecessary for the kernel tree as the library is > > > developed, especially if functionality is added that is not used by A > > > class cores (see comment below re data trace). > > > > Generally, if new hardware needs to be supported, the kernel tree will > > accept patches to support it. > > > > During development, the developers are free to maintain public git > > repos with their work-in-progress, as usual, for those seeking early > > access to the new feature. > > > > > >>> patchset is available now with a solution that follows what has > > > >>> already been done for dozens of other external library. There is no > > > >>> point in delaying the inclusion of the functionality when an > > > >>> end-to-end solution exists. > > > >> > > > >> See above: I'm not necessarily suggesting the code get refactored. > > > >> > > > >>> >> > Part of the > > > >>> >> > discussion was wanting to get things to the point where the tools using > > > >>> >> > the library were far enough along that we could be reasonably sure that > > > >>> > > > > >>> > Curious, what other tools are there? > > > >>> > > > >>> Ask around at ARM. > > > >> > > > >> I'm asking the person that claimed it. > > > > > > The intent is to use openCSD as a decoder within existing ARM tools - > > > though the library will need to be extended to decode Data trace > > > before this is possible. (ARM A class processors are instruction trace > > > only. ) > > > > So there are no other users in the linux ecosystem, meaning nothing > > is gained by packaging it in distros, and all the more reason to enable > > it by directly importing it into perf, so users get all the benefit > > without the wait and without the hassle of having to manually install > > the library and worry about compatibility between perf. > > Once again I think you're blowing the down side out of proportion. Like Mark > Brown said we have to start somewhere. Once we have this in work on proper > packaging will start immediately. Why package libopencsd at all, when only perf is using it? > > > Additionally the library test program itself is in fact a useful tool > > > for investigating trace capture / corruption / setup errors in > > > hardware. It is capable of listing trace packets from snapshots > > > captured by DS-5 or the open source CoreSight Access Library. > > > > Like I said above, the libopencsd files perf doesn't need don't need to > > be imported by the update script. > > > > > >>> >> > there weren't any problems that were going to require ABI breaks to fix > > > >>> >> > before pushing the library at distros since ABI churn isn't nice for > > > >>> >> > packagers to deal with. > > > >>> > > > > >>> > Why make perf the guinea pig? Whatever, this doesn't preclude > > > >>> > adding the code into the tree; it can be removed years from now when > > > >>> > libopencsd becomes ubiquitous among distros. > > > >>> > > > >>> The same can be said about proceeding the other way around - the > > > >>> openCSD library can be added to the kernel tree later if it is deemed > > > >>> necessary. Until then I really don't see why we'd prevent people from > > > >>> accessing the functionality. > > > >> > > > >> Again, I'm not suggesting the code be refactored... > > > >> > > > >>> >> > There's also a bit of a chicken and egg problem > > > >>> >> > in that it's a lot easier to get distros to package libraries that have > > > >>> >> > users available (some are not really bothered about this of course but > > > >>> >> > it still helps). > > > >>> >> > > > >>> >> Moreover including in the kernel tree every library that can > > > >>> >> potentially be used by the perf tools simply doesn't scale. > > > >>> > > > > >>> > This is a trace decoder library we're talking about: there are no > > > >>> > others in perf's system features autodetection list. And why wouldn't > > > >>> > adding such libraries scale? > > > >>> > > > >>> I don't see why a decoder library and say, libelf, need to be treated > > > >>> differently. > > > >> > > > >> libelf is a mature library based on an industry-wide standard, not to > > > >> mention already packaged by most (all?) distros. > > > > > > We re-factored the library name and the layout of the C-API necessary > > > header files in the latest release (v0.8) to allow for installation > > > into a linux system and the build changes made to perf. > > > Now, while it is true that we need to be careful that users are aware > > > they need to use the latest release, in general if the library is not > > > present on a system; if is not part of the distribution, it is very > > > easy to find using a simple search. > > > > The way it's being done in this patchseries, users are going to query > > their package managers for libopencsd and not find it: very > > misleading. To avoid having to manually install the library and > > inevitably have it go out of sync with perf, it'd behoove us to > > include the sources directly as of now. > > There's nothing going out of sync - until we have proper packaging > people simply need to recompile their perf utility. You can't guarantee that: Both the perf tool and libopencsd will continue to be developed. Without the control of which version of the library gets built with perf means that user copies will go out of sync if this patchseries goes in as-is. > As pointed out before that wouldn't be > hard to do for people using hardware assisted tracing. You mean people already used to building OpenCSD kernels, libraries, and perf from scratch? That's not saying much, esp. because by posting decoder support upstream, we should be catering to new - less unique - users. > > > Frankly whenever I appear to be > > > missing a library in linux the first thing I do is a web search, to > > > figure out what package I am missing (since it is not always obvious > > > to me what libraries lie in which packages). > > > > > > Google 'libopencsd' and you see the github HOWTO.md for OpenCSD very quickly. > > > > > > I think users interested in using the library will have the know-how > > > to find an install it if the perf build process marks it as missing. > > > The HOWTO will be maintained as part of the library to explain the > > > installation and integration with perf. > > > > We can avoid all of this. > > > > > Given all the above, I cannot see any engineering benefit from adding > > > the source to the kernel tree. We have a stable and maintainable > > > solution with the library added to the perf build as an external > > > dependency. > > > > That's not true: the perf and libopencsd sources can get out of sync, > > whereas if the perf-dependent files of libopencsd were maintained along > > with the perf source itself, that would not be possible. > > Not true: kernel subsystem get out of sync regularly, this wouldn't be any The kernel subsystem gets out of sync with what? The perf tool? That hasn't been my experience: When does that happen, and how regularly? Note that some of the kernel header files are copied into the perf tool, but are controllingly updated prior to every release. IIRC, on the Coresight mailing list, the first response to incoming bug reports is to ask the user to update to the latest OpenCSD-tree-maintained version of the kernel, library *and* perf tool, because that is the only tested/maintained version. You'd want the libopencsd sources in sync with perf to avoid those bug reports for that same reason. > different. Can we get upstream maintainer input on importing some libopencsd source files, much like how the kernel build system does in scripts/dtc? Thanks, Kim