> On Jan 15, 2019, at 9:56 AM, Arnaldo Carvalho de Melo <a...@kernel.org> 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 <songliubrav...@fb.com>
>> ---
>> 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

Reply via email to