lkml.org 
[lkml]   [2021]   [Apr]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v3 12/27] perf parse-events: Support no alias assigned event inside hybrid PMU
    From
    Date
    Hi Jiri,

    On 4/15/2021 10:11 PM, Jiri Olsa wrote:
    > On Thu, Apr 15, 2021 at 09:36:16PM +0800, Jin, Yao wrote:
    >
    > SNIP
    >
    >>>> + int n = 0;
    >>>> +
    >>>> + list_for_each(pos, list)
    >>>> + n++;
    >>>> +
    >>>> + return n;
    >>>> +}
    >>>> +
    >>>> +static int parse_events__with_hybrid_pmu(struct parse_events_state *parse_state,
    >>>> + const char *str, char *pmu_name,
    >>>> + bool *found, 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,
    >>>> + };
    >>>
    >>> could we add this pmu_name directly to __parse_events?
    >>>
    >>
    >> Do you suggest we directly call __parse_events()?
    >>
    >> int __parse_events(struct evlist *evlist, const char *str,
    >> struct parse_events_error *err, struct perf_pmu *fake_pmu)
    >>
    >> struct parse_events_state parse_state = {
    >> .list = LIST_HEAD_INIT(parse_state.list),
    >> .idx = evlist->core.nr_entries,
    >> .error = err,
    >> .evlist = evlist,
    >> .stoken = PE_START_EVENTS,
    >> .fake_pmu = fake_pmu,
    >> };
    >>
    >> But for parse_events__with_hybrid_pmu, we don't have valid evlist. So if we
    >> switch to __parse_events, evlist processing may be a problem.
    >
    > you should use parse_state->evlist no? but we can chec/make this
    > change in next itaration.. it's already lot of changes
    >
    > jirka
    >

    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();

    __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.

    Thanks
    Jin Yao

    \
     
     \ /
      Last update: 2021-04-15 16:59    [W:4.142 / U:0.128 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site