]> git.ipfire.org Git - thirdparty/kernel/linux.git/commitdiff
perf maps: Hide maps internals
authorIan Rogers <irogers@google.com>
Sat, 10 Feb 2024 03:17:45 +0000 (19:17 -0800)
committerNamhyung Kim <namhyung@kernel.org>
Mon, 12 Feb 2024 20:35:41 +0000 (12:35 -0800)
Move the struct into the C file. Add maps__equal to work around
exposing the struct for reference count checking. Add accessors for
the unwind_libunwind_ops. Move maps_list_node to its only use in
symbol.c.

Signed-off-by: Ian Rogers <irogers@google.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: K Prateek Nayak <kprateek.nayak@amd.com>
Cc: James Clark <james.clark@arm.com>
Cc: Vincent Whitchurch <vincent.whitchurch@axis.com>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Cc: Colin Ian King <colin.i.king@gmail.com>
Cc: Changbin Du <changbin.du@huawei.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Song Liu <song@kernel.org>
Cc: Leo Yan <leo.yan@linux.dev>
Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Cc: Liam Howlett <liam.howlett@oracle.com>
Cc: Artem Savkov <asavkov@redhat.com>
Cc: bpf@vger.kernel.org
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Link: https://lore.kernel.org/r/20240210031746.4057262-6-irogers@google.com
tools/perf/tests/thread-maps-share.c
tools/perf/util/callchain.c
tools/perf/util/maps.c
tools/perf/util/maps.h
tools/perf/util/symbol.c
tools/perf/util/thread.c
tools/perf/util/unwind-libdw.c
tools/perf/util/unwind-libunwind-local.c
tools/perf/util/unwind-libunwind.c

