Messages in this thread | | | Date | Fri, 27 May 2022 07:52:31 +0530 | Subject | Re: [PATCH v4 2/5] perf header: Parse non-cpu pmu capabilities | From | Ravi Bangoria <> |
| |
On 26-May-22 9:25 PM, Liang, Kan wrote: > > > On 5/26/2022 11:08 AM, Ravi Bangoria wrote: >> 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. > > I think this is the format for the internal string, not the format of the perf.data header.
Yeah I just realized that after replying. Anyway, thanks for clarifying. Will change internal format of HEADER_HYBRID_CPU_PMU_CAPS (as well as HEADER_CPU_PMU_CAPS), and replace HEADER_HYBRID_CPU_PMU_CAPS with HEADER_PMU_CAPS.
Thanks, Ravi
| |