linux-kernel - Re: [PATCH v4 2/5] perf header: Parse non-cpu pmu capabilities

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <05aa0287-ebc4-b85c-4012-f16870caf0b8@amd.com>
Date:   Fri, 27 May 2022 07:52:31 +0530
From:   Ravi Bangoria <ravi.bangoria@....com>
To:     "Liang, Kan" <kan.liang@...ux.intel.com>
Cc:     acme@...nel.org, irogers@...gle.com, peterz@...radead.org,
        rrichter@....com, mingo@...hat.com, mark.rutland@....com,
        jolsa@...nel.org, namhyung@...nel.org, tglx@...utronix.de,
        bp@...en8.de, james.clark@....com, leo.yan@...aro.org,
        ak@...ux.intel.com, eranian@...gle.com, like.xu.linux@...il.com,
        x86@...nel.org, linux-perf-users@...r.kernel.org,
        linux-kernel@...r.kernel.org, sandipan.das@....com,
        ananth.narayan@....com, kim.phillips@....com,
        santosh.shukla@....com, Ravi Bangoria <ravi.bangoria@....com>
Subject: Re: [PATCH v4 2/5] perf header: Parse non-cpu pmu capabilities

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