| 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
| ||
|
Message-ID: <10b48a82-fd30-4dd9-a19a-976e76dd0c8d@linux.intel.com>
Date: Wed, 6 Nov 2024 13:48:56 -0500
From: "Liang, Kan" <kan.liang@...ux.intel.com>
To: Ian Rogers <irogers@...gle.com>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Namhyung Kim <namhyung@...nel.org>, Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>, Adrian Hunter <adrian.hunter@...el.com>,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
Perry Taylor <perry.taylor@...el.com>, Samantha Alt
<samantha.alt@...el.com>, Caleb Biggers <caleb.biggers@...el.com>,
Weilin Wang <weilin.wang@...el.com>, Edward Baker <edward.baker@...el.com>
Subject: Re: [PATCH v4 06/22] perf jevents: Add tsx metric group for Intel
models
On 2024-11-06 1:15 p.m., Ian Rogers wrote:
> On Wed, Nov 6, 2024 at 9:53 AM Liang, Kan <kan.liang@...ux.intel.com> wrote:
>>
>>
>>
>> On 2024-09-26 1:50 p.m., Ian Rogers wrote:
>>> Allow duplicated metric to be dropped from json files. Detect when TSX
>>> is supported by a model by using the json events, use sysfs events at
>>> runtime as hypervisors, etc. may disable TSX.
>>>
>>> Add CheckPmu to metric to determine if which PMUs have been associated
>>> with the loaded events.
>>>
>>> Signed-off-by: Ian Rogers <irogers@...gle.com>
>>> ---
>>> tools/perf/pmu-events/intel_metrics.py | 52 +++++++++++++++++++++++++-
>>> 1 file changed, 51 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tools/perf/pmu-events/intel_metrics.py b/tools/perf/pmu-events/intel_metrics.py
>>> index f34b4230a4ee..58e243695f0a 100755
>>> --- a/tools/perf/pmu-events/intel_metrics.py
>>> +++ b/tools/perf/pmu-events/intel_metrics.py
>>> @@ -1,12 +1,13 @@
>>> #!/usr/bin/env python3
>>> # SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
>>> -from metric import (d_ratio, has_event, max, Event, JsonEncodeMetric,
>>> +from metric import (d_ratio, has_event, max, CheckPmu, Event, JsonEncodeMetric,
>>> JsonEncodeMetricGroupDescriptions, LoadEvents, Metric,
>>> MetricGroup, MetricRef, Select)
>>> import argparse
>>> import json
>>> import math
>>> import os
>>> +from typing import Optional
>>>
>>> # Global command line arguments.
>>> _args = None
>>> @@ -74,6 +75,54 @@ def Smi() -> MetricGroup:
>>> ], description = 'System Management Interrupt metrics')
>>>
>>>
>>> +def Tsx() -> Optional[MetricGroup]:
>>> + pmu = "cpu_core" if CheckPmu("cpu_core") else "cpu"
>>> + cycles = Event('cycles')
>>
>> Isn't the pmu prefix required for cycles as well?
>
> Makes sense.
>
>>> + cycles_in_tx = Event(f'{pmu}/cycles\-t/')
>>> + cycles_in_tx_cp = Event(f'{pmu}/cycles\-ct/')
>>> + try:
>>> + # Test if the tsx event is present in the json, prefer the
>>> + # sysfs version so that we can detect its presence at runtime.
>>> + transaction_start = Event("RTM_RETIRED.START")
>>> + transaction_start = Event(f'{pmu}/tx\-start/')
>>
>> What's the difference between this check and the later has_event() check?
>>
>> All the tsx related events are model-specific events. We should check
>> them all before using it.
>
> So if there is PMU in the Event name then the Event logic assumes you
> are using sysfs and doesn't check the event exists in json. As you
> say, I needed a way to detect does this model support TSX? I wanted to
> avoid a model lookup table, so I used the existence of
> RTM_RETIRED.START for a model as the way to determine if the model
> supports TSX. Once we know we have a model supporting TSX then we use
> the sysfs event name and has_event check, so that if the TSX and the
> event have been disabled the metric doesn't fail parsing.
>
> So, the first check is a compile time check of, "does this model have
> TSX?". The "has_event" check is a runtime thing where we want to see
> if the event exists in sysfs in case the TSX was disabled say in the
> BIOS.
>
Yes, that's sufficient.
But the "has_event" check seems very random.
For example,
>>>> + Metric('tsx_cycles_per_transaction',
>>>> + 'Number of cycles within a transaction divided by the number of transactions.',
>>>> + Select(cycles_in_tx / transaction_start,
>>>> + has_event(cycles_in_tx),
>>>> + 0),
>>>> + "cycles / transaction"),
I think both cycles_in_tx and transaction_start should be checked.
>>>> + Metric('tsx_cycles_per_elision',
>>>> + 'Number of cycles within a transaction divided by the number of elisions.',
>>>> + Select(cycles_in_tx / elision_start,
>>>> + has_event(elision_start),
>>>> + 0),
This one only checks the elision_start event.
Thanks,
Kan
> Thanks,
> Ian
>
>>
>> Thanks,
>> Kan
>>> + except:> + return None
>>> +
>>> + elision_start = None
>>> + try:
>>> + # Elision start isn't supported by all models, but we'll not
>>> + # generate the tsx_cycles_per_elision metric in that
>>> + # case. Again, prefer the sysfs encoding of the event.
>>> + elision_start = Event("HLE_RETIRED.START")
>>> + elision_start = Event(f'{pmu}/el\-start/')
>>> + except:
>>> + pass
>>> +
>>> + return MetricGroup('transaction', [
>>> + Metric('tsx_transactional_cycles',
>>> + 'Percentage of cycles within a transaction region.',
>>> + Select(cycles_in_tx / cycles, has_event(cycles_in_tx), 0),
>>> + '100%'),
>>> + Metric('tsx_aborted_cycles', 'Percentage of cycles in aborted transactions.',
>>> + Select(max(cycles_in_tx - cycles_in_tx_cp, 0) / cycles,
>>> + has_event(cycles_in_tx),
>>> + 0),
>>> + '100%'),
>>> + Metric('tsx_cycles_per_transaction',
>>> + 'Number of cycles within a transaction divided by the number of transactions.',
>>> + Select(cycles_in_tx / transaction_start,
>>> + has_event(cycles_in_tx),
>>> + 0),
>>> + "cycles / transaction"),
>>> + Metric('tsx_cycles_per_elision',
>>> + 'Number of cycles within a transaction divided by the number of elisions.',
>>> + Select(cycles_in_tx / elision_start,
>>> + has_event(elision_start),
>>> + 0),
>>> + "cycles / elision") if elision_start else None,
>>> + ], description="Breakdown of transactional memory statistics")
>>> +
>>> +
>>> def main() -> None:
>>> global _args
>>>
>>> @@ -100,6 +149,7 @@ def main() -> None:
>>> Idle(),
>>> Rapl(),
>>> Smi(),
>>> + Tsx(),
>>> ])
>>>
>>>
>>
>
Powered by blists - more mailing lists