Messages in this thread | | | Subject | Re: [PATCH v3 12/27] perf parse-events: Support no alias assigned event inside hybrid PMU | From | "Jin, Yao" <> | Date | Fri, 16 Apr 2021 09:57:49 +0800 |
| |
Hi Jiri,
On 4/16/2021 3:39 AM, Jiri Olsa wrote: > On Thu, Apr 15, 2021 at 10:53:33PM +0800, Jin, Yao wrote: > > SNIP > >> >> With my current code, >> >> static int parse_events__with_hybrid_pmu(struct parse_events_state *parse_state, >> const char *str, char *pmu_name, >> struct list_head *list) >> { >> struct parse_events_state ps = { >> .list = LIST_HEAD_INIT(ps.list), >> .stoken = PE_START_EVENTS, >> .pmu_name = pmu_name, >> .idx = parse_state->idx, >> }; >> int ret; >> >> ret = parse_events__scanner(str, &ps); >> perf_pmu__parse_cleanup(); >> >> if (!ret) { >> if (!list_empty(&ps.list)) { >> list_splice(&ps.list, list); >> parse_state->idx = ps.idx; >> } >> } >> >> return ret; >> } >> >> The new created evsels are added to the tail of list (ps.list) and ps.list >> is joined to the list (the parameter 'list'). >> >> If we want to reuse the __parse_events(), we may need to: >> >> struct evlist *evlist = evlist__new(); > > there's the original evlist pointer passed to the initial parser > that we should use no? >
Unfortunately the answer is no. :(
For "cpu_core/LLC-loads/", if we do the parser twice by just calling __parse_events, actually the __parse_events will be called two times.
int __parse_events(struct evlist *evlist, const char *str, struct parse_events_error *err, struct perf_pmu *fake_pmu, char *pmu_name) { struct parse_events_state parse_state = { .list = LIST_HEAD_INIT(parse_state.list), ... };
ret = parse_events__scanner(str, &parse_state); perf_pmu__parse_cleanup();
if (!ret && list_empty(&parse_state.list)) { WARN_ONCE(true, "WARNING: event parser found nothing\n"); return -1; } ... }
When returning to the first __parse_events,'parse_state.list' is an empty list so it would return "WARNING: event parser found nothing".
So in my patch, I pass a list pointer in and the new created evsels will be added to this list.
>> >> __parse_events(evlist, str, NULL, NULL); >> Add the evsels in evlist to the tail of list (the parameter 'list') >> evlist__delete(evlist); >> >> Is my understanding correct? >> >> Yes, we have to change the interface of __parse_events() by adding a new >> parameter 'pmu_name', which will bring much more changes. I agree to make >> this change in follow-up patches. > > let's check on this over the next version >
That's fine, thanks.
Thanks Jin Yao
> thanks, > jirka >
| |