Re: [PATCH v1 10/10] perf maps: Add context analysis annotations

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf maps: Recursive read-to-write deadlock in maps__load_first()
--

commit a66bec7dd23ce4df0be90007a645d5cd9c4e05dc
Author: Ian Rogers <irogers@google.com>

perf maps: Add context analysis annotations

This commit adds compiler context analysis annotations to the perf maps
subsystem. It annotates map struct fields and internal helper functions
to verify locking invariants at compile time.

> diff --git a/tools/perf/util/maps.c b/tools/perf/util/maps.c
> --- a/tools/perf/util/maps.c
> +++ b/tools/perf/util/maps.c

[ ... ]

> @@ -1495,7 +1495,7 @@ void maps__load_first(struct maps *maps)
>  {
>  	down_read(maps__lock(maps));
>  
> -	if (maps__nr_maps(maps) > 0)
> +	if (__maps__nr_maps(maps) > 0)
>  		map__load(maps__maps_by_address(maps)[0]);

[Severity: Medium]
Can this lead to a recursive read-to-write deadlock on maps__lock?

Here in maps__load_first(), maps__lock is acquired in read mode via
down_read() before calling map__load(). For kernel maps on x86_64,
map__load() triggers the x86_64 entry trampolines mapping:

map__load()
  -> dso__load()
    -> machine__map_x86_64_entry_trampolines()
      -> machine__create_extra_kernel_map()

The machine__create_extra_kernel_map() function then inserts a new map into
the same kernel maps array:

tools/perf/util/machine.c:machine__create_extra_kernel_map() {
    ...
    err = maps__insert(machine__kernel_maps(machine), map);
    ...
}

And maps__insert() attempts to acquire the exact same lock in write mode:

tools/perf/util/maps.c:maps__insert() {
    ...
    down_write(maps__lock(maps));
    ...
}

Since maps__load_first() still holds the read lock when map__load()
is called, wouldn't the down_write() in maps__insert() cause a
self-deadlock? Other APIs like maps__load_maps() copy the map array
and drop the read lock before loading to explicitly prevent this scenario.

>  
>  	up_read(maps__lock(maps));
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013182458.1234616-1-irogers@google.com?part=10