Re: [GIT PULL 00/21] perf/core improvements and fixes
* Arnaldo Carvalho de Melo wrote: > Hi Ingo, > > Please consider pullimg, > > - Arnaldo > > > Test results at the end of this message, as usual. > > The following changes since commit 5804b11034a21e4287daaf017c5ad60ad7af8d67: > > Merge tag 'perf-core-for-mingo-4.19-20180815' of > git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/urgent > (2018-08-18 13:11:51 +0200) > > are available in the Git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git > tags/perf-core-for-mingo-4.19-20180820 > > for you to fetch changes up to 78303650e4cd873c6c4276c6fe3e768ff0b46d22: > > tools arch: Update arch/x86/lib/memcpy_64.S copy used in 'perf bench mem > memcpy' (2018-08-20 10:17:14 -0300) > > > perf/core improvements and fixes: > > LLVM/clang/eBPF: (Arnaldo Carvalho de Melo) > > - Allow passing options to llc in addition to to clang. > > Hardware tracing: (Jack Henschel) > > - Improve error message for PMU address filters, clarifying availability of > that feature in hardware having hardware tracing such as Intel PT. > > Python interface: (Jiri Olsa) > > - Fix read_on_cpu() interface. > > ELF/DWARF libraries: (Jiri Olsa) > > - Fix handling of the combo compressed module file + decompressed associated > debuginfo file. > > Build (Rasmus Villemoes) > > - Disable parallelism for 'make clean', avoiding multiple submakes deleting > the same files and causing the build to fail on systems such as Yocto. > > Kernel ABI copies: (Arnaldo Carvalho de Melo) > > - Update tools's copy of x86's cpufeatures.h. > > - Update arch/x86/lib/memcpy_64.S copy used in 'perf bench mem memcpy'. > > Miscellaneous: (Steven Rostedt) > > - Change libtraceevent to SPDX License format. > > Signed-off-by: Arnaldo Carvalho de Melo > > > Arnaldo Carvalho de Melo (3): > perf llvm: Allow passing options to llc in addition to clang > tools arch x86: Update tools's copy of cpufeatures.h > tools arch: Update arch/x86/lib/memcpy_64.S copy used in 'perf bench > mem memcpy' > > Jack Henschel (1): > perf parser: Improve error message for PMU address filters > > Jiri Olsa (15): > perf tools: Get rid of dso__needs_decompress() call in > read_object_code() > perf tools: Get rid of dso__needs_decompress() call in > symbol__disassemble() > perf tools: Get rid of dso__needs_decompress() call in __open_dso() > perf tools: Make decompress_to_file() function static > perf tools: Make is_supported_compression() static > perf tools: Add compression id into 'struct kmod_path' > perf tools: Store compression id into struct dso > perf tools: Use compression id in decompress_kmodule() > perf tools: Move the temp file processing into decompress_kmodule > perf tools: Add is_compressed callback to compressions array > perf tools: Add lzma_is_compressed function > perf tools: Add gzip_is_compressed function > perf tools: Remove ext from struct kmod_path > perf mmap: Store real cpu number in 'struct perf_mmap' > perf python: Fix pyrf_evlist__read_on_cpu() interface > > Rasmus Villemoes (1): > perf tools: Disable parallelism for 'make clean' > > Steven Rostedt (VMware) (1): > tools lib traceevent: Change to SPDX License format > > tools/arch/x86/include/asm/cpufeatures.h | 3 +- > tools/arch/x86/lib/memcpy_64.S | 2 +- > tools/lib/traceevent/event-parse.c | 16 +--- > tools/lib/traceevent/event-plugin.c | 16 +--- > tools/lib/traceevent/event-utils.h | 16 +--- > tools/lib/traceevent/kbuffer-parse.c | 17 +--- > tools/lib/traceevent/parse-filter.c | 16 +--- > tools/lib/traceevent/parse-utils.c | 16 +--- > tools/lib/traceevent/trace-seq.c | 16 +--- > tools/perf/Makefile | 4 +- > tools/perf/tests/code-reading.c | 4 +- > tools/perf/tests/kmod-path.c | 136 > +++ > tools/perf/util/annotate.c | 4 +- > tools/perf/util/compress.h | 2 + > tools/perf/util/dso.c| 111 - > tools/perf/util/dso.h| 13 ++- > tools/perf/util/evlist.c | 2 +- > tools/perf/util/llvm-utils.c | 31 ++- > tools/perf/util/llvm-utils.h | 9 ++ > tools/perf/util/lzma.c | 20 + > tools/perf/util/machine.c| 4 +- > tools/perf/util/mmap.c | 3 +- > tools/perf/util/mmap.h | 3 +- > tools/perf/util/parse-events.c | 20 ++--- > tools/perf/util/python.c | 20 - > tools/perf/util/zlib.c | 18 > 26 files changed, 256 insertions(+), 266 deletions(-) Pulled,
Re: [GIT PULL 00/21] perf/core improvements and fixes
* Arnaldo Carvalho de Melo wrote: > Hi Ingo, > > Please consider pullimg, > > - Arnaldo > > > Test results at the end of this message, as usual. > > The following changes since commit 5804b11034a21e4287daaf017c5ad60ad7af8d67: > > Merge tag 'perf-core-for-mingo-4.19-20180815' of > git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/urgent > (2018-08-18 13:11:51 +0200) > > are available in the Git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git > tags/perf-core-for-mingo-4.19-20180820 > > for you to fetch changes up to 78303650e4cd873c6c4276c6fe3e768ff0b46d22: > > tools arch: Update arch/x86/lib/memcpy_64.S copy used in 'perf bench mem > memcpy' (2018-08-20 10:17:14 -0300) > > > perf/core improvements and fixes: > > LLVM/clang/eBPF: (Arnaldo Carvalho de Melo) > > - Allow passing options to llc in addition to to clang. > > Hardware tracing: (Jack Henschel) > > - Improve error message for PMU address filters, clarifying availability of > that feature in hardware having hardware tracing such as Intel PT. > > Python interface: (Jiri Olsa) > > - Fix read_on_cpu() interface. > > ELF/DWARF libraries: (Jiri Olsa) > > - Fix handling of the combo compressed module file + decompressed associated > debuginfo file. > > Build (Rasmus Villemoes) > > - Disable parallelism for 'make clean', avoiding multiple submakes deleting > the same files and causing the build to fail on systems such as Yocto. > > Kernel ABI copies: (Arnaldo Carvalho de Melo) > > - Update tools's copy of x86's cpufeatures.h. > > - Update arch/x86/lib/memcpy_64.S copy used in 'perf bench mem memcpy'. > > Miscellaneous: (Steven Rostedt) > > - Change libtraceevent to SPDX License format. > > Signed-off-by: Arnaldo Carvalho de Melo > > > Arnaldo Carvalho de Melo (3): > perf llvm: Allow passing options to llc in addition to clang > tools arch x86: Update tools's copy of cpufeatures.h > tools arch: Update arch/x86/lib/memcpy_64.S copy used in 'perf bench > mem memcpy' > > Jack Henschel (1): > perf parser: Improve error message for PMU address filters > > Jiri Olsa (15): > perf tools: Get rid of dso__needs_decompress() call in > read_object_code() > perf tools: Get rid of dso__needs_decompress() call in > symbol__disassemble() > perf tools: Get rid of dso__needs_decompress() call in __open_dso() > perf tools: Make decompress_to_file() function static > perf tools: Make is_supported_compression() static > perf tools: Add compression id into 'struct kmod_path' > perf tools: Store compression id into struct dso > perf tools: Use compression id in decompress_kmodule() > perf tools: Move the temp file processing into decompress_kmodule > perf tools: Add is_compressed callback to compressions array > perf tools: Add lzma_is_compressed function > perf tools: Add gzip_is_compressed function > perf tools: Remove ext from struct kmod_path > perf mmap: Store real cpu number in 'struct perf_mmap' > perf python: Fix pyrf_evlist__read_on_cpu() interface > > Rasmus Villemoes (1): > perf tools: Disable parallelism for 'make clean' > > Steven Rostedt (VMware) (1): > tools lib traceevent: Change to SPDX License format > > tools/arch/x86/include/asm/cpufeatures.h | 3 +- > tools/arch/x86/lib/memcpy_64.S | 2 +- > tools/lib/traceevent/event-parse.c | 16 +--- > tools/lib/traceevent/event-plugin.c | 16 +--- > tools/lib/traceevent/event-utils.h | 16 +--- > tools/lib/traceevent/kbuffer-parse.c | 17 +--- > tools/lib/traceevent/parse-filter.c | 16 +--- > tools/lib/traceevent/parse-utils.c | 16 +--- > tools/lib/traceevent/trace-seq.c | 16 +--- > tools/perf/Makefile | 4 +- > tools/perf/tests/code-reading.c | 4 +- > tools/perf/tests/kmod-path.c | 136 > +++ > tools/perf/util/annotate.c | 4 +- > tools/perf/util/compress.h | 2 + > tools/perf/util/dso.c| 111 - > tools/perf/util/dso.h| 13 ++- > tools/perf/util/evlist.c | 2 +- > tools/perf/util/llvm-utils.c | 31 ++- > tools/perf/util/llvm-utils.h | 9 ++ > tools/perf/util/lzma.c | 20 + > tools/perf/util/machine.c| 4 +- > tools/perf/util/mmap.c | 3 +- > tools/perf/util/mmap.h | 3 +- > tools/perf/util/parse-events.c | 20 ++--- > tools/perf/util/python.c | 20 - > tools/perf/util/zlib.c | 18 > 26 files changed, 256 insertions(+), 266 deletions(-) Pulled,
Re: [GIT PULL 00/21] perf/core improvements and fixes
* Arnaldo Carvalho de Melo wrote:
> Hi Ingo,
>
> Please consider pulling, contains a recently merged
> tip/perf/urgent,
>
> - Arnaldo
>
> Test results at the end of this message, as usual.
>
> The following changes since commit c2586cfbb905939b79b49a9121fb0a59a5668fd6:
>
> Merge remote-tracking branch 'tip/perf/urgent' into perf/core (2018-07-31
> 09:55:45 -0300)
>
> are available in the Git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git
> tags/perf-core-for-mingo-4.19-20180801
>
> for you to fetch changes up to b912885ab75c7c8aa841c615108afd755d0b97f8:
>
> perf trace: Do not require --no-syscalls to suppress strace like output
> (2018-08-01 16:20:28 -0300)
>
>
> perf/core improvements and fixes:
>
> perf trace: (Arnaldo Carvalho de Melo)
>
> - Do not require --no-syscalls to suppress strace like output, i.e.
>
> # perf trace -e sched:*switch
>
> will show just sched:sched_switch events, not strace-like formatted
> syscall events, use --syscalls to get the previous behaviour.
>
> If instead:
>
> # perf trace
>
> is used, i.e. no events specified, then --syscalls is implied and
> system wide strace like formatting will be applied to all syscalls.
>
> The behaviour when just a syscall subset is used with '-e' is unchanged:
>
> # perf trace -e *sleep,sched:*switch
>
> will work as before: just the 'nanosleep' syscall will be strace-like
> formatted plus the sched:sched_switch tracepoint event, system wide.
>
> - Allow string table generators to use a default header dir, allowing
> use of them without parameters to see the table it generates on
> stdout, e.g.:
>
> $ tools/perf/trace/beauty/kvm_ioctl.sh
> static const char *kvm_ioctl_cmds[] = {
> [0x00] = "GET_API_VERSION",
> [0x01] = "CREATE_VM",
> [0x02] = "GET_MSR_INDEX_LIST",
> [0x03] = "CHECK_EXTENSION",
>
> [0xe0] = "CREATE_DEVICE",
> [0xe1] = "SET_DEVICE_ATTR",
> [0xe2] = "GET_DEVICE_ATTR",
> [0xe3] = "HAS_DEVICE_ATTR",
> };
> $
>
> See 'ls tools/perf/trace/beauty/*.sh' to see the available string
> table generators.
>
> - Add a generator for IPPROTO_ socket's protocol constants.
>
> perf record: (Kan Liang)
>
> - Fix error out while applying initial delay and using LBR, due to
> the use of a PERF_TYPE_SOFTWARE/PERF_COUNT_SW_DUMMY event to track
> PERF_RECORD_MMAP events while waiting for the initial delay. Such
> events fail when configured asking PERF_SAMPLE_BRANCH_STACK in
> perf_event_attr.sample_type.
>
> perf c2c: (Jiri Olsa)
>
> - Fix report crash for empty browser, when processing a perf.data file
> without events of interest, either because not asked for in
> 'perf record' or because the workload didn't triggered such events.
>
> perf list: (Michael Petlan)
>
> - Align metric group description format with PMU event description.
>
> perf tests: (Sandipan Das)
>
> - Fix indexing when invoking subtests, which caused BPF tests to
> get results for the next test in the list, with the last one
> reporting a failure.
>
> eBPF:
>
> - Fix installation directory for header files included from eBPF proggies,
> avoiding clashing with relative paths used to build other software projects
> such as glibc. (Thomas Richter)
>
> - Show better message when failing to load an object. (Arnaldo Carvalho de
> Melo)
>
> General: (Christophe Leroy)
>
> - Allow overriding MAX_NR_CPUS at compile time, to make the tooling
> usable in systems with less memory, in time this has to be changed
> to properly allocate based on _NPROCESSORS_ONLN.
>
> Architecture specific:
>
> - Update arm64's ThunderX2 implementation defined pmu core events (Ganapatrao
> Kulkarni)
>
> - Fix complex event name parsing in 'perf test' for PowerPC, where the
> 'umask' event
> modifier isn't present. (Sandipan Das)
>
> CoreSight ARM hardware tracing: (Leo Yan)
>
> - Fix start tracing packet handling.
>
> - Support dummy address value for CS_ETM_TRACE_ON packet.
>
> - Generate branch sample when receiving a CS_ETM_TRACE_ON packet.
>
> - Generate branch sample for CS_ETM_TRACE_ON packet.
>
> Signed-off-by: Arnaldo Carvalho de Melo
>
>
> Arnaldo Carvalho de Melo (9):
> perf trace beauty: Default header_dir to cwd to work without parms
> tools include uapi: Grab a copy of linux/in.h
> perf beauty: Add a generator for IPPROTO_ socket's protocol constants
> perf trace beauty: Do not print NULL strarray entries
> perf trace beauty: Add beautifiers for 'socket''s 'protocol' arg
> perf trace: Beautify the AF_INET & AF_INET6 'socket' syscall 'protocol'
> args
> perf bpf: Show better message when failing to load an object
> perf bpf: Include uapi/linux/bpf.h from the 'perf trace' script's
Re: [GIT PULL 00/21] perf/core improvements and fixes
* Arnaldo Carvalho de Melo wrote:
> Hi Ingo,
>
> Please consider pulling, contains a recently merged
> tip/perf/urgent,
>
> - Arnaldo
>
> Test results at the end of this message, as usual.
>
> The following changes since commit c2586cfbb905939b79b49a9121fb0a59a5668fd6:
>
> Merge remote-tracking branch 'tip/perf/urgent' into perf/core (2018-07-31
> 09:55:45 -0300)
>
> are available in the Git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git
> tags/perf-core-for-mingo-4.19-20180801
>
> for you to fetch changes up to b912885ab75c7c8aa841c615108afd755d0b97f8:
>
> perf trace: Do not require --no-syscalls to suppress strace like output
> (2018-08-01 16:20:28 -0300)
>
>
> perf/core improvements and fixes:
>
> perf trace: (Arnaldo Carvalho de Melo)
>
> - Do not require --no-syscalls to suppress strace like output, i.e.
>
> # perf trace -e sched:*switch
>
> will show just sched:sched_switch events, not strace-like formatted
> syscall events, use --syscalls to get the previous behaviour.
>
> If instead:
>
> # perf trace
>
> is used, i.e. no events specified, then --syscalls is implied and
> system wide strace like formatting will be applied to all syscalls.
>
> The behaviour when just a syscall subset is used with '-e' is unchanged:
>
> # perf trace -e *sleep,sched:*switch
>
> will work as before: just the 'nanosleep' syscall will be strace-like
> formatted plus the sched:sched_switch tracepoint event, system wide.
>
> - Allow string table generators to use a default header dir, allowing
> use of them without parameters to see the table it generates on
> stdout, e.g.:
>
> $ tools/perf/trace/beauty/kvm_ioctl.sh
> static const char *kvm_ioctl_cmds[] = {
> [0x00] = "GET_API_VERSION",
> [0x01] = "CREATE_VM",
> [0x02] = "GET_MSR_INDEX_LIST",
> [0x03] = "CHECK_EXTENSION",
>
> [0xe0] = "CREATE_DEVICE",
> [0xe1] = "SET_DEVICE_ATTR",
> [0xe2] = "GET_DEVICE_ATTR",
> [0xe3] = "HAS_DEVICE_ATTR",
> };
> $
>
> See 'ls tools/perf/trace/beauty/*.sh' to see the available string
> table generators.
>
> - Add a generator for IPPROTO_ socket's protocol constants.
>
> perf record: (Kan Liang)
>
> - Fix error out while applying initial delay and using LBR, due to
> the use of a PERF_TYPE_SOFTWARE/PERF_COUNT_SW_DUMMY event to track
> PERF_RECORD_MMAP events while waiting for the initial delay. Such
> events fail when configured asking PERF_SAMPLE_BRANCH_STACK in
> perf_event_attr.sample_type.
>
> perf c2c: (Jiri Olsa)
>
> - Fix report crash for empty browser, when processing a perf.data file
> without events of interest, either because not asked for in
> 'perf record' or because the workload didn't triggered such events.
>
> perf list: (Michael Petlan)
>
> - Align metric group description format with PMU event description.
>
> perf tests: (Sandipan Das)
>
> - Fix indexing when invoking subtests, which caused BPF tests to
> get results for the next test in the list, with the last one
> reporting a failure.
>
> eBPF:
>
> - Fix installation directory for header files included from eBPF proggies,
> avoiding clashing with relative paths used to build other software projects
> such as glibc. (Thomas Richter)
>
> - Show better message when failing to load an object. (Arnaldo Carvalho de
> Melo)
>
> General: (Christophe Leroy)
>
> - Allow overriding MAX_NR_CPUS at compile time, to make the tooling
> usable in systems with less memory, in time this has to be changed
> to properly allocate based on _NPROCESSORS_ONLN.
>
> Architecture specific:
>
> - Update arm64's ThunderX2 implementation defined pmu core events (Ganapatrao
> Kulkarni)
>
> - Fix complex event name parsing in 'perf test' for PowerPC, where the
> 'umask' event
> modifier isn't present. (Sandipan Das)
>
> CoreSight ARM hardware tracing: (Leo Yan)
>
> - Fix start tracing packet handling.
>
> - Support dummy address value for CS_ETM_TRACE_ON packet.
>
> - Generate branch sample when receiving a CS_ETM_TRACE_ON packet.
>
> - Generate branch sample for CS_ETM_TRACE_ON packet.
>
> Signed-off-by: Arnaldo Carvalho de Melo
>
>
> Arnaldo Carvalho de Melo (9):
> perf trace beauty: Default header_dir to cwd to work without parms
> tools include uapi: Grab a copy of linux/in.h
> perf beauty: Add a generator for IPPROTO_ socket's protocol constants
> perf trace beauty: Do not print NULL strarray entries
> perf trace beauty: Add beautifiers for 'socket''s 'protocol' arg
> perf trace: Beautify the AF_INET & AF_INET6 'socket' syscall 'protocol'
> args
> perf bpf: Show better message when failing to load an object
> perf bpf: Include uapi/linux/bpf.h from the 'perf trace' script's
Re: [GIT PULL 00/21] perf/core improvements and fixes
On 4 March 2015 at 22:37, Ingo Molnar wrote: > > * Victor Kamensky wrote: > >> Hi Arnaldo, Ingo, >> >> What happened with this pull request? [...] > > This pull request was for v4.1, and I merged it in: Ok, I got it. Sorry I missed that before. > commit 8a26ce4e544659256349551283414df504889a59 > Merge: acba3c7e4652 726f3234dd12 > Author: Ingo Molnar > Date: Wed Feb 18 19:14:54 2015 +0100 > > Merge tag 'perf-core-for-mingo' of > git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/core > > Pull perf/core improvements and fixes from Arnaldo Carvalho de Melo: > > User visible changes: > >> [...] I already see in v4.0-rc2 changes additions to one requested >> by this pull request, but I don't see this series itself. >> >> For example e370a3d57664cd5e39c0b95d157ebc841b568409 >> "perf symbols: Define EM_AARCH64 for older OSes" by David >> is already in v4.0-rc2 and it is supposed to be addition to >> "perf symbols: Ignore mapping symbols on aarch64" that is part >> of this pull request but it did not make into v4.0-rcX yet. Looks >> quite strange. > > If some commits of the v4.1 queue are needed in v4.0 as well then they > should be cherry-picked back into the urgent queue. > > But maybe e370a3d57 was merged prematurely - in that case it appears > to be harmless and v4.1 will sort it out. Yes, this merge is harmless. Indeed it should be OK with v4.1. Thanks for explanation. - Victor > Arnaldo? > > Thanks, > > Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
* Victor Kamensky wrote: > Hi Arnaldo, Ingo, > > What happened with this pull request? [...] This pull request was for v4.1, and I merged it in: commit 8a26ce4e544659256349551283414df504889a59 Merge: acba3c7e4652 726f3234dd12 Author: Ingo Molnar Date: Wed Feb 18 19:14:54 2015 +0100 Merge tag 'perf-core-for-mingo' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/core Pull perf/core improvements and fixes from Arnaldo Carvalho de Melo: User visible changes: > [...] I already see in v4.0-rc2 changes additions to one requested > by this pull request, but I don't see this series itself. > > For example e370a3d57664cd5e39c0b95d157ebc841b568409 > "perf symbols: Define EM_AARCH64 for older OSes" by David > is already in v4.0-rc2 and it is supposed to be addition to > "perf symbols: Ignore mapping symbols on aarch64" that is part > of this pull request but it did not make into v4.0-rcX yet. Looks > quite strange. If some commits of the v4.1 queue are needed in v4.0 as well then they should be cherry-picked back into the urgent queue. But maybe e370a3d57 was merged prematurely - in that case it appears to be harmless and v4.1 will sort it out. Arnaldo? Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
Hi Arnaldo, Ingo,
What happened with this pull request? I already
see in v4.0-rc2 changes additions to one requested
by this pull request, but I don't see this series itself.
For example e370a3d57664cd5e39c0b95d157ebc841b568409
"perf symbols: Define EM_AARCH64 for older OSes" by David
is already in v4.0-rc2 and it is supposed to be addition to
"perf symbols: Ignore mapping symbols on aarch64" that is part
of this pull request but it did not make into v4.0-rcX yet. Looks
quite strange.
Thanks,
Victor
On 11 February 2015 at 13:08, Arnaldo Carvalho de Melo wrote:
> Hi Ingo,
>
> Please consider pulling,
>
> - Arnaldo
>
> The following changes since commit 2fde4f94e0a9531251e706fa57131b51b0df042e:
>
> perf: Decouple unthrottling and rotating (2015-02-04 08:07:16 +0100)
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git
> tags/perf-core-for-mingo
>
> for you to fetch changes up to 39f5704399042fff5f0d5f6af32bbbc3e787a897:
>
> perf tools: Define _GNU_SOURCE on pthread_attr_setaffinity_np feature check
> (2015-02-11 17:38:55 -0300)
>
>
> perf/core improvement and fixes:
>
> User visible:
>
> - No need to explicitely enable evsels for workload started from perf, let it
> be enabled via perf_event_attr.enable_on_exec, removing some events that
> take
> place in the 'perf trace' before a workload is really started by it.
> (Arnaldo Carvalho de Melo)
>
> - Fix to handle optimized not-inlined functions in 'perf probe' (Masami
> Hiramatsu)
>
> - Update 'perf probe' man page (Masami Hiramatsu)
>
> Infrastructure:
>
> Arnaldo Carvalho de Melo (4):
> - Introduce {trace_seq_do,event_format_}_fprintf functions to allow
> a default tracepoint field list printer to be used in tools that allows
> redirecting output to a file. (Arnaldo Carvalho de Melo)
>
> - The man page for pthread_attr_set_affinity_np states that _GNU_SOURCE
> must be defined before pthread.h, do it to fix the build in some
> systems (Josh Boyer)
>
> - Cleanups in 'perf buildid-cache' (Masami Hiramatsu)
>
> - Fix dso cache test case (Namhyung Kim)
>
> - Do Not rely on dso__data_read_offset() to open DSO (Namhyung Kim)
>
> - Make perf aware of tracefs (Steven Rostedt).
>
> - Fix build by defining STT_GNU_IFUNC for glibc 2.9 and older (Vinson Lee)
>
> - AArch64 symbol resolution fixes (Victor Kamensky)
>
> Signed-off-by: Arnaldo Carvalho de Melo
>
>
> Arnaldo Carvalho de Melo (4):
> tools lib traceevent: Introduce trace_seq_do_fprintf function
> perf tools: Introduce event_format__fprintf method
> perf trace: No need to enable evsels for workload started from perf
> perf evlist: Fix typo in comment
>
> Josh Boyer (1):
> perf tools: Define _GNU_SOURCE on pthread_attr_setaffinity_np feature
> check
>
> Masami Hiramatsu (4):
> perf probe: Fix to handle optimized not-inlined functions
> perf probe: Update man page
> perf buildid-cache: Remove unneeded debugdir parameters
> perf buildid-cache: Consolidate .build-id cache path generators
>
> Namhyung Kim (3):
> perf test: Fix dso cache testcase
> perf tests: Do not rely on dso__data_read_offset() to open dso
> perf tools: Fix a dso open fail message
>
> Steven Rostedt (Red Hat) (6):
> perf tools: Do not check debugfs MAGIC for tracing files
> tools lib fs: Add helper to find mounted file systems
> tools lib api fs: Add tracefs mount helper functions
> tools lib api debugfs: Add DEBUGFS_DEFAULT_PATH macro
> tools lib api fs: Add {tracefs,debugfs}_configured() functions
> perf tools: Make perf aware of tracefs
>
> Victor Kamensky (2):
> perf symbols: Ignore mapping symbols on aarch64
> perf symbols: debuglink should take symfs option into account
>
> Vinson Lee (1):
> perf symbols: Define STT_GNU_IFUNC for glibc 2.9 and older.
>
> tools/lib/api/Makefile| 4 ++
> tools/lib/api/fs/debugfs.c| 69 +++---
> tools/lib/api/fs/debugfs.h| 13 +
> tools/lib/api/fs/findfs.c | 63
> tools/lib/api/fs/findfs.h | 23
> tools/lib/api/fs/tracefs.c| 78 +
> tools/lib/api/fs/tracefs.h| 21 +++
> tools/lib/traceevent/event-parse.h| 2 +
> tools/lib/traceevent/trace-seq.c | 13 +++--
> tools/perf/Documentation/perf-probe.txt | 16 +-
> tools/perf/builtin-buildid-cache.c| 37 ++--
> tools/perf/builtin-trace.c| 4 +-
> tools/perf/config/feature-checks/Makefile | 2 +-
> tools/perf/tests/dso-data.c | 22 ---
> tools/perf/tests/open-syscall-all-cpus.c | 7 ++-
> tools/perf/tests/open-syscall.c | 7 ++-
>
Re: [GIT PULL 00/21] perf/core improvements and fixes
* Victor Kamensky victor.kamen...@linaro.org wrote: Hi Arnaldo, Ingo, What happened with this pull request? [...] This pull request was for v4.1, and I merged it in: commit 8a26ce4e544659256349551283414df504889a59 Merge: acba3c7e4652 726f3234dd12 Author: Ingo Molnar mi...@kernel.org Date: Wed Feb 18 19:14:54 2015 +0100 Merge tag 'perf-core-for-mingo' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/core Pull perf/core improvements and fixes from Arnaldo Carvalho de Melo: User visible changes: [...] I already see in v4.0-rc2 changes additions to one requested by this pull request, but I don't see this series itself. For example e370a3d57664cd5e39c0b95d157ebc841b568409 perf symbols: Define EM_AARCH64 for older OSes by David is already in v4.0-rc2 and it is supposed to be addition to perf symbols: Ignore mapping symbols on aarch64 that is part of this pull request but it did not make into v4.0-rcX yet. Looks quite strange. If some commits of the v4.1 queue are needed in v4.0 as well then they should be cherry-picked back into the urgent queue. But maybe e370a3d57 was merged prematurely - in that case it appears to be harmless and v4.1 will sort it out. Arnaldo? Thanks, Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
On 4 March 2015 at 22:37, Ingo Molnar mi...@kernel.org wrote: * Victor Kamensky victor.kamen...@linaro.org wrote: Hi Arnaldo, Ingo, What happened with this pull request? [...] This pull request was for v4.1, and I merged it in: Ok, I got it. Sorry I missed that before. commit 8a26ce4e544659256349551283414df504889a59 Merge: acba3c7e4652 726f3234dd12 Author: Ingo Molnar mi...@kernel.org Date: Wed Feb 18 19:14:54 2015 +0100 Merge tag 'perf-core-for-mingo' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/core Pull perf/core improvements and fixes from Arnaldo Carvalho de Melo: User visible changes: [...] I already see in v4.0-rc2 changes additions to one requested by this pull request, but I don't see this series itself. For example e370a3d57664cd5e39c0b95d157ebc841b568409 perf symbols: Define EM_AARCH64 for older OSes by David is already in v4.0-rc2 and it is supposed to be addition to perf symbols: Ignore mapping symbols on aarch64 that is part of this pull request but it did not make into v4.0-rcX yet. Looks quite strange. If some commits of the v4.1 queue are needed in v4.0 as well then they should be cherry-picked back into the urgent queue. But maybe e370a3d57 was merged prematurely - in that case it appears to be harmless and v4.1 will sort it out. Yes, this merge is harmless. Indeed it should be OK with v4.1. Thanks for explanation. - Victor Arnaldo? Thanks, Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
Hi Arnaldo, Ingo,
What happened with this pull request? I already
see in v4.0-rc2 changes additions to one requested
by this pull request, but I don't see this series itself.
For example e370a3d57664cd5e39c0b95d157ebc841b568409
perf symbols: Define EM_AARCH64 for older OSes by David
is already in v4.0-rc2 and it is supposed to be addition to
perf symbols: Ignore mapping symbols on aarch64 that is part
of this pull request but it did not make into v4.0-rcX yet. Looks
quite strange.
Thanks,
Victor
On 11 February 2015 at 13:08, Arnaldo Carvalho de Melo a...@kernel.org wrote:
Hi Ingo,
Please consider pulling,
- Arnaldo
The following changes since commit 2fde4f94e0a9531251e706fa57131b51b0df042e:
perf: Decouple unthrottling and rotating (2015-02-04 08:07:16 +0100)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git
tags/perf-core-for-mingo
for you to fetch changes up to 39f5704399042fff5f0d5f6af32bbbc3e787a897:
perf tools: Define _GNU_SOURCE on pthread_attr_setaffinity_np feature check
(2015-02-11 17:38:55 -0300)
perf/core improvement and fixes:
User visible:
- No need to explicitely enable evsels for workload started from perf, let it
be enabled via perf_event_attr.enable_on_exec, removing some events that
take
place in the 'perf trace' before a workload is really started by it.
(Arnaldo Carvalho de Melo)
- Fix to handle optimized not-inlined functions in 'perf probe' (Masami
Hiramatsu)
- Update 'perf probe' man page (Masami Hiramatsu)
Infrastructure:
Arnaldo Carvalho de Melo (4):
- Introduce {trace_seq_do,event_format_}_fprintf functions to allow
a default tracepoint field list printer to be used in tools that allows
redirecting output to a file. (Arnaldo Carvalho de Melo)
- The man page for pthread_attr_set_affinity_np states that _GNU_SOURCE
must be defined before pthread.h, do it to fix the build in some
systems (Josh Boyer)
- Cleanups in 'perf buildid-cache' (Masami Hiramatsu)
- Fix dso cache test case (Namhyung Kim)
- Do Not rely on dso__data_read_offset() to open DSO (Namhyung Kim)
- Make perf aware of tracefs (Steven Rostedt).
- Fix build by defining STT_GNU_IFUNC for glibc 2.9 and older (Vinson Lee)
- AArch64 symbol resolution fixes (Victor Kamensky)
Signed-off-by: Arnaldo Carvalho de Melo a...@redhat.com
Arnaldo Carvalho de Melo (4):
tools lib traceevent: Introduce trace_seq_do_fprintf function
perf tools: Introduce event_format__fprintf method
perf trace: No need to enable evsels for workload started from perf
perf evlist: Fix typo in comment
Josh Boyer (1):
perf tools: Define _GNU_SOURCE on pthread_attr_setaffinity_np feature
check
Masami Hiramatsu (4):
perf probe: Fix to handle optimized not-inlined functions
perf probe: Update man page
perf buildid-cache: Remove unneeded debugdir parameters
perf buildid-cache: Consolidate .build-id cache path generators
Namhyung Kim (3):
perf test: Fix dso cache testcase
perf tests: Do not rely on dso__data_read_offset() to open dso
perf tools: Fix a dso open fail message
Steven Rostedt (Red Hat) (6):
perf tools: Do not check debugfs MAGIC for tracing files
tools lib fs: Add helper to find mounted file systems
tools lib api fs: Add tracefs mount helper functions
tools lib api debugfs: Add DEBUGFS_DEFAULT_PATH macro
tools lib api fs: Add {tracefs,debugfs}_configured() functions
perf tools: Make perf aware of tracefs
Victor Kamensky (2):
perf symbols: Ignore mapping symbols on aarch64
perf symbols: debuglink should take symfs option into account
Vinson Lee (1):
perf symbols: Define STT_GNU_IFUNC for glibc 2.9 and older.
tools/lib/api/Makefile| 4 ++
tools/lib/api/fs/debugfs.c| 69 +++---
tools/lib/api/fs/debugfs.h| 13 +
tools/lib/api/fs/findfs.c | 63
tools/lib/api/fs/findfs.h | 23
tools/lib/api/fs/tracefs.c| 78 +
tools/lib/api/fs/tracefs.h| 21 +++
tools/lib/traceevent/event-parse.h| 2 +
tools/lib/traceevent/trace-seq.c | 13 +++--
tools/perf/Documentation/perf-probe.txt | 16 +-
tools/perf/builtin-buildid-cache.c| 37 ++--
tools/perf/builtin-trace.c| 4 +-
tools/perf/config/feature-checks/Makefile | 2 +-
tools/perf/tests/dso-data.c | 22 ---
tools/perf/tests/open-syscall-all-cpus.c | 7 ++-
tools/perf/tests/open-syscall.c | 7 ++-
tools/perf/tests/parse-events.c | 13 -
tools/perf/util/build-id.c| 96
Re: [GIT PULL 00/21] perf/core improvements and fixes
* Jiri Olsa wrote: > On Tue, Dec 10, 2013 at 12:07:59PM +0100, Ingo Molnar wrote: > > > > SNIP > > > > > Pulled, thanks Arnaldo! > > > > There's one detail I noticed about the recent trace-plugin changes: > > > > comet:~/tip/tools/perf> make install > > BUILD: Doing 'make -j12' parallel build > > SUBDIR Documentation > > INSTALL Documentation-man > > INSTALL GTK UI > > SUBDIR /home/mingo/tip/tools/lib/traceevent/ > > INSTALL binaries > > INSTALL plugin_jbd2.so > > INSTALL plugin_hrtimer.so > > INSTALL plugin_kmem.so > > INSTALL plugin_kvm.so > > INSTALL plugin_mac80211.so > > INSTALL plugin_sched_switch.so > > INSTALL plugin_function.so > > INSTALL plugin_xen.so > > INSTALL plugin_scsi.so > > INSTALL plugin_cfg80211.so > > INSTALL libexec > > INSTALL perf-archive > > INSTALL perl-scripts > > INSTALL python-scripts > > INSTALL perf_completion-script > > INSTALL tests > > > > those plugin installs are way too verbose, they should really be in a > > single summarized line, only saying something like: > > > > INSTALL plugins > > > > Just like we already sum up 'binaries', 'libexec', 'tests', etc. > > ok, TODO updated ;-) Consider it a regression! ;-) Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
On Tue, Dec 10, 2013 at 12:07:59PM +0100, Ingo Molnar wrote: > SNIP > > Pulled, thanks Arnaldo! > > There's one detail I noticed about the recent trace-plugin changes: > > comet:~/tip/tools/perf> make install > BUILD: Doing 'make -j12' parallel build > SUBDIR Documentation > INSTALL Documentation-man > INSTALL GTK UI > SUBDIR /home/mingo/tip/tools/lib/traceevent/ > INSTALL binaries > INSTALL plugin_jbd2.so > INSTALL plugin_hrtimer.so > INSTALL plugin_kmem.so > INSTALL plugin_kvm.so > INSTALL plugin_mac80211.so > INSTALL plugin_sched_switch.so > INSTALL plugin_function.so > INSTALL plugin_xen.so > INSTALL plugin_scsi.so > INSTALL plugin_cfg80211.so > INSTALL libexec > INSTALL perf-archive > INSTALL perl-scripts > INSTALL python-scripts > INSTALL perf_completion-script > INSTALL tests > > those plugin installs are way too verbose, they should really be in a > single summarized line, only saying something like: > > INSTALL plugins > > Just like we already sum up 'binaries', 'libexec', 'tests', etc. ok, TODO updated ;-) thanks, jirka -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
* Arnaldo Carvalho de Melo wrote:
> Em Tue, Dec 10, 2013 at 01:18:01PM +0100, Ingo Molnar escreveu:
> >
> > * Adrian Hunter wrote:
> >
> > > -void dso__set_short_name(struct dso *dso, const char *name)
> > > +void dso__set_short_name(struct dso *dso, const char *name, bool
> > > sname_alloc)
> > > {
> > > if (name == NULL)
> > > return;
> > > + if (dso->sname_alloc)
> > > + free((char *)dso->short_name);
> > > + dso->sname_alloc = sname_alloc;
> >
> > Calling the function option the same as the field name is asking for
> > trouble - I'd suggest 'new_sname_alloc' for the parameter, or so.
> >
> > And I'd also remove the 'const' from struct dso::short_name, it
> > probably does not help code generation, because 'dso' is passed in as
> > const in all the non-lifetime methods anyway.
>
> > That way the cast can be dropped from the free().
>
> Not that simple, there are multiple places that pass a constant
> short_name, for instance:
>
> machine__get_kernel()
> kernel = dso__kernel_findnew(machine, vmlinux_name,
>"[kernel]", DSO_TYPE_KERNEL);
> dso__set_short_name(dso, short_name);
>
> So dso->short_name will point to "[kernel]", which is a const char *.
Okay, I guess the free() cast is fine then.
Thanks,
Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
Em Tue, Dec 10, 2013 at 01:18:01PM +0100, Ingo Molnar escreveu:
>
> * Adrian Hunter wrote:
>
> > -void dso__set_short_name(struct dso *dso, const char *name)
> > +void dso__set_short_name(struct dso *dso, const char *name, bool
> > sname_alloc)
> > {
> > if (name == NULL)
> > return;
> > + if (dso->sname_alloc)
> > + free((char *)dso->short_name);
> > + dso->sname_alloc = sname_alloc;
>
> Calling the function option the same as the field name is asking for
> trouble - I'd suggest 'new_sname_alloc' for the parameter, or so.
>
> And I'd also remove the 'const' from struct dso::short_name, it
> probably does not help code generation, because 'dso' is passed in as
> const in all the non-lifetime methods anyway.
> That way the cast can be dropped from the free().
Not that simple, there are multiple places that pass a constant
short_name, for instance:
machine__get_kernel()
kernel = dso__kernel_findnew(machine, vmlinux_name,
"[kernel]", DSO_TYPE_KERNEL);
dso__set_short_name(dso, short_name);
So dso->short_name will point to "[kernel]", which is a const char *.
> Similar problems exist with the usage of 'short_name' - it overloads
> the field name which makes it somewhat confusing, and it's also
> sometimes inconsistently named, such as 'name' in
> dso__set_short_name().
>
> Ditto for 'long_name' handling.
>
> Also, the 'sname_alloc' name sucks, it does not make it obvious that
> it's related to 'short_name', hiding its true significance (and hiding
> the broken life time handling of the flag/pointer combo). I'd rename
> it to something more descriptive, like ->short_name_allocated - or I'd
> rename everything to 'sname'/'lname' naming for short/long names.
Ok, we can use rename it to short_name_alloc, like we have
short_name_len.
> Every time one runs into a crash like this it's a canary signal that
> cleanliness principles need hardening.
Hardening we go then!
> Thanks,
>
> Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
Em Tue, Dec 10, 2013 at 01:46:58PM +0100, Ingo Molnar escreveu: > * Ingo Molnar wrote: > > Every time one runs into a crash like this it's a canary signal that > > cleanliness principles need hardening. > > More observations about util/dso.c: > > - dso__binary_type_file() should probably pass in 'const struct dso' > > - dso__binary_type_file()'s filename string parameter should be named >'filename', not 'file' ... > > - build_id__sprintf() looks fragile: every single use of it appears >to follow this pattern: > > build_id__sprintf(x, sizeof(x), ...) > >this could be simplified (and eliminating the possibility to typo a >bug) by changing the function to __build_id__snprintf() and adding >a build_id__sprintf() wrapper macro around it: > > build_id__sprintf(x, ...) > >that generates the size itself. Right, like: int __perf_evlist__add_default_attrs(struct perf_evlist *evlist, struct perf_event_attr *attrs, size_t nr_attrs); #define perf_evlist__add_default_attrs(evlist, array) \ __perf_evlist__add_default_attrs(evlist, array, ARRAY_SIZE(array)) This is all a matter of being more dilligent and judicious at employing these and other good practices. But don't be shy to point anything (like you did here), as time permits we can go on doing patchkits to address things people notice. - Arnaldo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
* Ingo Molnar wrote:
>
> * Adrian Hunter wrote:
>
> > -void dso__set_short_name(struct dso *dso, const char *name)
> > +void dso__set_short_name(struct dso *dso, const char *name, bool
> > sname_alloc)
> > {
> > if (name == NULL)
> > return;
> > + if (dso->sname_alloc)
> > + free((char *)dso->short_name);
> > + dso->sname_alloc = sname_alloc;
>
> Calling the function option the same as the field name is asking for
> trouble - I'd suggest 'new_sname_alloc' for the parameter, or so.
>
> And I'd also remove the 'const' from struct dso::short_name, it
> probably does not help code generation, because 'dso' is passed in as
> const in all the non-lifetime methods anyway.
>
> That way the cast can be dropped from the free().
>
> Similar problems exist with the usage of 'short_name' - it overloads
> the field name which makes it somewhat confusing, and it's also
> sometimes inconsistently named, such as 'name' in
> dso__set_short_name().
>
> Ditto for 'long_name' handling.
>
> Also, the 'sname_alloc' name sucks, it does not make it obvious that
> it's related to 'short_name', hiding its true significance (and hiding
> the broken life time handling of the flag/pointer combo). I'd rename
> it to something more descriptive, like ->short_name_allocated - or I'd
> rename everything to 'sname'/'lname' naming for short/long names.
>
> Every time one runs into a crash like this it's a canary signal that
> cleanliness principles need hardening.
More observations about util/dso.c:
- dso__binary_type_file() should probably pass in 'const struct dso'
- dso__binary_type_file()'s filename string parameter should be named
'filename', not 'file' ...
- build_id__sprintf() looks fragile: every single use of it appears
to follow this pattern:
build_id__sprintf(x, sizeof(x), ...)
this could be simplified (and eliminating the possibility to typo a
bug) by changing the function to __build_id__snprintf() and adding
a build_id__sprintf() wrapper macro around it:
build_id__sprintf(x, ...)
that generates the size itself.
- dso__binary_type_file() is a method without a verb, so it's unclear
what it does. It probably wants to be renamed to
dso__set_binary_type_file() or so?
- dso_cache__find() probably wants to pass in a const rb_root.
- 'struct dso *pos' should probably be named 'struct dso *dso_pos' or
so - 'pos' is frequently used for integer variable names so its use
for an object iterator feels confusing.
- the 'head' argument of dsos__find() wants to be constified too I
guess
Thanks,
Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
* Arnaldo Carvalho de Melo wrote: > Em Tue, Dec 10, 2013 at 02:22:58PM +0200, Adrian Hunter escreveu: > > On 10/12/13 14:10, Arnaldo Carvalho de Melo wrote: > > > Em Tue, Dec 10, 2013 at 02:13:12PM +0200, Adrian Hunter escreveu: > > >> On 10/12/13 13:44, Arnaldo Carvalho de Melo wrote: > > >>> Em Tue, Dec 10, 2013 at 12:12:29PM +0100, Ingo Molnar escreveu: > > > > Hm, I've unpulled it because 'perf top' crashes on exit, in > > dso__delete(): > > >>> > > >>> 495 if (dso->sname_alloc) > > >>> 496 free((char *)dso->short_name) > > >>> > > >>> Yeah, must be that basename() patch from Stephane, I'll work on a fix > > >>> and resubmit this batch, thanks for the report. > > >> > > >> The problem is sname_alloc is not maintained. Perhaps it should be > > >> set in dso__set_short_name() e.g. > > > > > > Yeah, sounds better than having all callers manage that thing, quickie, > > > was this with Stephane's patch applied? > > > > Yes it was at > > e993d10caeb6dca690dbaf86e1981ba240d1414a > > perf symbols: fix bug in usage of the basename() function > > Yes, this is the buggy patch, my question was if Ingo did the > changes that streamlined the dso->sname_alloc management with > e993d10caeb6 applied to his working tree. My current perf/core head is: 789790791ad2 tools/perf/build: Fix install dependency which does not have e993d10c. [ Btw., a small nit: the capitalization of the commit title looks inconsistent. ] Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
Em Tue, Dec 10, 2013 at 09:22:13AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Dec 10, 2013 at 02:22:58PM +0200, Adrian Hunter escreveu:
> > On 10/12/13 14:10, Arnaldo Carvalho de Melo wrote:
> > > Em Tue, Dec 10, 2013 at 02:13:12PM +0200, Adrian Hunter escreveu:
> > >> On 10/12/13 13:44, Arnaldo Carvalho de Melo wrote:
> > >>> Em Tue, Dec 10, 2013 at 12:12:29PM +0100, Ingo Molnar escreveu:
> >
> > Hm, I've unpulled it because 'perf top' crashes on exit, in
> > dso__delete():
> > >>>
> > >>> 495 if (dso->sname_alloc)
> > >>> 496 free((char *)dso->short_name)
> > >>>
> > >>> Yeah, must be that basename() patch from Stephane, I'll work on a fix
> > >>> and resubmit this batch, thanks for the report.
> > >>
> > >> The problem is sname_alloc is not maintained. Perhaps it should be
> > >> set in dso__set_short_name() e.g.
> > >
> > > Yeah, sounds better than having all callers manage that thing, quickie,
> > > was this with Stephane's patch applied?
> >
> > Yes it was at
> > e993d10caeb6dca690dbaf86e1981ba240d1414a
> > perf symbols: fix bug in usage of the basename() function
>
> Yes, this is the buggy patch, my question was if Ingo did the changes
> that streamlined the dso->sname_alloc management with e993d10caeb6
> applied to his working tree.
Sorry Adrian, my bad, I thought Ingo had provided the patch, now I
realized my mistake, it was you 8-)
> - Arnaldo
>
> > > I think it should be done as a prep, then apply a modified version of
> > > Stephanes, that doesn't deal with the alloc flag (more than using 'true'
> > > to say it is a malloc'ed chunk).
> > >
> > > - Arnaldo
> > >
> > >> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> > >> index 9fae484..54ed980 100644
> > >> --- a/tools/perf/util/dso.c
> > >> +++ b/tools/perf/util/dso.c
> > >> @@ -379,7 +379,7 @@ struct dso *dso__kernel_findnew(struct machine
> > >> *machine,
> > >> const char *name,
> > >> * processing we had no idea this was the kernel dso.
> > >> */
> > >> if (dso != NULL) {
> > >> -dso__set_short_name(dso, short_name);
> > >> +dso__set_short_name(dso, short_name, false);
> > >> dso->kernel = dso_type;
> > >> }
> > >>
> > >> @@ -394,10 +394,13 @@ void dso__set_long_name(struct dso *dso, char
> > >> *name)
> > >> dso->long_name_len = strlen(name);
> > >> }
> > >>
> > >> -void dso__set_short_name(struct dso *dso, const char *name)
> > >> +void dso__set_short_name(struct dso *dso, const char *name, bool
> > >> sname_alloc)
> > >> {
> > >> if (name == NULL)
> > >> return;
> > >> +if (dso->sname_alloc)
> > >> +free((char *)dso->short_name);
> > >> +dso->sname_alloc = sname_alloc;
> > >> dso->short_name = name;
> > >> dso->short_name_len = strlen(name);
> > >> }
> > >> @@ -426,12 +429,7 @@ static void dso__set_basename(struct dso *dso)
> > >> if (!base)
> > >> return;
> > >>
> > >> -if (dso->sname_alloc)
> > >> -free((char *)dso->short_name);
> > >> -else
> > >> -dso->sname_alloc = 1;
> > >> -
> > >> -dso__set_short_name(dso, base);
> > >> +dso__set_short_name(dso, base, true);
> > >> }
> > >>
> > >> int dso__name_len(const struct dso *dso)
> > >> @@ -467,7 +465,7 @@ struct dso *dso__new(const char *name)
> > >> int i;
> > >> strcpy(dso->name, name);
> > >> dso__set_long_name(dso, dso->name);
> > >> -dso__set_short_name(dso, dso->name);
> > >> +dso__set_short_name(dso, dso->name, false);
> > >> for (i = 0; i < MAP__NR_TYPES; ++i)
> > >> dso->symbols[i] = dso->symbol_names[i] =
> > >> RB_ROOT;
> > >> dso->cache = RB_ROOT;
> > >> diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
> > >> index 384f2d9..166463e 100644
> > >> --- a/tools/perf/util/dso.h
> > >> +++ b/tools/perf/util/dso.h
> > >> @@ -110,7 +110,7 @@ static inline void dso__set_loaded(struct dso *dso,
> > >> enum
> > >> map_type type)
> > >> struct dso *dso__new(const char *name);
> > >> void dso__delete(struct dso *dso);
> > >>
> > >> -void dso__set_short_name(struct dso *dso, const char *name);
> > >> +void dso__set_short_name(struct dso *dso, const char *name, bool
> > >> sname_alloc);
> > >> void dso__set_long_name(struct dso *dso, char *name);
> > >>
> > >> int dso__name_len(const struct dso *dso);
> > >>
> > >>
> > >>>
> > >>> - Arnaldo
> > >>>
> > [Thread 0x770df700 (LWP 29561) exited]
> > *** Error in `/fast/mingo/tip/tools/perf/perf': munmap_chunk():
> > invalid pointer: 0x00587371 ***
> > === Backtrace: =
> > /lib64/libc.so.6[0x3e5907bbe7]
> > /fast/mingo/tip/tools/perf/perf(dso__delete+0xd9)[0x46da89]
> >
Re: [GIT PULL 00/21] perf/core improvements and fixes
Em Tue, Dec 10, 2013 at 02:22:58PM +0200, Adrian Hunter escreveu:
> On 10/12/13 14:10, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Dec 10, 2013 at 02:13:12PM +0200, Adrian Hunter escreveu:
> >> On 10/12/13 13:44, Arnaldo Carvalho de Melo wrote:
> >>> Em Tue, Dec 10, 2013 at 12:12:29PM +0100, Ingo Molnar escreveu:
>
> Hm, I've unpulled it because 'perf top' crashes on exit, in
> dso__delete():
> >>>
> >>> 495 if (dso->sname_alloc)
> >>> 496 free((char *)dso->short_name)
> >>>
> >>> Yeah, must be that basename() patch from Stephane, I'll work on a fix
> >>> and resubmit this batch, thanks for the report.
> >>
> >> The problem is sname_alloc is not maintained. Perhaps it should be
> >> set in dso__set_short_name() e.g.
> >
> > Yeah, sounds better than having all callers manage that thing, quickie,
> > was this with Stephane's patch applied?
>
> Yes it was at
> e993d10caeb6dca690dbaf86e1981ba240d1414a
> perf symbols: fix bug in usage of the basename() function
Yes, this is the buggy patch, my question was if Ingo did the changes
that streamlined the dso->sname_alloc management with e993d10caeb6
applied to his working tree.
- Arnaldo
> > I think it should be done as a prep, then apply a modified version of
> > Stephanes, that doesn't deal with the alloc flag (more than using 'true'
> > to say it is a malloc'ed chunk).
> >
> > - Arnaldo
> >
> >> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> >> index 9fae484..54ed980 100644
> >> --- a/tools/perf/util/dso.c
> >> +++ b/tools/perf/util/dso.c
> >> @@ -379,7 +379,7 @@ struct dso *dso__kernel_findnew(struct machine
> >> *machine,
> >> const char *name,
> >> * processing we had no idea this was the kernel dso.
> >> */
> >>if (dso != NULL) {
> >> - dso__set_short_name(dso, short_name);
> >> + dso__set_short_name(dso, short_name, false);
> >>dso->kernel = dso_type;
> >>}
> >>
> >> @@ -394,10 +394,13 @@ void dso__set_long_name(struct dso *dso, char *name)
> >>dso->long_name_len = strlen(name);
> >> }
> >>
> >> -void dso__set_short_name(struct dso *dso, const char *name)
> >> +void dso__set_short_name(struct dso *dso, const char *name, bool
> >> sname_alloc)
> >> {
> >>if (name == NULL)
> >>return;
> >> + if (dso->sname_alloc)
> >> + free((char *)dso->short_name);
> >> + dso->sname_alloc = sname_alloc;
> >>dso->short_name = name;
> >>dso->short_name_len = strlen(name);
> >> }
> >> @@ -426,12 +429,7 @@ static void dso__set_basename(struct dso *dso)
> >>if (!base)
> >>return;
> >>
> >> - if (dso->sname_alloc)
> >> - free((char *)dso->short_name);
> >> - else
> >> - dso->sname_alloc = 1;
> >> -
> >> - dso__set_short_name(dso, base);
> >> + dso__set_short_name(dso, base, true);
> >> }
> >>
> >> int dso__name_len(const struct dso *dso)
> >> @@ -467,7 +465,7 @@ struct dso *dso__new(const char *name)
> >>int i;
> >>strcpy(dso->name, name);
> >>dso__set_long_name(dso, dso->name);
> >> - dso__set_short_name(dso, dso->name);
> >> + dso__set_short_name(dso, dso->name, false);
> >>for (i = 0; i < MAP__NR_TYPES; ++i)
> >>dso->symbols[i] = dso->symbol_names[i] = RB_ROOT;
> >>dso->cache = RB_ROOT;
> >> diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
> >> index 384f2d9..166463e 100644
> >> --- a/tools/perf/util/dso.h
> >> +++ b/tools/perf/util/dso.h
> >> @@ -110,7 +110,7 @@ static inline void dso__set_loaded(struct dso *dso,
> >> enum
> >> map_type type)
> >> struct dso *dso__new(const char *name);
> >> void dso__delete(struct dso *dso);
> >>
> >> -void dso__set_short_name(struct dso *dso, const char *name);
> >> +void dso__set_short_name(struct dso *dso, const char *name, bool
> >> sname_alloc);
> >> void dso__set_long_name(struct dso *dso, char *name);
> >>
> >> int dso__name_len(const struct dso *dso);
> >>
> >>
> >>>
> >>> - Arnaldo
> >>>
> [Thread 0x770df700 (LWP 29561) exited]
> *** Error in `/fast/mingo/tip/tools/perf/perf': munmap_chunk(): invalid
> pointer: 0x00587371 ***
> === Backtrace: =
> /lib64/libc.so.6[0x3e5907bbe7]
> /fast/mingo/tip/tools/perf/perf(dso__delete+0xd9)[0x46da89]
> /fast/mingo/tip/tools/perf/perf(machines__exit+0xad)[0x482e7d]
> /fast/mingo/tip/tools/perf/perf(perf_session__delete+0xb6)[0x488c66]
> /fast/mingo/tip/tools/perf/perf(cmd_top+0xf44)[0x4345f4]
> /fast/mingo/tip/tools/perf/perf[0x419f95]
> /fast/mingo/tip/tools/perf/perf(main+0x600)[0x419830]
> /lib64/libc.so.6(__libc_start_main+0xf5)[0x3e59021b45]
> /fast/mingo/tip/tools/perf/perf[0x4198fd]
> === Memory map:
>
> Program received signal SIGABRT, Aborted.
> 0x003e590359e9 in raise () from /lib64/libc.so.6
> Missing separate debuginfos, use:
Re: [GIT PULL 00/21] perf/core improvements and fixes
* Adrian Hunter wrote:
> -void dso__set_short_name(struct dso *dso, const char *name)
> +void dso__set_short_name(struct dso *dso, const char *name, bool sname_alloc)
> {
> if (name == NULL)
> return;
> + if (dso->sname_alloc)
> + free((char *)dso->short_name);
> + dso->sname_alloc = sname_alloc;
Calling the function option the same as the field name is asking for
trouble - I'd suggest 'new_sname_alloc' for the parameter, or so.
And I'd also remove the 'const' from struct dso::short_name, it
probably does not help code generation, because 'dso' is passed in as
const in all the non-lifetime methods anyway.
That way the cast can be dropped from the free().
Similar problems exist with the usage of 'short_name' - it overloads
the field name which makes it somewhat confusing, and it's also
sometimes inconsistently named, such as 'name' in
dso__set_short_name().
Ditto for 'long_name' handling.
Also, the 'sname_alloc' name sucks, it does not make it obvious that
it's related to 'short_name', hiding its true significance (and hiding
the broken life time handling of the flag/pointer combo). I'd rename
it to something more descriptive, like ->short_name_allocated - or I'd
rename everything to 'sname'/'lname' naming for short/long names.
Every time one runs into a crash like this it's a canary signal that
cleanliness principles need hardening.
Thanks,
Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
On 10/12/13 14:10, Arnaldo Carvalho de Melo wrote:
> Em Tue, Dec 10, 2013 at 02:13:12PM +0200, Adrian Hunter escreveu:
>> On 10/12/13 13:44, Arnaldo Carvalho de Melo wrote:
>>> Em Tue, Dec 10, 2013 at 12:12:29PM +0100, Ingo Molnar escreveu:
Hm, I've unpulled it because 'perf top' crashes on exit, in
dso__delete():
>>>
>>> 495 if (dso->sname_alloc)
>>> 496 free((char *)dso->short_name)
>>>
>>> Yeah, must be that basename() patch from Stephane, I'll work on a fix
>>> and resubmit this batch, thanks for the report.
>>
>> The problem is sname_alloc is not maintained. Perhaps it should be
>> set in dso__set_short_name() e.g.
>
> Yeah, sounds better than having all callers manage that thing, quickie,
> was this with Stephane's patch applied?
Yes it was at
e993d10caeb6dca690dbaf86e1981ba240d1414a
perf symbols: fix bug in usage of the basename() function
>
> I think it should be done as a prep, then apply a modified version of
> Stephanes, that doesn't deal with the alloc flag (more than using 'true'
> to say it is a malloc'ed chunk).
>
> - Arnaldo
>
>> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
>> index 9fae484..54ed980 100644
>> --- a/tools/perf/util/dso.c
>> +++ b/tools/perf/util/dso.c
>> @@ -379,7 +379,7 @@ struct dso *dso__kernel_findnew(struct machine *machine,
>> const char *name,
>> * processing we had no idea this was the kernel dso.
>> */
>> if (dso != NULL) {
>> -dso__set_short_name(dso, short_name);
>> +dso__set_short_name(dso, short_name, false);
>> dso->kernel = dso_type;
>> }
>>
>> @@ -394,10 +394,13 @@ void dso__set_long_name(struct dso *dso, char *name)
>> dso->long_name_len = strlen(name);
>> }
>>
>> -void dso__set_short_name(struct dso *dso, const char *name)
>> +void dso__set_short_name(struct dso *dso, const char *name, bool
>> sname_alloc)
>> {
>> if (name == NULL)
>> return;
>> +if (dso->sname_alloc)
>> +free((char *)dso->short_name);
>> +dso->sname_alloc = sname_alloc;
>> dso->short_name = name;
>> dso->short_name_len = strlen(name);
>> }
>> @@ -426,12 +429,7 @@ static void dso__set_basename(struct dso *dso)
>> if (!base)
>> return;
>>
>> -if (dso->sname_alloc)
>> -free((char *)dso->short_name);
>> -else
>> -dso->sname_alloc = 1;
>> -
>> -dso__set_short_name(dso, base);
>> +dso__set_short_name(dso, base, true);
>> }
>>
>> int dso__name_len(const struct dso *dso)
>> @@ -467,7 +465,7 @@ struct dso *dso__new(const char *name)
>> int i;
>> strcpy(dso->name, name);
>> dso__set_long_name(dso, dso->name);
>> -dso__set_short_name(dso, dso->name);
>> +dso__set_short_name(dso, dso->name, false);
>> for (i = 0; i < MAP__NR_TYPES; ++i)
>> dso->symbols[i] = dso->symbol_names[i] = RB_ROOT;
>> dso->cache = RB_ROOT;
>> diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
>> index 384f2d9..166463e 100644
>> --- a/tools/perf/util/dso.h
>> +++ b/tools/perf/util/dso.h
>> @@ -110,7 +110,7 @@ static inline void dso__set_loaded(struct dso *dso, enum
>> map_type type)
>> struct dso *dso__new(const char *name);
>> void dso__delete(struct dso *dso);
>>
>> -void dso__set_short_name(struct dso *dso, const char *name);
>> +void dso__set_short_name(struct dso *dso, const char *name, bool
>> sname_alloc);
>> void dso__set_long_name(struct dso *dso, char *name);
>>
>> int dso__name_len(const struct dso *dso);
>>
>>
>>>
>>> - Arnaldo
>>>
[Thread 0x770df700 (LWP 29561) exited]
*** Error in `/fast/mingo/tip/tools/perf/perf': munmap_chunk(): invalid
pointer: 0x00587371 ***
=== Backtrace: =
/lib64/libc.so.6[0x3e5907bbe7]
/fast/mingo/tip/tools/perf/perf(dso__delete+0xd9)[0x46da89]
/fast/mingo/tip/tools/perf/perf(machines__exit+0xad)[0x482e7d]
/fast/mingo/tip/tools/perf/perf(perf_session__delete+0xb6)[0x488c66]
/fast/mingo/tip/tools/perf/perf(cmd_top+0xf44)[0x4345f4]
/fast/mingo/tip/tools/perf/perf[0x419f95]
/fast/mingo/tip/tools/perf/perf(main+0x600)[0x419830]
/lib64/libc.so.6(__libc_start_main+0xf5)[0x3e59021b45]
/fast/mingo/tip/tools/perf/perf[0x4198fd]
=== Memory map:
Program received signal SIGABRT, Aborted.
0x003e590359e9 in raise () from /lib64/libc.so.6
Missing separate debuginfos, use: debuginfo-install
audit-libs-2.3.2-1.fc19.x86_64 bzip2-libs-1.0.6-8.fc19.x86_64
elfutils-libelf-0.156-5.fc19.x86_64 elfutils-libs-0.156-5.fc19.x86_64
glibc-2.17-19.fc19.x86_64 libgcc-4.8.2-1.fc19.x86_64
libunwind-1.1-2.fc19.x86_64 nss-softokn-freebl-3.15.2-2.fc19.x86_64
numactl-libs-2.0.8-4.fc19.x86_64 perl-libs-5.16.3-266.fc19.x86_64
python-libs-2.7.5-9.fc19.x86_64 slang-2.2.4-8.fc19.x86_64
Re: [GIT PULL 00/21] perf/core improvements and fixes
Em Tue, Dec 10, 2013 at 02:13:12PM +0200, Adrian Hunter escreveu:
> On 10/12/13 13:44, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Dec 10, 2013 at 12:12:29PM +0100, Ingo Molnar escreveu:
> >>
> >> Hm, I've unpulled it because 'perf top' crashes on exit, in
> >> dso__delete():
> >
> > 495 if (dso->sname_alloc)
> > 496 free((char *)dso->short_name)
> >
> > Yeah, must be that basename() patch from Stephane, I'll work on a fix
> > and resubmit this batch, thanks for the report.
>
> The problem is sname_alloc is not maintained. Perhaps it should be
> set in dso__set_short_name() e.g.
Yeah, sounds better than having all callers manage that thing, quickie,
was this with Stephane's patch applied?
I think it should be done as a prep, then apply a modified version of
Stephanes, that doesn't deal with the alloc flag (more than using 'true'
to say it is a malloc'ed chunk).
- Arnaldo
> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> index 9fae484..54ed980 100644
> --- a/tools/perf/util/dso.c
> +++ b/tools/perf/util/dso.c
> @@ -379,7 +379,7 @@ struct dso *dso__kernel_findnew(struct machine *machine,
> const char *name,
>* processing we had no idea this was the kernel dso.
>*/
> if (dso != NULL) {
> - dso__set_short_name(dso, short_name);
> + dso__set_short_name(dso, short_name, false);
> dso->kernel = dso_type;
> }
>
> @@ -394,10 +394,13 @@ void dso__set_long_name(struct dso *dso, char *name)
> dso->long_name_len = strlen(name);
> }
>
> -void dso__set_short_name(struct dso *dso, const char *name)
> +void dso__set_short_name(struct dso *dso, const char *name, bool sname_alloc)
> {
> if (name == NULL)
> return;
> + if (dso->sname_alloc)
> + free((char *)dso->short_name);
> + dso->sname_alloc = sname_alloc;
> dso->short_name = name;
> dso->short_name_len = strlen(name);
> }
> @@ -426,12 +429,7 @@ static void dso__set_basename(struct dso *dso)
> if (!base)
> return;
>
> - if (dso->sname_alloc)
> - free((char *)dso->short_name);
> - else
> - dso->sname_alloc = 1;
> -
> - dso__set_short_name(dso, base);
> + dso__set_short_name(dso, base, true);
> }
>
> int dso__name_len(const struct dso *dso)
> @@ -467,7 +465,7 @@ struct dso *dso__new(const char *name)
> int i;
> strcpy(dso->name, name);
> dso__set_long_name(dso, dso->name);
> - dso__set_short_name(dso, dso->name);
> + dso__set_short_name(dso, dso->name, false);
> for (i = 0; i < MAP__NR_TYPES; ++i)
> dso->symbols[i] = dso->symbol_names[i] = RB_ROOT;
> dso->cache = RB_ROOT;
> diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
> index 384f2d9..166463e 100644
> --- a/tools/perf/util/dso.h
> +++ b/tools/perf/util/dso.h
> @@ -110,7 +110,7 @@ static inline void dso__set_loaded(struct dso *dso, enum
> map_type type)
> struct dso *dso__new(const char *name);
> void dso__delete(struct dso *dso);
>
> -void dso__set_short_name(struct dso *dso, const char *name);
> +void dso__set_short_name(struct dso *dso, const char *name, bool
> sname_alloc);
> void dso__set_long_name(struct dso *dso, char *name);
>
> int dso__name_len(const struct dso *dso);
>
>
> >
> > - Arnaldo
> >
> >> [Thread 0x770df700 (LWP 29561) exited]
> >> *** Error in `/fast/mingo/tip/tools/perf/perf': munmap_chunk(): invalid
> >> pointer: 0x00587371 ***
> >> === Backtrace: =
> >> /lib64/libc.so.6[0x3e5907bbe7]
> >> /fast/mingo/tip/tools/perf/perf(dso__delete+0xd9)[0x46da89]
> >> /fast/mingo/tip/tools/perf/perf(machines__exit+0xad)[0x482e7d]
> >> /fast/mingo/tip/tools/perf/perf(perf_session__delete+0xb6)[0x488c66]
> >> /fast/mingo/tip/tools/perf/perf(cmd_top+0xf44)[0x4345f4]
> >> /fast/mingo/tip/tools/perf/perf[0x419f95]
> >> /fast/mingo/tip/tools/perf/perf(main+0x600)[0x419830]
> >> /lib64/libc.so.6(__libc_start_main+0xf5)[0x3e59021b45]
> >> /fast/mingo/tip/tools/perf/perf[0x4198fd]
> >> === Memory map:
> >>
> >> Program received signal SIGABRT, Aborted.
> >> 0x003e590359e9 in raise () from /lib64/libc.so.6
> >> Missing separate debuginfos, use: debuginfo-install
> >> audit-libs-2.3.2-1.fc19.x86_64 bzip2-libs-1.0.6-8.fc19.x86_64
> >> elfutils-libelf-0.156-5.fc19.x86_64 elfutils-libs-0.156-5.fc19.x86_64
> >> glibc-2.17-19.fc19.x86_64 libgcc-4.8.2-1.fc19.x86_64
> >> libunwind-1.1-2.fc19.x86_64 nss-softokn-freebl-3.15.2-2.fc19.x86_64
> >> numactl-libs-2.0.8-4.fc19.x86_64 perl-libs-5.16.3-266.fc19.x86_64
> >> python-libs-2.7.5-9.fc19.x86_64 slang-2.2.4-8.fc19.x86_64
> >> xz-libs-5.1.2-4alpha.fc19.x86_64 zlib-1.2.7-10.fc19.x86_64
> >> (gdb)
> >> (gdb) bt
> >> #0 0x003e590359e9 in raise () from /lib64/libc.so.6
> >> #1 0x003e590370f8 in abort () from /lib64/libc.so.6
> >> #2 0x003e59075d17 in
Re: [GIT PULL 00/21] perf/core improvements and fixes
* Arnaldo Carvalho de Melo wrote:
> Em Tue, Dec 10, 2013 at 12:47:57PM +0100, Ingo Molnar escreveu:
> > * Arnaldo Carvalho de Melo wrote:
> > > Em Tue, Dec 10, 2013 at 12:12:29PM +0100, Ingo Molnar escreveu:
> > > > Hm, I've unpulled it because 'perf top' crashes on exit, in
> > > > dso__delete():
>
> > > 495 if (dso->sname_alloc)
> > > 496 free((char *)dso->short_name)
>
> > Btw., instead of trusting flags I'd argue that using the pointer as a
> > flag and clearing the pointer too is a much more robust freeing
> > pattern in general:
>
> > if (dso->short_name) {
> > free(dso->short_name);
> > dso->short_name = NULL;
> > }
> >
> > or so ...
>
> This is not an unusual idiom, if you look at
> tools/perf/util/ev{list,sel}.c, for instance, you'll see it in many
> destructors.
>
> In this case there is a micro optimization where sometimes the
> shortname is just a pointer to the tail part of the long name, hence
> the flag.
Sounds fair.
[btw., a tiny nit remains: the cast is probably unnecessary, free()
will take any pointer.]
Thanks,
Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
On 10/12/13 13:44, Arnaldo Carvalho de Melo wrote:
> Em Tue, Dec 10, 2013 at 12:12:29PM +0100, Ingo Molnar escreveu:
>>
>> Hm, I've unpulled it because 'perf top' crashes on exit, in
>> dso__delete():
>
> 495 if (dso->sname_alloc)
> 496 free((char *)dso->short_name)
>
> Yeah, must be that basename() patch from Stephane, I'll work on a fix
> and resubmit this batch, thanks for the report.
The problem is sname_alloc is not maintained. Perhaps it should be
set in dso__set_short_name() e.g.
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 9fae484..54ed980 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -379,7 +379,7 @@ struct dso *dso__kernel_findnew(struct machine *machine,
const char *name,
* processing we had no idea this was the kernel dso.
*/
if (dso != NULL) {
- dso__set_short_name(dso, short_name);
+ dso__set_short_name(dso, short_name, false);
dso->kernel = dso_type;
}
@@ -394,10 +394,13 @@ void dso__set_long_name(struct dso *dso, char *name)
dso->long_name_len = strlen(name);
}
-void dso__set_short_name(struct dso *dso, const char *name)
+void dso__set_short_name(struct dso *dso, const char *name, bool sname_alloc)
{
if (name == NULL)
return;
+ if (dso->sname_alloc)
+ free((char *)dso->short_name);
+ dso->sname_alloc = sname_alloc;
dso->short_name = name;
dso->short_name_len = strlen(name);
}
@@ -426,12 +429,7 @@ static void dso__set_basename(struct dso *dso)
if (!base)
return;
- if (dso->sname_alloc)
- free((char *)dso->short_name);
- else
- dso->sname_alloc = 1;
-
- dso__set_short_name(dso, base);
+ dso__set_short_name(dso, base, true);
}
int dso__name_len(const struct dso *dso)
@@ -467,7 +465,7 @@ struct dso *dso__new(const char *name)
int i;
strcpy(dso->name, name);
dso__set_long_name(dso, dso->name);
- dso__set_short_name(dso, dso->name);
+ dso__set_short_name(dso, dso->name, false);
for (i = 0; i < MAP__NR_TYPES; ++i)
dso->symbols[i] = dso->symbol_names[i] = RB_ROOT;
dso->cache = RB_ROOT;
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index 384f2d9..166463e 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -110,7 +110,7 @@ static inline void dso__set_loaded(struct dso *dso, enum
map_type type)
struct dso *dso__new(const char *name);
void dso__delete(struct dso *dso);
-void dso__set_short_name(struct dso *dso, const char *name);
+void dso__set_short_name(struct dso *dso, const char *name, bool sname_alloc);
void dso__set_long_name(struct dso *dso, char *name);
int dso__name_len(const struct dso *dso);
>
> - Arnaldo
>
>> [Thread 0x770df700 (LWP 29561) exited]
>> *** Error in `/fast/mingo/tip/tools/perf/perf': munmap_chunk(): invalid
>> pointer: 0x00587371 ***
>> === Backtrace: =
>> /lib64/libc.so.6[0x3e5907bbe7]
>> /fast/mingo/tip/tools/perf/perf(dso__delete+0xd9)[0x46da89]
>> /fast/mingo/tip/tools/perf/perf(machines__exit+0xad)[0x482e7d]
>> /fast/mingo/tip/tools/perf/perf(perf_session__delete+0xb6)[0x488c66]
>> /fast/mingo/tip/tools/perf/perf(cmd_top+0xf44)[0x4345f4]
>> /fast/mingo/tip/tools/perf/perf[0x419f95]
>> /fast/mingo/tip/tools/perf/perf(main+0x600)[0x419830]
>> /lib64/libc.so.6(__libc_start_main+0xf5)[0x3e59021b45]
>> /fast/mingo/tip/tools/perf/perf[0x4198fd]
>> === Memory map:
>>
>> Program received signal SIGABRT, Aborted.
>> 0x003e590359e9 in raise () from /lib64/libc.so.6
>> Missing separate debuginfos, use: debuginfo-install
>> audit-libs-2.3.2-1.fc19.x86_64 bzip2-libs-1.0.6-8.fc19.x86_64
>> elfutils-libelf-0.156-5.fc19.x86_64 elfutils-libs-0.156-5.fc19.x86_64
>> glibc-2.17-19.fc19.x86_64 libgcc-4.8.2-1.fc19.x86_64
>> libunwind-1.1-2.fc19.x86_64 nss-softokn-freebl-3.15.2-2.fc19.x86_64
>> numactl-libs-2.0.8-4.fc19.x86_64 perl-libs-5.16.3-266.fc19.x86_64
>> python-libs-2.7.5-9.fc19.x86_64 slang-2.2.4-8.fc19.x86_64
>> xz-libs-5.1.2-4alpha.fc19.x86_64 zlib-1.2.7-10.fc19.x86_64
>> (gdb)
>> (gdb) bt
>> #0 0x003e590359e9 in raise () from /lib64/libc.so.6
>> #1 0x003e590370f8 in abort () from /lib64/libc.so.6
>> #2 0x003e59075d17 in __libc_message () from /lib64/libc.so.6
>> #3 0x003e5907bbe7 in malloc_printerr () from /lib64/libc.so.6
>> #4 0x0046da89 in dso__delete (dso=0x8e46f0) at util/dso.c:496
>> #5 0x00482e7d in dsos__delete (dsos=0x8e4490) at util/machine.c:72
>> #6 machine__exit (machine=) at util/machine.c:103
>> #7 machines__exit (machines=machines@entry=0x8e4438) at util/machine.c:123
>> #8 0x00488c66 in perf_session__delete (session=0x8e4360) at
>> util/session.c:155
>> #9 0x004345f4 in __cmd_top
Re: [GIT PULL 00/21] perf/core improvements and fixes
Em Tue, Dec 10, 2013 at 12:47:57PM +0100, Ingo Molnar escreveu:
> * Arnaldo Carvalho de Melo wrote:
> > Em Tue, Dec 10, 2013 at 12:12:29PM +0100, Ingo Molnar escreveu:
> > > Hm, I've unpulled it because 'perf top' crashes on exit, in
> > > dso__delete():
> > 495 if (dso->sname_alloc)
> > 496 free((char *)dso->short_name)
> Btw., instead of trusting flags I'd argue that using the pointer as a
> flag and clearing the pointer too is a much more robust freeing
> pattern in general:
> if (dso->short_name) {
> free(dso->short_name);
> dso->short_name = NULL;
> }
>
> or so ...
This is not an unusual idiom, if you look at tools/perf/util/ev{list,sel}.c,
for instance, you'll see it in many destructors.
In this case there is a micro optimization where sometimes the shortname
is just a pointer to the tail part of the long name, hence the flag.
- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
* Arnaldo Carvalho de Melo wrote:
> Em Tue, Dec 10, 2013 at 12:12:29PM +0100, Ingo Molnar escreveu:
> >
> > Hm, I've unpulled it because 'perf top' crashes on exit, in
> > dso__delete():
>
> 495 if (dso->sname_alloc)
> 496 free((char *)dso->short_name)
Btw., instead of trusting flags I'd argue that using the pointer as a
flag and clearing the pointer too is a much more robust freeing
pattern in general:
if (dso->short_name) {
free(dso->short_name);
dso->short_name = NULL;
}
or so ...
Thanks,
Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
Em Tue, Dec 10, 2013 at 12:12:29PM +0100, Ingo Molnar escreveu: > > Hm, I've unpulled it because 'perf top' crashes on exit, in > dso__delete(): 495 if (dso->sname_alloc) 496 free((char *)dso->short_name) Yeah, must be that basename() patch from Stephane, I'll work on a fix and resubmit this batch, thanks for the report. - Arnaldo > [Thread 0x770df700 (LWP 29561) exited] > *** Error in `/fast/mingo/tip/tools/perf/perf': munmap_chunk(): invalid > pointer: 0x00587371 *** > === Backtrace: = > /lib64/libc.so.6[0x3e5907bbe7] > /fast/mingo/tip/tools/perf/perf(dso__delete+0xd9)[0x46da89] > /fast/mingo/tip/tools/perf/perf(machines__exit+0xad)[0x482e7d] > /fast/mingo/tip/tools/perf/perf(perf_session__delete+0xb6)[0x488c66] > /fast/mingo/tip/tools/perf/perf(cmd_top+0xf44)[0x4345f4] > /fast/mingo/tip/tools/perf/perf[0x419f95] > /fast/mingo/tip/tools/perf/perf(main+0x600)[0x419830] > /lib64/libc.so.6(__libc_start_main+0xf5)[0x3e59021b45] > /fast/mingo/tip/tools/perf/perf[0x4198fd] > === Memory map: > > Program received signal SIGABRT, Aborted. > 0x003e590359e9 in raise () from /lib64/libc.so.6 > Missing separate debuginfos, use: debuginfo-install > audit-libs-2.3.2-1.fc19.x86_64 bzip2-libs-1.0.6-8.fc19.x86_64 > elfutils-libelf-0.156-5.fc19.x86_64 elfutils-libs-0.156-5.fc19.x86_64 > glibc-2.17-19.fc19.x86_64 libgcc-4.8.2-1.fc19.x86_64 > libunwind-1.1-2.fc19.x86_64 nss-softokn-freebl-3.15.2-2.fc19.x86_64 > numactl-libs-2.0.8-4.fc19.x86_64 perl-libs-5.16.3-266.fc19.x86_64 > python-libs-2.7.5-9.fc19.x86_64 slang-2.2.4-8.fc19.x86_64 > xz-libs-5.1.2-4alpha.fc19.x86_64 zlib-1.2.7-10.fc19.x86_64 > (gdb) > (gdb) bt > #0 0x003e590359e9 in raise () from /lib64/libc.so.6 > #1 0x003e590370f8 in abort () from /lib64/libc.so.6 > #2 0x003e59075d17 in __libc_message () from /lib64/libc.so.6 > #3 0x003e5907bbe7 in malloc_printerr () from /lib64/libc.so.6 > #4 0x0046da89 in dso__delete (dso=0x8e46f0) at util/dso.c:496 > #5 0x00482e7d in dsos__delete (dsos=0x8e4490) at util/machine.c:72 > #6 machine__exit (machine=) at util/machine.c:103 > #7 machines__exit (machines=machines@entry=0x8e4438) at util/machine.c:123 > #8 0x00488c66 in perf_session__delete (session=0x8e4360) at > util/session.c:155 > #9 0x004345f4 in __cmd_top (top=0x7fffb140) at builtin-top.c:985 > #10 cmd_top (argc=, argv=, prefix= out>) at builtin-top.c:1210 > #11 0x00419f95 in run_builtin (p=p@entry=0x7ece88 , > argc=argc@entry=2, argv=argv@entry=0x7fffe420) at perf.c:319 > #12 0x00419830 in handle_internal_command (argv=0x7fffe420, > argc=2) at perf.c:376 > #13 run_argv (argv=0x7fffe220, argcp=0x7fffe22c) at perf.c:420 > #14 main (argc=2, argv=0x7fffe420) at perf.c:529 > (gdb) > > Running it on an up-to-date installation of Fedora 19. > > Thanks, > > Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
Hm, I've unpulled it because 'perf top' crashes on exit, in dso__delete(): [Thread 0x770df700 (LWP 29561) exited] *** Error in `/fast/mingo/tip/tools/perf/perf': munmap_chunk(): invalid pointer: 0x00587371 *** === Backtrace: = /lib64/libc.so.6[0x3e5907bbe7] /fast/mingo/tip/tools/perf/perf(dso__delete+0xd9)[0x46da89] /fast/mingo/tip/tools/perf/perf(machines__exit+0xad)[0x482e7d] /fast/mingo/tip/tools/perf/perf(perf_session__delete+0xb6)[0x488c66] /fast/mingo/tip/tools/perf/perf(cmd_top+0xf44)[0x4345f4] /fast/mingo/tip/tools/perf/perf[0x419f95] /fast/mingo/tip/tools/perf/perf(main+0x600)[0x419830] /lib64/libc.so.6(__libc_start_main+0xf5)[0x3e59021b45] /fast/mingo/tip/tools/perf/perf[0x4198fd] === Memory map: Program received signal SIGABRT, Aborted. 0x003e590359e9 in raise () from /lib64/libc.so.6 Missing separate debuginfos, use: debuginfo-install audit-libs-2.3.2-1.fc19.x86_64 bzip2-libs-1.0.6-8.fc19.x86_64 elfutils-libelf-0.156-5.fc19.x86_64 elfutils-libs-0.156-5.fc19.x86_64 glibc-2.17-19.fc19.x86_64 libgcc-4.8.2-1.fc19.x86_64 libunwind-1.1-2.fc19.x86_64 nss-softokn-freebl-3.15.2-2.fc19.x86_64 numactl-libs-2.0.8-4.fc19.x86_64 perl-libs-5.16.3-266.fc19.x86_64 python-libs-2.7.5-9.fc19.x86_64 slang-2.2.4-8.fc19.x86_64 xz-libs-5.1.2-4alpha.fc19.x86_64 zlib-1.2.7-10.fc19.x86_64 (gdb) (gdb) bt #0 0x003e590359e9 in raise () from /lib64/libc.so.6 #1 0x003e590370f8 in abort () from /lib64/libc.so.6 #2 0x003e59075d17 in __libc_message () from /lib64/libc.so.6 #3 0x003e5907bbe7 in malloc_printerr () from /lib64/libc.so.6 #4 0x0046da89 in dso__delete (dso=0x8e46f0) at util/dso.c:496 #5 0x00482e7d in dsos__delete (dsos=0x8e4490) at util/machine.c:72 #6 machine__exit (machine=) at util/machine.c:103 #7 machines__exit (machines=machines@entry=0x8e4438) at util/machine.c:123 #8 0x00488c66 in perf_session__delete (session=0x8e4360) at util/session.c:155 #9 0x004345f4 in __cmd_top (top=0x7fffb140) at builtin-top.c:985 #10 cmd_top (argc=, argv=, prefix=) at builtin-top.c:1210 #11 0x00419f95 in run_builtin (p=p@entry=0x7ece88 , argc=argc@entry=2, argv=argv@entry=0x7fffe420) at perf.c:319 #12 0x00419830 in handle_internal_command (argv=0x7fffe420, argc=2) at perf.c:376 #13 run_argv (argv=0x7fffe220, argcp=0x7fffe22c) at perf.c:420 #14 main (argc=2, argv=0x7fffe420) at perf.c:529 (gdb) Running it on an up-to-date installation of Fedora 19. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
* Arnaldo Carvalho de Melo wrote: > From: Arnaldo Carvalho de Melo > > Hi Ingo, > > Please consider pulling, > > Best Regards, > > -Arnaldo > > The following changes since commit 6d65894bc028d0342829ea1e64c9e9efad571124: > > tools lib traceevent: Update kvm plugin with is_writable_pte helper > (2013-12-04 15:38:14 -0300) > > are available in the git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux > tags/perf-core-for-mingo > > for you to fetch changes up to e993d10caeb6dca690dbaf86e1981ba240d1414a: > > perf symbols: fix bug in usage of the basename() function (2013-12-09 > 15:41:59 -0300) > > > perf/core improvements and fixes: > > . Add an option in 'perf script' to print the source line number, from Adrian > Hunter > > . Fix symoff printing in callchains in 'perf script', from Adrian Hunter. > > . Assorted mmap_pages handling fixes, from Adrian Hunter. > > . Fix summary percentage when processing files in 'perf trace', fom David > Ahern. > > . Handle old kernels where the "raw_syscalls" tracepoints were called plan > "syscalls", > in 'perf trace', from David Ahern. > > . Several man pages typo fixes from Dongsheng Yang. > > . Add '-v' option to 'perf kvm', from Dongsheng Yang. > > . Make perf kvm diff support --guestmount, from Dongsheng Yang. > > . Get rid of several die() calls in libtraceevent, from Namhyung Kim. > > . Use basename() in a more robust way, to avoid problems related to different > system library implementations for that function, from Stephane Eranian. > > Signed-off-by: Arnaldo Carvalho de Melo > > > Adrian Hunter (6): > perf script: Fix symoff printing in callchains > perf script: Add an option to print the source line number > perf record: Fix display of incorrect mmap pages > perf evlist: Remove unnecessary parentheses > perf evlist: Fix max mmap_pages > perf evlist: Fix mmap pages rounding to power of 2 > > David Ahern (2): > perf trace: Add support for syscalls vs raw_syscalls > perf trace: Fix summary percentage when processing files > > Dongsheng Yang (6): > perf kvm: Introduce option -v for perf kvm command. > perf kvm: Fix bug in 'stat report' > perf archive: Remove duplicated 'runs' in man page > perf annotate: Fix typo > perf kvm: Move code to generate filename for perf-kvm to function. > perf kvm: Make perf kvm diff support --guestmount. > > Namhyung Kim (5): > tools lib traceevent: Get rid of malloc_or_die() in > pevent_filter_alloc() > tools lib traceevent: Get rid of malloc_or_die() in add_event() > tools lib traceevent: Get rid of die() in create_arg_item() > tools lib traceevent: Get rid of malloc_or_die() in > pevent_filter_add_filter_str() > tools lib traceevent: Get rid of die() in pevent_filter_clear_trivial() > > Stephane Eranian (1): > perf symbols: fix bug in usage of the basename() function > > Steven Rostedt (1): > tools lib traceevent: Report better error message on bad function args > > tools/lib/traceevent/event-parse.c| 28 +-- > tools/lib/traceevent/event-parse.h| 2 +- > tools/lib/traceevent/parse-filter.c | 57 > --- > tools/perf/Documentation/perf-archive.txt | 6 ++-- > tools/perf/Documentation/perf-kvm.txt | 7 ++-- > tools/perf/Documentation/perf-script.txt | 2 +- > tools/perf/builtin-annotate.c | 2 +- > tools/perf/builtin-diff.c | 3 +- > tools/perf/builtin-kvm.c | 11 +++--- > tools/perf/builtin-record.c | 2 +- > tools/perf/builtin-script.c | 10 ++ > tools/perf/builtin-trace.c| 32 +++-- > tools/perf/util/dso.c | 29 +++- > tools/perf/util/evlist.c | 10 +++--- > tools/perf/util/map.c | 17 + > tools/perf/util/map.h | 2 ++ > tools/perf/util/session.c | 15 +++- > tools/perf/util/session.h | 1 + > tools/perf/util/util.c| 14 > tools/perf/util/util.h| 14 > 20 files changed, 214 insertions(+), 50 deletions(-) Pulled, thanks Arnaldo! There's one detail I noticed about the recent trace-plugin changes: comet:~/tip/tools/perf> make install BUILD: Doing 'make -j12' parallel build SUBDIR Documentation INSTALL Documentation-man INSTALL GTK UI SUBDIR /home/mingo/tip/tools/lib/traceevent/ INSTALL binaries INSTALL plugin_jbd2.so INSTALL plugin_hrtimer.so INSTALL plugin_kmem.so INSTALL plugin_kvm.so INSTALL plugin_mac80211.so INSTALL plugin_sched_switch.so INSTALL plugin_function.so INSTALL plugin_xen.so
Re: [GIT PULL 00/21] perf/core improvements and fixes
* Arnaldo Carvalho de Melo a...@infradead.org wrote: From: Arnaldo Carvalho de Melo a...@ghostprotocols.net Hi Ingo, Please consider pulling, Best Regards, -Arnaldo The following changes since commit 6d65894bc028d0342829ea1e64c9e9efad571124: tools lib traceevent: Update kvm plugin with is_writable_pte helper (2013-12-04 15:38:14 -0300) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux tags/perf-core-for-mingo for you to fetch changes up to e993d10caeb6dca690dbaf86e1981ba240d1414a: perf symbols: fix bug in usage of the basename() function (2013-12-09 15:41:59 -0300) perf/core improvements and fixes: . Add an option in 'perf script' to print the source line number, from Adrian Hunter . Fix symoff printing in callchains in 'perf script', from Adrian Hunter. . Assorted mmap_pages handling fixes, from Adrian Hunter. . Fix summary percentage when processing files in 'perf trace', fom David Ahern. . Handle old kernels where the raw_syscalls tracepoints were called plan syscalls, in 'perf trace', from David Ahern. . Several man pages typo fixes from Dongsheng Yang. . Add '-v' option to 'perf kvm', from Dongsheng Yang. . Make perf kvm diff support --guestmount, from Dongsheng Yang. . Get rid of several die() calls in libtraceevent, from Namhyung Kim. . Use basename() in a more robust way, to avoid problems related to different system library implementations for that function, from Stephane Eranian. Signed-off-by: Arnaldo Carvalho de Melo a...@redhat.com Adrian Hunter (6): perf script: Fix symoff printing in callchains perf script: Add an option to print the source line number perf record: Fix display of incorrect mmap pages perf evlist: Remove unnecessary parentheses perf evlist: Fix max mmap_pages perf evlist: Fix mmap pages rounding to power of 2 David Ahern (2): perf trace: Add support for syscalls vs raw_syscalls perf trace: Fix summary percentage when processing files Dongsheng Yang (6): perf kvm: Introduce option -v for perf kvm command. perf kvm: Fix bug in 'stat report' perf archive: Remove duplicated 'runs' in man page perf annotate: Fix typo perf kvm: Move code to generate filename for perf-kvm to function. perf kvm: Make perf kvm diff support --guestmount. Namhyung Kim (5): tools lib traceevent: Get rid of malloc_or_die() in pevent_filter_alloc() tools lib traceevent: Get rid of malloc_or_die() in add_event() tools lib traceevent: Get rid of die() in create_arg_item() tools lib traceevent: Get rid of malloc_or_die() in pevent_filter_add_filter_str() tools lib traceevent: Get rid of die() in pevent_filter_clear_trivial() Stephane Eranian (1): perf symbols: fix bug in usage of the basename() function Steven Rostedt (1): tools lib traceevent: Report better error message on bad function args tools/lib/traceevent/event-parse.c| 28 +-- tools/lib/traceevent/event-parse.h| 2 +- tools/lib/traceevent/parse-filter.c | 57 --- tools/perf/Documentation/perf-archive.txt | 6 ++-- tools/perf/Documentation/perf-kvm.txt | 7 ++-- tools/perf/Documentation/perf-script.txt | 2 +- tools/perf/builtin-annotate.c | 2 +- tools/perf/builtin-diff.c | 3 +- tools/perf/builtin-kvm.c | 11 +++--- tools/perf/builtin-record.c | 2 +- tools/perf/builtin-script.c | 10 ++ tools/perf/builtin-trace.c| 32 +++-- tools/perf/util/dso.c | 29 +++- tools/perf/util/evlist.c | 10 +++--- tools/perf/util/map.c | 17 + tools/perf/util/map.h | 2 ++ tools/perf/util/session.c | 15 +++- tools/perf/util/session.h | 1 + tools/perf/util/util.c| 14 tools/perf/util/util.h| 14 20 files changed, 214 insertions(+), 50 deletions(-) Pulled, thanks Arnaldo! There's one detail I noticed about the recent trace-plugin changes: comet:~/tip/tools/perf make install BUILD: Doing 'make -j12' parallel build SUBDIR Documentation INSTALL Documentation-man INSTALL GTK UI SUBDIR /home/mingo/tip/tools/lib/traceevent/ INSTALL binaries INSTALL plugin_jbd2.so INSTALL plugin_hrtimer.so INSTALL plugin_kmem.so INSTALL plugin_kvm.so INSTALL plugin_mac80211.so INSTALL plugin_sched_switch.so INSTALL plugin_function.so INSTALL plugin_xen.so INSTALL plugin_scsi.so INSTALL plugin_cfg80211.so
Re: [GIT PULL 00/21] perf/core improvements and fixes
Hm, I've unpulled it because 'perf top' crashes on exit, in dso__delete(): [Thread 0x770df700 (LWP 29561) exited] *** Error in `/fast/mingo/tip/tools/perf/perf': munmap_chunk(): invalid pointer: 0x00587371 *** === Backtrace: = /lib64/libc.so.6[0x3e5907bbe7] /fast/mingo/tip/tools/perf/perf(dso__delete+0xd9)[0x46da89] /fast/mingo/tip/tools/perf/perf(machines__exit+0xad)[0x482e7d] /fast/mingo/tip/tools/perf/perf(perf_session__delete+0xb6)[0x488c66] /fast/mingo/tip/tools/perf/perf(cmd_top+0xf44)[0x4345f4] /fast/mingo/tip/tools/perf/perf[0x419f95] /fast/mingo/tip/tools/perf/perf(main+0x600)[0x419830] /lib64/libc.so.6(__libc_start_main+0xf5)[0x3e59021b45] /fast/mingo/tip/tools/perf/perf[0x4198fd] === Memory map: Program received signal SIGABRT, Aborted. 0x003e590359e9 in raise () from /lib64/libc.so.6 Missing separate debuginfos, use: debuginfo-install audit-libs-2.3.2-1.fc19.x86_64 bzip2-libs-1.0.6-8.fc19.x86_64 elfutils-libelf-0.156-5.fc19.x86_64 elfutils-libs-0.156-5.fc19.x86_64 glibc-2.17-19.fc19.x86_64 libgcc-4.8.2-1.fc19.x86_64 libunwind-1.1-2.fc19.x86_64 nss-softokn-freebl-3.15.2-2.fc19.x86_64 numactl-libs-2.0.8-4.fc19.x86_64 perl-libs-5.16.3-266.fc19.x86_64 python-libs-2.7.5-9.fc19.x86_64 slang-2.2.4-8.fc19.x86_64 xz-libs-5.1.2-4alpha.fc19.x86_64 zlib-1.2.7-10.fc19.x86_64 (gdb) (gdb) bt #0 0x003e590359e9 in raise () from /lib64/libc.so.6 #1 0x003e590370f8 in abort () from /lib64/libc.so.6 #2 0x003e59075d17 in __libc_message () from /lib64/libc.so.6 #3 0x003e5907bbe7 in malloc_printerr () from /lib64/libc.so.6 #4 0x0046da89 in dso__delete (dso=0x8e46f0) at util/dso.c:496 #5 0x00482e7d in dsos__delete (dsos=0x8e4490) at util/machine.c:72 #6 machine__exit (machine=optimized out) at util/machine.c:103 #7 machines__exit (machines=machines@entry=0x8e4438) at util/machine.c:123 #8 0x00488c66 in perf_session__delete (session=0x8e4360) at util/session.c:155 #9 0x004345f4 in __cmd_top (top=0x7fffb140) at builtin-top.c:985 #10 cmd_top (argc=optimized out, argv=optimized out, prefix=optimized out) at builtin-top.c:1210 #11 0x00419f95 in run_builtin (p=p@entry=0x7ece88 commands+264, argc=argc@entry=2, argv=argv@entry=0x7fffe420) at perf.c:319 #12 0x00419830 in handle_internal_command (argv=0x7fffe420, argc=2) at perf.c:376 #13 run_argv (argv=0x7fffe220, argcp=0x7fffe22c) at perf.c:420 #14 main (argc=2, argv=0x7fffe420) at perf.c:529 (gdb) Running it on an up-to-date installation of Fedora 19. Thanks, Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
Em Tue, Dec 10, 2013 at 12:12:29PM +0100, Ingo Molnar escreveu: Hm, I've unpulled it because 'perf top' crashes on exit, in dso__delete(): 495 if (dso-sname_alloc) 496 free((char *)dso-short_name) Yeah, must be that basename() patch from Stephane, I'll work on a fix and resubmit this batch, thanks for the report. - Arnaldo [Thread 0x770df700 (LWP 29561) exited] *** Error in `/fast/mingo/tip/tools/perf/perf': munmap_chunk(): invalid pointer: 0x00587371 *** === Backtrace: = /lib64/libc.so.6[0x3e5907bbe7] /fast/mingo/tip/tools/perf/perf(dso__delete+0xd9)[0x46da89] /fast/mingo/tip/tools/perf/perf(machines__exit+0xad)[0x482e7d] /fast/mingo/tip/tools/perf/perf(perf_session__delete+0xb6)[0x488c66] /fast/mingo/tip/tools/perf/perf(cmd_top+0xf44)[0x4345f4] /fast/mingo/tip/tools/perf/perf[0x419f95] /fast/mingo/tip/tools/perf/perf(main+0x600)[0x419830] /lib64/libc.so.6(__libc_start_main+0xf5)[0x3e59021b45] /fast/mingo/tip/tools/perf/perf[0x4198fd] === Memory map: Program received signal SIGABRT, Aborted. 0x003e590359e9 in raise () from /lib64/libc.so.6 Missing separate debuginfos, use: debuginfo-install audit-libs-2.3.2-1.fc19.x86_64 bzip2-libs-1.0.6-8.fc19.x86_64 elfutils-libelf-0.156-5.fc19.x86_64 elfutils-libs-0.156-5.fc19.x86_64 glibc-2.17-19.fc19.x86_64 libgcc-4.8.2-1.fc19.x86_64 libunwind-1.1-2.fc19.x86_64 nss-softokn-freebl-3.15.2-2.fc19.x86_64 numactl-libs-2.0.8-4.fc19.x86_64 perl-libs-5.16.3-266.fc19.x86_64 python-libs-2.7.5-9.fc19.x86_64 slang-2.2.4-8.fc19.x86_64 xz-libs-5.1.2-4alpha.fc19.x86_64 zlib-1.2.7-10.fc19.x86_64 (gdb) (gdb) bt #0 0x003e590359e9 in raise () from /lib64/libc.so.6 #1 0x003e590370f8 in abort () from /lib64/libc.so.6 #2 0x003e59075d17 in __libc_message () from /lib64/libc.so.6 #3 0x003e5907bbe7 in malloc_printerr () from /lib64/libc.so.6 #4 0x0046da89 in dso__delete (dso=0x8e46f0) at util/dso.c:496 #5 0x00482e7d in dsos__delete (dsos=0x8e4490) at util/machine.c:72 #6 machine__exit (machine=optimized out) at util/machine.c:103 #7 machines__exit (machines=machines@entry=0x8e4438) at util/machine.c:123 #8 0x00488c66 in perf_session__delete (session=0x8e4360) at util/session.c:155 #9 0x004345f4 in __cmd_top (top=0x7fffb140) at builtin-top.c:985 #10 cmd_top (argc=optimized out, argv=optimized out, prefix=optimized out) at builtin-top.c:1210 #11 0x00419f95 in run_builtin (p=p@entry=0x7ece88 commands+264, argc=argc@entry=2, argv=argv@entry=0x7fffe420) at perf.c:319 #12 0x00419830 in handle_internal_command (argv=0x7fffe420, argc=2) at perf.c:376 #13 run_argv (argv=0x7fffe220, argcp=0x7fffe22c) at perf.c:420 #14 main (argc=2, argv=0x7fffe420) at perf.c:529 (gdb) Running it on an up-to-date installation of Fedora 19. Thanks, Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
* Arnaldo Carvalho de Melo a...@ghostprotocols.net wrote:
Em Tue, Dec 10, 2013 at 12:12:29PM +0100, Ingo Molnar escreveu:
Hm, I've unpulled it because 'perf top' crashes on exit, in
dso__delete():
495 if (dso-sname_alloc)
496 free((char *)dso-short_name)
Btw., instead of trusting flags I'd argue that using the pointer as a
flag and clearing the pointer too is a much more robust freeing
pattern in general:
if (dso-short_name) {
free(dso-short_name);
dso-short_name = NULL;
}
or so ...
Thanks,
Ingo
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
Em Tue, Dec 10, 2013 at 12:47:57PM +0100, Ingo Molnar escreveu:
* Arnaldo Carvalho de Melo a...@ghostprotocols.net wrote:
Em Tue, Dec 10, 2013 at 12:12:29PM +0100, Ingo Molnar escreveu:
Hm, I've unpulled it because 'perf top' crashes on exit, in
dso__delete():
495 if (dso-sname_alloc)
496 free((char *)dso-short_name)
Btw., instead of trusting flags I'd argue that using the pointer as a
flag and clearing the pointer too is a much more robust freeing
pattern in general:
if (dso-short_name) {
free(dso-short_name);
dso-short_name = NULL;
}
or so ...
This is not an unusual idiom, if you look at tools/perf/util/ev{list,sel}.c,
for instance, you'll see it in many destructors.
In this case there is a micro optimization where sometimes the shortname
is just a pointer to the tail part of the long name, hence the flag.
- Arnaldo
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
On 10/12/13 13:44, Arnaldo Carvalho de Melo wrote:
Em Tue, Dec 10, 2013 at 12:12:29PM +0100, Ingo Molnar escreveu:
Hm, I've unpulled it because 'perf top' crashes on exit, in
dso__delete():
495 if (dso-sname_alloc)
496 free((char *)dso-short_name)
Yeah, must be that basename() patch from Stephane, I'll work on a fix
and resubmit this batch, thanks for the report.
The problem is sname_alloc is not maintained. Perhaps it should be
set in dso__set_short_name() e.g.
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 9fae484..54ed980 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -379,7 +379,7 @@ struct dso *dso__kernel_findnew(struct machine *machine,
const char *name,
* processing we had no idea this was the kernel dso.
*/
if (dso != NULL) {
- dso__set_short_name(dso, short_name);
+ dso__set_short_name(dso, short_name, false);
dso-kernel = dso_type;
}
@@ -394,10 +394,13 @@ void dso__set_long_name(struct dso *dso, char *name)
dso-long_name_len = strlen(name);
}
-void dso__set_short_name(struct dso *dso, const char *name)
+void dso__set_short_name(struct dso *dso, const char *name, bool sname_alloc)
{
if (name == NULL)
return;
+ if (dso-sname_alloc)
+ free((char *)dso-short_name);
+ dso-sname_alloc = sname_alloc;
dso-short_name = name;
dso-short_name_len = strlen(name);
}
@@ -426,12 +429,7 @@ static void dso__set_basename(struct dso *dso)
if (!base)
return;
- if (dso-sname_alloc)
- free((char *)dso-short_name);
- else
- dso-sname_alloc = 1;
-
- dso__set_short_name(dso, base);
+ dso__set_short_name(dso, base, true);
}
int dso__name_len(const struct dso *dso)
@@ -467,7 +465,7 @@ struct dso *dso__new(const char *name)
int i;
strcpy(dso-name, name);
dso__set_long_name(dso, dso-name);
- dso__set_short_name(dso, dso-name);
+ dso__set_short_name(dso, dso-name, false);
for (i = 0; i MAP__NR_TYPES; ++i)
dso-symbols[i] = dso-symbol_names[i] = RB_ROOT;
dso-cache = RB_ROOT;
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index 384f2d9..166463e 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -110,7 +110,7 @@ static inline void dso__set_loaded(struct dso *dso, enum
map_type type)
struct dso *dso__new(const char *name);
void dso__delete(struct dso *dso);
-void dso__set_short_name(struct dso *dso, const char *name);
+void dso__set_short_name(struct dso *dso, const char *name, bool sname_alloc);
void dso__set_long_name(struct dso *dso, char *name);
int dso__name_len(const struct dso *dso);
- Arnaldo
[Thread 0x770df700 (LWP 29561) exited]
*** Error in `/fast/mingo/tip/tools/perf/perf': munmap_chunk(): invalid
pointer: 0x00587371 ***
=== Backtrace: =
/lib64/libc.so.6[0x3e5907bbe7]
/fast/mingo/tip/tools/perf/perf(dso__delete+0xd9)[0x46da89]
/fast/mingo/tip/tools/perf/perf(machines__exit+0xad)[0x482e7d]
/fast/mingo/tip/tools/perf/perf(perf_session__delete+0xb6)[0x488c66]
/fast/mingo/tip/tools/perf/perf(cmd_top+0xf44)[0x4345f4]
/fast/mingo/tip/tools/perf/perf[0x419f95]
/fast/mingo/tip/tools/perf/perf(main+0x600)[0x419830]
/lib64/libc.so.6(__libc_start_main+0xf5)[0x3e59021b45]
/fast/mingo/tip/tools/perf/perf[0x4198fd]
=== Memory map:
Program received signal SIGABRT, Aborted.
0x003e590359e9 in raise () from /lib64/libc.so.6
Missing separate debuginfos, use: debuginfo-install
audit-libs-2.3.2-1.fc19.x86_64 bzip2-libs-1.0.6-8.fc19.x86_64
elfutils-libelf-0.156-5.fc19.x86_64 elfutils-libs-0.156-5.fc19.x86_64
glibc-2.17-19.fc19.x86_64 libgcc-4.8.2-1.fc19.x86_64
libunwind-1.1-2.fc19.x86_64 nss-softokn-freebl-3.15.2-2.fc19.x86_64
numactl-libs-2.0.8-4.fc19.x86_64 perl-libs-5.16.3-266.fc19.x86_64
python-libs-2.7.5-9.fc19.x86_64 slang-2.2.4-8.fc19.x86_64
xz-libs-5.1.2-4alpha.fc19.x86_64 zlib-1.2.7-10.fc19.x86_64
(gdb)
(gdb) bt
#0 0x003e590359e9 in raise () from /lib64/libc.so.6
#1 0x003e590370f8 in abort () from /lib64/libc.so.6
#2 0x003e59075d17 in __libc_message () from /lib64/libc.so.6
#3 0x003e5907bbe7 in malloc_printerr () from /lib64/libc.so.6
#4 0x0046da89 in dso__delete (dso=0x8e46f0) at util/dso.c:496
#5 0x00482e7d in dsos__delete (dsos=0x8e4490) at util/machine.c:72
#6 machine__exit (machine=optimized out) at util/machine.c:103
#7 machines__exit (machines=machines@entry=0x8e4438) at util/machine.c:123
#8 0x00488c66 in perf_session__delete (session=0x8e4360) at
util/session.c:155
#9 0x004345f4 in __cmd_top (top=0x7fffb140) at builtin-top.c:985
#10 cmd_top (argc=optimized out, argv=optimized out,
Re: [GIT PULL 00/21] perf/core improvements and fixes
* Arnaldo Carvalho de Melo a...@ghostprotocols.net wrote:
Em Tue, Dec 10, 2013 at 12:47:57PM +0100, Ingo Molnar escreveu:
* Arnaldo Carvalho de Melo a...@ghostprotocols.net wrote:
Em Tue, Dec 10, 2013 at 12:12:29PM +0100, Ingo Molnar escreveu:
Hm, I've unpulled it because 'perf top' crashes on exit, in
dso__delete():
495 if (dso-sname_alloc)
496 free((char *)dso-short_name)
Btw., instead of trusting flags I'd argue that using the pointer as a
flag and clearing the pointer too is a much more robust freeing
pattern in general:
if (dso-short_name) {
free(dso-short_name);
dso-short_name = NULL;
}
or so ...
This is not an unusual idiom, if you look at
tools/perf/util/ev{list,sel}.c, for instance, you'll see it in many
destructors.
In this case there is a micro optimization where sometimes the
shortname is just a pointer to the tail part of the long name, hence
the flag.
Sounds fair.
[btw., a tiny nit remains: the cast is probably unnecessary, free()
will take any pointer.]
Thanks,
Ingo
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
Em Tue, Dec 10, 2013 at 02:13:12PM +0200, Adrian Hunter escreveu:
On 10/12/13 13:44, Arnaldo Carvalho de Melo wrote:
Em Tue, Dec 10, 2013 at 12:12:29PM +0100, Ingo Molnar escreveu:
Hm, I've unpulled it because 'perf top' crashes on exit, in
dso__delete():
495 if (dso-sname_alloc)
496 free((char *)dso-short_name)
Yeah, must be that basename() patch from Stephane, I'll work on a fix
and resubmit this batch, thanks for the report.
The problem is sname_alloc is not maintained. Perhaps it should be
set in dso__set_short_name() e.g.
Yeah, sounds better than having all callers manage that thing, quickie,
was this with Stephane's patch applied?
I think it should be done as a prep, then apply a modified version of
Stephanes, that doesn't deal with the alloc flag (more than using 'true'
to say it is a malloc'ed chunk).
- Arnaldo
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 9fae484..54ed980 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -379,7 +379,7 @@ struct dso *dso__kernel_findnew(struct machine *machine,
const char *name,
* processing we had no idea this was the kernel dso.
*/
if (dso != NULL) {
- dso__set_short_name(dso, short_name);
+ dso__set_short_name(dso, short_name, false);
dso-kernel = dso_type;
}
@@ -394,10 +394,13 @@ void dso__set_long_name(struct dso *dso, char *name)
dso-long_name_len = strlen(name);
}
-void dso__set_short_name(struct dso *dso, const char *name)
+void dso__set_short_name(struct dso *dso, const char *name, bool sname_alloc)
{
if (name == NULL)
return;
+ if (dso-sname_alloc)
+ free((char *)dso-short_name);
+ dso-sname_alloc = sname_alloc;
dso-short_name = name;
dso-short_name_len = strlen(name);
}
@@ -426,12 +429,7 @@ static void dso__set_basename(struct dso *dso)
if (!base)
return;
- if (dso-sname_alloc)
- free((char *)dso-short_name);
- else
- dso-sname_alloc = 1;
-
- dso__set_short_name(dso, base);
+ dso__set_short_name(dso, base, true);
}
int dso__name_len(const struct dso *dso)
@@ -467,7 +465,7 @@ struct dso *dso__new(const char *name)
int i;
strcpy(dso-name, name);
dso__set_long_name(dso, dso-name);
- dso__set_short_name(dso, dso-name);
+ dso__set_short_name(dso, dso-name, false);
for (i = 0; i MAP__NR_TYPES; ++i)
dso-symbols[i] = dso-symbol_names[i] = RB_ROOT;
dso-cache = RB_ROOT;
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index 384f2d9..166463e 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -110,7 +110,7 @@ static inline void dso__set_loaded(struct dso *dso, enum
map_type type)
struct dso *dso__new(const char *name);
void dso__delete(struct dso *dso);
-void dso__set_short_name(struct dso *dso, const char *name);
+void dso__set_short_name(struct dso *dso, const char *name, bool
sname_alloc);
void dso__set_long_name(struct dso *dso, char *name);
int dso__name_len(const struct dso *dso);
- Arnaldo
[Thread 0x770df700 (LWP 29561) exited]
*** Error in `/fast/mingo/tip/tools/perf/perf': munmap_chunk(): invalid
pointer: 0x00587371 ***
=== Backtrace: =
/lib64/libc.so.6[0x3e5907bbe7]
/fast/mingo/tip/tools/perf/perf(dso__delete+0xd9)[0x46da89]
/fast/mingo/tip/tools/perf/perf(machines__exit+0xad)[0x482e7d]
/fast/mingo/tip/tools/perf/perf(perf_session__delete+0xb6)[0x488c66]
/fast/mingo/tip/tools/perf/perf(cmd_top+0xf44)[0x4345f4]
/fast/mingo/tip/tools/perf/perf[0x419f95]
/fast/mingo/tip/tools/perf/perf(main+0x600)[0x419830]
/lib64/libc.so.6(__libc_start_main+0xf5)[0x3e59021b45]
/fast/mingo/tip/tools/perf/perf[0x4198fd]
=== Memory map:
Program received signal SIGABRT, Aborted.
0x003e590359e9 in raise () from /lib64/libc.so.6
Missing separate debuginfos, use: debuginfo-install
audit-libs-2.3.2-1.fc19.x86_64 bzip2-libs-1.0.6-8.fc19.x86_64
elfutils-libelf-0.156-5.fc19.x86_64 elfutils-libs-0.156-5.fc19.x86_64
glibc-2.17-19.fc19.x86_64 libgcc-4.8.2-1.fc19.x86_64
libunwind-1.1-2.fc19.x86_64 nss-softokn-freebl-3.15.2-2.fc19.x86_64
numactl-libs-2.0.8-4.fc19.x86_64 perl-libs-5.16.3-266.fc19.x86_64
python-libs-2.7.5-9.fc19.x86_64 slang-2.2.4-8.fc19.x86_64
xz-libs-5.1.2-4alpha.fc19.x86_64 zlib-1.2.7-10.fc19.x86_64
(gdb)
(gdb) bt
#0 0x003e590359e9 in raise () from /lib64/libc.so.6
#1 0x003e590370f8 in abort () from /lib64/libc.so.6
#2 0x003e59075d17 in __libc_message () from /lib64/libc.so.6
#3 0x003e5907bbe7 in malloc_printerr () from /lib64/libc.so.6
#4 0x0046da89 in dso__delete (dso=0x8e46f0) at util/dso.c:496
#5 0x00482e7d in
Re: [GIT PULL 00/21] perf/core improvements and fixes
On 10/12/13 14:10, Arnaldo Carvalho de Melo wrote:
Em Tue, Dec 10, 2013 at 02:13:12PM +0200, Adrian Hunter escreveu:
On 10/12/13 13:44, Arnaldo Carvalho de Melo wrote:
Em Tue, Dec 10, 2013 at 12:12:29PM +0100, Ingo Molnar escreveu:
Hm, I've unpulled it because 'perf top' crashes on exit, in
dso__delete():
495 if (dso-sname_alloc)
496 free((char *)dso-short_name)
Yeah, must be that basename() patch from Stephane, I'll work on a fix
and resubmit this batch, thanks for the report.
The problem is sname_alloc is not maintained. Perhaps it should be
set in dso__set_short_name() e.g.
Yeah, sounds better than having all callers manage that thing, quickie,
was this with Stephane's patch applied?
Yes it was at
e993d10caeb6dca690dbaf86e1981ba240d1414a
perf symbols: fix bug in usage of the basename() function
I think it should be done as a prep, then apply a modified version of
Stephanes, that doesn't deal with the alloc flag (more than using 'true'
to say it is a malloc'ed chunk).
- Arnaldo
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 9fae484..54ed980 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -379,7 +379,7 @@ struct dso *dso__kernel_findnew(struct machine *machine,
const char *name,
* processing we had no idea this was the kernel dso.
*/
if (dso != NULL) {
-dso__set_short_name(dso, short_name);
+dso__set_short_name(dso, short_name, false);
dso-kernel = dso_type;
}
@@ -394,10 +394,13 @@ void dso__set_long_name(struct dso *dso, char *name)
dso-long_name_len = strlen(name);
}
-void dso__set_short_name(struct dso *dso, const char *name)
+void dso__set_short_name(struct dso *dso, const char *name, bool
sname_alloc)
{
if (name == NULL)
return;
+if (dso-sname_alloc)
+free((char *)dso-short_name);
+dso-sname_alloc = sname_alloc;
dso-short_name = name;
dso-short_name_len = strlen(name);
}
@@ -426,12 +429,7 @@ static void dso__set_basename(struct dso *dso)
if (!base)
return;
-if (dso-sname_alloc)
-free((char *)dso-short_name);
-else
-dso-sname_alloc = 1;
-
-dso__set_short_name(dso, base);
+dso__set_short_name(dso, base, true);
}
int dso__name_len(const struct dso *dso)
@@ -467,7 +465,7 @@ struct dso *dso__new(const char *name)
int i;
strcpy(dso-name, name);
dso__set_long_name(dso, dso-name);
-dso__set_short_name(dso, dso-name);
+dso__set_short_name(dso, dso-name, false);
for (i = 0; i MAP__NR_TYPES; ++i)
dso-symbols[i] = dso-symbol_names[i] = RB_ROOT;
dso-cache = RB_ROOT;
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index 384f2d9..166463e 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -110,7 +110,7 @@ static inline void dso__set_loaded(struct dso *dso, enum
map_type type)
struct dso *dso__new(const char *name);
void dso__delete(struct dso *dso);
-void dso__set_short_name(struct dso *dso, const char *name);
+void dso__set_short_name(struct dso *dso, const char *name, bool
sname_alloc);
void dso__set_long_name(struct dso *dso, char *name);
int dso__name_len(const struct dso *dso);
- Arnaldo
[Thread 0x770df700 (LWP 29561) exited]
*** Error in `/fast/mingo/tip/tools/perf/perf': munmap_chunk(): invalid
pointer: 0x00587371 ***
=== Backtrace: =
/lib64/libc.so.6[0x3e5907bbe7]
/fast/mingo/tip/tools/perf/perf(dso__delete+0xd9)[0x46da89]
/fast/mingo/tip/tools/perf/perf(machines__exit+0xad)[0x482e7d]
/fast/mingo/tip/tools/perf/perf(perf_session__delete+0xb6)[0x488c66]
/fast/mingo/tip/tools/perf/perf(cmd_top+0xf44)[0x4345f4]
/fast/mingo/tip/tools/perf/perf[0x419f95]
/fast/mingo/tip/tools/perf/perf(main+0x600)[0x419830]
/lib64/libc.so.6(__libc_start_main+0xf5)[0x3e59021b45]
/fast/mingo/tip/tools/perf/perf[0x4198fd]
=== Memory map:
Program received signal SIGABRT, Aborted.
0x003e590359e9 in raise () from /lib64/libc.so.6
Missing separate debuginfos, use: debuginfo-install
audit-libs-2.3.2-1.fc19.x86_64 bzip2-libs-1.0.6-8.fc19.x86_64
elfutils-libelf-0.156-5.fc19.x86_64 elfutils-libs-0.156-5.fc19.x86_64
glibc-2.17-19.fc19.x86_64 libgcc-4.8.2-1.fc19.x86_64
libunwind-1.1-2.fc19.x86_64 nss-softokn-freebl-3.15.2-2.fc19.x86_64
numactl-libs-2.0.8-4.fc19.x86_64 perl-libs-5.16.3-266.fc19.x86_64
python-libs-2.7.5-9.fc19.x86_64 slang-2.2.4-8.fc19.x86_64
xz-libs-5.1.2-4alpha.fc19.x86_64 zlib-1.2.7-10.fc19.x86_64
(gdb)
(gdb) bt
#0 0x003e590359e9 in raise () from /lib64/libc.so.6
#1 0x003e590370f8 in abort () from /lib64/libc.so.6
#2 0x003e59075d17 in __libc_message () from /lib64/libc.so.6
#3 0x003e5907bbe7 in malloc_printerr () from
Re: [GIT PULL 00/21] perf/core improvements and fixes
* Adrian Hunter adrian.hun...@intel.com wrote:
-void dso__set_short_name(struct dso *dso, const char *name)
+void dso__set_short_name(struct dso *dso, const char *name, bool sname_alloc)
{
if (name == NULL)
return;
+ if (dso-sname_alloc)
+ free((char *)dso-short_name);
+ dso-sname_alloc = sname_alloc;
Calling the function option the same as the field name is asking for
trouble - I'd suggest 'new_sname_alloc' for the parameter, or so.
And I'd also remove the 'const' from struct dso::short_name, it
probably does not help code generation, because 'dso' is passed in as
const in all the non-lifetime methods anyway.
That way the cast can be dropped from the free().
Similar problems exist with the usage of 'short_name' - it overloads
the field name which makes it somewhat confusing, and it's also
sometimes inconsistently named, such as 'name' in
dso__set_short_name().
Ditto for 'long_name' handling.
Also, the 'sname_alloc' name sucks, it does not make it obvious that
it's related to 'short_name', hiding its true significance (and hiding
the broken life time handling of the flag/pointer combo). I'd rename
it to something more descriptive, like -short_name_allocated - or I'd
rename everything to 'sname'/'lname' naming for short/long names.
Every time one runs into a crash like this it's a canary signal that
cleanliness principles need hardening.
Thanks,
Ingo
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
Em Tue, Dec 10, 2013 at 02:22:58PM +0200, Adrian Hunter escreveu:
On 10/12/13 14:10, Arnaldo Carvalho de Melo wrote:
Em Tue, Dec 10, 2013 at 02:13:12PM +0200, Adrian Hunter escreveu:
On 10/12/13 13:44, Arnaldo Carvalho de Melo wrote:
Em Tue, Dec 10, 2013 at 12:12:29PM +0100, Ingo Molnar escreveu:
Hm, I've unpulled it because 'perf top' crashes on exit, in
dso__delete():
495 if (dso-sname_alloc)
496 free((char *)dso-short_name)
Yeah, must be that basename() patch from Stephane, I'll work on a fix
and resubmit this batch, thanks for the report.
The problem is sname_alloc is not maintained. Perhaps it should be
set in dso__set_short_name() e.g.
Yeah, sounds better than having all callers manage that thing, quickie,
was this with Stephane's patch applied?
Yes it was at
e993d10caeb6dca690dbaf86e1981ba240d1414a
perf symbols: fix bug in usage of the basename() function
Yes, this is the buggy patch, my question was if Ingo did the changes
that streamlined the dso-sname_alloc management with e993d10caeb6
applied to his working tree.
- Arnaldo
I think it should be done as a prep, then apply a modified version of
Stephanes, that doesn't deal with the alloc flag (more than using 'true'
to say it is a malloc'ed chunk).
- Arnaldo
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 9fae484..54ed980 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -379,7 +379,7 @@ struct dso *dso__kernel_findnew(struct machine
*machine,
const char *name,
* processing we had no idea this was the kernel dso.
*/
if (dso != NULL) {
- dso__set_short_name(dso, short_name);
+ dso__set_short_name(dso, short_name, false);
dso-kernel = dso_type;
}
@@ -394,10 +394,13 @@ void dso__set_long_name(struct dso *dso, char *name)
dso-long_name_len = strlen(name);
}
-void dso__set_short_name(struct dso *dso, const char *name)
+void dso__set_short_name(struct dso *dso, const char *name, bool
sname_alloc)
{
if (name == NULL)
return;
+ if (dso-sname_alloc)
+ free((char *)dso-short_name);
+ dso-sname_alloc = sname_alloc;
dso-short_name = name;
dso-short_name_len = strlen(name);
}
@@ -426,12 +429,7 @@ static void dso__set_basename(struct dso *dso)
if (!base)
return;
- if (dso-sname_alloc)
- free((char *)dso-short_name);
- else
- dso-sname_alloc = 1;
-
- dso__set_short_name(dso, base);
+ dso__set_short_name(dso, base, true);
}
int dso__name_len(const struct dso *dso)
@@ -467,7 +465,7 @@ struct dso *dso__new(const char *name)
int i;
strcpy(dso-name, name);
dso__set_long_name(dso, dso-name);
- dso__set_short_name(dso, dso-name);
+ dso__set_short_name(dso, dso-name, false);
for (i = 0; i MAP__NR_TYPES; ++i)
dso-symbols[i] = dso-symbol_names[i] = RB_ROOT;
dso-cache = RB_ROOT;
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index 384f2d9..166463e 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -110,7 +110,7 @@ static inline void dso__set_loaded(struct dso *dso,
enum
map_type type)
struct dso *dso__new(const char *name);
void dso__delete(struct dso *dso);
-void dso__set_short_name(struct dso *dso, const char *name);
+void dso__set_short_name(struct dso *dso, const char *name, bool
sname_alloc);
void dso__set_long_name(struct dso *dso, char *name);
int dso__name_len(const struct dso *dso);
- Arnaldo
[Thread 0x770df700 (LWP 29561) exited]
*** Error in `/fast/mingo/tip/tools/perf/perf': munmap_chunk(): invalid
pointer: 0x00587371 ***
=== Backtrace: =
/lib64/libc.so.6[0x3e5907bbe7]
/fast/mingo/tip/tools/perf/perf(dso__delete+0xd9)[0x46da89]
/fast/mingo/tip/tools/perf/perf(machines__exit+0xad)[0x482e7d]
/fast/mingo/tip/tools/perf/perf(perf_session__delete+0xb6)[0x488c66]
/fast/mingo/tip/tools/perf/perf(cmd_top+0xf44)[0x4345f4]
/fast/mingo/tip/tools/perf/perf[0x419f95]
/fast/mingo/tip/tools/perf/perf(main+0x600)[0x419830]
/lib64/libc.so.6(__libc_start_main+0xf5)[0x3e59021b45]
/fast/mingo/tip/tools/perf/perf[0x4198fd]
=== Memory map:
Program received signal SIGABRT, Aborted.
0x003e590359e9 in raise () from /lib64/libc.so.6
Missing separate debuginfos, use: debuginfo-install
audit-libs-2.3.2-1.fc19.x86_64 bzip2-libs-1.0.6-8.fc19.x86_64
elfutils-libelf-0.156-5.fc19.x86_64 elfutils-libs-0.156-5.fc19.x86_64
glibc-2.17-19.fc19.x86_64 libgcc-4.8.2-1.fc19.x86_64
libunwind-1.1-2.fc19.x86_64 nss-softokn-freebl-3.15.2-2.fc19.x86_64
numactl-libs-2.0.8-4.fc19.x86_64 perl-libs-5.16.3-266.fc19.x86_64
python-libs-2.7.5-9.fc19.x86_64 slang-2.2.4-8.fc19.x86_64
Re: [GIT PULL 00/21] perf/core improvements and fixes
Em Tue, Dec 10, 2013 at 09:22:13AM -0300, Arnaldo Carvalho de Melo escreveu:
Em Tue, Dec 10, 2013 at 02:22:58PM +0200, Adrian Hunter escreveu:
On 10/12/13 14:10, Arnaldo Carvalho de Melo wrote:
Em Tue, Dec 10, 2013 at 02:13:12PM +0200, Adrian Hunter escreveu:
On 10/12/13 13:44, Arnaldo Carvalho de Melo wrote:
Em Tue, Dec 10, 2013 at 12:12:29PM +0100, Ingo Molnar escreveu:
Hm, I've unpulled it because 'perf top' crashes on exit, in
dso__delete():
495 if (dso-sname_alloc)
496 free((char *)dso-short_name)
Yeah, must be that basename() patch from Stephane, I'll work on a fix
and resubmit this batch, thanks for the report.
The problem is sname_alloc is not maintained. Perhaps it should be
set in dso__set_short_name() e.g.
Yeah, sounds better than having all callers manage that thing, quickie,
was this with Stephane's patch applied?
Yes it was at
e993d10caeb6dca690dbaf86e1981ba240d1414a
perf symbols: fix bug in usage of the basename() function
Yes, this is the buggy patch, my question was if Ingo did the changes
that streamlined the dso-sname_alloc management with e993d10caeb6
applied to his working tree.
Sorry Adrian, my bad, I thought Ingo had provided the patch, now I
realized my mistake, it was you 8-)
- Arnaldo
I think it should be done as a prep, then apply a modified version of
Stephanes, that doesn't deal with the alloc flag (more than using 'true'
to say it is a malloc'ed chunk).
- Arnaldo
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 9fae484..54ed980 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -379,7 +379,7 @@ struct dso *dso__kernel_findnew(struct machine
*machine,
const char *name,
* processing we had no idea this was the kernel dso.
*/
if (dso != NULL) {
-dso__set_short_name(dso, short_name);
+dso__set_short_name(dso, short_name, false);
dso-kernel = dso_type;
}
@@ -394,10 +394,13 @@ void dso__set_long_name(struct dso *dso, char
*name)
dso-long_name_len = strlen(name);
}
-void dso__set_short_name(struct dso *dso, const char *name)
+void dso__set_short_name(struct dso *dso, const char *name, bool
sname_alloc)
{
if (name == NULL)
return;
+if (dso-sname_alloc)
+free((char *)dso-short_name);
+dso-sname_alloc = sname_alloc;
dso-short_name = name;
dso-short_name_len = strlen(name);
}
@@ -426,12 +429,7 @@ static void dso__set_basename(struct dso *dso)
if (!base)
return;
-if (dso-sname_alloc)
-free((char *)dso-short_name);
-else
-dso-sname_alloc = 1;
-
-dso__set_short_name(dso, base);
+dso__set_short_name(dso, base, true);
}
int dso__name_len(const struct dso *dso)
@@ -467,7 +465,7 @@ struct dso *dso__new(const char *name)
int i;
strcpy(dso-name, name);
dso__set_long_name(dso, dso-name);
-dso__set_short_name(dso, dso-name);
+dso__set_short_name(dso, dso-name, false);
for (i = 0; i MAP__NR_TYPES; ++i)
dso-symbols[i] = dso-symbol_names[i] =
RB_ROOT;
dso-cache = RB_ROOT;
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index 384f2d9..166463e 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -110,7 +110,7 @@ static inline void dso__set_loaded(struct dso *dso,
enum
map_type type)
struct dso *dso__new(const char *name);
void dso__delete(struct dso *dso);
-void dso__set_short_name(struct dso *dso, const char *name);
+void dso__set_short_name(struct dso *dso, const char *name, bool
sname_alloc);
void dso__set_long_name(struct dso *dso, char *name);
int dso__name_len(const struct dso *dso);
- Arnaldo
[Thread 0x770df700 (LWP 29561) exited]
*** Error in `/fast/mingo/tip/tools/perf/perf': munmap_chunk():
invalid pointer: 0x00587371 ***
=== Backtrace: =
/lib64/libc.so.6[0x3e5907bbe7]
/fast/mingo/tip/tools/perf/perf(dso__delete+0xd9)[0x46da89]
/fast/mingo/tip/tools/perf/perf(machines__exit+0xad)[0x482e7d]
/fast/mingo/tip/tools/perf/perf(perf_session__delete+0xb6)[0x488c66]
/fast/mingo/tip/tools/perf/perf(cmd_top+0xf44)[0x4345f4]
/fast/mingo/tip/tools/perf/perf[0x419f95]
/fast/mingo/tip/tools/perf/perf(main+0x600)[0x419830]
/lib64/libc.so.6(__libc_start_main+0xf5)[0x3e59021b45]
/fast/mingo/tip/tools/perf/perf[0x4198fd]
=== Memory map:
Program received signal SIGABRT, Aborted.
Re: [GIT PULL 00/21] perf/core improvements and fixes
* Arnaldo Carvalho de Melo a...@ghostprotocols.net wrote: Em Tue, Dec 10, 2013 at 02:22:58PM +0200, Adrian Hunter escreveu: On 10/12/13 14:10, Arnaldo Carvalho de Melo wrote: Em Tue, Dec 10, 2013 at 02:13:12PM +0200, Adrian Hunter escreveu: On 10/12/13 13:44, Arnaldo Carvalho de Melo wrote: Em Tue, Dec 10, 2013 at 12:12:29PM +0100, Ingo Molnar escreveu: Hm, I've unpulled it because 'perf top' crashes on exit, in dso__delete(): 495 if (dso-sname_alloc) 496 free((char *)dso-short_name) Yeah, must be that basename() patch from Stephane, I'll work on a fix and resubmit this batch, thanks for the report. The problem is sname_alloc is not maintained. Perhaps it should be set in dso__set_short_name() e.g. Yeah, sounds better than having all callers manage that thing, quickie, was this with Stephane's patch applied? Yes it was at e993d10caeb6dca690dbaf86e1981ba240d1414a perf symbols: fix bug in usage of the basename() function Yes, this is the buggy patch, my question was if Ingo did the changes that streamlined the dso-sname_alloc management with e993d10caeb6 applied to his working tree. My current perf/core head is: 789790791ad2 tools/perf/build: Fix install dependency which does not have e993d10c. [ Btw., a small nit: the capitalization of the commit title looks inconsistent. ] Thanks, Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
* Ingo Molnar mi...@kernel.org wrote:
* Adrian Hunter adrian.hun...@intel.com wrote:
-void dso__set_short_name(struct dso *dso, const char *name)
+void dso__set_short_name(struct dso *dso, const char *name, bool
sname_alloc)
{
if (name == NULL)
return;
+ if (dso-sname_alloc)
+ free((char *)dso-short_name);
+ dso-sname_alloc = sname_alloc;
Calling the function option the same as the field name is asking for
trouble - I'd suggest 'new_sname_alloc' for the parameter, or so.
And I'd also remove the 'const' from struct dso::short_name, it
probably does not help code generation, because 'dso' is passed in as
const in all the non-lifetime methods anyway.
That way the cast can be dropped from the free().
Similar problems exist with the usage of 'short_name' - it overloads
the field name which makes it somewhat confusing, and it's also
sometimes inconsistently named, such as 'name' in
dso__set_short_name().
Ditto for 'long_name' handling.
Also, the 'sname_alloc' name sucks, it does not make it obvious that
it's related to 'short_name', hiding its true significance (and hiding
the broken life time handling of the flag/pointer combo). I'd rename
it to something more descriptive, like -short_name_allocated - or I'd
rename everything to 'sname'/'lname' naming for short/long names.
Every time one runs into a crash like this it's a canary signal that
cleanliness principles need hardening.
More observations about util/dso.c:
- dso__binary_type_file() should probably pass in 'const struct dso'
- dso__binary_type_file()'s filename string parameter should be named
'filename', not 'file' ...
- build_id__sprintf() looks fragile: every single use of it appears
to follow this pattern:
build_id__sprintf(x, sizeof(x), ...)
this could be simplified (and eliminating the possibility to typo a
bug) by changing the function to __build_id__snprintf() and adding
a build_id__sprintf() wrapper macro around it:
build_id__sprintf(x, ...)
that generates the size itself.
- dso__binary_type_file() is a method without a verb, so it's unclear
what it does. It probably wants to be renamed to
dso__set_binary_type_file() or so?
- dso_cache__find() probably wants to pass in a const rb_root.
- 'struct dso *pos' should probably be named 'struct dso *dso_pos' or
so - 'pos' is frequently used for integer variable names so its use
for an object iterator feels confusing.
- the 'head' argument of dsos__find() wants to be constified too I
guess
Thanks,
Ingo
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
Em Tue, Dec 10, 2013 at 01:46:58PM +0100, Ingo Molnar escreveu: * Ingo Molnar mi...@kernel.org wrote: Every time one runs into a crash like this it's a canary signal that cleanliness principles need hardening. More observations about util/dso.c: - dso__binary_type_file() should probably pass in 'const struct dso' - dso__binary_type_file()'s filename string parameter should be named 'filename', not 'file' ... - build_id__sprintf() looks fragile: every single use of it appears to follow this pattern: build_id__sprintf(x, sizeof(x), ...) this could be simplified (and eliminating the possibility to typo a bug) by changing the function to __build_id__snprintf() and adding a build_id__sprintf() wrapper macro around it: build_id__sprintf(x, ...) that generates the size itself. Right, like: int __perf_evlist__add_default_attrs(struct perf_evlist *evlist, struct perf_event_attr *attrs, size_t nr_attrs); #define perf_evlist__add_default_attrs(evlist, array) \ __perf_evlist__add_default_attrs(evlist, array, ARRAY_SIZE(array)) This is all a matter of being more dilligent and judicious at employing these and other good practices. But don't be shy to point anything (like you did here), as time permits we can go on doing patchkits to address things people notice. - Arnaldo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
Em Tue, Dec 10, 2013 at 01:18:01PM +0100, Ingo Molnar escreveu:
* Adrian Hunter adrian.hun...@intel.com wrote:
-void dso__set_short_name(struct dso *dso, const char *name)
+void dso__set_short_name(struct dso *dso, const char *name, bool
sname_alloc)
{
if (name == NULL)
return;
+ if (dso-sname_alloc)
+ free((char *)dso-short_name);
+ dso-sname_alloc = sname_alloc;
Calling the function option the same as the field name is asking for
trouble - I'd suggest 'new_sname_alloc' for the parameter, or so.
And I'd also remove the 'const' from struct dso::short_name, it
probably does not help code generation, because 'dso' is passed in as
const in all the non-lifetime methods anyway.
That way the cast can be dropped from the free().
Not that simple, there are multiple places that pass a constant
short_name, for instance:
machine__get_kernel()
kernel = dso__kernel_findnew(machine, vmlinux_name,
[kernel], DSO_TYPE_KERNEL);
dso__set_short_name(dso, short_name);
So dso-short_name will point to [kernel], which is a const char *.
Similar problems exist with the usage of 'short_name' - it overloads
the field name which makes it somewhat confusing, and it's also
sometimes inconsistently named, such as 'name' in
dso__set_short_name().
Ditto for 'long_name' handling.
Also, the 'sname_alloc' name sucks, it does not make it obvious that
it's related to 'short_name', hiding its true significance (and hiding
the broken life time handling of the flag/pointer combo). I'd rename
it to something more descriptive, like -short_name_allocated - or I'd
rename everything to 'sname'/'lname' naming for short/long names.
Ok, we can use rename it to short_name_alloc, like we have
short_name_len.
Every time one runs into a crash like this it's a canary signal that
cleanliness principles need hardening.
Hardening we go then!
Thanks,
Ingo
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
* Arnaldo Carvalho de Melo a...@ghostprotocols.net wrote:
Em Tue, Dec 10, 2013 at 01:18:01PM +0100, Ingo Molnar escreveu:
* Adrian Hunter adrian.hun...@intel.com wrote:
-void dso__set_short_name(struct dso *dso, const char *name)
+void dso__set_short_name(struct dso *dso, const char *name, bool
sname_alloc)
{
if (name == NULL)
return;
+ if (dso-sname_alloc)
+ free((char *)dso-short_name);
+ dso-sname_alloc = sname_alloc;
Calling the function option the same as the field name is asking for
trouble - I'd suggest 'new_sname_alloc' for the parameter, or so.
And I'd also remove the 'const' from struct dso::short_name, it
probably does not help code generation, because 'dso' is passed in as
const in all the non-lifetime methods anyway.
That way the cast can be dropped from the free().
Not that simple, there are multiple places that pass a constant
short_name, for instance:
machine__get_kernel()
kernel = dso__kernel_findnew(machine, vmlinux_name,
[kernel], DSO_TYPE_KERNEL);
dso__set_short_name(dso, short_name);
So dso-short_name will point to [kernel], which is a const char *.
Okay, I guess the free() cast is fine then.
Thanks,
Ingo
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
On Tue, Dec 10, 2013 at 12:07:59PM +0100, Ingo Molnar wrote: SNIP Pulled, thanks Arnaldo! There's one detail I noticed about the recent trace-plugin changes: comet:~/tip/tools/perf make install BUILD: Doing 'make -j12' parallel build SUBDIR Documentation INSTALL Documentation-man INSTALL GTK UI SUBDIR /home/mingo/tip/tools/lib/traceevent/ INSTALL binaries INSTALL plugin_jbd2.so INSTALL plugin_hrtimer.so INSTALL plugin_kmem.so INSTALL plugin_kvm.so INSTALL plugin_mac80211.so INSTALL plugin_sched_switch.so INSTALL plugin_function.so INSTALL plugin_xen.so INSTALL plugin_scsi.so INSTALL plugin_cfg80211.so INSTALL libexec INSTALL perf-archive INSTALL perl-scripts INSTALL python-scripts INSTALL perf_completion-script INSTALL tests those plugin installs are way too verbose, they should really be in a single summarized line, only saying something like: INSTALL plugins Just like we already sum up 'binaries', 'libexec', 'tests', etc. ok, TODO updated ;-) thanks, jirka -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
* Jiri Olsa jo...@redhat.com wrote: On Tue, Dec 10, 2013 at 12:07:59PM +0100, Ingo Molnar wrote: SNIP Pulled, thanks Arnaldo! There's one detail I noticed about the recent trace-plugin changes: comet:~/tip/tools/perf make install BUILD: Doing 'make -j12' parallel build SUBDIR Documentation INSTALL Documentation-man INSTALL GTK UI SUBDIR /home/mingo/tip/tools/lib/traceevent/ INSTALL binaries INSTALL plugin_jbd2.so INSTALL plugin_hrtimer.so INSTALL plugin_kmem.so INSTALL plugin_kvm.so INSTALL plugin_mac80211.so INSTALL plugin_sched_switch.so INSTALL plugin_function.so INSTALL plugin_xen.so INSTALL plugin_scsi.so INSTALL plugin_cfg80211.so INSTALL libexec INSTALL perf-archive INSTALL perl-scripts INSTALL python-scripts INSTALL perf_completion-script INSTALL tests those plugin installs are way too verbose, they should really be in a single summarized line, only saying something like: INSTALL plugins Just like we already sum up 'binaries', 'libexec', 'tests', etc. ok, TODO updated ;-) Consider it a regression! ;-) Thanks, Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
* Arnaldo Carvalho de Melo wrote: > From: Arnaldo Carvalho de Melo > > Hi Ingo, > > Please consider pulling, another batch, more to come soon, > > - Arnaldo > > The following changes since commit 5ec4c599a52362896c3e7c6a31ba6145dca9c6f5: > > perf: Do not compute time values unnecessarily (2013-08-16 17:55:52 +0200) > > are available in the git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux > tags/perf-core-for-mingo > > for you to fetch changes up to 456da532a5fb04f8a79622df7dd49e84e04f31a8: > > tools lib traceevent: Fixup jobserver setup (2013-08-27 11:05:55 -0300) > > > perf/core improvements and fixes: > > . Don't install scripting files files when perl/python support is disabled. > > . Support ! in -e expressions in 'perf trace', to filter a list of syscalls. > > . Add --verbose and -o/--output options to 'perf trace'. > > . Introduce better formatting of syscall arguments in 'perf trace', > including so far beautifiers for mmap, madvise, syscall return > values. > > . Fixup jobserver setup in libtraceevent makefile. > > . Debug improvements from Adrian Hunter. > > . Try to increase the file descriptor limits on EMFILE, from Andi Kleen. > > . Remove unused force option in 'perf kvm', from David Ahern. > > . Make 'perf trace' command line arguments consistent with 'perf record', > from David Ahern. > > . Fix correlation of samples coming after PERF_RECORD_EXIT event, from > David Ahern. > > Signed-off-by: Arnaldo Carvalho de Melo > > > Adrian Hunter (3): > perf tools: Re-implement debug print function for linking python/perf.so > perf tools: Add debug prints > perf tools: Add pid to struct thread > > Andi Kleen (1): > perf tools: Try to increase the file descriptor limits on EMFILE > > Arnaldo Carvalho de Melo (14): > perf trace: Implement -o/--output filename > perf tools: Don't install scripting files files when disabled > perf trace: Support ! in -e expressions > perf trace: Add --verbose option > perf trace: Hide sys_exit messages about syscall id = -1 > perf trace: Introduce syscall arg formatters > perf trace: Simplify sys_exit return printing > perf trace: Allow printing syscall return values in hex > perf trace: Add aliases to remaining syscalls of the sys_enter_newfoo > perf trace: Allow overiding the formatting of syscall fields > perf trace: Add beautifier for mmap prot parm > perf trace: Add beautifier for mmap flags parm > perf trace: Add beautifier for madvise behaviour/advice parm > tools lib traceevent: Fixup jobserver setup > > David Ahern (3): > perf kvm: Remove force option to cmd_record > perf trace: Make command line arguments consistent with perf-record > perf tools: Sample after exit loses thread correlation > > tools/lib/traceevent/Makefile | 2 +- > tools/perf/Documentation/perf-trace.txt | 16 ++ > tools/perf/Makefile | 6 +- > tools/perf/builtin-kvm.c| 1 - > tools/perf/builtin-trace.c | 379 > ++-- > tools/perf/util/evlist.c| 3 + > tools/perf/util/evsel.c | 93 > tools/perf/util/machine.c | 50 +++-- > tools/perf/util/python.c| 20 ++ > tools/perf/util/thread.c| 3 +- > tools/perf/util/thread.h| 8 +- > 11 files changed, 488 insertions(+), 93 deletions(-) Pulled, thanks a lot Arnaldo! Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
* Arnaldo Carvalho de Melo a...@infradead.org wrote: From: Arnaldo Carvalho de Melo a...@ghostprotocols.net Hi Ingo, Please consider pulling, another batch, more to come soon, - Arnaldo The following changes since commit 5ec4c599a52362896c3e7c6a31ba6145dca9c6f5: perf: Do not compute time values unnecessarily (2013-08-16 17:55:52 +0200) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux tags/perf-core-for-mingo for you to fetch changes up to 456da532a5fb04f8a79622df7dd49e84e04f31a8: tools lib traceevent: Fixup jobserver setup (2013-08-27 11:05:55 -0300) perf/core improvements and fixes: . Don't install scripting files files when perl/python support is disabled. . Support ! in -e expressions in 'perf trace', to filter a list of syscalls. . Add --verbose and -o/--output options to 'perf trace'. . Introduce better formatting of syscall arguments in 'perf trace', including so far beautifiers for mmap, madvise, syscall return values. . Fixup jobserver setup in libtraceevent makefile. . Debug improvements from Adrian Hunter. . Try to increase the file descriptor limits on EMFILE, from Andi Kleen. . Remove unused force option in 'perf kvm', from David Ahern. . Make 'perf trace' command line arguments consistent with 'perf record', from David Ahern. . Fix correlation of samples coming after PERF_RECORD_EXIT event, from David Ahern. Signed-off-by: Arnaldo Carvalho de Melo a...@redhat.com Adrian Hunter (3): perf tools: Re-implement debug print function for linking python/perf.so perf tools: Add debug prints perf tools: Add pid to struct thread Andi Kleen (1): perf tools: Try to increase the file descriptor limits on EMFILE Arnaldo Carvalho de Melo (14): perf trace: Implement -o/--output filename perf tools: Don't install scripting files files when disabled perf trace: Support ! in -e expressions perf trace: Add --verbose option perf trace: Hide sys_exit messages about syscall id = -1 perf trace: Introduce syscall arg formatters perf trace: Simplify sys_exit return printing perf trace: Allow printing syscall return values in hex perf trace: Add aliases to remaining syscalls of the sys_enter_newfoo perf trace: Allow overiding the formatting of syscall fields perf trace: Add beautifier for mmap prot parm perf trace: Add beautifier for mmap flags parm perf trace: Add beautifier for madvise behaviour/advice parm tools lib traceevent: Fixup jobserver setup David Ahern (3): perf kvm: Remove force option to cmd_record perf trace: Make command line arguments consistent with perf-record perf tools: Sample after exit loses thread correlation tools/lib/traceevent/Makefile | 2 +- tools/perf/Documentation/perf-trace.txt | 16 ++ tools/perf/Makefile | 6 +- tools/perf/builtin-kvm.c| 1 - tools/perf/builtin-trace.c | 379 ++-- tools/perf/util/evlist.c| 3 + tools/perf/util/evsel.c | 93 tools/perf/util/machine.c | 50 +++-- tools/perf/util/python.c| 20 ++ tools/perf/util/thread.c| 3 +- tools/perf/util/thread.h| 8 +- 11 files changed, 488 insertions(+), 93 deletions(-) Pulled, thanks a lot Arnaldo! Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
* Arnaldo Carvalho de Melo wrote:
> From: Arnaldo Carvalho de Melo
>
> Hi Ingo,
>
> Please consider pulling,
>
> Flushing it out now before processing another batch.
>
> - Arnaldo
>
> The following changes since commit 0a3d23a2568ed5e73bd4fb532dc672fa9f03b1f1:
>
> Merge tag 'perf-core-for-mingo' of
> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/core
> (2013-08-12 10:14:47 +0200)
>
> are available in the git repository at:
>
>
> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux
> tags/perf-core-for-mingo
>
> for you to fetch changes up to 2ae3a312c0ccd8ff615372f00aab1700aac27474:
>
> perf trace: Allow specifying which syscalls to trace (2013-08-14 11:44:21
> -0300)
>
>
> perf/core improvements and fixes:
>
> . Allow specifying syscalls in 'perf trace', a la strace.
>
> . Simplify symbol filtering by doing it at machine class level,
> from Adrian Hunter.
>
> . Add option to 'perf kvm' to print only events that exceed a specified time
> duration, from David Ahern.
>
> . 'perf sched' improvements, including removing some tracepoints that provide
> the same information as the PERF_RECORD_{FORK,EXIT} events.
>
> . Improve stack trace printing, from David Ahern.
>
> . Update documentation with live command, from David Ahern
>
> . Fix 'perf test' compile failure on do_sort_something, from David Ahern.
>
> . Improve robustness of topology parsing code, from Stephane Eranian.
>
> Signed-off-by: Arnaldo Carvalho de Melo
>
>
> Adrian Hunter (8):
> perf machine: Add symbol filter to struct machine
> perf top: Set the machines symbol filter
> perf report: Set the machines symbol filter
> perf mem: Remove unused symbol filter member
> perf annotate: Set the machines symbol filter
> perf tools: Remove filter parameter of perf_event__preprocess_sample()
> perf tools: Remove filter parameter of thread__find_addr_location()
> perf tools: Remove filter parameter of thread__find_addr_map()
>
> Arnaldo Carvalho de Melo (1):
> perf trace: Allow specifying which syscalls to trace
>
> David Ahern (11):
> perf kvm: Option to print events that exceed a duration
> perf kvm: Update documentation with live command
> perf sched: Simplify arguments to read_events
> perf sched: Remove thread lookup in sample handler
> perf sched: Remove sched_process_exit tracepoint
> perf sched: Remove sched_process_fork tracepoint
> perf tool: Simplify options to perf_evsel__print_ip
> perf evsel: Add option to print stack trace on single line
> perf evsel: Add option to limit stack depth in callchain dumps
> perf session: Change perf_session__has_traces to actually check for
> tracepoints
> perf tests: Fix compile failure on do_sort_something
>
> Stephane Eranian (1):
> perf tools: Improve robustness of topology parsing code
>
> tools/perf/Documentation/perf-kvm.txt | 46 +++-
> tools/perf/Documentation/perf-trace.txt | 4 ++
> tools/perf/builtin-annotate.c | 5 +-
> tools/perf/builtin-diff.c | 2 +-
> tools/perf/builtin-inject.c | 2 +-
> tools/perf/builtin-kvm.c| 25 +++--
> tools/perf/builtin-mem.c| 4 +-
> tools/perf/builtin-report.c | 7 ++-
> tools/perf/builtin-sched.c | 94
> ++---
> tools/perf/builtin-script.c | 35 +---
> tools/perf/builtin-top.c| 5 +-
> tools/perf/builtin-trace.c | 52 +++---
> tools/perf/perf.h | 3 ++
> tools/perf/tests/code-reading.c | 13 +++--
> tools/perf/tests/hists_link.c | 4 +-
> tools/perf/util/build-id.c | 2 +-
> tools/perf/util/event.c | 20 +++
> tools/perf/util/event.h | 3 +-
> tools/perf/util/header.c| 11 ++--
> tools/perf/util/machine.c | 28 --
> tools/perf/util/machine.h | 5 ++
> tools/perf/util/session.c | 40 +-
> tools/perf/util/session.h | 8 ++-
> tools/perf/util/thread.h| 5 +-
> tools/perf/util/unwind.c| 6 +--
> 25 files changed, 294 insertions(+), 135 deletions(-)
Pulled, thanks Arnaldo!
Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
* Arnaldo Carvalho de Melo a...@infradead.org wrote:
From: Arnaldo Carvalho de Melo a...@ghostprotocols.net
Hi Ingo,
Please consider pulling,
Flushing it out now before processing another batch.
- Arnaldo
The following changes since commit 0a3d23a2568ed5e73bd4fb532dc672fa9f03b1f1:
Merge tag 'perf-core-for-mingo' of
git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/core
(2013-08-12 10:14:47 +0200)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux
tags/perf-core-for-mingo
for you to fetch changes up to 2ae3a312c0ccd8ff615372f00aab1700aac27474:
perf trace: Allow specifying which syscalls to trace (2013-08-14 11:44:21
-0300)
perf/core improvements and fixes:
. Allow specifying syscalls in 'perf trace', a la strace.
. Simplify symbol filtering by doing it at machine class level,
from Adrian Hunter.
. Add option to 'perf kvm' to print only events that exceed a specified time
duration, from David Ahern.
. 'perf sched' improvements, including removing some tracepoints that provide
the same information as the PERF_RECORD_{FORK,EXIT} events.
. Improve stack trace printing, from David Ahern.
. Update documentation with live command, from David Ahern
. Fix 'perf test' compile failure on do_sort_something, from David Ahern.
. Improve robustness of topology parsing code, from Stephane Eranian.
Signed-off-by: Arnaldo Carvalho de Melo a...@redhat.com
Adrian Hunter (8):
perf machine: Add symbol filter to struct machine
perf top: Set the machines symbol filter
perf report: Set the machines symbol filter
perf mem: Remove unused symbol filter member
perf annotate: Set the machines symbol filter
perf tools: Remove filter parameter of perf_event__preprocess_sample()
perf tools: Remove filter parameter of thread__find_addr_location()
perf tools: Remove filter parameter of thread__find_addr_map()
Arnaldo Carvalho de Melo (1):
perf trace: Allow specifying which syscalls to trace
David Ahern (11):
perf kvm: Option to print events that exceed a duration
perf kvm: Update documentation with live command
perf sched: Simplify arguments to read_events
perf sched: Remove thread lookup in sample handler
perf sched: Remove sched_process_exit tracepoint
perf sched: Remove sched_process_fork tracepoint
perf tool: Simplify options to perf_evsel__print_ip
perf evsel: Add option to print stack trace on single line
perf evsel: Add option to limit stack depth in callchain dumps
perf session: Change perf_session__has_traces to actually check for
tracepoints
perf tests: Fix compile failure on do_sort_something
Stephane Eranian (1):
perf tools: Improve robustness of topology parsing code
tools/perf/Documentation/perf-kvm.txt | 46 +++-
tools/perf/Documentation/perf-trace.txt | 4 ++
tools/perf/builtin-annotate.c | 5 +-
tools/perf/builtin-diff.c | 2 +-
tools/perf/builtin-inject.c | 2 +-
tools/perf/builtin-kvm.c| 25 +++--
tools/perf/builtin-mem.c| 4 +-
tools/perf/builtin-report.c | 7 ++-
tools/perf/builtin-sched.c | 94
++---
tools/perf/builtin-script.c | 35 +---
tools/perf/builtin-top.c| 5 +-
tools/perf/builtin-trace.c | 52 +++---
tools/perf/perf.h | 3 ++
tools/perf/tests/code-reading.c | 13 +++--
tools/perf/tests/hists_link.c | 4 +-
tools/perf/util/build-id.c | 2 +-
tools/perf/util/event.c | 20 +++
tools/perf/util/event.h | 3 +-
tools/perf/util/header.c| 11 ++--
tools/perf/util/machine.c | 28 --
tools/perf/util/machine.h | 5 ++
tools/perf/util/session.c | 40 +-
tools/perf/util/session.h | 8 ++-
tools/perf/util/thread.h| 5 +-
tools/perf/util/unwind.c| 6 +--
25 files changed, 294 insertions(+), 135 deletions(-)
Pulled, thanks Arnaldo!
Ingo
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
* Arnaldo Carvalho de Melo wrote: > Hi Ingo, > > Please consider pulling. > > Namhyung, Jiri, the 'group report' patches are at acme/perf/group, > will send a pull req later if it survives further testing. > > - Arnaldo > > The following changes since commit a2d28d0c198b65fac28ea6212f5f8edc77b29c27: > > Merge tag 'perf-core-for-mingo' of > git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/core > (2013-01-25 11:34:00 +0100) > > are available in the git repository at: > > > git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux > tags/perf-core-for-mingo > > for you to fetch changes up to 5809fde040de2afa477a6c593ce2e8fd2c11d9d3: > > perf header: Fix double fclose() on do_write(fd, xxx) failure (2013-01-30 > 10:40:44 -0300) > > > perf/core improvements and fixes: > > . Fix some leaks in exit paths. > > . Use memdup where applicable > > . Remove some die() calls, allowing callers to handle exit paths > gracefully. > > . Correct typo in tools Makefile, fix from Borislav Petkov. > > . Add 'perf bench numa mem' NUMA performance measurement suite, from Ingo > Molnar. > > . Handle dynamic array's element size properly, fix from Jiri Olsa. > > . Fix memory leaks on evsel->counts, from Namhyung Kim. > > . Make numa benchmark optional, allowing the build in machines where required > numa libraries are not present, fix from Peter Hurley. > > . Add interval printing in 'perf stat', from Stephane Eranian. > > . Fix compile warnings in tests/attr.c, from Sukadev Bhattiprolu. > > . Fix double free, pclose instead of fclose, leaks and double fclose errors > found with the cppcheck tool, from Thomas Jarosch. > > Signed-off-by: Arnaldo Carvalho de Melo > > > Arnaldo Carvalho de Melo (8): > perf tools: Stop using 'self' in strlist > perf tools: Stop using 'self' in map.[ch] > perf tools: Use memdup in map__clone > perf kmem: Use memdup() > perf header: Stop using die() calls when processing tracing data > perf ui browser: Free browser->helpline() on ui_browser__hide() > perf tests: Call machine__exit in the vmlinux matches kallsyms test > perf tests: Fix leaks on PERF_RECORD_* test > > Borislav Petkov (1): > tools: Correct typo in tools Makefile > > Ingo Molnar (1): > perf: Add 'perf bench numa mem' NUMA performance measurement suite > > Jiri Olsa (1): > tools lib traceevent: Handle dynamic array's element size properly > > Namhyung Kim (1): > perf evsel: Fix memory leaks on evsel->counts > > Peter Hurley (1): > perf tools: Make numa benchmark optional > > Stephane Eranian (2): > perf evsel: Add prev_raw_count field > perf stat: Add interval printing > > Sukadev Bhattiprolu (1): > perf tools, powerpc: Fix compile warnings in tests/attr.c > > Thomas Jarosch (5): > perf tools: Fix possible double free on error > perf sort: Use pclose() instead of fclose() on pipe stream > perf tools: Fix memory leak on error > perf header: Fix memory leak for the "Not caching a kptr_restrict'ed > /proc/kallsyms" case > perf header: Fix double fclose() on do_write(fd, xxx) failure > > tools/Makefile |2 +- > tools/lib/traceevent/event-parse.c | 39 +- > tools/perf/Documentation/perf-stat.txt |4 + > tools/perf/Makefile | 13 + > tools/perf/arch/common.c |1 + > tools/perf/bench/bench.h |1 + > tools/perf/bench/numa.c | 1731 > ++ > tools/perf/builtin-bench.c | 17 + > tools/perf/builtin-kmem.c|6 +- > tools/perf/builtin-stat.c| 158 ++- > tools/perf/config/feature-tests.mak | 11 + > tools/perf/tests/attr.c |5 + > tools/perf/tests/open-syscall-all-cpus.c |1 + > tools/perf/tests/perf-record.c | 12 +- > tools/perf/tests/vmlinux-kallsyms.c |4 +- > tools/perf/ui/browser.c |2 + > tools/perf/util/event.c |4 +- > tools/perf/util/evsel.c | 31 + > tools/perf/util/evsel.h |2 + > tools/perf/util/header.c | 25 +- > tools/perf/util/map.c| 118 +- > tools/perf/util/map.h| 24 +- > tools/perf/util/sort.c |7 +- > tools/perf/util/strlist.c| 54 +- > tools/perf/util/strlist.h| 42 +- > 25 files changed, 2154 insertions(+), 160 deletions(-) > create mode 100644 tools/perf/bench/numa.c Pulled, thanks a lot Arnaldo! Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More
Re: [GIT PULL 00/21] perf/core improvements and fixes
* Arnaldo Carvalho de Melo a...@infradead.org wrote: Hi Ingo, Please consider pulling. Namhyung, Jiri, the 'group report' patches are at acme/perf/group, will send a pull req later if it survives further testing. - Arnaldo The following changes since commit a2d28d0c198b65fac28ea6212f5f8edc77b29c27: Merge tag 'perf-core-for-mingo' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/core (2013-01-25 11:34:00 +0100) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux tags/perf-core-for-mingo for you to fetch changes up to 5809fde040de2afa477a6c593ce2e8fd2c11d9d3: perf header: Fix double fclose() on do_write(fd, xxx) failure (2013-01-30 10:40:44 -0300) perf/core improvements and fixes: . Fix some leaks in exit paths. . Use memdup where applicable . Remove some die() calls, allowing callers to handle exit paths gracefully. . Correct typo in tools Makefile, fix from Borislav Petkov. . Add 'perf bench numa mem' NUMA performance measurement suite, from Ingo Molnar. . Handle dynamic array's element size properly, fix from Jiri Olsa. . Fix memory leaks on evsel-counts, from Namhyung Kim. . Make numa benchmark optional, allowing the build in machines where required numa libraries are not present, fix from Peter Hurley. . Add interval printing in 'perf stat', from Stephane Eranian. . Fix compile warnings in tests/attr.c, from Sukadev Bhattiprolu. . Fix double free, pclose instead of fclose, leaks and double fclose errors found with the cppcheck tool, from Thomas Jarosch. Signed-off-by: Arnaldo Carvalho de Melo a...@redhat.com Arnaldo Carvalho de Melo (8): perf tools: Stop using 'self' in strlist perf tools: Stop using 'self' in map.[ch] perf tools: Use memdup in map__clone perf kmem: Use memdup() perf header: Stop using die() calls when processing tracing data perf ui browser: Free browser-helpline() on ui_browser__hide() perf tests: Call machine__exit in the vmlinux matches kallsyms test perf tests: Fix leaks on PERF_RECORD_* test Borislav Petkov (1): tools: Correct typo in tools Makefile Ingo Molnar (1): perf: Add 'perf bench numa mem' NUMA performance measurement suite Jiri Olsa (1): tools lib traceevent: Handle dynamic array's element size properly Namhyung Kim (1): perf evsel: Fix memory leaks on evsel-counts Peter Hurley (1): perf tools: Make numa benchmark optional Stephane Eranian (2): perf evsel: Add prev_raw_count field perf stat: Add interval printing Sukadev Bhattiprolu (1): perf tools, powerpc: Fix compile warnings in tests/attr.c Thomas Jarosch (5): perf tools: Fix possible double free on error perf sort: Use pclose() instead of fclose() on pipe stream perf tools: Fix memory leak on error perf header: Fix memory leak for the Not caching a kptr_restrict'ed /proc/kallsyms case perf header: Fix double fclose() on do_write(fd, xxx) failure tools/Makefile |2 +- tools/lib/traceevent/event-parse.c | 39 +- tools/perf/Documentation/perf-stat.txt |4 + tools/perf/Makefile | 13 + tools/perf/arch/common.c |1 + tools/perf/bench/bench.h |1 + tools/perf/bench/numa.c | 1731 ++ tools/perf/builtin-bench.c | 17 + tools/perf/builtin-kmem.c|6 +- tools/perf/builtin-stat.c| 158 ++- tools/perf/config/feature-tests.mak | 11 + tools/perf/tests/attr.c |5 + tools/perf/tests/open-syscall-all-cpus.c |1 + tools/perf/tests/perf-record.c | 12 +- tools/perf/tests/vmlinux-kallsyms.c |4 +- tools/perf/ui/browser.c |2 + tools/perf/util/event.c |4 +- tools/perf/util/evsel.c | 31 + tools/perf/util/evsel.h |2 + tools/perf/util/header.c | 25 +- tools/perf/util/map.c| 118 +- tools/perf/util/map.h| 24 +- tools/perf/util/sort.c |7 +- tools/perf/util/strlist.c| 54 +- tools/perf/util/strlist.h| 42 +- 25 files changed, 2154 insertions(+), 160 deletions(-) create mode 100644 tools/perf/bench/numa.c Pulled, thanks a lot Arnaldo! Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at
Re: [GIT PULL 00/21] perf/core improvements and fixes
* Arnaldo Carvalho de Melo wrote: > Hi Ingo, > > Please consider pulling. > > - Arnaldo > > The following changes since commit 8dfec403e39b7c37fd6e8813bacc01da1e1210ab: > > perf tests: Removing 'optional' field (2012-11-05 14:03:59 -0300) > > are available in the git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux > tags/perf-core-for-mingo > > for you to fetch changes up to 27f94d52394003d444a383eaf8d4824daf32432e: > > tools lib traceevent: Use 'const' in variables pointing to const strings > (2012-11-09 17:42:47 -0300) > > > perf/core improvements and fixes: > > . Add a 'link' method for hists, so that we can have the leader with > buckets for all the entries in all the hists. This new method > is now used in the default 'diff' output, making the sum of the 'baseline' > column be 100%, eliminating blind spots. Now we need to use this > for 'diff' with > 2 perf.data files and for multi event 'report' and > 'annotate'. > > . libtraceevent fixes for compiler warnings trying to make perf it build > on some distros, like fedora 14, 32-bit, some of the warnings really > pointed to real bugs. > > . Remove temp dir on failure in 'perf test', fix from Jiri Olsa. > > . Fixes for handling data, stack mmaps, from Namhyung Kim. > > . Fix live annotation bug related to recent objdump lookup patches, from > Namhyung Kim > > . Don't try to follow jump target on PLT symbols in the annotation browser, > fix from Namhyung Kim. > > . Fix leak on hist_entry delete, from Namhyung Kim. > > . Fix a CPU_ALLOC related build error on builtin-test, from Zheng Liu. > > Signed-off-by: Arnaldo Carvalho de Melo > > > Andi Kleen (1): > perf tools: Add arbitary aliases and support names with - > > Arnaldo Carvalho de Melo (10): > perf diff: Start moving to support matching more than two hists > perf diff: Move hists__match to the hists lib > perf hists: Introduce hists__link > perf diff: Use hists__link when not pairing just with baseline > perf machine: Move more methods to machine.[ch] > tools lib traceevent: Add __maybe_unused to unused parameters > tools lib traceevent: Avoid comparisions between signed/unsigned > tools lib traceevent: No need to check for < 0 on an unsigned enum > tools lib traceevent: Handle INVALID_ARG_TYPE errno in pevent_strerror > tools lib traceevent: Use 'const' in variables pointing to const strings > > Jiri Olsa (2): > perf tests: Move attr.py temp dir cleanup into finally section > perf tools: Add LIBDW_DIR Makefile variable to for alternate libdw > > Namhyung Kim (7): > perf machine: Set kernel data mapping length > perf tools: Fix detection of stack area > perf hists: Free branch_info when freeing hist_entry > perf tools: Don't try to lookup objdump for live mode > perf annotate: Whitespace fixups > perf annotate: Don't try to follow jump target on PLT symbols > perf annotate: Merge same lines in summary view > > Zheng Liu (1): > perf test: fix a build error on builtin-test > > tools/lib/traceevent/event-parse.c | 22 ++-- > tools/perf/Makefile| 12 ++- > tools/perf/arch/common.c |7 ++ > tools/perf/builtin-diff.c | 48 ++--- > tools/perf/tests/attr.py | 30 +++--- > tools/perf/tests/builtin-test.c| 39 +++ > tools/perf/tests/dso-data.c|1 + > tools/perf/ui/browsers/annotate.c | 12 +++ > tools/perf/ui/hist.c | 10 +- > tools/perf/util/annotate.c | 69 ++-- > tools/perf/util/annotate.h |1 + > tools/perf/util/dso.c |1 + > tools/perf/util/hist.c | 100 ++ > tools/perf/util/hist.h |3 + > tools/perf/util/machine.c | 205 > ++-- > tools/perf/util/machine.h | 131 ++- > tools/perf/util/map.c | 181 +-- > tools/perf/util/map.h | 93 > tools/perf/util/parse-events.l |2 + > tools/perf/util/session.h |5 +- > tools/perf/util/sort.h | 27 - > tools/perf/util/symbol.c |1 + > tools/perf/util/symbol.h | 20 > 23 files changed, 604 insertions(+), 416 deletions(-) Pulled, thanks Arnaldo! Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
* Arnaldo Carvalho de Melo a...@infradead.org wrote: Hi Ingo, Please consider pulling. - Arnaldo The following changes since commit 8dfec403e39b7c37fd6e8813bacc01da1e1210ab: perf tests: Removing 'optional' field (2012-11-05 14:03:59 -0300) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux tags/perf-core-for-mingo for you to fetch changes up to 27f94d52394003d444a383eaf8d4824daf32432e: tools lib traceevent: Use 'const' in variables pointing to const strings (2012-11-09 17:42:47 -0300) perf/core improvements and fixes: . Add a 'link' method for hists, so that we can have the leader with buckets for all the entries in all the hists. This new method is now used in the default 'diff' output, making the sum of the 'baseline' column be 100%, eliminating blind spots. Now we need to use this for 'diff' with 2 perf.data files and for multi event 'report' and 'annotate'. . libtraceevent fixes for compiler warnings trying to make perf it build on some distros, like fedora 14, 32-bit, some of the warnings really pointed to real bugs. . Remove temp dir on failure in 'perf test', fix from Jiri Olsa. . Fixes for handling data, stack mmaps, from Namhyung Kim. . Fix live annotation bug related to recent objdump lookup patches, from Namhyung Kim . Don't try to follow jump target on PLT symbols in the annotation browser, fix from Namhyung Kim. . Fix leak on hist_entry delete, from Namhyung Kim. . Fix a CPU_ALLOC related build error on builtin-test, from Zheng Liu. Signed-off-by: Arnaldo Carvalho de Melo a...@redhat.com Andi Kleen (1): perf tools: Add arbitary aliases and support names with - Arnaldo Carvalho de Melo (10): perf diff: Start moving to support matching more than two hists perf diff: Move hists__match to the hists lib perf hists: Introduce hists__link perf diff: Use hists__link when not pairing just with baseline perf machine: Move more methods to machine.[ch] tools lib traceevent: Add __maybe_unused to unused parameters tools lib traceevent: Avoid comparisions between signed/unsigned tools lib traceevent: No need to check for 0 on an unsigned enum tools lib traceevent: Handle INVALID_ARG_TYPE errno in pevent_strerror tools lib traceevent: Use 'const' in variables pointing to const strings Jiri Olsa (2): perf tests: Move attr.py temp dir cleanup into finally section perf tools: Add LIBDW_DIR Makefile variable to for alternate libdw Namhyung Kim (7): perf machine: Set kernel data mapping length perf tools: Fix detection of stack area perf hists: Free branch_info when freeing hist_entry perf tools: Don't try to lookup objdump for live mode perf annotate: Whitespace fixups perf annotate: Don't try to follow jump target on PLT symbols perf annotate: Merge same lines in summary view Zheng Liu (1): perf test: fix a build error on builtin-test tools/lib/traceevent/event-parse.c | 22 ++-- tools/perf/Makefile| 12 ++- tools/perf/arch/common.c |7 ++ tools/perf/builtin-diff.c | 48 ++--- tools/perf/tests/attr.py | 30 +++--- tools/perf/tests/builtin-test.c| 39 +++ tools/perf/tests/dso-data.c|1 + tools/perf/ui/browsers/annotate.c | 12 +++ tools/perf/ui/hist.c | 10 +- tools/perf/util/annotate.c | 69 ++-- tools/perf/util/annotate.h |1 + tools/perf/util/dso.c |1 + tools/perf/util/hist.c | 100 ++ tools/perf/util/hist.h |3 + tools/perf/util/machine.c | 205 ++-- tools/perf/util/machine.h | 131 ++- tools/perf/util/map.c | 181 +-- tools/perf/util/map.h | 93 tools/perf/util/parse-events.l |2 + tools/perf/util/session.h |5 +- tools/perf/util/sort.h | 27 - tools/perf/util/symbol.c |1 + tools/perf/util/symbol.h | 20 23 files changed, 604 insertions(+), 416 deletions(-) Pulled, thanks Arnaldo! Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
On Mon, 12 Nov 2012 13:01:39 -0300, Arnaldo Carvalho de Melo wrote: > Em Mon, Nov 12, 2012 at 02:55:46PM +0100, Jiri Olsa escreveu: >> On Mon, Nov 12, 2012 at 11:10:52AM +0900, Namhyung Kim wrote: >> > On Fri, 9 Nov 2012 18:42:49 -0300, Arnaldo Carvalho de Melo wrote: >> > > . Add a 'link' method for hists, so that we can have the leader with >> > > buckets for all the entries in all the hists. This new method >> > > is now used in the default 'diff' output, making the sum of the >> > > 'baseline' >> > > column be 100%, eliminating blind spots. Now we need to use this >> > > for 'diff' with > 2 perf.data files and for multi event 'report' and >> > > 'annotate'. > >> > I'm not sure it can be used for group report at least in its current >> > form. IIUC it connects multiple hist entries using a list head and >> > create a dummy entry in the leader if need be. But it didn't handle >> > non-leader entries so it's hard to tell which is which if less entries >> > are present only. For example consider following case: > >> >leader member1 member2 >> >A A A >> >B >> >C >> >D > >> > where leader, member1 and member2 are evsel/hists and A, B, C and D are >> > hist entries. After 'linking' the entries the leader will have >> > following linkage: > >> >leader >> >A -> A -> A >> >B >> >C (dummy) ->C >> >D (dummy) -> D > >> > In this case, for entry A the leader can determine which entry came from >> > which hists by looking its order in the list. For entry B the leader >> > can use zero value for them since the list is empty. However for >> > entries C and D, it cannot know which one is the right hists unless it >> > records a hist index or creates dummy entry and insert it in a correct >> > order (looks far from an optimal solution). Am I missing something? > >> there's hists pointer in hist_entry if that's what you look for > > And from there to evsel->idx. In your patchset you even introduce > hists_2_evsel(), right? Ah, okay. I worried about a possiblity of non-consecutive event groups for some reason, but that's not gonna happen in the future? Thanks, Namhyung -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
Em Mon, Nov 12, 2012 at 02:55:46PM +0100, Jiri Olsa escreveu: > On Mon, Nov 12, 2012 at 11:10:52AM +0900, Namhyung Kim wrote: > > On Fri, 9 Nov 2012 18:42:49 -0300, Arnaldo Carvalho de Melo wrote: > > > . Add a 'link' method for hists, so that we can have the leader with > > > buckets for all the entries in all the hists. This new method > > > is now used in the default 'diff' output, making the sum of the > > > 'baseline' > > > column be 100%, eliminating blind spots. Now we need to use this > > > for 'diff' with > 2 perf.data files and for multi event 'report' and > > > 'annotate'. > > I'm not sure it can be used for group report at least in its current > > form. IIUC it connects multiple hist entries using a list head and > > create a dummy entry in the leader if need be. But it didn't handle > > non-leader entries so it's hard to tell which is which if less entries > > are present only. For example consider following case: > > leader member1 member2 > > A A A > > B > > C > > D > > where leader, member1 and member2 are evsel/hists and A, B, C and D are > > hist entries. After 'linking' the entries the leader will have > > following linkage: > > leader > > A -> A -> A > > B > > C (dummy) ->C > > D (dummy) -> D > > In this case, for entry A the leader can determine which entry came from > > which hists by looking its order in the list. For entry B the leader > > can use zero value for them since the list is empty. However for > > entries C and D, it cannot know which one is the right hists unless it > > records a hist index or creates dummy entry and insert it in a correct > > order (looks far from an optimal solution). Am I missing something? > there's hists pointer in hist_entry if that's what you look for And from there to evsel->idx. In your patchset you even introduce hists_2_evsel(), right? - Arnaldo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
On Mon, Nov 12, 2012 at 11:10:52AM +0900, Namhyung Kim wrote: > Hi Arnaldo, > > On Fri, 9 Nov 2012 18:42:49 -0300, Arnaldo Carvalho de Melo wrote: > > Hi Ingo, > > > > Please consider pulling. > > > > - Arnaldo > > > > The following changes since commit 8dfec403e39b7c37fd6e8813bacc01da1e1210ab: > > > > perf tests: Removing 'optional' field (2012-11-05 14:03:59 -0300) > > > > are available in the git repository at: > > > > git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux > > tags/perf-core-for-mingo > > > > for you to fetch changes up to 27f94d52394003d444a383eaf8d4824daf32432e: > > > > tools lib traceevent: Use 'const' in variables pointing to const strings > > (2012-11-09 17:42:47 -0300) > > > > > > perf/core improvements and fixes: > > > > . Add a 'link' method for hists, so that we can have the leader with > > buckets for all the entries in all the hists. This new method > > is now used in the default 'diff' output, making the sum of the 'baseline' > > column be 100%, eliminating blind spots. Now we need to use this > > for 'diff' with > 2 perf.data files and for multi event 'report' and > > 'annotate'. > > I'm not sure it can be used for group report at least in its current > form. IIUC it connects multiple hist entries using a list head and > create a dummy entry in the leader if need be. But it didn't handle > non-leader entries so it's hard to tell which is which if less entries > are present only. For example consider following case: > > leader member1 member2 > A A A > B > C > D > > where leader, member1 and member2 are evsel/hists and A, B, C and D are > hist entries. After 'linking' the entries the leader will have > following linkage: > > leader > A -> A -> A > B > C (dummy) ->C > D (dummy) -> D > > In this case, for entry A the leader can determine which entry came from > which hists by looking its order in the list. For entry B the leader > can use zero value for them since the list is empty. However for > entries C and D, it cannot know which one is the right hists unless it > records a hist index or creates dummy entry and insert it in a correct > order (looks far from an optimal solution). Am I missing something? there's hists pointer in hist_entry if that's what you look for jirka -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
On Mon, Nov 12, 2012 at 11:10:52AM +0900, Namhyung Kim wrote: Hi Arnaldo, On Fri, 9 Nov 2012 18:42:49 -0300, Arnaldo Carvalho de Melo wrote: Hi Ingo, Please consider pulling. - Arnaldo The following changes since commit 8dfec403e39b7c37fd6e8813bacc01da1e1210ab: perf tests: Removing 'optional' field (2012-11-05 14:03:59 -0300) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux tags/perf-core-for-mingo for you to fetch changes up to 27f94d52394003d444a383eaf8d4824daf32432e: tools lib traceevent: Use 'const' in variables pointing to const strings (2012-11-09 17:42:47 -0300) perf/core improvements and fixes: . Add a 'link' method for hists, so that we can have the leader with buckets for all the entries in all the hists. This new method is now used in the default 'diff' output, making the sum of the 'baseline' column be 100%, eliminating blind spots. Now we need to use this for 'diff' with 2 perf.data files and for multi event 'report' and 'annotate'. I'm not sure it can be used for group report at least in its current form. IIUC it connects multiple hist entries using a list head and create a dummy entry in the leader if need be. But it didn't handle non-leader entries so it's hard to tell which is which if less entries are present only. For example consider following case: leader member1 member2 A A A B C D where leader, member1 and member2 are evsel/hists and A, B, C and D are hist entries. After 'linking' the entries the leader will have following linkage: leader A - A - A B C (dummy) -C D (dummy) - D In this case, for entry A the leader can determine which entry came from which hists by looking its order in the list. For entry B the leader can use zero value for them since the list is empty. However for entries C and D, it cannot know which one is the right hists unless it records a hist index or creates dummy entry and insert it in a correct order (looks far from an optimal solution). Am I missing something? there's hists pointer in hist_entry if that's what you look for jirka -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
Em Mon, Nov 12, 2012 at 02:55:46PM +0100, Jiri Olsa escreveu: On Mon, Nov 12, 2012 at 11:10:52AM +0900, Namhyung Kim wrote: On Fri, 9 Nov 2012 18:42:49 -0300, Arnaldo Carvalho de Melo wrote: . Add a 'link' method for hists, so that we can have the leader with buckets for all the entries in all the hists. This new method is now used in the default 'diff' output, making the sum of the 'baseline' column be 100%, eliminating blind spots. Now we need to use this for 'diff' with 2 perf.data files and for multi event 'report' and 'annotate'. I'm not sure it can be used for group report at least in its current form. IIUC it connects multiple hist entries using a list head and create a dummy entry in the leader if need be. But it didn't handle non-leader entries so it's hard to tell which is which if less entries are present only. For example consider following case: leader member1 member2 A A A B C D where leader, member1 and member2 are evsel/hists and A, B, C and D are hist entries. After 'linking' the entries the leader will have following linkage: leader A - A - A B C (dummy) -C D (dummy) - D In this case, for entry A the leader can determine which entry came from which hists by looking its order in the list. For entry B the leader can use zero value for them since the list is empty. However for entries C and D, it cannot know which one is the right hists unless it records a hist index or creates dummy entry and insert it in a correct order (looks far from an optimal solution). Am I missing something? there's hists pointer in hist_entry if that's what you look for And from there to evsel-idx. In your patchset you even introduce hists_2_evsel(), right? - Arnaldo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
On Mon, 12 Nov 2012 13:01:39 -0300, Arnaldo Carvalho de Melo wrote: Em Mon, Nov 12, 2012 at 02:55:46PM +0100, Jiri Olsa escreveu: On Mon, Nov 12, 2012 at 11:10:52AM +0900, Namhyung Kim wrote: On Fri, 9 Nov 2012 18:42:49 -0300, Arnaldo Carvalho de Melo wrote: . Add a 'link' method for hists, so that we can have the leader with buckets for all the entries in all the hists. This new method is now used in the default 'diff' output, making the sum of the 'baseline' column be 100%, eliminating blind spots. Now we need to use this for 'diff' with 2 perf.data files and for multi event 'report' and 'annotate'. I'm not sure it can be used for group report at least in its current form. IIUC it connects multiple hist entries using a list head and create a dummy entry in the leader if need be. But it didn't handle non-leader entries so it's hard to tell which is which if less entries are present only. For example consider following case: leader member1 member2 A A A B C D where leader, member1 and member2 are evsel/hists and A, B, C and D are hist entries. After 'linking' the entries the leader will have following linkage: leader A - A - A B C (dummy) -C D (dummy) - D In this case, for entry A the leader can determine which entry came from which hists by looking its order in the list. For entry B the leader can use zero value for them since the list is empty. However for entries C and D, it cannot know which one is the right hists unless it records a hist index or creates dummy entry and insert it in a correct order (looks far from an optimal solution). Am I missing something? there's hists pointer in hist_entry if that's what you look for And from there to evsel-idx. In your patchset you even introduce hists_2_evsel(), right? Ah, okay. I worried about a possiblity of non-consecutive event groups for some reason, but that's not gonna happen in the future? Thanks, Namhyung -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
Hi Arnaldo, On Fri, 9 Nov 2012 18:42:49 -0300, Arnaldo Carvalho de Melo wrote: > Hi Ingo, > > Please consider pulling. > > - Arnaldo > > The following changes since commit 8dfec403e39b7c37fd6e8813bacc01da1e1210ab: > > perf tests: Removing 'optional' field (2012-11-05 14:03:59 -0300) > > are available in the git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux > tags/perf-core-for-mingo > > for you to fetch changes up to 27f94d52394003d444a383eaf8d4824daf32432e: > > tools lib traceevent: Use 'const' in variables pointing to const strings > (2012-11-09 17:42:47 -0300) > > > perf/core improvements and fixes: > > . Add a 'link' method for hists, so that we can have the leader with > buckets for all the entries in all the hists. This new method > is now used in the default 'diff' output, making the sum of the 'baseline' > column be 100%, eliminating blind spots. Now we need to use this > for 'diff' with > 2 perf.data files and for multi event 'report' and > 'annotate'. I'm not sure it can be used for group report at least in its current form. IIUC it connects multiple hist entries using a list head and create a dummy entry in the leader if need be. But it didn't handle non-leader entries so it's hard to tell which is which if less entries are present only. For example consider following case: leader member1 member2 A A A B C D where leader, member1 and member2 are evsel/hists and A, B, C and D are hist entries. After 'linking' the entries the leader will have following linkage: leader A -> A -> A B C (dummy) ->C D (dummy) -> D In this case, for entry A the leader can determine which entry came from which hists by looking its order in the list. For entry B the leader can use zero value for them since the list is empty. However for entries C and D, it cannot know which one is the right hists unless it records a hist index or creates dummy entry and insert it in a correct order (looks far from an optimal solution). Am I missing something? Thanks, Namhyung -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL 00/21] perf/core improvements and fixes
Hi Arnaldo, On Fri, 9 Nov 2012 18:42:49 -0300, Arnaldo Carvalho de Melo wrote: Hi Ingo, Please consider pulling. - Arnaldo The following changes since commit 8dfec403e39b7c37fd6e8813bacc01da1e1210ab: perf tests: Removing 'optional' field (2012-11-05 14:03:59 -0300) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux tags/perf-core-for-mingo for you to fetch changes up to 27f94d52394003d444a383eaf8d4824daf32432e: tools lib traceevent: Use 'const' in variables pointing to const strings (2012-11-09 17:42:47 -0300) perf/core improvements and fixes: . Add a 'link' method for hists, so that we can have the leader with buckets for all the entries in all the hists. This new method is now used in the default 'diff' output, making the sum of the 'baseline' column be 100%, eliminating blind spots. Now we need to use this for 'diff' with 2 perf.data files and for multi event 'report' and 'annotate'. I'm not sure it can be used for group report at least in its current form. IIUC it connects multiple hist entries using a list head and create a dummy entry in the leader if need be. But it didn't handle non-leader entries so it's hard to tell which is which if less entries are present only. For example consider following case: leader member1 member2 A A A B C D where leader, member1 and member2 are evsel/hists and A, B, C and D are hist entries. After 'linking' the entries the leader will have following linkage: leader A - A - A B C (dummy) -C D (dummy) - D In this case, for entry A the leader can determine which entry came from which hists by looking its order in the list. For entry B the leader can use zero value for them since the list is empty. However for entries C and D, it cannot know which one is the right hists unless it records a hist index or creates dummy entry and insert it in a correct order (looks far from an optimal solution). Am I missing something? Thanks, Namhyung -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
