Re: [PATCH v7 perf, bpf-next 6/8] perf util: handle PERF_RECORD_KSYMBOL
From: Song Liu
Date: Tue Jan 15 2019 - 14:58:58 EST
> On Jan 15, 2019, at 9:56 AM, Arnaldo Carvalho de Melo <acme@xxxxxxxxxx> wrote:
>
> Em Thu, Jan 10, 2019 at 04:19:31PM -0800, Song Liu escreveu:
>> This patch handles PERF_RECORD_KSYMBOL in perf record/report.
>> Specifically, map and symbol are created for ksymbol register, and
>> removed for ksymbol unregister.
>>
>> This patch also set perf_event_attr.ksymbol properly. The flag is
>> ON by default.
>>
>> Signed-off-by: Song Liu <songliubraving@xxxxxx>
>> ---
>> tools/perf/util/event.c | 21 +++++++++++++++
>> tools/perf/util/event.h | 20 ++++++++++++++
>> tools/perf/util/evsel.c | 9 +++++++
>> tools/perf/util/evsel.h | 1 +
>> tools/perf/util/machine.c | 57 +++++++++++++++++++++++++++++++++++++++
>> tools/perf/util/machine.h | 3 +++
>> tools/perf/util/session.c | 4 +++
>> tools/perf/util/tool.h | 4 ++-
>> 8 files changed, 118 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
>> index 937a5a4f71cc..3c8a6a8dd260 100644
>> --- a/tools/perf/util/event.c
>> +++ b/tools/perf/util/event.c
>> @@ -24,6 +24,7 @@
>> #include "symbol/kallsyms.h"
>> #include "asm/bug.h"
>> #include "stat.h"
>> +#include "session.h"
>>
>> #define DEFAULT_PROC_MAP_PARSE_TIMEOUT 500
>>
>> @@ -45,6 +46,7 @@ static const char *perf_event__names[] = {
>> [PERF_RECORD_SWITCH] = "SWITCH",
>> [PERF_RECORD_SWITCH_CPU_WIDE] = "SWITCH_CPU_WIDE",
>> [PERF_RECORD_NAMESPACES] = "NAMESPACES",
>> + [PERF_RECORD_KSYMBOL] = "KSYMBOL",
>> [PERF_RECORD_HEADER_ATTR] = "ATTR",
>> [PERF_RECORD_HEADER_EVENT_TYPE] = "EVENT_TYPE",
>> [PERF_RECORD_HEADER_TRACING_DATA] = "TRACING_DATA",
>> @@ -1329,6 +1331,14 @@ int perf_event__process_switch(struct perf_tool *tool __maybe_unused,
>> return machine__process_switch_event(machine, event);
>> }
>>
>> +int perf_event__process_ksymbol(struct perf_tool *tool __maybe_unused,
>> + union perf_event *event,
>> + struct perf_sample *sample __maybe_unused,
>> + struct machine *machine)
>> +{
>> + return machine__process_ksymbol(machine, event, sample);
>> +}
>> +
>> size_t perf_event__fprintf_mmap(union perf_event *event, FILE *fp)
>> {
>> return fprintf(fp, " %d/%d: [%#" PRIx64 "(%#" PRIx64 ") @ %#" PRIx64 "]: %c %s\n",
>> @@ -1461,6 +1471,14 @@ static size_t perf_event__fprintf_lost(union perf_event *event, FILE *fp)
>> return fprintf(fp, " lost %" PRIu64 "\n", event->lost.lost);
>> }
>>
>> +size_t perf_event__fprintf_ksymbol(union perf_event *event, FILE *fp)
>> +{
>> + return fprintf(fp, " ksymbol event with addr %lx len %u type %u flags 0x%x name %s\n",
>> + event->ksymbol_event.addr, event->ksymbol_event.len,
>> + event->ksymbol_event.ksym_type,
>> + event->ksymbol_event.flags, event->ksymbol_event.name);
>> +}
>> +
>> size_t perf_event__fprintf(union perf_event *event, FILE *fp)
>> {
>> size_t ret = fprintf(fp, "PERF_RECORD_%s",
>> @@ -1496,6 +1514,9 @@ size_t perf_event__fprintf(union perf_event *event, FILE *fp)
>> case PERF_RECORD_LOST:
>> ret += perf_event__fprintf_lost(event, fp);
>> break;
>> + case PERF_RECORD_KSYMBOL:
>> + ret += perf_event__fprintf_ksymbol(event, fp);
>> + break;
>> default:
>> ret += fprintf(fp, "\n");
>> }
>> diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
>> index eb95f3384958..018322f2a13e 100644
>> --- a/tools/perf/util/event.h
>> +++ b/tools/perf/util/event.h
>> @@ -5,6 +5,7 @@
>> #include <limits.h>
>> #include <stdio.h>
>> #include <linux/kernel.h>
>> +#include <linux/bpf.h>
>>
>> #include "../perf.h"
>> #include "build-id.h"
>> @@ -84,6 +85,19 @@ struct throttle_event {
>> u64 stream_id;
>> };
>>
>> +#ifndef KSYM_NAME_LEN
>> +#define KSYM_NAME_LEN 256
>> +#endif
>> +
>> +struct ksymbol_event {
>> + struct perf_event_header header;
>> + u64 addr;
>> + u32 len;
>> + u16 ksym_type;
>> + u16 flags;
>> + char name[KSYM_NAME_LEN];
>> +};
>> +
>> #define PERF_SAMPLE_MASK \
>> (PERF_SAMPLE_IP | PERF_SAMPLE_TID | \
>> PERF_SAMPLE_TIME | PERF_SAMPLE_ADDR | \
>> @@ -651,6 +665,7 @@ union perf_event {
>> struct stat_round_event stat_round;
>> struct time_conv_event time_conv;
>> struct feature_event feat;
>> + struct ksymbol_event ksymbol_event;
>> };
>>
>> void perf_event__print_totals(void);
>> @@ -748,6 +763,10 @@ int perf_event__process_exit(struct perf_tool *tool,
>> union perf_event *event,
>> struct perf_sample *sample,
>> struct machine *machine);
>> +int perf_event__process_ksymbol(struct perf_tool *tool,
>> + union perf_event *event,
>> + struct perf_sample *sample,
>> + struct machine *machine);
>> int perf_tool__process_synth_event(struct perf_tool *tool,
>> union perf_event *event,
>> struct machine *machine,
>> @@ -811,6 +830,7 @@ size_t perf_event__fprintf_switch(union perf_event *event, FILE *fp);
>> size_t perf_event__fprintf_thread_map(union perf_event *event, FILE *fp);
>> size_t perf_event__fprintf_cpu_map(union perf_event *event, FILE *fp);
>> size_t perf_event__fprintf_namespaces(union perf_event *event, FILE *fp);
>> +size_t perf_event__fprintf_ksymbol(union perf_event *event, FILE *fp);
>> size_t perf_event__fprintf(union perf_event *event, FILE *fp);
>>
>> int kallsyms__get_function_start(const char *kallsyms_filename,
>> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
>> index dbc0466db368..de34ce875648 100644
>> --- a/tools/perf/util/evsel.c
>> +++ b/tools/perf/util/evsel.c
>> @@ -1035,6 +1035,7 @@ void perf_evsel__config(struct perf_evsel *evsel, struct record_opts *opts,
>> attr->mmap = track;
>> attr->mmap2 = track && !perf_missing_features.mmap2;
>> attr->comm = track;
>> + attr->ksymbol = track && !perf_missing_features.ksymbol;
>>
>> if (opts->record_namespaces)
>> attr->namespaces = track;
>> @@ -1652,6 +1653,7 @@ int perf_event_attr__fprintf(FILE *fp, struct perf_event_attr *attr,
>> PRINT_ATTRf(context_switch, p_unsigned);
>> PRINT_ATTRf(write_backward, p_unsigned);
>> PRINT_ATTRf(namespaces, p_unsigned);
>> + PRINT_ATTRf(ksymbol, p_unsigned);
>>
>> PRINT_ATTRn("{ wakeup_events, wakeup_watermark }", wakeup_events, p_unsigned);
>> PRINT_ATTRf(bp_type, p_unsigned);
>> @@ -1811,6 +1813,8 @@ int perf_evsel__open(struct perf_evsel *evsel, struct cpu_map *cpus,
>> PERF_SAMPLE_BRANCH_NO_CYCLES);
>> if (perf_missing_features.group_read && evsel->attr.inherit)
>> evsel->attr.read_format &= ~(PERF_FORMAT_GROUP|PERF_FORMAT_ID);
>> + if (perf_missing_features.ksymbol)
>> + evsel->attr.ksymbol = 0;
>> retry_sample_id:
>> if (perf_missing_features.sample_id_all)
>> evsel->attr.sample_id_all = 0;
>> @@ -1955,6 +1959,11 @@ int perf_evsel__open(struct perf_evsel *evsel, struct cpu_map *cpus,
>> perf_missing_features.exclude_guest = true;
>> pr_debug2("switching off exclude_guest, exclude_host\n");
>> goto fallback_missing_features;
>> + } else if (!perf_missing_features.ksymbol &&
>> + evsel->attr.ksymbol) {
>> + perf_missing_features.ksymbol = true;
>> + pr_debug2("switching off ksymbol\n");
>> + goto fallback_missing_features;
>
>
> Please move this to the top, see the comment there:
>
> if (err != -EINVAL || cpu > 0 || thread > 0)
> goto out_close;
>
> /*
> * Must probe features in the order they were added to the
> * perf_event_attr interface.
> */
> if (!perf_missing_features.write_backward && evsel->attr.write_backward) {
> perf_missing_features.write_backward = true;
> pr_debug2("switching off write_backward\n");
> goto out_close;
> } else if (!perf_missing_features.clockid_wrong && evsel->attr.use_clockid) {
> perf_missing_features.clockid_wrong = true;
> pr_debug2("switching off clockid\n");
> goto fallback_missing_features;
>
>
> So that when one asks for, say, attr.write_backward _and_ attr.ksymbol,
> we first disable attr.ksymbol, to try the fallback, as it was added
> after attr.write_backward.
>
> Think about an older kernel where attr.write_backward is present but not
> attr.ksymbol.
>
> Ditto for the attr.bpf_event.
>
Thanks Arnaldo!
I will include the fix in next version.
Song