Messages in this thread | | | Subject | Re: [PATCH v13 2/8] perf evlist: Introduce aux evlist | From | "Wangnan (F)" <> | Date | Mon, 11 Jul 2016 18:20:31 +0800 |
| |
On 2016/7/8 22:46, Jiri Olsa wrote: > 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
I don't like it either, but aux_evlist is the easiest way to do this work. Other potential solutions require heavy API changes.
Thank you.
| |