Messages in this thread | | | Date | Fri, 8 Jul 2016 16:46:54 +0200 | From | Jiri Olsa <> | Subject | Re: [PATCH v13 2/8] perf evlist: Introduce aux evlist |
| |
On Wed, Jul 06, 2016 at 08:16:52PM +0800, Wangnan (F) wrote: > > > On 2016/7/6 19:36, Jiri Olsa wrote: > > On Mon, Jul 04, 2016 at 06:20:03AM +0000, Wang Nan wrote: > > > > SNIP > > > > > +struct perf_evlist *perf_evlist__new_aux(struct perf_evlist *parent) > > > +{ > > > + struct perf_evlist *evlist; > > > + > > > + if (perf_evlist__is_aux(parent)) { > > > + pr_err("Internal error: create aux evlist from another aux evlist\n"); > > > + return NULL; > > > + } > > > + > > > + evlist = zalloc(sizeof(*evlist)); > > > + if (!evlist) > > > + return NULL; > > > + > > > + perf_evlist__init(evlist, parent->cpus, parent->threads); > > > + evlist->parent = parent; > > > + INIT_LIST_HEAD(&evlist->list); > > > + list_add(&evlist->list, &parent->children); > > I understand there's some reason for separating maps with and > > without overwrite set, but I'm missing it.. why is that? > > You are asking overwrite, not write_backward? > > Overwrite mapping needs to be mapped without PROT_WRITE, so its > control page is also read only, so perf_evlist__mmap_consume() is > not able to use, and there's no way to tell kernel to where we have > read. Kernel overwrite old records when its full. Compare with normal > mapping: perf uses perf_evlist__mmap_consume() to tell kernel the > last byte it has read, so kernel stop writing data to it when it full, > and issues LOST event. This is the reason we need to separate maps > with and without overwrite set. > > For write backward: kernel write data in different direction, so > requires map separation.
I dont like the idea of duplicating whole perf_evlist in order just to map some events with overwrite/backward
perf_evlist carries all the other info about events, not just memory maping..
I think it'd be better to do it some other way, like:
- we have mmaps for events/evsels, so you're able to map it differently with or without PROT_WRITE even in current design.. there's struct perf_mmap that can carry that info then it's the matter of reading/processing those maps that needs to change.. new perf_evlist interface
- we could keep separate struct perf_mmap arrays for forward and backward/overwrite maps
- ...
I understand both mapping need different treatment, but I think that should be encapsulated within the struct perf_evlist interface
thanks, jirka
| |