Re: [PATCH v5 perf, bpf-next 1/7] perf, bpf: Introduce PERF_RECORD_KSYMBOL

From: Peter Zijlstra
Date: Tue Jan 08 2019 - 13:31:35 EST


On Thu, Dec 20, 2018 at 10:28:58AM -0800, Song Liu wrote:

> @@ -648,11 +649,18 @@ struct perf_event_mmap_page {
> * PERF_RECORD_MISC_COMM_EXEC - PERF_RECORD_COMM event
> * PERF_RECORD_MISC_FORK_EXEC - PERF_RECORD_FORK event (perf internal)
> * PERF_RECORD_MISC_SWITCH_OUT - PERF_RECORD_SWITCH* events
> + * PERF_RECORD_MISC_KSYMBOL_* - PERF_RECORD_KSYMBOL event
> */
> #define PERF_RECORD_MISC_MMAP_DATA (1 << 13)
> #define PERF_RECORD_MISC_COMM_EXEC (1 << 13)
> #define PERF_RECORD_MISC_FORK_EXEC (1 << 13)
> #define PERF_RECORD_MISC_SWITCH_OUT (1 << 13)
> +
> +#define PERF_RECORD_MISC_KSYMBOL_UNREGISTER (1 << 3)
> +#define PERF_RECORD_MISC_KSYMBOL_TYPE_MASK (7 << 4)
> +#define PERF_RECORD_MISC_KSYMBOL_TYPE_UNKNOWN (0 << 4)
> +#define PERF_RECORD_MISC_KSYMBOL_TYPE_BPF (1 << 4)

So this gives us 8 possible types, of which 2 are claimed.

I suppose we already know of FTRACE_TRAMPOLINE and MODULE, which
accounts for half the space.

> +
> /*
> * These PERF_RECORD_MISC_* flags below are safely reused
> * for the following events:
> @@ -965,6 +973,19 @@ enum perf_event_type {
> */
> PERF_RECORD_NAMESPACES = 16,
>
> + /*
> + * Record ksymbol register/unregister events:
> + *
> + * struct {
> + * struct perf_event_header header;
> + * u64 addr;
> + * u64 len;
> + * char name[];
> + * struct sample_id sample_id;
> + * };
> + */
> + PERF_RECORD_KSYMBOL = 17,

Do we really need u64 length? That is, are we willing to consider
symbols larger than 4G ?

Could we not reclaim the top 32 or 16 bits thereof for that type thing
instead of using a few misc bits?