linux-stable.git/tools/perf/util, branch v6.9.2

perf lock contention: Add a missing NULL check

2024-04-11T17:30:06+00:00

I got a report for a failure in BPF verifier on a recent kernel with
perf lock contention command.  It checks task->sighand->siglock without
checking if sighand is NULL or not.  Let's add one.

  ; if (&curr->sighand->siglock == (void *)lock)
  265: (79) r1 = *(u64 *)(r0 +2624)     ; frame1: R0_w=trusted_ptr_task_struct(off=0,imm=0)
                                        ;         R1_w=rcu_ptr_or_null_sighand_struct(off=0,imm=0)
  266: (b7) r2 = 0                      ; frame1: R2_w=0
  267: (0f) r1 += r2
  R1 pointer arithmetic on rcu_ptr_or_null_ prohibited, null-check it first
  processed 164 insns (limit 1000000) max_states_per_insn 1 total_states 15 peak_states 15 mark_read 5
  -- END PROG LOAD LOG --
  libbpf: prog 'contention_end': failed to load: -13
  libbpf: failed to load object 'lock_contention_bpf'
  libbpf: failed to load BPF skeleton 'lock_contention_bpf': -13
  Failed to load lock-contention BPF skeleton
  lock contention BPF setup failed
  lock contention did not detect any lock contention

Fixes: 1811e82767dcc ("perf lock contention: Track and show siglock with address")
Reviewed-by: Ian Rogers 
Acked-by: Arnaldo Carvalho de Melo 
Cc: Song Liu 
Cc: bpf@vger.kernel.org
Signed-off-by: Namhyung Kim 
Link: https://lore.kernel.org/r/20240409225542.1870999-1-namhyung@kernel.org

perf annotate: Make sure to call symbol__annotate2() in TUI

2024-04-11T17:14:58+00:00

The symbol__annotate2() initializes some data structures needed by TUI.
It has a logic to prevent calling it multiple times by checking if it
has the annotated source.  But data type profiling uses a different
code (symbol__annotate) to allocate the annotated lines in advance.
So TUI missed to call symbol__annotate2() when it shows the annotation
browser.

Make symbol__annotate() reentrant and handle that situation properly.
This fixes a crash in the annotation browser started by perf report in
TUI like below.

  $ perf report -s type,sym --tui
  # and press 'a' key and then move down

Fixes: 81e57deec325 ("perf report: Support data type profiling")
Reviewed-by: Ian Rogers 
Signed-off-by: Namhyung Kim 
Link: https://lore.kernel.org/r/20240405211800.1412920-2-namhyung@kernel.org

perf annotate: Add comments in the data structures

2024-03-07T04:25:48+00:00

Reviewed-by: Ian Rogers 
Reviewed-by: Arnaldo Carvalho de Melo 
Tested-by: Arnaldo Carvalho de Melo 
Cc: Andi Kleen 
Signed-off-by: Namhyung Kim 
Link: https://lore.kernel.org/r/20240304230815.1440583-5-namhyung@kernel.org

perf annotate: Remove sym_hist.addr[] array

2024-03-07T04:25:36+00:00

It's not used anymore and the code is coverted to use a hash map.  Now
sym_hist has a static size, so no need to have sizeof_sym_hist in the
struct annotated_source.

Reviewed-by: Ian Rogers 
Reviewed-by: Arnaldo Carvalho de Melo 
Tested-by: Arnaldo Carvalho de Melo 
Cc: Andi Kleen 
Signed-off-by: Namhyung Kim 
Link: https://lore.kernel.org/r/20240304230815.1440583-4-namhyung@kernel.org

perf annotate: Calculate instruction overhead using hashmap

2024-03-07T04:25:20+00:00

Use annotated_source.samples hashmap instead of addr array in the
struct sym_hist.

Reviewed-by: Ian Rogers 
Reviewed-by: Arnaldo Carvalho de Melo 
Tested-by: Arnaldo Carvalho de Melo 
Cc: Andi Kleen 
Signed-off-by: Namhyung Kim 
Link: https://lore.kernel.org/r/20240304230815.1440583-3-namhyung@kernel.org

perf annotate: Add a hashmap for symbol histogram

2024-03-07T04:24:55+00:00

Now symbol histogram uses an array to save per-offset sample counts.
But it wastes a lot of memory if the symbol has a few samples only.
Add a hashmap to save values only for actual samples.

For now, it has duplicate histogram (one in the existing array and
another in the new hash map).  Once it can convert to use the hash
in all places, we can get rid of the array later.

Reviewed-by: Ian Rogers 
Reviewed-by: Arnaldo Carvalho de Melo 
Tested-by: Arnaldo Carvalho de Melo 
Cc: Andi Kleen 
Signed-off-by: Namhyung Kim 
Link: https://lore.kernel.org/r/20240304230815.1440583-2-namhyung@kernel.org

perf threads: Reduce table size from 256 to 8

2024-03-04T06:52:13+00:00

The threads data structure is an array of hashmaps, previously
rbtrees. The two levels allows for a fixed outer array where access is
guarded by rw_semaphores. Commit 91e467bc568f ("perf machine: Use
hashtable for machine threads") sized the outer table at 256 entries
to avoid future scalability problems, however, this means the threads
struct is sized at 30,720 bytes. As the hashmaps allow O(1) access for
the common find/insert/remove operations, lower the number of entries
to 8. This reduces the size overhead to 960 bytes.

Signed-off-by: Ian Rogers 
Acked-by: Namhyung Kim 
Cc: Yang Jihong 
Cc: Oliver Upton 
Signed-off-by: Namhyung Kim 
Link: https://lore.kernel.org/r/20240301053646.1449657-8-irogers@google.com

perf threads: Switch from rbtree to hashmap

2024-03-04T06:52:04+00:00

The rbtree provides a sorting on entries but this is unused. Switch to
using hashmap for O(1) rather than O(log n) find/insert/remove
complexity.

Signed-off-by: Ian Rogers 
Acked-by: Namhyung Kim 
Cc: Yang Jihong 
Cc: Oliver Upton 
Signed-off-by: Namhyung Kim 
Link: https://lore.kernel.org/r/20240301053646.1449657-7-irogers@google.com

perf threads: Move threads to its own files

2024-03-04T06:51:55+00:00

Move threads out of machine and into its own file.

Signed-off-by: Ian Rogers 
Acked-by: Namhyung Kim 
Cc: Yang Jihong 
Cc: Oliver Upton 
Signed-off-by: Namhyung Kim 
Link: https://lore.kernel.org/r/20240301053646.1449657-6-irogers@google.com

perf machine: Move machine's threads into its own abstraction

2024-03-04T06:51:44+00:00

Move thread_rb_node into the machine.c file. This hides the
implementation of threads from the rest of the code allowing for it to
be refactored.

Locking discipline is tightened up in this change. As the lock is now
encapsulated in threads, the findnew function requires holding it (as
it already did in machine). Rather than do conditionals with locks
based on whether the thread should be created (which could potentially
be error prone with a read lock match with a write unlock), have a
separate threads__find that won't create the thread and only holds the
read lock. This effectively duplicates the findnew logic, with the
existing findnew logic only operating under a write lock assuming
creation is necessary as a previous find failed. The creation may
still fail with the write lock due to another thread. The duplication
is removed in a later next patch that delegates the implementation to
hashtable.

Signed-off-by: Ian Rogers 
Acked-by: Namhyung Kim 
Cc: Yang Jihong 
Cc: Oliver Upton 
Signed-off-by: Namhyung Kim 
Link: https://lore.kernel.org/r/20240301053646.1449657-5-irogers@google.com