index 7fa6f7c568e2f9b0721c21621fc04b92617bd94e..e9ecd30a5c058076b9eb8f1aa4f5950e3663cbea 100644 (file)
@@ -46,9 +46,9 @@ static int test__thread_maps_share(struct test_suite *test __maybe_unused, int s
        TEST_ASSERT_EQUAL("wrong refcnt", refcount_read(maps__refcnt(maps)), 4);
 
        /* test the maps pointer is shared */
-       TEST_ASSERT_VAL("maps don't match", RC_CHK_EQUAL(maps, thread__maps(t1)));
-       TEST_ASSERT_VAL("maps don't match", RC_CHK_EQUAL(maps, thread__maps(t2)));
-       TEST_ASSERT_VAL("maps don't match", RC_CHK_EQUAL(maps, thread__maps(t3)));
+       TEST_ASSERT_VAL("maps don't match", maps__equal(maps, thread__maps(t1)));
+       TEST_ASSERT_VAL("maps don't match", maps__equal(maps, thread__maps(t2)));
+       TEST_ASSERT_VAL("maps don't match", maps__equal(maps, thread__maps(t3)));
 
        /*
         * Verify the other leader was created by previous call.
@@ -73,7 +73,7 @@ static int test__thread_maps_share(struct test_suite *test __maybe_unused, int s
        other_maps = thread__maps(other);
        TEST_ASSERT_EQUAL("wrong refcnt", refcount_read(maps__refcnt(other_maps)), 2);
 
-       TEST_ASSERT_VAL("maps don't match", RC_CHK_EQUAL(other_maps, thread__maps(other_leader)));
+       TEST_ASSERT_VAL("maps don't match", maps__equal(other_maps, thread__maps(other_leader)));
 
        /* release thread group */
        thread__put(t3);
index 8262f69118dbbd6ee13ad32855886a49d23f4ab6..7517d16c02ec976adb3387f5a75bac71bf812934 100644 (file)
@@ -1157,7 +1157,7 @@ int fill_callchain_info(struct addr_location *al, struct callchain_cursor_node *
                if (al->map == NULL)
                        goto out;
        }
-       if (RC_CHK_EQUAL(al->maps, machine__kernel_maps(machine))) {
+       if (maps__equal(al->maps, machine__kernel_maps(machine))) {
                if (machine__is_host(machine)) {
                        al->cpumode = PERF_RECORD_MISC_KERNEL;
                        al->level = 'k';
index df0c8041899e3ea71673bb78aa0ceaae2c929824..439cefab112a24078c55c94ab4157d16824de33b 100644 (file)
@@ -6,9 +6,63 @@
 #include "dso.h"
 #include "map.h"
 #include "maps.h"
+#include "rwsem.h"
 #include "thread.h"
 #include "ui/ui.h"
 #include "unwind.h"
+#include <internal/rc_check.h>
+
+/*
+ * Locking/sorting note:
+ *
+ * Sorting is done with the write lock, iteration and binary searching happens
+ * under the read lock requiring being sorted. There is a race between sorting
+ * releasing the write lock and acquiring the read lock for iteration/searching
+ * where another thread could insert and break the sorting of the maps. In
+ * practice inserting maps should be rare meaning that the race shouldn't lead
+ * to live lock. Removal of maps doesn't break being sorted.
+ */
+
+DECLARE_RC_STRUCT(maps) {
+       struct rw_semaphore lock;
+       /**
+        * @maps_by_address: array of maps sorted by their starting address if
+        * maps_by_address_sorted is true.
+        */
+       struct map       **maps_by_address;
+       /**
+        * @maps_by_name: optional array of maps sorted by their dso name if
+        * maps_by_name_sorted is true.
+        */
+       struct map       **maps_by_name;
+       struct machine   *machine;
+#ifdef HAVE_LIBUNWIND_SUPPORT
+       void            *addr_space;
+       const struct unwind_libunwind_ops *unwind_libunwind_ops;
+#endif
+       refcount_t       refcnt;
+       /**
+        * @nr_maps: number of maps_by_address, and possibly maps_by_name,
+        * entries that contain maps.
+        */
+       unsigned int     nr_maps;
+       /**
+        * @nr_maps_allocated: number of entries in maps_by_address and possibly
+        * maps_by_name.
+        */
+       unsigned int     nr_maps_allocated;
+       /**
+        * @last_search_by_name_idx: cache of last found by name entry's index
+        * as frequent searches for the same dso name are common.
+        */
+       unsigned int     last_search_by_name_idx;
+       /** @maps_by_address_sorted: is maps_by_address sorted. */
+       bool             maps_by_address_sorted;
+       /** @maps_by_name_sorted: is maps_by_name sorted. */
+       bool             maps_by_name_sorted;
+       /** @ends_broken: does the map contain a map where end values are unset/unsorted? */
+       bool             ends_broken;
+};
 
 static void check_invariants(const struct maps *maps __maybe_unused)
 {
@@ -118,6 +172,43 @@ static void maps__set_maps_by_name_sorted(struct maps *maps, bool value)
        RC_CHK_ACCESS(maps)->maps_by_name_sorted = value;
 }
 
+struct machine *maps__machine(const struct maps *maps)
+{
+       return RC_CHK_ACCESS(maps)->machine;
+}
+
+unsigned int maps__nr_maps(const struct maps *maps)
+{
+       return RC_CHK_ACCESS(maps)->nr_maps;
+}
+
+refcount_t *maps__refcnt(struct maps *maps)
+{
+       return &RC_CHK_ACCESS(maps)->refcnt;
+}
+
+#ifdef HAVE_LIBUNWIND_SUPPORT
+void *maps__addr_space(const struct maps *maps)
+{
+       return RC_CHK_ACCESS(maps)->addr_space;
+}
+
+void maps__set_addr_space(struct maps *maps, void *addr_space)
+{
+       RC_CHK_ACCESS(maps)->addr_space = addr_space;
+}
+
+const struct unwind_libunwind_ops *maps__unwind_libunwind_ops(const struct maps *maps)
+{
+       return RC_CHK_ACCESS(maps)->unwind_libunwind_ops;
+}
+
+void maps__set_unwind_libunwind_ops(struct maps *maps, const struct unwind_libunwind_ops *ops)
+{
+       RC_CHK_ACCESS(maps)->unwind_libunwind_ops = ops;
+}
+#endif
+
 static struct rw_semaphore *maps__lock(struct maps *maps)
 {
        /*
@@ -453,6 +544,11 @@ bool maps__empty(struct maps *maps)
        return maps__nr_maps(maps) == 0;
 }
 
+bool maps__equal(struct maps *a, struct maps *b)
+{
+       return RC_CHK_EQUAL(a, b);
+}
+
 int maps__for_each_map(struct maps *maps, int (*cb)(struct map *map, void *data), void *data)
 {
        bool done = false;
index df9dd5a0e3c0e671c7962ccc0260d644ea58efe9..4bcba136ffe5c164b4b8d789115c7fed7c91c67a 100644 (file)
@@ -3,80 +3,15 @@
 #define __PERF_MAPS_H
 
 #include <linux/refcount.h>
-#include <linux/rbtree.h>
 #include <stdio.h>
 #include <stdbool.h>
 #include <linux/types.h>
-#include "rwsem.h"
-#include <internal/rc_check.h>
 
 struct ref_reloc_sym;
 struct machine;
 struct map;
 struct maps;
 
-struct map_list_node {
-       struct list_head node;
-       struct map *map;
-};
-
-static inline struct map_list_node *map_list_node__new(void)
-{
-       return malloc(sizeof(struct map_list_node));
-}
-
-/*
- * Locking/sorting note:
- *
- * Sorting is done with the write lock, iteration and binary searching happens
- * under the read lock requiring being sorted. There is a race between sorting
- * releasing the write lock and acquiring the read lock for iteration/searching
- * where another thread could insert and break the sorting of the maps. In
- * practice inserting maps should be rare meaning that the race shouldn't lead
- * to live lock. Removal of maps doesn't break being sorted.
- */
-
-DECLARE_RC_STRUCT(maps) {
-       struct rw_semaphore lock;
-       /**
-        * @maps_by_address: array of maps sorted by their starting address if
-        * maps_by_address_sorted is true.
-        */
-       struct map       **maps_by_address;
-       /**
-        * @maps_by_name: optional array of maps sorted by their dso name if
-        * maps_by_name_sorted is true.
-        */
-       struct map       **maps_by_name;
-       struct machine   *machine;
-#ifdef HAVE_LIBUNWIND_SUPPORT
-       void            *addr_space;
-       const struct unwind_libunwind_ops *unwind_libunwind_ops;
-#endif
-       refcount_t       refcnt;
-       /**
-        * @nr_maps: number of maps_by_address, and possibly maps_by_name,
-        * entries that contain maps.
-        */
-       unsigned int     nr_maps;
-       /**
-        * @nr_maps_allocated: number of entries in maps_by_address and possibly
-        * maps_by_name.
-        */
-       unsigned int     nr_maps_allocated;
-       /**
-        * @last_search_by_name_idx: cache of last found by name entry's index
-        * as frequent searches for the same dso name are common.
-        */
-       unsigned int     last_search_by_name_idx;
-       /** @maps_by_address_sorted: is maps_by_address sorted. */
-       bool             maps_by_address_sorted;
-       /** @maps_by_name_sorted: is maps_by_name sorted. */
-       bool             maps_by_name_sorted;
-       /** @ends_broken: does the map contain a map where end values are unset/unsorted? */
-       bool             ends_broken;
-};
-
 #define KMAP_NAME_LEN 256
 
 struct kmap {
@@ -100,36 +35,22 @@ static inline void __maps__zput(struct maps **map)
 
 #define maps__zput(map) __maps__zput(&map)
 
+bool maps__equal(struct maps *a, struct maps *b);
+
 /* Iterate over map calling cb for each entry. */
 int maps__for_each_map(struct maps *maps, int (*cb)(struct map *map, void *data), void *data);
 /* Iterate over map removing an entry if cb returns true. */
 void maps__remove_maps(struct maps *maps, bool (*cb)(struct map *map, void *data), void *data);
 
-static inline struct machine *maps__machine(struct maps *maps)
-{
-       return RC_CHK_ACCESS(maps)->machine;
-}
-
-static inline unsigned int maps__nr_maps(const struct maps *maps)
-{
-       return RC_CHK_ACCESS(maps)->nr_maps;
-}
-
-static inline refcount_t *maps__refcnt(struct maps *maps)
-{
-       return &RC_CHK_ACCESS(maps)->refcnt;
-}
+struct machine *maps__machine(const struct maps *maps);
+unsigned int maps__nr_maps(const struct maps *maps);
+refcount_t *maps__refcnt(struct maps *maps);
 
 #ifdef HAVE_LIBUNWIND_SUPPORT
-static inline void *maps__addr_space(struct maps *maps)
-{
-       return RC_CHK_ACCESS(maps)->addr_space;
-}
-
-static inline const struct unwind_libunwind_ops *maps__unwind_libunwind_ops(const struct maps *maps)
-{
-       return RC_CHK_ACCESS(maps)->unwind_libunwind_ops;
-}
+void *maps__addr_space(const struct maps *maps);
+void maps__set_addr_space(struct maps *maps, void *addr_space);
+const struct unwind_libunwind_ops *maps__unwind_libunwind_ops(const struct maps *maps);
+void maps__set_unwind_libunwind_ops(struct maps *maps, const struct unwind_libunwind_ops *ops);
 #endif
 
 size_t maps__fprintf(struct maps *maps, FILE *fp);
index 0785a54e832e4b2f70d2c9a4d0fa69a77ba45a81..35975189999b1f0e4362c5fc62ed5dd047781975 100644 (file)
@@ -63,6 +63,16 @@ struct symbol_conf symbol_conf = {
        .res_sample             = 0,
 };
 
+struct map_list_node {
+       struct list_head node;
+       struct map *map;
+};
+
+static struct map_list_node *map_list_node__new(void)
+{
+       return malloc(sizeof(struct map_list_node));
+}
+
 static enum dso_binary_type binary_type_symtab[] = {
        DSO_BINARY_TYPE__KALLSYMS,
        DSO_BINARY_TYPE__GUEST_KALLSYMS,
index 89c47a5098e289b14e9807a4ea894e15f9e8ce4a..c59ab4d79163efc7e9b9e47308cbd1415955091b 100644 (file)
@@ -383,7 +383,7 @@ static int thread__clone_maps(struct thread *thread, struct thread *parent, bool
        if (thread__pid(thread) == thread__pid(parent))
                return thread__prepare_access(thread);
 
-       if (RC_CHK_EQUAL(thread__maps(thread), thread__maps(parent))) {
+       if (maps__equal(thread__maps(thread), thread__maps(parent))) {
                pr_debug("broken map groups on thread %d/%d parent %d/%d\n",
                         thread__pid(thread), thread__tid(thread),
                         thread__pid(parent), thread__tid(parent));
index 6013335a8daea58a4fe19c0b4a5041e522f6707d..b38d322734b4a7fa0b2981bfb9e90cb26284dfdd 100644 (file)
@@ -263,7 +263,7 @@ int unwind__get_entries(unwind_entry_cb_t cb, void *arg,
        struct unwind_info *ui, ui_buf = {
                .sample         = data,
                .thread         = thread,
-               .machine        = RC_CHK_ACCESS(thread__maps(thread))->machine,
+               .machine        = maps__machine((thread__maps(thread))),
                .cb             = cb,
                .arg            = arg,
                .max_stack      = max_stack,
index dac536e28360a2481956360ec1753d2079488925..6a5ac0faa6f42114d428c715c3812fbbac7d407d 100644 (file)
@@ -706,7 +706,7 @@ static int _unwind__prepare_access(struct maps *maps)
 {
        void *addr_space = unw_create_addr_space(&accessors, 0);
 
-       RC_CHK_ACCESS(maps)->addr_space = addr_space;
+       maps__set_addr_space(maps, addr_space);
        if (!addr_space) {
                pr_err("unwind: Can't create unwind address space.\n");
                return -ENOMEM;
index 76cd63de80a8efef85cf91645b4d77a746f6c32b..2728eb4f13eab7943450db5b6ab01fd257e1f415 100644 (file)
@@ -12,11 +12,6 @@ struct unwind_libunwind_ops __weak *local_unwind_libunwind_ops;
 struct unwind_libunwind_ops __weak *x86_32_unwind_libunwind_ops;
 struct unwind_libunwind_ops __weak *arm64_unwind_libunwind_ops;
 
-static void unwind__register_ops(struct maps *maps, struct unwind_libunwind_ops *ops)
-{
-       RC_CHK_ACCESS(maps)->unwind_libunwind_ops = ops;
-}
-
 int unwind__prepare_access(struct maps *maps, struct map *map, bool *initialized)
 {
        const char *arch;
@@ -60,7 +55,7 @@ int unwind__prepare_access(struct maps *maps, struct map *map, bool *initialized
                return 0;
        }
 out_register:
-       unwind__register_ops(maps, ops);
+       maps__set_unwind_libunwind_ops(maps, ops);
 
        err = maps__unwind_libunwind_ops(maps)->prepare_access(maps);
        if (initialized)