All of lore.kernel.org
 help / color / mirror / Atom feed
From: Namhyung Kim <namhyung@kernel.org>
To: Ian Rogers <irogers@google.com>
Cc: acme@kernel.org, adrian.hunter@intel.com, bvanassche@acm.org,
	elver@google.com, james.clark@linaro.org, jolsa@kernel.org,
	justinstitt@google.com, linux-kernel@vger.kernel.org,
	linux-perf-users@vger.kernel.org, llvm@lists.linux.dev,
	mingo@redhat.com, morbo@google.com, nathan@kernel.org,
	ndesaulniers@google.com, peterz@infradead.org
Subject: Re: [PATCH v11 00/20] tools/perf: Adopt kernel compiler-context-analysis.h annotations
Date: Tue, 10 Nov 2026 14:58:12 -0800	[thread overview]
Message-ID: <avOhhIL1EExpcJBi@google.com> (raw)
In-Reply-To: <20261107044102.1506626-1-irogers@google.com>

Hi Ian,

On Fri, Nov 06, 2026 at 08:40:41PM -0800, Ian Rogers wrote:
> The perf tool previously added Clang thread-safety annotations in
> tools/perf/util/mutex.h (GUARDED_BY, LOCKABLE, EXCLUSIVE_LOCK_FUNCTION,
> UNLOCK_FUNCTION, etc.). Since then, the Linux kernel has adopted a
> unified set of static context analysis macros and attributes in
> include/linux/compiler-context-analysis.h (context_lock_struct,
> __guarded_by, __acquires, __releases, __must_hold, etc.).
> 
> This series makes tools/include/linux/compiler-context-analysis.h
> functional by synchronizing it and the required compiler_types.h helpers
> with the kernel headers, converts tools/perf from its custom mutex.h
> macros to the kernel-style compiler-context-analysis.h annotations, and
> expands context analysis coverage across comm, dso/dsos, thread/threads,
> config, intel-tpebs, env, and maps. Along the way, separate preparatory
> patches fix an unlocked __dsos__add() call and TOCTOU race in
> __machine__addnew_vdso(), skip BPF and OOL DSOs in
> dsos__find_kernel_dso(), ensure DSOs created in auxtrace, machine, and
> symbol helpers belong to a dsos collection, avoid dropping
> _dso__data_open_lock in dso__list_del() under REFCNT_CHECKING, switch
> dso__data_open_lock and tpebs_mtx to DEFINE_MUTEX(), add a read-lock
> fast path to dsos__findnew_id(), fix an unlocked read of
> tpebs_reader_thread and double-close of tpebs_cmd.out in intel-tpebs,
> avoid a potential read-to-write deadlock in maps__load_first(), and fix
> map index lookup fallback and removal in maps.
> 
> This series was developed and reviewed with assistance from an AI coding
> agent (Gemini 3.1 Pro) and reviewed with Sashiko.

Is there any chance other tools than perf can be affected by this
change? 

How can I check this?  Does it require specific build flags?  Does it
work with GCC as well?  Can we extend the build test to validate this
automatically?

Thanks,
Namhyung

