- 16 Apr, 2024 3 commits
-
-
Namhyung Kim authored
In match_var_offset(), it just checked the end address of the variable with the given offset because it assumed the register holds a pointer to the data type and the offset starts from the base. But I found some cases that the stack pointer (rsp = reg7) register is used to pointer a stack variable while the frame base is maintained by a different register (rbp = reg6). In that case, it cannot simply use the stack pointer as it cannot guarantee that it points to the frame base. So it needs to check both boundaries of the variable location. Before: ----------------------------------------------------------- find data type for 0x7c(reg7) at tcp_getsockopt+0xb62 CU for net/ipv4/tcp.c (die:0x7b5f516) frame base: cfa=0 fbreg=6 no pointer or no type check variable "tss" failed (die: 0x7b95801) variable location: base reg7, offset=0x110 type='struct scm_timestamping_internal' size=0x30 (die:0x7b8c126) So the current code just checks register number for the non-PC and non-FB registers and assuming it has offset 0. But this variable has offset 0x110 so it should not match to this. After: ----------------------------------------------------------- find data type for 0x7c(reg7) at tcp_getsockopt+0xb62 CU for net/ipv4/tcp.c (die:0x7b5f516) frame base: cfa=0 fbreg=6 no pointer or no type check variable "zc" failed (die: 0x7b9580a) variable location: base=reg7, offset=0x40 type='struct tcp_zerocopy_receive' size=0x40 (die:7b947f4) Now it find the correct variable "zc". It was located at reg7 + 0x40 and the size if 0x40 which means it should cover [0x40, 0x80). And the access was for reg7 + 0x7c so it found the right one. But it still failed to use the variable and it would be handled in the next patch. Signed-off-by:
Namhyung Kim <namhyung@kernel.org> Cc: Adrian Hunter <adrian.hunter@intel.com> Cc: Ian Rogers <irogers@google.com> Cc: Ingo Molnar <mingo@kernel.org> Cc: Jiri Olsa <jolsa@kernel.org> Cc: Kan Liang <kan.liang@linux.intel.com> Cc: Masami Hiramatsu <mhiramat@kernel.org> Cc: Peter Zijlstra <peterz@infradead.org> Link: https://lore.kernel.org/r/20240412183310.2518474-4-namhyung@kernel.orgSigned-off-by:
Arnaldo Carvalho de Melo <acme@redhat.com>
-
Namhyung Kim authored
In match_var_offset(), it checks the offset range with the target type only for non-pointer types. But it also needs to check the pointer types with the target type. This is because there can be more than one pointer variable located in the same register. Let's look at the following example. It's looking up a variable for reg3 at tcp_get_info+0x62. It found "sk" variable but it wasn't the right one since it accesses beyond the target type (struct 'sock' in this case) size. ----------------------------------------------------------- find data type for 0x7bc(reg3) at tcp_get_info+0x62 CU for net/ipv4/tcp.c (die:0x7b5f516) frame base: cfa=0 fbreg=6 offset: 1980 is bigger than size: 760 check variable "sk" failed (die: 0x7b92b2c) variable location: reg3 type='struct sock' size=0x2f8 (die:0x7b63c3ab) Actually there was another variable "tp" in the function and it's located at the same (reg3) because it's just type-casted like below. void tcp_get_info(struct sock *sk, struct tcp_info *info) { const struct tcp_sock *tp = tcp_sk(sk); ... The 'struct tcp_sock' contains the 'struct sock' at offset 0 so it can just use the same address as a pointer to tcp_sock. That means it should match variables correctly by checking the offset and size. Actually it cannot distinguish if the offset was smaller than the size of the original struct sock. But I think it's fine as they are the same at that part. So let's check the target type size and retry if it doesn't match. Now it succeeded to find the correct variable. ----------------------------------------------------------- find data type for 0x7bc(reg3) at tcp_get_info+0x62 CU for net/ipv4/tcp.c (die:0x7b5f516) frame base: cfa=0 fbreg=6 found "tp" in scope=1/1 (die: 0x7b92b16) type_offset=0x7bc variable location: reg3 type='struct tcp_sock' size=0xa68 (die:0x7b81380) Fixes: bc10db8e ("perf annotate-data: Support stack variables") Signed-off-by:
-
Namhyung Kim authored
To verify it found the correct variable, let's add the location expression to the debug message. $ perf --debug type-profile annotate --data-type ... ----------------------------------------------------------- find data type for 0xaf0(reg15) at schedule+0xeb CU for kernel/sched/core.c (die:0x1180523) frame base: cfa=0 fbreg=6 found "rq" in scope=3/4 (die: 0x11b6a00) type_offset=0xaf0 variable location: reg15 type='struct rq' size=0xfc0 (die:0x11892e2) ----------------------------------------------------------- find data type for 0x7bc(reg3) at tcp_get_info+0x62 CU for net/ipv4/tcp.c (die:0x7b5f516) frame base: cfa=0 fbreg=6 offset: 1980 is bigger than size: 760 check variable "sk" failed (die: 0x7b92b2c) variable location: reg3 type='struct sock' size=0x2f8 (die:0x7b63c3ab) ----------------------------------------------------------- ... The first case is fine. It looked up a data type in r15 with offset of 0xaf0 at schedule+0xeb. It found the CU die and the frame base info and the variable "rq" was found in the scope 3/4. Its location is the r15 register and the type size is 0xfc0 which includes 0xaf0. But the second case is not good. It looked up a data type in rbx (reg3) with offset 0x7bc. It found a CU and the frame base which is good so far. And it also found a variable "sk" but the access offset is bigger than the type size (1980 vs. 760 or 0x7bc vs. 0x2f8). The variable has the right location (reg3) but I need to figure out why it accesses beyond what it's supposed to. Signed-off-by:
-
- 12 Apr, 2024 29 commits
-
-
Ian Rogers authored
Name benchmarks with _ret at the end to avoid creating a new set of benchmarks. Signed-off-by:
Ian Rogers <irogers@google.com> Acked-by:
Jiri Olsa <jolsa@kernel.org> Cc: Adrian Hunter <adrian.hunter@intel.com> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> Cc: Andrei Vagin <avagin@google.com> Cc: Ingo Molnar <mingo@redhat.com> Cc: Kan Liang <kan.liang@linux.intel.com> Cc: Kees Kook <keescook@chromium.org> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Namhyung Kim <namhyung@kernel.org> Cc: Peter Zijlstra <peterz@infradead.org> Link: https://lore.kernel.org/r/20240406040911.1603801-2-irogers@google.comSigned-off-by:
-
Ian Rogers authored
bpf_program__attach_uprobe_opts will search LD_LIBRARY_PATH and so specifying `/lib64` is unnecessary and causes failures for libc.so.6 paths like `/lib/x86_64-linux-gnu/libc.so.6`. Fixes: 7b47623b ("perf bench uprobe trace_printk: Add entry attaching an BPF program that does a trace_printk") Signed-off-by:
-
Ian Rogers authored
Add shell check to scripts generating perf trace lookup tables. Fix quoting issue in arch_errno_names.sh. Reviewed-by:
James Clark <james.clark@arm.com> Signed-off-by:
-
Ian Rogers authored
Add shellcheck to generate-cmdlist.sh to avoid basic shell script mistakes. Reviewed-by:
-
Ian Rogers authored
Add shellcheck for: tools/perf/arch/x86/tests/gen-insn-x86-dat.sh tools/perf/arch/x86/entry/syscalls/syscalltbl.sh Address a minor quoting issue. Reviewed-by:
-
Ian Rogers authored
Address shell check errors/warnings in perf-archive.sh and perf-completion.sh. Reviewed-by:
-
Ian Rogers authored
Events like for sapphirerapids have '\r' in the uncore descriptions. The non-escaped versions of this fail JSON validation the the 'perf list' test. Signed-off-by:
-
Ian Rogers authored
Switch loops within dsos.c, add a version that isn't locked. Switch some unlocked loops to hold the read lock. Signed-off-by:
-
Ian Rogers authored
Move dso and dso_id functions to dso.c to match the struct declarations. Signed-off-by:
-
Ian Rogers authored
To better abstract the dsos internals, introduce dsos__for_each_dso that does a callback on each dso. This also means the read lock can be correctly held. Signed-off-by:
-
Ian Rogers authored
Move more functionality into dsos.c generally from machine.c, renaming functions to match their new usage. The find function is made to always "get" before returning a dso. Reduce the scope of locks in vdso to match the locking paradigm. Signed-off-by:
-
Ian Rogers authored
Move functions from machine and build-id to dsos. Pass 'struct dsos' rather than internal state. Rename some functions to better represent which data structure they operate on. Signed-off-by:
-
Adrian Hunter authored
evlist__config() might mess up the debug output consumed by test "Test per-thread recording" in "Miscellaneous Intel PT testing". Move it out from between the debug prints: "perf record opening and mmapping events" and "perf record done opening and mmapping events" Fixes: da406202 ("perf tools: Add debug messages and comments for testing") Closes: https://lore.kernel.org/linux-perf-users/ZhVfc5jYLarnGzKa@x1/Reported-by:
Arnaldo Carvalho de Melo <acme@kernel.org> Signed-off-by:
Adrian Hunter <adrian.hunter@intel.com> Tested-by:
-
Namhyung Kim authored
In some data file, I see the following messages repeated. It seems it doesn't have DSOs in the system and the dso->binary_type is set to DSO_BINARY_TYPE__NOT_FOUND. Let's skip them to avoid the followings. No output from objdump --start-address=0x0000000000000000 --stop-address=0x00000000000000d4 -d --no-show-raw-insn -C "$1" Error running objdump --start-address=0x0000000000000000 --stop-address=0x0000000000000631 -d --no-show-raw-insn -C "$1" ... Closes: https://lore.kernel.org/linux-perf-users/15e1a2847b8cebab4de57fc68e033086aa6980ce.camel@yandex.ru/Reported-by:
Konstantin Kharlamov <Hi-Angel@yandex.ru> Reviewed-by:
-
Namhyung Kim authored
The data type profiling alone doesn't need the sample histogram for functions. It only needs the histogram for the types. Let's remove the condition in the report_callback to check if data type profiling is selected and make sure the annotation has the 'struct annotated_source' instantiated before calling symbol__disassemble(). Reviewed-by:
-
Namhyung Kim authored
When the hist entry has the type info, it should be able to display the annotation browser for the type like in `perf annotate --data-type`. Reviewed-by:
-
Namhyung Kim authored
Like in stdio, it should print all events in a group together. Committer notes: Collect it: root@number:~# perf record -a -e '{cpu_core/mem-loads,ldlat=30/P,cpu_core/mem-stores/P}' ^C[ perf record: Woken up 8 times to write data ] [ perf record: Captured and wrote 4.980 MB perf.data (55825 samples) ] root@number:~# Then do it in stdio: root@number:~# perf annotate --stdio --data-type Annotate type: 'union ' in /usr/lib64/libc.so.6 (1131 samples): event[0] = cpu_core/mem-loads,ldlat=30/P event[1] = cpu_core/mem-stores/P ============================================================================ Percent offset size field 100.00 100.00 0 40 union { 100.00 100.00 0 40 struct __pthread_mutex_s __data { 48.61 23.46 0 4 int __lock; 0.00 0.48 4 4 unsigned int __count; 6.38 41.32 8 4 int __owner; 8.74 34.02 12 4 unsigned int __nusers; 35.66 0.26 16 4 int __kind; 0.61 0.45 20 2 short int __spins; 0.00 0.00 22 2 short int __elision; 0.00 0.00 24 16 __pthread_list_t __list { 0.00 0.00 24 8 struct __pthread_internal_list* __prev; 0.00 0.00 32 8 struct __pthread_internal_list* __next; }; }; 0.00 0.00 0 0 char* __size; 48.61 23.94 0 8 long int __align; }; Now with TUI before this patch: root@number:~# perf annotate --tui --data-type Annotate type: 'union ' (790 samples) Percent Offset Size Field 100.00 0 40 union { 100.00 0 40 struct __pthread_mutex_s __data { 48.61 0 4 int __lock; 0.00 4 4 unsigned int __count; 6.38 8 4 int __owner; 8.74 12 4 unsigned int __nusers; 35.66 16 4 int __kind; 0.61 20 2 short int __spins; 0.00 22 2 short int __elision; 0.00 24 16 __pthread_list_t __list { 0.00 24 8 struct __pthread_internal_list* __prev; 0.00 32 8 struct __pthread_internal_list* __next; 0.00 0 0 char* __size; 48.61 0 8 long int __align; }; And now after this patch: Annotate type: 'union ' (790 samples) Percent Offset Size Field 100.00 100.00 0 40 union { 100.00 100.00 0 40 struct __pthread_mutex_s __data { 48.61 23.46 0 4 int __lock; 0.00 0.48 4 4 unsigned int __count; 6.38 41.32 8 4 int __owner; 8.74 34.02 12 4 unsigned int __nusers; 35.66 0.26 16 4 int __kind; 0.61 0.45 20 2 short int __spins; 0.00 0.00 22 2 short int __elision; 0.00 0.00 24 16 __pthread_list_t __list { 0.00 0.00 24 8 struct __pthread_internal_list* __prev; 0.00 0.00 32 8 struct __pthread_internal_list* __next; }; }; 0.00 0.00 0 0 char* __size; 48.61 23.94 0 8 long int __align; }; On a followup patch the --tui output should have this that is present in --stdio: And the --stdio has all the missing info in TUI: Annotate type: 'union ' in /usr/lib64/libc.so.6 (1131 samples): event[0] = cpu_core/mem-loads,ldlat=30/P event[1] = cpu_core/mem-stores/P Reviewed-by: -
Namhyung Kim authored
Support data type profiling output on TUI. Testing from Arnaldo: First make sure that the debug information for your workload binaries in embedded in them by building it with '-g' or install the debuginfo packages, since our workload is 'find': root@number:~# type find find is hashed (/usr/bin/find) root@number:~# rpm -qf /usr/bin/find findutils-4.9.0-5.fc39.x86_64 root@number:~# dnf debuginfo-install findutils <SNIP> root@number:~# Then collect some data: root@number:~# echo 1 > /proc/sys/vm/drop_caches root@number:~# perf mem record find / > /dev/null [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 0.331 MB perf.data (3982 samples) ] root@number:~# Finally do data-type annotation with the following command, that will default, as 'perf report' to the --tui mode, with lines colored to highlight the hotspots, etc. root@number:~# perf annotate --data-type Annotate type: 'struct predicate' (58 samples) Percent Offset Size Field 100.00 0 312 struct predicate { 0.00 0 8 PRED_FUNC pred_func; 0.00 8 8 char* p_name; 0.00 16 4 enum predicate_type p_type; 0.00 20 4 enum predicate_precedence p_prec; 0.00 24 1 _Bool side_effects; 0.00 25 1 _Bool no_default_print; 0.00 26 1 _Bool need_stat; 0.00 27 1 _Bool need_type; 0.00 28 1 _Bool need_inum; 0.00 32 4 enum EvaluationCost p_cost; 0.00 36 4 float est_success_rate; 0.00 40 1 _Bool literal_control_chars; 0.00 41 1 _Bool artificial; 0.00 48 8 char* arg_text; <SNIP> Reviewed-by: -
Namhyung Kim authored
And move the related code into util/annotate-data.c file. Reviewed-by:
-
Namhyung Kim authored
Like 'perf report', it can take a while to process samples. Show a progress window to inform users how that it is not stuck. Reviewed-by:
-
Namhyung Kim authored
It's a pseudo data type and has no field. Fixes: b3c95109 ("perf annotate-data: Add stack canary type") Closes: https://lore.kernel.org/lkml/Zhb6jJneP36Z-or0@x1Reported-by:
-
James Clark authored
This check can be done with uname which is more portable. At the same time re-arrange it into a standard if statement so that it's more readable. Reviewed-by:
-
James Clark authored
In a debug build there is validation that mmap lists are sorted when taking a lock. In machine__update_kernel_mmap() the start and end addresses are updated resulting in an unsorted list before the map is removed from the list. When the map is removed, the lock is taken which triggers the validation and the failure: $ perf test "object code reading" --- start --- perf: util/maps.c:88: check_invariants: Assertion `map__start(prev) <= map__start(map)' failed. Aborted Fix it by updating the addresses after removal, but before insertion. The bug depends on the ordering and type of debug info on the system and doesn't reproduce everywhere. Fixes: 659ad349 ("perf maps: Switch from rbtree to lazily sorted array for addresses") Reviewed-by:
-
James Clark authored
PERF_PMU_CAP_EXTENDED_HW_TYPE results in multiple events being opened on heterogeneous systems. Currently this test only sets its required attributes on the first event. Not disabling enable_on_exec on the other events causes the test to fail because the forked objdump processes are sampled. No tracking event is opened so Perf only knows about its own mappings causing the objdump samples to give the following error: $ perf test -vvv "object code reading" Reading object code for memory address: 0xffff9aaa55ec thread__find_map failed ---- end(-1) ---- 24: Object code reading : FAILED! Fixes: 251aa040 ("perf parse-events: Wildcard most "numeric" events") Reviewed-by:
-
James Clark authored
To prevent anyone from seeing a test failure appear as a regression and thinking that it was caused by their code change, insert some noise into the loop which makes it immune to sampling bias issues (errata 1694299). The "test data symbol" test can fail with any unrelated change that shifts the loop into an unfortunate position in the Perf binary which is almost impossible to debug as the root cause of the test failure. Ultimately it's caused by the referenced errata. Fixes: 60abedb8 ("perf test: Introduce script for data symbol testing") Reviewed-by:
-
Ian Rogers authored
Rather than place metrics without a metric group in "No_group" place them in a a metric group that is their name. Still allow such metrics to be selected if "No_group" is passed, this change just impacts perf list. Reviewed-by:
Kan Liang <kan.liang@linux.intel.com> Signed-off-by:
-
Namhyung Kim authored
Now symbol__annotate() is reentrant and it doesn't need to remove non-instruction lines. Let's get rid of symbol__ensure_annotate() and call symbol__annotate() directly. Also we can use it to get the arch pointer instead of calling evsel__get_arch() directly. Reviewed-by:
-
Namhyung Kim authored
For data type profiling, it removed non-instruction lines from the list of annotation lines. It was to simplify the implementation dealing with instructions like to calculate the PC-relative address and to search the shortest path to the target instruction or basic block. But it means that it removes all the comments and debug information in the annotate output like source file name and line numbers. To support both code annotation and data type annotation, it'd be better to keep the non-instruction lines as well. So this change is to skip those lines during the data type profiling and to display them in the normal perf annotate output. No function changes intended (other than having more lines). Reviewed-by:
-
Namhyung Kim authored
The recent change in the global variable handling added a bug to miss setting the return value even if it found a data type. Also add the type name in the debug message. Fixes: 1ebb5e17 ("perf annotate-data: Add get_global_var_type()") Reviewed-by:
-
- 08 Apr, 2024 8 commits
-
-
Ian Rogers authored
Add non-blocking function to check if a 'struct child_process' has completed. If the process has completed the exit code is stored in the 'struct child_process' so that finish_command() returns it. Signed-off-by:
-
Namhyung Kim authored
It's only used in 'perf annotate' output which means functions with actual samples. No need to consume memory for every symbol ('struct annotation'). Signed-off-by: -
Namhyung Kim authored
It's only used in 'perf annotate' output which means functions with actual samples. No need to consume memory for every symbol ('struct annotation'). Signed-off-by: -
Namhyung Kim authored
It's only used in 'perf annotate' output which means functions with actual samples. No need to consume memory for every symbol ('struct annotation'). Signed-off-by: -
Namhyung Kim authored
It's only used in 'perf annotate' output which means functions with actual samples. No need to consume memory for every symbol ('struct annotation'). Also move the 'max_line_len' field into it as it's related. Signed-off-by: -
Namhyung Kim authored
The struct annotated_source.offsets[] is to save pointers to annotation_line at each offset. We can use annotated_source__get_line() helper instead so let's get rid of the array. Signed-off-by:
-
Namhyung Kim authored
In some places, it checks annotated (disasm) lines for each byte. But as it already has a list of disasm lines, it'd be better to traverse the list entries instead of checking every offset with linear search (by annotated_source__get_line() helper). Signed-off-by:
-
Namhyung Kim authored
It's a helper function to get annotation_line at the given offset without using the offsets array. The goal is to get rid of the offsets array altogether. It just does the linear search but I think it's better to save memory as it won't be called in a hot path. Signed-off-by:
-
