So that it can indicate the it needs to collect data mappings. This is
needed especially for kernel maps.
At first, I just wanted to add it to struct machines only and to use the
machine->machines to check the value. But it turned out that some of
machine didn't belong to any machines (eg. some test codes), so I just
copied the value to individual struct machine.
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/builtin-inject.c | 2 +-
tools/perf/builtin-record.c | 2 +-
tools/perf/tests/vmlinux-kallsyms.c | 4 ++--
tools/perf/util/machine.c | 12 +++++++-----
tools/perf/util/machine.h | 10 ++++++++--
tools/perf/util/session.c | 4 ++--
tools/perf/util/session.h | 4 ++--
7 files changed, 23 insertions(+), 15 deletions(-)
diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index c8cf2fdd9cff..481adaa97a68 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -2325,7 +2325,7 @@ int cmd_inject(int argc, const char **argv)
}
inject.session = __perf_session__new(&data, repipe,
- output_fd(&inject),
+ output_fd(&inject), false,
&inject.tool);
if (IS_ERR(inject.session)) {
ret = PTR_ERR(inject.session);
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index aec18db7ff23..b4d0154dcb18 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -2373,7 +2373,7 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
signal(SIGUSR2, SIG_IGN);
}
- session = perf_session__new(data, tool);
+ session = __perf_session__new(data, false, -1, rec->opts.sample_address, tool);
if (IS_ERR(session)) {
pr_err("Perf session creation failed.\n");
return PTR_ERR(session);
diff --git a/tools/perf/tests/vmlinux-kallsyms.c b/tools/perf/tests/vmlinux-kallsyms.c
index 1078a93b01aa..1f1ba3ae88aa 100644
--- a/tools/perf/tests/vmlinux-kallsyms.c
+++ b/tools/perf/tests/vmlinux-kallsyms.c
@@ -131,8 +131,8 @@ static int test__vmlinux_matches_kallsyms(struct test_suite *test __maybe_unused
* Init the machines that will hold kernel, modules obtained from
* both vmlinux + .ko files and from /proc/kallsyms split by modules.
*/
- machine__init(&kallsyms, "", HOST_KERNEL_ID);
- machine__init(&vmlinux, "", HOST_KERNEL_ID);
+ machine__init(&kallsyms, "", HOST_KERNEL_ID, false);
+ machine__init(&vmlinux, "", HOST_KERNEL_ID, false);
maps = machine__kernel_maps(&vmlinux);
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 4e62843d51b7..ddc0a2130caf 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -110,7 +110,7 @@ static void thread__set_guest_comm(struct thread *thread, pid_t pid)
thread__set_comm(thread, comm, 0);
}
-int machine__init(struct machine *machine, const char *root_dir, pid_t pid)
+int machine__init(struct machine *machine, const char *root_dir, pid_t pid, bool has_data_mmap)
{
int err = -ENOMEM;
@@ -128,6 +128,7 @@ int machine__init(struct machine *machine, const char *root_dir, pid_t pid)
machine->env = NULL;
machine->pid = pid;
+ machine->has_data_mmap = has_data_mmap;
machine->id_hdr_size = 0;
machine->kptr_restrict_warned = false;
@@ -170,7 +171,7 @@ struct machine *machine__new_host(void)
struct machine *machine = malloc(sizeof(*machine));
if (machine != NULL) {
- machine__init(machine, "", HOST_KERNEL_ID);
+ machine__init(machine, "", HOST_KERNEL_ID, false);
if (machine__create_kernel_maps(machine) < 0)
goto out_delete;
@@ -272,10 +273,11 @@ void machine__delete(struct machine *machine)
}
}
-void machines__init(struct machines *machines)
+void __machines__init(struct machines *machines, bool have_data_mmap)
{
- machine__init(&machines->host, "", HOST_KERNEL_ID);
+ machine__init(&machines->host, "", HOST_KERNEL_ID, have_data_mmap);
machines->guests = RB_ROOT_CACHED;
+ machines->have_data_mmaps = have_data_mmap;
}
void machines__exit(struct machines *machines)
@@ -295,7 +297,7 @@ struct machine *machines__add(struct machines *machines, pid_t pid,
if (machine == NULL)
return NULL;
- if (machine__init(machine, root_dir, pid) != 0) {
+ if (machine__init(machine, root_dir, pid, machines->have_data_mmaps) != 0) {
free(machine);
return NULL;
}
diff --git a/tools/perf/util/machine.h b/tools/perf/util/machine.h
index d034ecaf89c1..f54e5c888a99 100644
--- a/tools/perf/util/machine.h
+++ b/tools/perf/util/machine.h
@@ -46,6 +46,7 @@ struct machine {
bool comm_exec;
bool kptr_restrict_warned;
bool single_address_space;
+ bool has_data_mmap;
char *root_dir;
char *mmap_name;
char *kallsyms_filename;
@@ -160,9 +161,14 @@ typedef void (*machine__process_t)(struct machine *machine, void *data);
struct machines {
struct machine host;
struct rb_root_cached guests;
+ bool have_data_mmaps;
};
-void machines__init(struct machines *machines);
+void __machines__init(struct machines *machines, bool data_mmap);
+static inline void machines__init(struct machines *machines)
+{
+ __machines__init(machines, false);
+}
void machines__exit(struct machines *machines);
void machines__process_guests(struct machines *machines,
@@ -181,7 +187,7 @@ void machines__set_comm_exec(struct machines *machines, bool comm_exec);
struct machine *machine__new_host(void);
struct machine *machine__new_kallsyms(void);
-int machine__init(struct machine *machine, const char *root_dir, pid_t pid);
+int machine__init(struct machine *machine, const char *root_dir, pid_t pid, bool has_data_mmap);
void machine__exit(struct machine *machine);
void machine__delete_threads(struct machine *machine);
void machine__delete(struct machine *machine);
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 00d18c74c090..e09a02ec8f28 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -191,7 +191,7 @@ static int ordered_events__deliver_event(struct ordered_events *oe,
}
struct perf_session *__perf_session__new(struct perf_data *data,
- bool repipe, int repipe_fd,
+ bool repipe, int repipe_fd, bool data_mmap,
struct perf_tool *tool)
{
int ret = -ENOMEM;
@@ -205,7 +205,7 @@ struct perf_session *__perf_session__new(struct perf_data *data,
session->decomp_data.zstd_decomp = &session->zstd_data;
session->active_decomp = &session->decomp_data;
INIT_LIST_HEAD(&session->auxtrace_index);
- machines__init(&session->machines);
+ __machines__init(&session->machines, data_mmap);
ordered_events__init(&session->ordered_events,
ordered_events__deliver_event, NULL);
diff --git a/tools/perf/util/session.h b/tools/perf/util/session.h
index ee3715e8563b..465610e81e95 100644
--- a/tools/perf/util/session.h
+++ b/tools/perf/util/session.h
@@ -64,13 +64,13 @@ struct decomp {
struct perf_tool;
struct perf_session *__perf_session__new(struct perf_data *data,
- bool repipe, int repipe_fd,
+ bool repipe, int repipe_fd, bool data_mmap,
struct perf_tool *tool);
static inline struct perf_session *perf_session__new(struct perf_data *data,
struct perf_tool *tool)
{
- return __perf_session__new(data, false, -1, tool);
+ return __perf_session__new(data, false, -1, false, tool);
}
void perf_session__delete(struct perf_session *session);
--
2.41.0.185.g7c58973941-goog
The kallsyms__get_symbol_start() to get any symbol address from
kallsyms. The existing kallsyms__get_function_start() only allows text
symbols so create this to allow data symbols too.
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/util/event.c | 30 +++++++++++++++++++++++++++---
tools/perf/util/event.h | 2 ++
2 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 3860b0c74829..6fdda0eb3854 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -93,8 +93,8 @@ struct process_symbol_args {
u64 start;
};
-static int find_symbol_cb(void *arg, const char *name, char type,
- u64 start)
+static int find_func_symbol_cb(void *arg, const char *name, char type,
+ u64 start)
{
struct process_symbol_args *args = arg;
@@ -110,12 +110,36 @@ static int find_symbol_cb(void *arg, const char *name, char type,
return 1;
}
+static int find_any_symbol_cb(void *arg, const char *name,
+ char type __maybe_unused, u64 start)
+{
+ struct process_symbol_args *args = arg;
+
+ if (strcmp(name, args->name))
+ return 0;
+
+ args->start = start;
+ return 1;
+}
+
int kallsyms__get_function_start(const char *kallsyms_filename,
const char *symbol_name, u64 *addr)
{
struct process_symbol_args args = { .name = symbol_name, };
- if (kallsyms__parse(kallsyms_filename, &args, find_symbol_cb) <= 0)
+ if (kallsyms__parse(kallsyms_filename, &args, find_func_symbol_cb) <= 0)
+ return -1;
+
+ *addr = args.start;
+ return 0;
+}
+
+int kallsyms__get_symbol_start(const char *kallsyms_filename,
+ const char *symbol_name, u64 *addr)
+{
+ struct process_symbol_args args = { .name = symbol_name, };
+
+ if (kallsyms__parse(kallsyms_filename, &args, find_any_symbol_cb) <= 0)
return -1;
*addr = args.start;
diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index de20e01c9d72..d8bcee2e9b93 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -360,6 +360,8 @@ size_t perf_event__fprintf(union perf_event *event, struct machine *machine, FIL
int kallsyms__get_function_start(const char *kallsyms_filename,
const char *symbol_name, u64 *addr);
+int kallsyms__get_symbol_start(const char *kallsyms_filename,
+ const char *symbol_name, u64 *addr);
void event_attr_init(struct perf_event_attr *attr);
--
2.41.0.185.g7c58973941-goog
On Tue, Jun 20, 2023 at 1:18 PM Namhyung Kim <[email protected]> wrote:
>
> So that it can indicate the it needs to collect data mappings. This is
> needed especially for kernel maps.
>
> At first, I just wanted to add it to struct machines only and to use the
> machine->machines to check the value. But it turned out that some of
> machine didn't belong to any machines (eg. some test codes), so I just
> copied the value to individual struct machine.
>
> Signed-off-by: Namhyung Kim <[email protected]>
Acked-by: Ian Rogers <[email protected]>
> ---
> tools/perf/builtin-inject.c | 2 +-
> tools/perf/builtin-record.c | 2 +-
> tools/perf/tests/vmlinux-kallsyms.c | 4 ++--
> tools/perf/util/machine.c | 12 +++++++-----
> tools/perf/util/machine.h | 10 ++++++++--
> tools/perf/util/session.c | 4 ++--
> tools/perf/util/session.h | 4 ++--
> 7 files changed, 23 insertions(+), 15 deletions(-)
>
> diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> index c8cf2fdd9cff..481adaa97a68 100644
> --- a/tools/perf/builtin-inject.c
> +++ b/tools/perf/builtin-inject.c
> @@ -2325,7 +2325,7 @@ int cmd_inject(int argc, const char **argv)
> }
>
> inject.session = __perf_session__new(&data, repipe,
> - output_fd(&inject),
> + output_fd(&inject), false,
> &inject.tool);
> if (IS_ERR(inject.session)) {
> ret = PTR_ERR(inject.session);
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index aec18db7ff23..b4d0154dcb18 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -2373,7 +2373,7 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
> signal(SIGUSR2, SIG_IGN);
> }
>
> - session = perf_session__new(data, tool);
> + session = __perf_session__new(data, false, -1, rec->opts.sample_address, tool);
> if (IS_ERR(session)) {
> pr_err("Perf session creation failed.\n");
> return PTR_ERR(session);
> diff --git a/tools/perf/tests/vmlinux-kallsyms.c b/tools/perf/tests/vmlinux-kallsyms.c
> index 1078a93b01aa..1f1ba3ae88aa 100644
> --- a/tools/perf/tests/vmlinux-kallsyms.c
> +++ b/tools/perf/tests/vmlinux-kallsyms.c
> @@ -131,8 +131,8 @@ static int test__vmlinux_matches_kallsyms(struct test_suite *test __maybe_unused
> * Init the machines that will hold kernel, modules obtained from
> * both vmlinux + .ko files and from /proc/kallsyms split by modules.
> */
> - machine__init(&kallsyms, "", HOST_KERNEL_ID);
> - machine__init(&vmlinux, "", HOST_KERNEL_ID);
> + machine__init(&kallsyms, "", HOST_KERNEL_ID, false);
> + machine__init(&vmlinux, "", HOST_KERNEL_ID, false);
>
> maps = machine__kernel_maps(&vmlinux);
>
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index 4e62843d51b7..ddc0a2130caf 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -110,7 +110,7 @@ static void thread__set_guest_comm(struct thread *thread, pid_t pid)
> thread__set_comm(thread, comm, 0);
> }
>
> -int machine__init(struct machine *machine, const char *root_dir, pid_t pid)
> +int machine__init(struct machine *machine, const char *root_dir, pid_t pid, bool has_data_mmap)
> {
> int err = -ENOMEM;
>
> @@ -128,6 +128,7 @@ int machine__init(struct machine *machine, const char *root_dir, pid_t pid)
> machine->env = NULL;
>
> machine->pid = pid;
> + machine->has_data_mmap = has_data_mmap;
>
> machine->id_hdr_size = 0;
> machine->kptr_restrict_warned = false;
> @@ -170,7 +171,7 @@ struct machine *machine__new_host(void)
> struct machine *machine = malloc(sizeof(*machine));
>
> if (machine != NULL) {
> - machine__init(machine, "", HOST_KERNEL_ID);
> + machine__init(machine, "", HOST_KERNEL_ID, false);
nit: when passing constants it can be nice to name the parameter, so
here "/*data_mmap=*/false". There are some checkers based on this
like:
https://clang.llvm.org/extra/clang-tidy/checks/bugprone/argument-comment.html
Thanks,
Ian
>
> if (machine__create_kernel_maps(machine) < 0)
> goto out_delete;
> @@ -272,10 +273,11 @@ void machine__delete(struct machine *machine)
> }
> }
>
> -void machines__init(struct machines *machines)
> +void __machines__init(struct machines *machines, bool have_data_mmap)
> {
> - machine__init(&machines->host, "", HOST_KERNEL_ID);
> + machine__init(&machines->host, "", HOST_KERNEL_ID, have_data_mmap);
> machines->guests = RB_ROOT_CACHED;
> + machines->have_data_mmaps = have_data_mmap;
> }
>
> void machines__exit(struct machines *machines)
> @@ -295,7 +297,7 @@ struct machine *machines__add(struct machines *machines, pid_t pid,
> if (machine == NULL)
> return NULL;
>
> - if (machine__init(machine, root_dir, pid) != 0) {
> + if (machine__init(machine, root_dir, pid, machines->have_data_mmaps) != 0) {
> free(machine);
> return NULL;
> }
> diff --git a/tools/perf/util/machine.h b/tools/perf/util/machine.h
> index d034ecaf89c1..f54e5c888a99 100644
> --- a/tools/perf/util/machine.h
> +++ b/tools/perf/util/machine.h
> @@ -46,6 +46,7 @@ struct machine {
> bool comm_exec;
> bool kptr_restrict_warned;
> bool single_address_space;
> + bool has_data_mmap;
> char *root_dir;
> char *mmap_name;
> char *kallsyms_filename;
> @@ -160,9 +161,14 @@ typedef void (*machine__process_t)(struct machine *machine, void *data);
> struct machines {
> struct machine host;
> struct rb_root_cached guests;
> + bool have_data_mmaps;
> };
>
> -void machines__init(struct machines *machines);
> +void __machines__init(struct machines *machines, bool data_mmap);
> +static inline void machines__init(struct machines *machines)
> +{
> + __machines__init(machines, false);
> +}
> void machines__exit(struct machines *machines);
>
> void machines__process_guests(struct machines *machines,
> @@ -181,7 +187,7 @@ void machines__set_comm_exec(struct machines *machines, bool comm_exec);
>
> struct machine *machine__new_host(void);
> struct machine *machine__new_kallsyms(void);
> -int machine__init(struct machine *machine, const char *root_dir, pid_t pid);
> +int machine__init(struct machine *machine, const char *root_dir, pid_t pid, bool has_data_mmap);
> void machine__exit(struct machine *machine);
> void machine__delete_threads(struct machine *machine);
> void machine__delete(struct machine *machine);
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index 00d18c74c090..e09a02ec8f28 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -191,7 +191,7 @@ static int ordered_events__deliver_event(struct ordered_events *oe,
> }
>
> struct perf_session *__perf_session__new(struct perf_data *data,
> - bool repipe, int repipe_fd,
> + bool repipe, int repipe_fd, bool data_mmap,
> struct perf_tool *tool)
> {
> int ret = -ENOMEM;
> @@ -205,7 +205,7 @@ struct perf_session *__perf_session__new(struct perf_data *data,
> session->decomp_data.zstd_decomp = &session->zstd_data;
> session->active_decomp = &session->decomp_data;
> INIT_LIST_HEAD(&session->auxtrace_index);
> - machines__init(&session->machines);
> + __machines__init(&session->machines, data_mmap);
> ordered_events__init(&session->ordered_events,
> ordered_events__deliver_event, NULL);
>
> diff --git a/tools/perf/util/session.h b/tools/perf/util/session.h
> index ee3715e8563b..465610e81e95 100644
> --- a/tools/perf/util/session.h
> +++ b/tools/perf/util/session.h
> @@ -64,13 +64,13 @@ struct decomp {
> struct perf_tool;
>
> struct perf_session *__perf_session__new(struct perf_data *data,
> - bool repipe, int repipe_fd,
> + bool repipe, int repipe_fd, bool data_mmap,
> struct perf_tool *tool);
>
> static inline struct perf_session *perf_session__new(struct perf_data *data,
> struct perf_tool *tool)
> {
> - return __perf_session__new(data, false, -1, tool);
> + return __perf_session__new(data, false, -1, false, tool);
> }
>
> void perf_session__delete(struct perf_session *session);
> --
> 2.41.0.185.g7c58973941-goog
>
On Tue, Jun 20, 2023 at 1:18 PM Namhyung Kim <[email protected]> wrote:
>
> The kallsyms__get_symbol_start() to get any symbol address from
> kallsyms. The existing kallsyms__get_function_start() only allows text
> symbols so create this to allow data symbols too.
>
> Signed-off-by: Namhyung Kim <[email protected]>
Acked-by: Ian Rogers <[email protected]>
Thanks,
Ian
> ---
> tools/perf/util/event.c | 30 +++++++++++++++++++++++++++---
> tools/perf/util/event.h | 2 ++
> 2 files changed, 29 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> index 3860b0c74829..6fdda0eb3854 100644
> --- a/tools/perf/util/event.c
> +++ b/tools/perf/util/event.c
> @@ -93,8 +93,8 @@ struct process_symbol_args {
> u64 start;
> };
>
> -static int find_symbol_cb(void *arg, const char *name, char type,
> - u64 start)
> +static int find_func_symbol_cb(void *arg, const char *name, char type,
> + u64 start)
> {
> struct process_symbol_args *args = arg;
>
> @@ -110,12 +110,36 @@ static int find_symbol_cb(void *arg, const char *name, char type,
> return 1;
> }
>
> +static int find_any_symbol_cb(void *arg, const char *name,
> + char type __maybe_unused, u64 start)
> +{
> + struct process_symbol_args *args = arg;
> +
> + if (strcmp(name, args->name))
> + return 0;
> +
> + args->start = start;
> + return 1;
> +}
> +
> int kallsyms__get_function_start(const char *kallsyms_filename,
> const char *symbol_name, u64 *addr)
> {
> struct process_symbol_args args = { .name = symbol_name, };
>
> - if (kallsyms__parse(kallsyms_filename, &args, find_symbol_cb) <= 0)
> + if (kallsyms__parse(kallsyms_filename, &args, find_func_symbol_cb) <= 0)
> + return -1;
> +
> + *addr = args.start;
> + return 0;
> +}
> +
> +int kallsyms__get_symbol_start(const char *kallsyms_filename,
> + const char *symbol_name, u64 *addr)
> +{
> + struct process_symbol_args args = { .name = symbol_name, };
> +
> + if (kallsyms__parse(kallsyms_filename, &args, find_any_symbol_cb) <= 0)
> return -1;
>
> *addr = args.start;
> diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
> index de20e01c9d72..d8bcee2e9b93 100644
> --- a/tools/perf/util/event.h
> +++ b/tools/perf/util/event.h
> @@ -360,6 +360,8 @@ size_t perf_event__fprintf(union perf_event *event, struct machine *machine, FIL
>
> int kallsyms__get_function_start(const char *kallsyms_filename,
> const char *symbol_name, u64 *addr);
> +int kallsyms__get_symbol_start(const char *kallsyms_filename,
> + const char *symbol_name, u64 *addr);
>
> void event_attr_init(struct perf_event_attr *attr);
>
> --
> 2.41.0.185.g7c58973941-goog
>