Messages in this thread | | | Date | Thu, 26 May 2022 20:38:02 +0530 | Subject | Re: [PATCH v4 2/5] perf header: Parse non-cpu pmu capabilities | From | Ravi Bangoria <> |
| |
Hi Kan,
[...]
>> +static int write_pmu_caps(struct feat_fd *ff, struct evlist *evlist __maybe_unused) >> +{ >> + struct perf_pmu_caps *caps = NULL; >> + struct perf_pmu *pmu = NULL; >> + u32 nr_pmus = 0; >> + int ret; >> + >> + while ((pmu = perf_pmu__scan(pmu))) { >> + if (!pmu->name || !strncmp(pmu->name, "cpu", 3) || >> + perf_pmu__caps_parse(pmu) <= 0) >> + continue; >> + nr_pmus++; >> + } >> + >> + ret = do_write(ff, &nr_pmus, sizeof(nr_pmus)); >> + if (ret < 0) >> + return ret; >> + >> + if (!nr_pmus) >> + return 0; >> + >> + while ((pmu = perf_pmu__scan(pmu))) { >> + if (!pmu->name || !strncmp(pmu->name, "cpu", 3) || !pmu->nr_caps) >> + continue; >> + >> + ret = do_write_string(ff, pmu->name); >> + if (ret < 0) >> + return ret; >> + >> + ret = do_write(ff, &pmu->nr_caps, sizeof(pmu->nr_caps)); >> + if (ret < 0) >> + return ret; >> + >> + list_for_each_entry(caps, &pmu->caps, list) { >> + ret = do_write_string(ff, caps->name); >> + if (ret < 0) >> + return ret; >> + >> + ret = do_write_string(ff, caps->value); >> + if (ret < 0) >> + return ret; >> + } >> + } > > The write_per_cpu_pmu_caps() also does a similar thing. Can we factor out a generic write_pmu_caps() which works for both cpu and non-cpu pmu capabilities?
I might be able to do this but..
> It seems the print_pmu_caps()/process_pmu_caps() can also does similar factor out.
not this, see below..
> Actually, more aggressively, why not use the HEADER_PMU_CAPS to replace the HEADER_HYBRID_CPU_PMU_CAPS? The HEADER_HYBRID_CPU_PMU_CAPS is the last header feature. It seems doable. We can always write/print the "cpu_" kind of PMU first to be compatible with the old tools.
There are some differences in how capabilities are stored in perf.data header as well as perf_env. In case of HEADER_CPU_PMU_CAPS or HEADER_HYBRID_CPU_PMU_CAPS, all capabilities are stored in a single string separated by NULL character. Whereas, in case of HEADER_PMU_CAPS, they are stored as an array of strings. The reason for this difference is, searching in an array is far easier compared to searching in a NULL separated string. So, I don't think I can extend HEADER_HYBRID_CPU_PMU_CAPS as HEADER_PMU_CAPS without adding complexity in perf_env__find_pmu_cap().
Thanks for the review, Ravi
| |