1. 28 Aug, 2024 25 commits
  2. 26 Aug, 2024 1 commit
  3. 22 Aug, 2024 6 commits
    • Arnaldo Carvalho de Melo's avatar
      perf python: Disable -Wno-cast-function-type-mismatch if present on clang · 00dc5146
      Arnaldo Carvalho de Melo authored
      The -Wcast-function-type-mismatch option was introduced in clang 19 and
      its enabled by default, since we use -Werror, and python bindings do
      casts that are valid but trips this warning, disable it if present.
      
      Closes: https://lore.kernel.org/all/CA+icZUXoJ6BS3GMhJHV3aZWyb5Cz2haFneX0C5pUMUUhG-UVKQ@mail.gmail.comReported-by: default avatarSedat Dilek <sedat.dilek@gmail.com>
      Tested-by: default avatarSedat Dilek <sedat.dilek@gmail.com>
      Cc: Ian Rogers <irogers@google.com>
      Cc: Ingo Molnar <mingo@redhat.com>
      Cc: Namhyung Kim <namhyung@kernel.org>
      Cc: Nathan Chancellor <nathan@kernel.org>
      Cc: Peter Zijlstra <peterz@infradead.org>
      Cc: stable@vger.kernel.org # To allow building with the upcoming clang 19
      Link: https://lore.kernel.org/lkml/CA+icZUVtHn8X1Tb_Y__c-WswsO0K8U9uy3r2MzKXwTA5THtL7w@mail.gmail.comSigned-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      00dc5146
    • Arnaldo Carvalho de Melo's avatar
      perf python: Allow checking for the existence of warning options in clang · b8116230
      Arnaldo Carvalho de Melo authored
      We'll need to check if an warning option introduced in clang 19 is
      available on the clang version being used, so cover the error message
      emitted when testing for a -W option.
      Tested-by: default avatarSedat Dilek <sedat.dilek@gmail.com>
      Cc: Ian Rogers <irogers@google.com>
      Cc: Ingo Molnar <mingo@redhat.com>
      Cc: Namhyung Kim <namhyung@kernel.org>
      Cc: Nathan Chancellor <nathan@kernel.org>
      Cc: Peter Zijlstra <peterz@infradead.org>
      Link: https://lore.kernel.org/lkml/CA+icZUVtHn8X1Tb_Y__c-WswsO0K8U9uy3r2MzKXwTA5THtL7w@mail.gmail.comSigned-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      b8116230
    • Namhyung Kim's avatar
      perf annotate-data: Copy back variable types after move · 1cfd01eb
      Namhyung Kim authored
      In some cases, compilers don't set the location expression in DWARF
      precisely.  For instance, it may assign a variable to a register after
      copying it from a different register.  Then it should use the register
      for the new type but still uses the old register.  This makes hard to
      track the type information properly.
      
      This is an example I found in __tcp_transmit_skb().  The first argument
      (sk) of this function is a pointer to sock and there's a variable (tp)
      for tcp_sock.
      
        static int __tcp_transmit_skb(struct sock *sk, struct sk_buff *skb,
        				int clone_it, gfp_t gfp_mask, u32 rcv_nxt)
        {
        	...
        	struct tcp_sock *tp;
      
        	BUG_ON(!skb || !tcp_skb_pcount(skb));
        	tp = tcp_sk(sk);
        	prior_wstamp = tp->tcp_wstamp_ns;
        	tp->tcp_wstamp_ns = max(tp->tcp_wstamp_ns, tp->tcp_clock_cache);
        	...
      
      So it basically calls tcp_sk(sk) to get the tcp_sock pointer from sk.
      But it turned out to be the same value because tcp_sock embeds sock as
      the first member.  The sk is located in reg5 (RDI) and tp is in reg3
      (RBX).  The offset of tcp_wstamp_ns is 0x748 and tcp_clock_cache is
      0x750.  So you need to use RBX (reg3) to access the fields in the
      tcp_sock.  But the code used RDI (reg5) as it has the same value.
      
        $ pahole --hex -C tcp_sock vmlinux | grep -e 748 -e 750
      	u64                tcp_wstamp_ns;        /* 0x748   0x8 */
      	u64                tcp_clock_cache;      /* 0x750   0x8 */
      
      And this is the disassembly of the part of the function.
      
        <__tcp_transmit_skb>:
        ...
        44:  mov    %rdi, %rbx
        47:  mov    0x748(%rdi), %rsi
        4e:  mov    0x750(%rdi), %rax
        55:  cmp    %rax, %rsi
      
      Because compiler put the debug info to RBX, it only knows RDI is a
      pointer to sock and accessing those two fields resulted in error
      due to offset being beyond the type size.
      
        -----------------------------------------------------------
        find data type for 0x748(reg5) at __tcp_transmit_skb+0x63
        CU for net/ipv4/tcp_output.c (die:0x817f543)
        frame base: cfa=0 fbreg=6
        scope: [1/1] (die:81aac3e)
        bb: [0 - 30]
        var [0] -0x98(stack) type='struct tcp_out_options' size=0x28 (die:0x81af3df)
        var [5] reg8 type='unsigned int' size=0x4 (die:0x8180ed6)
        var [5] reg2 type='unsigned int' size=0x4 (die:0x8180ed6)
        var [5] reg1 type='int' size=0x4 (die:0x818059e)
        var [5] reg4 type='struct sk_buff*' size=0x8 (die:0x8181360)
        var [5] reg5 type='struct sock*' size=0x8 (die:0x8181a0c)                   <<<--- the first argument ('sk' at %RDI)
        mov [19] reg8 -> -0xa8(stack) type='unsigned int' size=0x4 (die:0x8180ed6)
        mov [20] stack canary -> reg0
        mov [29] reg0 -> -0x30(stack) stack canary
        bb: [36 - 3e]
        mov [36] reg4 -> reg15 type='struct sk_buff*' size=0x8 (die:0x8181360)
        bb: [44 - 63]
        mov [44] reg5 -> reg3 type='struct sock*' size=0x8 (die:0x8181a0c)          <<<--- calling tcp_sk()
        var [47] reg3 type='struct tcp_sock*' size=0x8 (die:0x819eead)              <<<--- new variable ('tp' at %RBX)
        var [4e] reg4 type='unsigned long long' size=0x8 (die:0x8180edd)
        mov [58] reg4 -> -0xc0(stack) type='unsigned long long' size=0x8 (die:0x8180edd)
        chk [63] reg5 offset=0x748 ok=1 kind=1 (struct sock*) : offset bigger than size    <<<--- access with old variable
        final result: offset bigger than size
      
      While it's a fault in the compiler, we could work around this issue by
      using the type of new variable when it's copied directly.  So I've added
      copied_from field in the register state to track those direct register
      to register copies.  After that new register gets a new type and the old
      register still has the same type, it'll update (copy it back) the type
      of the old register.
      
      For example, if we can update type of reg5 at __tcp_transmit_skb+0x47,
      we can find the target type of the instruction at 0x63 like below:
      
        -----------------------------------------------------------
        find data type for 0x748(reg5) at __tcp_transmit_skb+0x63
        ...
        bb: [44 - 63]
        mov [44] reg5 -> reg3 type='struct sock*' size=0x8 (die:0x8181a0c)
        var [47] reg3 type='struct tcp_sock*' size=0x8 (die:0x819eead)
        var [47] copyback reg5 type='struct tcp_sock*' size=0x8 (die:0x819eead)     <<<--- here
        mov [47] 0x748(reg5) -> reg4 type='unsigned long long' size=0x8 (die:0x8180edd)
        mov [4e] 0x750(reg5) -> reg0 type='unsigned long long' size=0x8 (die:0x8180edd)
        mov [58] reg4 -> -0xc0(stack) type='unsigned long long' size=0x8 (die:0x8180edd)
        chk [63] reg5 offset=0x748 ok=1 kind=1 (struct tcp_sock*) : Good!           <<<--- new type
        found by insn track: 0x748(reg5) type-offset=0x748
        final result:  type='struct tcp_sock' size=0xa98 (die:0x819eeb2)
      Signed-off-by: default avatarNamhyung Kim <namhyung@kernel.org>
      Cc: Adrian Hunter <adrian.hunter@intel.com>
      Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.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: Peter Zijlstra <peterz@infradead.org>
      Link: https://lore.kernel.org/r/20240821232628.353177-5-namhyung@kernel.orgSigned-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      1cfd01eb
    • Namhyung Kim's avatar
      perf annotate-data: Update stack slot for the store · 895891da
      Namhyung Kim authored
      When checking the match variable at the target instruction, it might not
      have any information if it's a first write to a stack slot.  In this
      case it could spill a register value into the stack so the type info is
      in the source operand.
      
      But currently it's hard to get the operand from the checking function.
      Let's process the instruction and retry to get the type info from the
      stack if there's no information already.
      
      This is an example of __tcp_transmit_skb().  The instructions are
      
        <__tcp_transmit_skb>:
         0: nopl   0x0(%rax, %rax, 1)
         5: push   %rbp
         6: mov    %rsp, %rbp
         9: push   %r15
         b: push   %r14
         d: push   %r13
         f: push   %r12
        11: push   %rbx
        12: sub    $0x98, %rsp
        19: mov    %r8d, -0xa8(%rbp)
        ...
      
      It cannot find any variable at -0xa8(%rbp) at this point.
        -----------------------------------------------------------
        find data type for -0xa8(reg6) at __tcp_transmit_skb+0x19
        CU for net/ipv4/tcp_output.c (die:0x817f543)
        frame base: cfa=0 fbreg=6
        scope: [1/1] (die:81aac3e)
        bb: [0 - 19]
        var [0] -0x98(stack) type='struct tcp_out_options' size=0x28 (die:0x81af3df)
        var [5] reg8 type='unsigned int' size=0x4 (die:0x8180ed6)
        var [5] reg2 type='unsigned int' size=0x4 (die:0x8180ed6)
        var [5] reg1 type='int' size=0x4 (die:0x818059e)
        var [5] reg4 type='struct sk_buff*' size=0x8 (die:0x8181360)
        var [5] reg5 type='struct sock*' size=0x8 (die:0x8181a0c)
        chk [19] reg6 offset=-0xa8 ok=0 kind=0 fbreg : no type information
        no type information
      
      And it was able to find the type after processing the 'mov' instruction.
        -----------------------------------------------------------
        find data type for -0xa8(reg6) at __tcp_transmit_skb+0x19
        CU for net/ipv4/tcp_output.c (die:0x817f543)
        frame base: cfa=0 fbreg=6
        scope: [1/1] (die:81aac3e)
        bb: [0 - 19]
        var [0] -0x98(stack) type='struct tcp_out_options' size=0x28 (die:0x81af3df)
        var [5] reg8 type='unsigned int' size=0x4 (die:0x8180ed6)
        var [5] reg2 type='unsigned int' size=0x4 (die:0x8180ed6)
        var [5] reg1 type='int' size=0x4 (die:0x818059e)
        var [5] reg4 type='struct sk_buff*' size=0x8 (die:0x8181360)
        var [5] reg5 type='struct sock*' size=0x8 (die:0x8181a0c)
        chk [19] reg6 offset=-0xa8 ok=0 kind=0 fbreg : retry                    <<<--- here
        mov [19] reg8 -> -0xa8(stack) type='unsigned int' size=0x4 (die:0x8180ed6)
        chk [19] reg6 offset=-0xa8 ok=0 kind=0 fbreg : Good!
        found by insn track: -0xa8(reg6) type-offset=0
        final result:  type='unsigned int' size=0x4 (die:0x8180ed6)
      Signed-off-by: default avatarNamhyung Kim <namhyung@kernel.org>
      Cc: Adrian Hunter <adrian.hunter@intel.com>
      Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.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: Peter Zijlstra <peterz@infradead.org>
      Link: https://lore.kernel.org/r/20240821232628.353177-4-namhyung@kernel.orgSigned-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      895891da
    • Namhyung Kim's avatar
      perf annotate-data: Update debug messages · a0d57c60
      Namhyung Kim authored
      In check_matching_type(), it'd be easier to display the typename in
      question if it's available.
      
      For example, check out the line starts with 'chk'.
        -----------------------------------------------------------
        find data type for 0x10(reg0) at cpuacct_charge+0x13
        CU for kernel/sched/build_utility.c (die:0x137ee0b)
        frame base: cfa=1 fbreg=7
        scope: [3/3] (die:13d9632)
        bb: [c - 13]
        var [c] reg5 type='struct task_struct*' size=0x8 (die:0x1381230)
        mov [c] 0xdf8(reg5) -> reg0 type='struct css_set*' size=0x8 (die:0x1385c56)
        chk [13] reg0 offset=0x10 ok=1 kind=1 (struct css_set*) : Good!         <<<--- here
        found by insn track: 0x10(reg0) type-offset=0x10
        final result:  type='struct css_set' size=0x250 (die:0x1385b0e)
      
      Another example:
        -----------------------------------------------------------
        find data type for 0x8(reg0) at menu_select+0x279
        CU for drivers/cpuidle/governors/menu.c (die:0x7b0fe79)
        frame base: cfa=1 fbreg=7
        scope: [2/2] (die:7b11010)
        bb: [273 - 277]
        bb: [279 - 279]
        chk [279] reg0 offset=0x8 ok=0 kind=0 cfa : no type information
        scope: [1/2] (die:7b10cbc)
        bb: [0 - 64]
        ...
        mov [26a] imm=0xffffffff -> reg15
        bb: [273 - 277]
        bb: [279 - 279]
        chk [279] reg0 offset=0x8 ok=1 kind=1 (long long unsigned int) : no/void pointer    <<<--- here
        final result: no/void pointer
      
      Also change some places to print negative offsets properly.
      
      Before:
        -----------------------------------------------------------
        find data type for 0xffffff40(reg6) at __tcp_transmit_skb+0x58
      
      After:
        -----------------------------------------------------------
        find data type for -0xc0(reg6) at __tcp_transmit_skb+0x58
      Signed-off-by: default avatarNamhyung Kim <namhyung@kernel.org>
      Cc: Adrian Hunter <adrian.hunter@intel.com>
      Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.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: Peter Zijlstra <peterz@infradead.org>
      Link: https://lore.kernel.org/r/20240821232628.353177-3-namhyung@kernel.orgSigned-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      a0d57c60
    • Namhyung Kim's avatar
      perf dwarf-aux: Handle bitfield members from pointer access · a11b4222
      Namhyung Kim authored
      The __die_find_member_offset_cb() missed to handle bitfield members
      which don't have DW_AT_data_member_location.  Like in adding member
      types in __add_member_cb() it should fallback to check the bit offset
      when it resolves the member type for an offset.
      
      Fixes: 437683a9 ("perf dwarf-aux: Handle type transfer for memory access")
      Signed-off-by: default avatarNamhyung Kim <namhyung@kernel.org>
      Cc: Adrian Hunter <adrian.hunter@intel.com>
      Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.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: Namhyung Kim <namhyung@kernel.org>
      Cc: Peter Zijlstra <peterz@infradead.org>
      Link: https://lore.kernel.org/r/20240821232628.353177-2-namhyung@kernel.orgSigned-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      a11b4222
  4. 21 Aug, 2024 6 commits
    • Namhyung Kim's avatar
      perf annotate-data: Add 'typecln' sort key · fd45d52e
      Namhyung Kim authored
      Sometimes it's useful to organize member fields in cache-line boundary.
      
      The 'typecln' sort key is short for type-cacheline and to show samples
      in each cacheline.  The cacheline size is fixed to 64 for now, but it
      can read the actual size once it saves the value from sysfs.
      
      For example, you maybe want to which cacheline in a target is hot or
      cold.  The following shows members in the cfs_rq's first cache line.
      
        $ perf report -s type,typecln,typeoff -H
        ...
        -    2.67%        struct cfs_rq
           +    1.23%        struct cfs_rq: cache-line 2
           +    0.57%        struct cfs_rq: cache-line 4
           +    0.46%        struct cfs_rq: cache-line 6
           -    0.41%        struct cfs_rq: cache-line 0
                   0.39%        struct cfs_rq +0x14 (h_nr_running)
                   0.02%        struct cfs_rq +0x38 (tasks_timeline.rb_leftmost)
        ...
      
      Committer testing:
      
        # root@number:~# perf report -s type,typecln,typeoff -H --stdio
        # Total Lost Samples: 0
        #
        # Samples: 5K of event 'cpu_atom/mem-loads,ldlat=5/P'
        # Event count (approx.): 312251
        #
        #       Overhead  Data Type / Data Type Cacheline / Data Type Offset
        # ..............  ..................................................
        #
        <SNIP>
             0.07%        struct sigaction
                0.05%        struct sigaction: cache-line 1
                   0.02%        struct sigaction +0x58 (sa_mask)
                   0.02%        struct sigaction +0x78 (sa_mask)
                0.03%        struct sigaction: cache-line 0
                   0.02%        struct sigaction +0x38 (sa_mask)
                   0.01%        struct sigaction +0x8 (sa_mask)
        <SNIP>
      Signed-off-by: default avatarNamhyung Kim <namhyung@kernel.org>
      Tested-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      Cc: Adrian Hunter <adrian.hunter@intel.com>
      Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.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: Peter Zijlstra <peterz@infradead.org>
      Link: https://lore.kernel.org/r/20240819233603.54941-2-namhyung@kernel.orgSigned-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      fd45d52e
    • Namhyung Kim's avatar
      perf annotate-data: Show offset and size in hex · 7a5c2170
      Namhyung Kim authored
      It'd be better to have them in hex to check cacheline alignment.
      
       Percent     offset       size  field
        100.00          0      0x1c0  struct cfs_rq    {
          0.00          0       0x10      struct load_weight  load {
          0.00          0        0x8          long unsigned int       weight;
          0.00        0x8        0x4          u32     inv_weight;
                                          };
          0.00       0x10        0x4      unsigned int        nr_running;
         14.56       0x14        0x4      unsigned int        h_nr_running;
          0.00       0x18        0x4      unsigned int        idle_nr_running;
          0.00       0x1c        0x4      unsigned int        idle_h_nr_running;
        ...
      
      Committer notes:
      
      Justification from Namhyung when asked about why it would be "better":
      
      Cache line sizes are power of 2 so it'd be natural to use hex and
      check whether an offset is in the same boundary.  Also 'perf annotate'
      shows instruction offsets in hex.
      
      >
      > Maybe this should be selectable?
      
      I can add an option and/or a config if you want.
      Signed-off-by: default avatarNamhyung Kim <namhyung@kernel.org>
      Tested-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      Cc: Adrian Hunter <adrian.hunter@intel.com>
      Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.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: Peter Zijlstra <peterz@infradead.org>
      Link: https://lore.kernel.org/r/20240819233603.54941-1-namhyung@kernel.orgSigned-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      7a5c2170
    • Yang Ruibin's avatar
      perf bpf: Remove redundant check that map is NULL · ce66d7c7
      Yang Ruibin authored
      The check that map is NULL is already done in the bpf_map__fd(map) and
      returns an errno, which does not run further checks.
      
      In addition, even if the check for map is run, the return is a pointer,
      which is not consistent with the err_number returned by bpf_map__fd(map).
      Signed-off-by: default avatarYang Ruibin <11162571@vivo.com>
      Cc: Adrian Hunter <adrian.hunter@intel.com>
      Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
      Cc: Ian Rogers <irogers@google.com>
      Cc: Ingo Molnar <mingo@redhat.com>
      Cc: Jiri Olsa <jolsa@kernel.org>
      Cc: Kan Liang <kan.liang@linux.intel.com>
      Cc: Mark Rutland <mark.rutland@arm.com>
      Cc: Namhyung Kim <namhyung@kernel.org>
      Cc: Peter Zijlstra <peterz@infradead.org>
      Cc: Stephen Rothwell <sfr@canb.auug.org.au>
      Cc: opensource.kernel@vivo.com
      Link: https://lore.kernel.org/r/20240821101500.4568-1-11162571@vivo.comSigned-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      ce66d7c7
    • Namhyung Kim's avatar
      perf annotate-data: Fix percpu pointer check · 4d6d6e0f
      Namhyung Kim authored
      In check_matching_type(), it checks the type state of the register in a
      wrong order.  When it's the percpu pointer, it should check the type for
      the pointer, but it checks the CFA bit first and thought it has no type
      in the stack slot.  This resulted in no type info.
      
        -----------------------------------------------------------
        find data type for 0x28(reg1) at hrtimer_reprogram+0x88
        CU for kernel/time/hrtimer.c (die:0x18f219f)
        frame base: cfa=1 fbreg=7
        ...
        add [72] percpu 0x24500 -> reg1 pointer type='struct hrtimer_cpu_base' size=0x240 (die:0x18f6d46)
        bb: [7a - 7e]
        bb: [80 - 86]                        (here)
        bb: [88 - 88]                         vvv
        chk [88] reg1 offset=0x28 ok=1 kind=4 cfa : no type information
        no type information
      
      Here, instruction at 0x72 found reg1 has a (percpu) pointer and got the
      correct type.  But when it checks the final result, it wrongly thought
      it was stack variable because it checks the cfa bit first.
      
      After changing the order of state check:
        -----------------------------------------------------------
        find data type for 0x28(reg1) at hrtimer_reprogram+0x88
        CU for kernel/time/hrtimer.c (die:0x18f219f)
        frame base: cfa=1 fbreg=7
        ...                                     (here)
                                              vvvvvvvvvv
        chk [88] reg1 offset=0x28 ok=1 kind=4 percpu ptr : Good!
        found by insn track: 0x28(reg1) type-offset=0x28
        final type: type='struct hrtimer_cpu_base' size=0x240 (die:0x18f6d46)
      Signed-off-by: default avatarNamhyung Kim <namhyung@kernel.org>
      Cc: Adrian Hunter <adrian.hunter@intel.com>
      Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.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: Peter Zijlstra <peterz@infradead.org>
      Link: https://lore.kernel.org/r/20240821065408.285548-3-namhyung@kernel.orgSigned-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      4d6d6e0f
    • Namhyung Kim's avatar
      perf annotate-data: Prefer struct/union over base type · 4a32a972
      Namhyung Kim authored
      Sometimes a compound type can have a single field and the size is the
      same as the base type.  But it's still preferred as struct or union
      could carry more information than the base type.
      
      Also put a slight priority on the typedef for the same reason.
      Signed-off-by: default avatarNamhyung Kim <namhyung@kernel.org>
      Cc: Adrian Hunter <adrian.hunter@intel.com>
      Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.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: Peter Zijlstra <peterz@infradead.org>
      Link: https://lore.kernel.org/r/20240821065408.285548-2-namhyung@kernel.orgSigned-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      4a32a972
    • Namhyung Kim's avatar
      perf annotate-data: Fix missing constant copy · 922ec313
      Namhyung Kim authored
      I found it missed to copy the immediate constant when it moves the
      register value.  This could result in a wrong type inference since the
      address for the per-cpu variable would be 0 always.
      
      Fixes: eb9190af ("perf annotate-data: Handle ADD instructions")
      Signed-off-by: default avatarNamhyung Kim <namhyung@kernel.org>
      Cc: Adrian Hunter <adrian.hunter@intel.com>
      Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.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: Peter Zijlstra <peterz@infradead.org>
      Link: https://lore.kernel.org/r/20240821065408.285548-1-namhyung@kernel.orgSigned-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      922ec313
  5. 20 Aug, 2024 2 commits
    • Ian Rogers's avatar
      perf cap: Tidy up and improve capability testing · e25ebda7
      Ian Rogers authored
      Remove dependence on libcap. libcap is only used to query whether a
      capability is supported, which is just 1 capget system call.
      
      If the capget system call fails, fall back on root permission
      checking. Previously if libcap fails then the permission is assumed
      not present which may be pessimistic/wrong.
      
      Add a used_root out argument to perf_cap__capable to say whether the
      fall back root check was used. This allows the correct error message,
      "root" vs "users with the CAP_PERFMON or CAP_SYS_ADMIN capability", to
      be selected.
      
      Tidy uses of perf_cap__capable so that tests aren't repeated if capget
      isn't supported.
      Signed-off-by: default avatarIan Rogers <irogers@google.com>
      Cc: Adrian Hunter <adrian.hunter@intel.com>
      Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
      Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
      Cc: Changbin Du <changbin.du@huawei.com>
      Cc: Ingo Molnar <mingo@redhat.com>
      Cc: James Clark <james.clark@arm.com>
      Cc: Jiri Olsa <jolsa@kernel.org>
      Cc: Kan Liang <kan.liang@linux.intel.com>
      Cc: Leo Yan <leo.yan@linux.dev>
      Cc: Mark Rutland <mark.rutland@arm.com>
      Cc: Namhyung Kim <namhyung@kernel.org>
      Cc: Oliver Upton <oliver.upton@linux.dev>
      Cc: Peter Zijlstra <peterz@infradead.org>
      Link: https://lore.kernel.org/r/20240806220614.831914-1-irogers@google.comSigned-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      e25ebda7
    • Namhyung Kim's avatar
      perf annotate-data: Set bitfield member offset and size properly · 8b1042c4
      Namhyung Kim authored
      The bitfield members might not have DW_AT_data_member_location.  Let's
      use DW_AT_data_bit_offset to set the member offset correct.  Also use
      DW_AT_bit_size for the name like in a C program.
      
      Before:
        Annotate type: 'struct sk_buff' (1 samples)
              Percent     Offset       Size  Field
        -      100.00          0        232  struct sk_buff {
        +        0.00          0         24      union  ;
        +        0.00         24          8      union  ;
        +        0.00         32          8      union  ;
                 0.00         40         48      char[] cb;
        +        0.00         88         16      union  ;
                 0.00        104          8      long unsigned int      _nfct;
               100.00        112          4      unsigned int   len;
                 0.00        116          4      unsigned int   data_len;
                 0.00        120          2      __u16  mac_len;
                 0.00        122          2      __u16  hdr_len;
                 0.00        124          2      __u16  queue_mapping;
                 0.00        126          0      __u8[] __cloned_offset;
                 0.00          0          1      __u8   cloned;
                 0.00          0          1      __u8   nohdr;
                 0.00          0          1      __u8   fclone;
                 0.00          0          1      __u8   peeked;
                 0.00          0          1      __u8   head_frag;
                 0.00          0          1      __u8   pfmemalloc;
                 0.00          0          1      __u8   pp_recycle;
                 0.00        127          1      __u8   active_extensions;
        +        0.00        128         60      union  ;
                 0.00        188          4      sk_buff_data_t tail;
                 0.00        192          4      sk_buff_data_t end;
                 0.00        200          8      unsigned char* head;
      
      After:
      
        Annotate type: 'struct sk_buff' (1 samples)
              Percent     Offset       Size  Field
        -      100.00          0        232  struct sk_buff {
        +        0.00          0         24      union  ;
        +        0.00         24          8      union  ;
        +        0.00         32          8      union  ;
                 0.00         40         48      char[] cb
        +        0.00         88         16      union  ;
                 0.00        104          8      long unsigned int      _nfct;
               100.00        112          4      unsigned int   len;
                 0.00        116          4      unsigned int   data_len;
                 0.00        120          2      __u16  mac_len;
                 0.00        122          2      __u16  hdr_len;
                 0.00        124          2      __u16  queue_mapping;
                 0.00        126          0      __u8[] __cloned_offset;
                 0.00        126          1      __u8   cloned:1;
                 0.00        126          1      __u8   nohdr:1;
                 0.00        126          1      __u8   fclone:2;
                 0.00        126          1      __u8   peeked:1;
                 0.00        126          1      __u8   head_frag:1;
                 0.00        126          1      __u8   pfmemalloc:1;
                 0.00        126          1      __u8   pp_recycle:1;
                 0.00        127          1      __u8   active_extensions;
        +        0.00        128         60      union  ;
                 0.00        188          4      sk_buff_data_t tail;
                 0.00        192          4      sk_buff_data_t end;
                 0.00        200          8      unsigned char* head;
      
      Commiter notes:
      
      Collect some data:
      
        root@number:~# perf mem record -a --ldlat 5 -- ping -s 8193 -f 192.168.86.1
        Memory events are enabled on a subset of CPUs: 16-27
        PING 192.168.86.1 (192.168.86.1) 8193(8221) bytes of data.
        .^C
        --- 192.168.86.1 ping statistics ---
        13881 packets transmitted, 13880 received, 0.00720409% packet loss, time 8664ms
        rtt min/avg/max/mdev = 0.510/0.599/7.768/0.115 ms, ipg/ewma 0.624/0.593 ms
        [ perf record: Woken up 8 times to write data ]
        [ perf record: Captured and wrote 14.877 MB perf.data (46785 samples) ]
      
        root@number:~#
        root@number:~# perf evlist
        cpu_atom/mem-loads,ldlat=5/P
        cpu_atom/mem-stores/P
        dummy:u
        root@number:~# perf evlist -v
        cpu_atom/mem-loads,ldlat=5/P: type: 10 (cpu_atom), size: 136, config: 0x5d0 (mem-loads), { sample_period, sample_freq }: 4000, sample_type: IP|TID|TIME|ADDR|CPU|PERIOD|IDENTIFIER|DATA_SRC|WEIGHT_STRUCT, read_format: ID|LOST, disabled: 1, inherit: 1, freq: 1, precise_ip: 3, sample_id_all: 1, { bp_addr, config1 }: 0x7
        cpu_atom/mem-stores/P: type: 10 (cpu_atom), size: 136, config: 0x6d0 (mem-stores), { sample_period, sample_freq }: 4000, sample_type: IP|TID|TIME|ADDR|CPU|PERIOD|IDENTIFIER|DATA_SRC|WEIGHT_STRUCT, read_format: ID|LOST, disabled: 1, inherit: 1, freq: 1, precise_ip: 3, sample_id_all: 1
        dummy:u: type: 1 (software), size: 136, config: 0x9 (PERF_COUNT_SW_DUMMY), { sample_period, sample_freq }: 1, sample_type: IP|TID|TIME|ADDR|CPU|IDENTIFIER|DATA_SRC|WEIGHT_STRUCT, read_format: ID|LOST, inherit: 1, exclude_kernel: 1, exclude_hv: 1, mmap: 1, comm: 1, task: 1, mmap_data: 1, sample_id_all: 1, exclude_guest: 1, mmap2: 1, comm_exec: 1, ksymbol: 1, bpf_event: 1
        root@number:~#
      
      Ok, now lets see what changes from before this patch to after it:
      
        root@number:~# perf annotate --data-type > /tmp/before
      
      Apply the patch, build:
      
        root@number:~# perf annotate --data-type > /tmp/after
      
      The first hunk of the diff, for a glib data structure, in userspace,
      look at those bitfields:
      
        root@number:~# diff -u10 /tmp/before /tmp/after | head -20
        --- /tmp/before	2024-08-20 17:29:58.306765780 -0300
        +++ /tmp/after	2024-08-20 17:33:13.210582596 -0300
        @@ -163,22 +163,22 @@
      
         Annotate type: 'GHashTable' in /usr/lib64/libglib-2.0.so.0.8000.3 (1 samples):
         ============================================================================
          Percent     offset       size  field
           100.00          0         96  GHashTable	 {
             0.00          0          8      gsize	size;
             0.00          8          4      gint	mod;
           100.00         12          4      guint	mask;
             0.00         16          4      guint	nnodes;
             0.00         20          4      guint	noccupied;
        -    0.00          0          4      guint	have_big_keys;
        -    0.00          0          4      guint	have_big_values;
        +    0.00         24          1      guint	have_big_keys:1;
        +    0.00         24          1      guint	have_big_values:1;
             0.00         32          8      gpointer	keys;
             0.00         40          8      guint*	hashes;
             0.00         48          8      gpointer	values;
        root@number:~#
      
      As advertised :-)
      Signed-off-by: default avatarNamhyung Kim <namhyung@kernel.org>
      Tested-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      Cc: Adrian Hunter <adrian.hunter@intel.com>
      Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
      Cc: Ian Rogers <irogers@google.com>
      Cc: Ingo Molnar <mingo@kernel.org>
      Cc: Jiri Olsa <jolsa@kernel.org>
      Cc: Peter Zijlstra <peterz@infradead.org>
      Link: https://lore.kernel.org/r/20240815223823.2402285-1-namhyung@kernel.orgSigned-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      8b1042c4