All of lore.kernel.org
 help / color / mirror / Atom feed
From: Namhyung Kim <namhyung@kernel.org>
To: Zecheng Li <zecheng@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@kernel.org>, Ian Rogers <irogers@google.com>,
	Adrian Hunter <adrian.hunter@intel.com>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	Xu Liu <xliuprof@google.com>,
	linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 0/9] perf tools: Some improvements on data type profiler
Date: Wed, 22 Oct 2025 09:41:57 +0900	[thread overview]
Message-ID: <aPgoVTfCFxqTpCaK@google.com> (raw)
In-Reply-To: <20251013181607.2745653-1-zecheng@google.com>

Hello,

On Mon, Oct 13, 2025 at 06:15:57PM +0000, Zecheng Li wrote:
> Hi all,
> 
> I've identified several missing data type annotations within the perf
> tools when annotating the Linux kernel. This patch series improves the
> coverage and correctness of data type annotations.
> 
> Some patches from the previous version of this series were
> cherry-picked. This revision adds new improvements based on feedback and
> further development.
> 
> Here's a breakdown of the changes in this revision:
> 
> Patch 1 skips annotations for LEA instructions in x86, as these do not
> involve memory access. It now returns NO_TYPE.
> 
> Patches 2 implements the TSR_KIND_POINTER to represent registers holding
> memory addresses of the type. We are using the size of void* to get the
> pointer size. This could be improved to use an architecture dependent
> pointer size, but may require more work.
> 
> Patches 3-5 implement a basic approach for register offset tracking that
> supports add, sub, and lea operations. The register state is invalidated
> when an unsupported arithmetic instruction is encountered. This revision
> uses TSR_KIND_POINTER to avoid finding the pointer type in DWARF and
> preserves the pointer offset information in the stack state.

I've applied up to this to perf-tools-next, will review the rest later.

Thanks,
Namhyung

> 
> Patches 6-8 split patch 8 from v2 with some minor improvements. It skips
> check_variable when the type is found directly by register, since
> sufficient checking is already performed in match_var_offset.
> check_variable lacks some DWARF information to correctly determine if a
> variable is valid. I also found it is able to find members for
> typedef'd types so I preserve them in match_var_offset.
> 
> Patch 9 implements support for DW_OP_piece. Currently, this is allowed
> in check_allowed_ops but is handled like other single location
> expressions. This patch splits any expression containing DW_OP_piece
> into multiple parts and handle them separately.
> 
> I have tested each patch on a vmlinux and manually checked the results.
> After applying all patches, there are less missing or incorrect
> annotations. No obvious regressions were observed.
> 
> v4:
> Merged patch in v3:
> perf annotate: Rename TSR_KIND_POINTER to TSR_KIND_PERCPU_POINTER
> 
> Updated patches 1-5 based on the feedback from Namhyung.
> 
> v3:
> https://lore.kernel.org/all/20250917195808.2514277-1-zecheng@google.com/
> Merged patches in v2:
> 
> perf dwarf-aux: Use signed variable types in match_var_offset
> perf dwarf-aux: More accurate variable type match for breg
> perf dwarf-aux: Better variable collection for insn tracking
> perf dwarf-aux: Skip check_variable for die_find_variable_by_reg
> 
> v2:
> https://lore.kernel.org/all/20250825195412.223077-1-zecheng@google.com/
> 1. update the match_var_offset function signature to s64
> 2. correct the comment for is_breg_access_indirect. Use simpler logic to
> match the expressions we support.
> 3. add is_reg_var_addr to indicate whether a register holds an address
> of the variable. This defers the type dereference logic to
> update_var_state.
> 4. invalidate register state for unsupported instructions.
> 5. include two new patches related to improving data type profiler.
> 
> v1:
> https://lore.kernel.org/linux-perf-users/20250725202809.1230085-1-zecheng@google.com/
> 
> Zecheng Li (9):
>   perf annotate: Skip annotating data types to lea instructions
>   perf annotate: Track address registers via TSR_KIND_POINTER
>   perf annotate: Track arithmetic instructions on pointers
>   perf annotate: Save pointer offset in stack state
>   perf annotate: Invalidate register states for untracked instructions
>   perf dwarf-aux: Skip check_variable for die_find_variable_by_reg
>   perf dwarf-aux: Preserve typedefs in match_var_offset
>   perf annotate: Improve type comparison from different scopes
>   perf dwarf-aux: Support DW_OP_piece expressions
> 
>  tools/perf/arch/x86/annotate/instructions.c | 183 +++++++++++++-
>  tools/perf/util/annotate-data.c             | 102 ++++++--
>  tools/perf/util/annotate-data.h             |  14 +-
>  tools/perf/util/annotate.c                  |  20 ++
>  tools/perf/util/dwarf-aux.c                 | 266 +++++++++++++++-----
>  tools/perf/util/dwarf-aux.h                 |   2 +-
>  6 files changed, 493 insertions(+), 94 deletions(-)
> 
> -- 
> 2.51.0
> 

      parent reply	other threads:[~2025-10-22  0:42 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-13 18:15 [PATCH v4 0/9] perf tools: Some improvements on data type profiler Zecheng Li
2025-10-13 18:15 ` [PATCH v4 1/9] perf annotate: Skip annotating data types to lea instructions Zecheng Li
2025-10-14 18:40   ` Arnaldo Carvalho de Melo
2025-10-20  4:29     ` Namhyung Kim
2025-10-13 18:15 ` [PATCH v4 2/9] perf annotate: Track address registers via TSR_KIND_POINTER Zecheng Li
2025-10-13 18:16 ` [PATCH v4 3/9] perf annotate: Track arithmetic instructions on pointers Zecheng Li
2025-10-13 18:16 ` [PATCH v4 4/9] perf annotate: Save pointer offset in stack state Zecheng Li
2025-10-13 18:16 ` [PATCH v4 5/9] perf annotate: Invalidate register states for untracked instructions Zecheng Li
2025-10-13 18:16 ` [PATCH v4 6/9] perf dwarf-aux: Skip check_variable for die_find_variable_by_reg Zecheng Li
2025-10-13 18:16 ` [PATCH v4 7/9] perf dwarf-aux: Preserve typedefs in match_var_offset Zecheng Li
2025-10-25 17:40   ` Namhyung Kim
2025-10-13 18:16 ` [PATCH v4 8/9] perf annotate: Improve type comparison from different scopes Zecheng Li
2025-10-25 17:46   ` Namhyung Kim
2025-10-13 18:16 ` [PATCH v4 9/9] perf dwarf-aux: Support DW_OP_piece expressions Zecheng Li
2025-10-25 17:57   ` Namhyung Kim
2025-10-22  0:41 ` Namhyung Kim [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aPgoVTfCFxqTpCaK@google.com \
    --to=namhyung@kernel.org \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=irogers@google.com \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mhiramat@kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=xliuprof@google.com \
    --cc=zecheng@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.