> 
> v11:
>   - Rebase onto perf-tools-next.
>   - Add Fixes tags to patches 7, 15, and 18.
>   - Serialize tpebs_send_record_cmd() with teardown via
>     tpebs_cmd_in_flight and tpebs_cmd_cond, check
>     check_if_command_finished(&tpebs_cmd) under tpebs_mtx only when not
>     stopping, guard evsel__tpebs_open() against a concurrent tpebs_stop()
>     with a generation counter (tpebs_gen) and tpebs_stopping across
>     closing the fds and finish_command(), and annotate tpebs_cmd,
>     control_fd, ack_fd, tpebs_stopping, tpebs_cmd_in_flight, and
>     tpebs_gen with __guarded_by(&tpebs_mtx) in patch 17.
> 
> v10:
>   - Split the maps__load_first() lock inversion fix (patch 18) from the
>     maps index lookup and removal fixes (patch 19).
>   - Fall back to the linear pointer scan in maps__by_address_index() and
>     maps__by_name_index() when bsearch() returns NULL, check
>     map__start(prev) < map__start(new) in __maps__insert(), factor out
>     __maps__remove_at() so maps__remove_maps() removes at index i
>     directly without risking an infinite loop, guard against -1 index
>     wraparound in maps__find_next_entry(), refresh sort invariants in
>     maps__fixup_end() and machine__create_kernel_maps(), and add a unit
>     test in tests/maps.c.
> 
> v9:
>   - Rebase onto perf-tools-next (resolving the conflict in
>     tools/include/linux/compiler.h with ASSERT_STATIC_STORAGE() and
>     converting the newly added dso__set_symsrc_filename() lock
>     annotation in tools/perf/util/dso.h).
>   - Move __must_hold_shared(&dsos->lock) on __dsos__find() from patch 13
>     into patch 7 where __dsos__find() is introduced so every commit in
>     the series builds cleanly with -Wthread-safety.
> 
> v8:
>   - Set DSO_BINARY_TYPE__BPF_IMAGE for non-OOL ksymbol DSOs in
>     machine__process_ksymbol_register() and skip
>     DSO_BINARY_TYPE__BPF_PROG_INFO, DSO_BINARY_TYPE__BPF_IMAGE, and
>     DSO_BINARY_TYPE__OOL in dsos__find_kernel_dso_cb() so that
>     dsos__find_kernel_dso() does not match ksymbol DSOs in
>     machine->dsos (patch 8).
>   - Split the dsos__add() additions in auxtrace, machine, and symbol into
>     a standalone patch (patch 9) before "perf dso: Pass dso_data to
>     close_data_fd() and dso__list_del()" (patch 10).
> 
> v7:
>   - Add newly created DSOs in machine__process_ksymbol_register() and
>     dso__split_kallsyms() to machine->dsos so assert(dso__dsos(dso)) in
>     dso__list_add() always holds.
>   - Add __guarded_by(&config_mutex) to function-local static parser
>     buffers in parse_value() and perf_parse_file().
>   - Remove from maps_by_name before calling
>     map__put(maps_by_address[address_idx]) in __maps__remove() so
>     callers such as maps__remove_maps() that pass maps_by_address[i] as
>     map do not use a freed handle in maps__by_name_index() under
>     REFCNT_CHECKING.
> 
> v6:
>   - Rebase onto perf-tools-next.
> 
> v5:
>   - Hold machine->dsos.lock in write mode across machine->vdso_info
>     initialization and temporary file creation in
>     machine__findnew_vdso() and __machine__findnew_compat(), and
>     annotate __machine__addnew_vdso() with
>     __must_hold(&machine->dsos.lock).
>   - Reset tpebs_cmd.pid = 0 and return early without setting
>     t->started = true if start_command() fails in
>     evsel__tpebs_start_perf_record().
> 
> v4:
>   - Place the DSO loaded in addr_filter__resolve_syms() into a dsos
>     collection so dso__dsos(dso) is populated.
>   - Join tpebs_reader_thread on error in evsel__tpebs_open(), document
>     why tpebs_cmd.pid is cleared before pthread_join() in tpebs_stop(),
>     and include <pthread.h> and <signal.h> in util/intel-tpebs.c.
>   - Include <string.h> in util/maps.c.
> 
> v3:
>   - Rebase onto perf-tools-next (converting the newly added
>     dso__find_binary_type() lock annotations in util/dso.c).
>   - Separate bug fixes, DEFINE_MUTEX() cleanups, and improvements (vdso,
>     dso_data, dso__data_open_lock, dsos__findnew_id, intel-tpebs,
>     tpebs_mtx, maps) from the annotation patches.
>   - Move AI coding assistant attribution to the cover letter and drop
>     per-commit Assisted-by tags per linux-next
>     Documentation/process/coding-assistants.rst.
>   - Add Marco Elver's Reviewed-by to patch 1.
>   - Define __diag_clang_23() and Clang ASM_INPUT_G/ASM_INPUT_RM/
>     ASM_OUTPUT_RM overrides in tools/include/linux/compiler-gcc.h.
>   - Probe for -Wthread-safety-pointer/-Wthread-safety-beta in
>     Makefile.config.
>   - Pass write_locked to __dsos__find().
>   - Pass struct dso_data * to close_data_fd() and dso__list_del() to
>     remove the REFCNT_CHECKING dso pointer from struct dso_data and
>     avoid dropping dso__data_open_lock in dso__list_del(), and include
>     <assert.h> in util/dso.c.
>   - Re-validate DSO lookup under the write lock in dsos__findnew_id()
>     via __dsos__findnew_id(), and handle NULL dso_id in
>     dso__needs_improve_id(), __dso__improve_id(), and dso_id__cmp().
>   - Reset tpebs_cmd.out = -1 if start_command() fails and include
>     <stdint.h>, <stdlib.h>, <string.h>, and <unistd.h> in
>     util/intel-tpebs.c.
>   - Bounds-check the index in __maps__remove() (freeing maps_by_name if
>     out of bounds).
> 
> v2:
>   - Use -c rather than -E when probing for
>     -fexperimental-late-parse-attributes in Makefile.config to avoid
>     -Werror,-Wunused-command-line-argument.
>   - Acquire machine->dsos.lock in write mode across duplicate checking,
>     dso__set_long_name(), and __dsos__add() in __machine__addnew_vdso().
>   - Avoid dropping dso__data_open_lock in dso__list_del() under
>     REFCNT_CHECKING.
>   - Pass tpebs_cmd.out directly to __sample_reader() and set
>     tpebs_cmd.out = -1 in evsel__tpebs_open() to avoid a double-close in
>     tpebs_stop(), and clarify the config_set_mutex comment.
>   - Drop maps__lock before calling map__load() in maps__load_first() to
>     avoid a read-to-write self-deadlock when loading kernel maps inserts
>     additional trampoline maps.
> 
> Ian Rogers (20):
>   compiler-context-analysis: Avoid unused parameter warning in
>     _context_unsafe_alias()
>   tools headers compiler: Synchronize compiler_types.h with the kernel
>     sources
>   tools headers: Synchronize linux/compiler-context-analysis.h with the
>     kernel sources
>   perf mutex: Use kernel style compiler-context-analysis.h annotations
>   perf build: Enable additional Clang thread-safety flags
>   perf comm: Add __guarded_by annotations to struct comm_strs
>   perf vdso: Fix unlocked __dsos__add() and race in
>     __machine__addnew_vdso()
>   perf dsos: Skip BPF and OOL DSOs in dsos__find_kernel_dso()
>   perf dsos: Ensure DSOs in auxtrace, machine, and symbol are in dsos
>   perf dso: Pass dso_data to close_data_fd() and dso__list_del()
>   perf dso: Use DEFINE_MUTEX for dso__data_open_lock
>   perf dsos: Use read lock fast path in dsos__findnew_id()
>   perf dso: Expand context analysis annotations
>   perf thread: Expand context analysis annotations to threads and
>     namespaces_lock
>   perf intel-tpebs: Fix race in tpebs_stop() and tpebs_cmd.out fd
>     handling
>   perf intel-tpebs: Use DEFINE_MUTEX for tpebs_mtx
>   perf config, intel-tpebs, env: Add __guarded_by and __must_hold
>     annotations
>   perf maps: Fix lock inversion in maps__load_first()
>   perf maps: Fix map index lookup fallback and removal
>   perf maps: Add context analysis annotations
> 
>  include/linux/compiler-context-analysis.h     |   2 +-
>  .../include/linux/compiler-context-analysis.h | 518 ++++++++++++++++--
>  tools/include/linux/compiler-gcc.h            |  72 ++-
>  tools/include/linux/compiler.h                |  50 +-
>  tools/include/linux/compiler_types.h          | 206 ++++++-
>  tools/perf/Makefile.config                    |   8 +-
>  tools/perf/builtin-sched.c                    |  16 +-
>  tools/perf/builtin-top.c                      |   4 +-
>  tools/perf/check-headers.sh                   |   1 +
>  tools/perf/tests/dso-data.c                   |   4 +
>  tools/perf/tests/maps.c                       | 140 +++++
>  tools/perf/util/annotate.c                    |   4 +-
>  tools/perf/util/annotate.h                    |   8 +-
>  tools/perf/util/auxtrace.c                    |  16 +-
>  tools/perf/util/bpf-event.h                   |   5 +-
>  tools/perf/util/comm.c                        |  10 +-
>  tools/perf/util/config.c                      |  47 +-
>  tools/perf/util/dso.c                         | 140 +++--
>  tools/perf/util/dso.h                         |  17 +-
>  tools/perf/util/dsos.c                        |  34 +-
>  tools/perf/util/dsos.h                        |  12 +-
>  tools/perf/util/env.c                         |   1 +
>  tools/perf/util/env.h                         |  19 +-
>  tools/perf/util/intel-tpebs.c                 | 206 ++++---
>  tools/perf/util/libbfd.c                      |   4 +-
>  tools/perf/util/machine.c                     |  34 +-
>  tools/perf/util/maps.c                        | 216 +++++---
>  tools/perf/util/mutex.c                       |   4 +-
>  tools/perf/util/mutex.h                       |  90 +--
>  tools/perf/util/rwsem.c                       |   8 +-
>  tools/perf/util/rwsem.h                       |  10 +-
>  tools/perf/util/symbol.c                      |   4 +
>  tools/perf/util/thread.c                      |  10 +-
>  tools/perf/util/thread.h                      |  30 +-
>  tools/perf/util/threads.c                     |   4 +
>  tools/perf/util/threads.h                     |   6 +-
>  tools/perf/util/vdso.c                        |  53 +-
>  37 files changed, 1477 insertions(+), 536 deletions(-)
> 
> 
> base-commit: 13a8d580433a3b72b10be34f0e26787fe5659d3c
> -- 
> 2.56.0.455.g6c5a503b38-goog
> 

  parent reply	other threads:[~2026-11-10 22:58 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-07  4:40 [PATCH v11 00/20] tools/perf: Adopt kernel compiler-context-analysis.h annotations Ian Rogers
2026-11-07  4:40 ` [PATCH v11 01/20] compiler-context-analysis: Avoid unused parameter warning in _context_unsafe_alias() Ian Rogers
2026-11-09 19:55   ` sashiko-bot
2026-11-07  4:40 ` [PATCH v11 02/20] tools headers compiler: Synchronize compiler_types.h with the kernel sources Ian Rogers
2026-11-09 20:01   ` sashiko-bot
2026-11-07  4:40 ` [PATCH v11 03/20] tools headers: Synchronize linux/compiler-context-analysis.h " Ian Rogers
2026-11-09 20:04   ` sashiko-bot
2026-11-07  4:40 ` [PATCH v11 04/20] perf mutex: Use kernel style compiler-context-analysis.h annotations Ian Rogers
2026-11-09 20:11   ` sashiko-bot
2026-11-07  4:40 ` [PATCH v11 05/20] perf build: Enable additional Clang thread-safety flags Ian Rogers
2026-11-09 20:23   ` sashiko-bot
2026-11-07  4:40 ` [PATCH v11 06/20] perf comm: Add __guarded_by annotations to struct comm_strs Ian Rogers
2026-11-09 20:37   ` sashiko-bot
2026-11-07  4:40 ` [PATCH v11 07/20] perf vdso: Fix unlocked __dsos__add() and race in __machine__addnew_vdso() Ian Rogers
2026-11-09 20:41   ` sashiko-bot
2026-11-07  4:40 ` [PATCH v11 08/20] perf dsos: Skip BPF and OOL DSOs in dsos__find_kernel_dso() Ian Rogers
2026-11-09 20:59   ` sashiko-bot
2026-11-07  4:40 ` [PATCH v11 09/20] perf dsos: Ensure DSOs in auxtrace, machine, and symbol are in dsos Ian Rogers
2026-11-09 21:08   ` sashiko-bot
2026-11-07  4:40 ` [PATCH v11 10/20] perf dso: Pass dso_data to close_data_fd() and dso__list_del() Ian Rogers
2026-11-09 21:13   ` sashiko-bot
2026-11-07  4:40 ` [PATCH v11 11/20] perf dso: Use DEFINE_MUTEX for dso__data_open_lock Ian Rogers
2026-11-09 21:14   ` sashiko-bot
2026-11-07  4:40 ` [PATCH v11 12/20] perf dsos: Use read lock fast path in dsos__findnew_id() Ian Rogers
2026-11-09 21:19   ` sashiko-bot
2026-11-07  4:40 ` [PATCH v11 13/20] perf dso: Expand context analysis annotations Ian Rogers
2026-11-09 21:21   ` sashiko-bot
2026-11-07  4:40 ` [PATCH v11 14/20] perf thread: Expand context analysis annotations to threads and namespaces_lock Ian Rogers
2026-11-09 21:25   ` sashiko-bot
2026-11-07  4:40 ` [PATCH v11 15/20] perf intel-tpebs: Fix race in tpebs_stop() and tpebs_cmd.out fd handling Ian Rogers
2026-11-09 21:50   ` sashiko-bot
2026-11-07  4:40 ` [PATCH v11 16/20] perf intel-tpebs: Use DEFINE_MUTEX for tpebs_mtx Ian Rogers
2026-11-09 21:58   ` sashiko-bot
2026-11-07  4:40 ` [PATCH v11 17/20] perf config, intel-tpebs, env: Add __guarded_by and __must_hold annotations Ian Rogers
2026-11-09 22:06   ` sashiko-bot
2026-11-07  4:40 ` [PATCH v11 18/20] perf maps: Fix lock inversion in maps__load_first() Ian Rogers
2026-11-09 22:10   ` sashiko-bot
2026-11-07  4:41 ` [PATCH v11 19/20] perf maps: Fix map index lookup fallback and removal Ian Rogers
2026-11-09 22:17   ` sashiko-bot
2026-11-07  4:41 ` [PATCH v11 20/20] perf maps: Add context analysis annotations Ian Rogers
2026-11-09 22:24   ` sashiko-bot
2026-11-10 22:58 ` Namhyung Kim [this message]
2026-11-10 23:14   ` [PATCH v11 00/20] tools/perf: Adopt kernel compiler-context-analysis.h annotations Ian Rogers

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=avOhhIL1EExpcJBi@google.com \
    --to=namhyung@kernel.org \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=bvanassche@acm.org \
    --cc=elver@google.com \
    --cc=irogers@google.com \
    --cc=james.clark@linaro.org \
    --cc=jolsa@kernel.org \
    --cc=justinstitt@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=llvm@lists.linux.dev \
    --cc=mingo@redhat.com \
    --cc=morbo@google.com \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=peterz@infradead.org \
    /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.