On Fri, Dec 14, 2018 at 10:48:57AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Dec 13, 2018 at 09:48:57PM +0000, Song Liu escreveu:
> > I guess you are looking for something for all ksym add/delete events, like;
> >
> > /*
> > * PERF_RECORD_KSYMBOL
> > *
> > * struct {
> > * struct perf_event_header header;
> > * u64 addr;
> > * u32 len;
> > * u16 ksym_type;
> > * u16 flags;
> > * char name[];
> > * struct sample_id sample_id;
> > * };
> > */
Yes, something like that.
> Can't this reuse PERF_RECORD_MMAP2 with some bit in the header to mean
> that the name is the symbol name, not a path to some ELF/whatever? The
> ksym type could be encoded in the prot field, PROT_EXEC for functions,
> PROT_READ for read only data, PROT_WRITE for rw data.
>
> If we do it that way older tools will show the DSO name and an
> unresolved symbol, and even an indication if its a function or data,
> which is better than not showing anything when processing a new
> PERF_RECORD_KSYMBOL.
>
> New tools, seeing the perf_event_attr.header bit will know that this is
> a "map" with just one symbol and will show that for both DSO name and
> symbol.
That confuses me; the DSO for ksyms is [kernel|$modname] after all. And
BPF would like to have multiple symbols per 'program', so I can imagine
it would want to do something like:
[bpf-progname1] function1
[bpf-progname1] function2
[bpf-progname2] progname2
The first being an bpf proglet with multiple functions, the second a
'legacy' bpf proglet with only a single function.
Trouble is; both PERF_RECORD_KSYM and MMAP* only have a single name[]
field. Now, I suppose we could add:
char modname[MODULE_NAME_LEN]
or:
u16 modlen;
char modname[modlen];
or something along those lines.
Similarly; I would not expect the ftrace trampolines to all have a
different module name.
> > We can use ksym_type to encode BPF_EVENT, trampolines, or other type of
> > ksym.
> > We can use flags or header.misc to encode ksym add/delete. Is this right?
> >
> > If we go this direction, shall we reserve a few more bytes in it for
> > different
> > types to use, like:
> >
> > /*
> > * PERF_RECORD_KSYMBOL
> > *
> > * struct {
> > * struct perf_event_header header;
> > * u64 addr;
> > * u32 len;
> > * u16 ksym_type;
> > * u16 flags;
> > * u64 data[2];
> > * char name[];
> > * struct sample_id sample_id;
> > * };
> > */
Right; elsewhere you proposed keeping PERF_RECORD_BPF_EVENT for that;
which I think is clearer.
I think you can keep much of the current patches for that in fact.
