| 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: <37726d03-c988-5f26-b133-d48408f30e89@linux.intel.com>
Date: Tue, 29 Jun 2021 13:50:19 -0400
From: "Liang, Kan" <kan.liang@...ux.intel.com>
To: Greg KH <gregkh@...uxfoundation.org>
Cc: peterz@...radead.org, mingo@...hat.com, acme@...nel.org,
linux-kernel@...r.kernel.org, eranian@...gle.com,
namhyung@...nel.org, jolsa@...hat.com, ak@...ux.intel.com,
yao.jin@...ux.intel.com,
"Antonov, Alexander" <alexander.antonov@...ux.intel.com>
Subject: Re: [PATCH V2 2/6] perf/x86/intel/uncore: Add alias PMU name
On 6/29/2021 2:15 AM, Greg KH wrote:
> On Mon, Jun 28, 2021 at 01:17:39PM -0700, kan.liang@...ux.intel.com wrote:
>> From: Kan Liang <kan.liang@...ux.intel.com>
>>
>> A perf PMU may have two PMU names. For example, Intel Sapphire Rapids
>> server supports the discovery mechanism. Without the platform-specific
>> support, an uncore PMU is named by a type ID plus a box ID, e.g.,
>> uncore_type_0_0, because the real name of the uncore PMU cannot be
>> retrieved from the discovery table. With the platform-specific support
>> later, perf has the mapping information from a type ID to a specific
>> uncore unit. Just like the previous platforms, the uncore PMU is named
>> by the real PMU name, e.g., uncore_cha_0. The user scripts which work
>> well with the old numeric name may not work anymore.
>>
>> Add a new attribute "alias" to indicate the old numeric name. The
>> following userspace perf tool patch will handle both names. The user
>> scripts should work properly with the updated perf tool.
>>
>> Signed-off-by: Kan Liang <kan.liang@...ux.intel.com>
>> Cc: gregkh@...uxfoundation.org
>> ---
>> .../testing/sysfs-bus-event_source-devices-uncore | 13 ++++++++
>> arch/x86/events/intel/uncore.c | 19 ++++++++----
>> arch/x86/events/intel/uncore.h | 1 +
>> arch/x86/events/intel/uncore_snbep.c | 35 +++++++++++++++++++++-
>> 4 files changed, 61 insertions(+), 7 deletions(-)
>> create mode 100644 Documentation/ABI/testing/sysfs-bus-event_source-devices-uncore
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-event_source-devices-uncore b/Documentation/ABI/testing/sysfs-bus-event_source-devices-uncore
>> new file mode 100644
>> index 0000000..b56e8f0
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-bus-event_source-devices-uncore
>> @@ -0,0 +1,13 @@
>> +What: /sys/bus/event_source/devices/uncore_*/alias
>> +Date: June 2021
>> +KernelVersion: 5.15
>> +Contact: Linux kernel mailing list <linux-kernel@...r.kernel.org>
>> +Description: Read-only. An attribute to describe the alias name of
>> + the uncore PMU if an alias exists on some platforms.
>> + The 'perf(1)' tool should treat both names the same.
>> + They both can be used to access the uncore PMU.
>> +
>> + Example:
>> +
>> + $ cat /sys/devices/uncore_cha_2/alias
>> + uncore_type_0_2
>> diff --git a/arch/x86/events/intel/uncore.c b/arch/x86/events/intel/uncore.c
>> index 7087ce7..ff07472 100644
>> --- a/arch/x86/events/intel/uncore.c
>> +++ b/arch/x86/events/intel/uncore.c
>> @@ -846,6 +846,18 @@ static const struct attribute_group uncore_pmu_attr_group = {
>> .attrs = uncore_pmu_attrs,
>> };
>>
>> +void uncore_get_alias_name(char *pmu_name, struct intel_uncore_pmu *pmu)
>> +{
>> + struct intel_uncore_type *type = pmu->type;
>> +
>> + if (type->num_boxes == 1)
>> + sprintf(pmu_name, "uncore_type_%u", type->type_id);
>> + else {
>> + sprintf(pmu_name, "uncore_type_%u_%d",
>> + type->type_id, type->box_ids[pmu->pmu_idx]);
>> + }
>> +}
>> +
>> static void uncore_get_pmu_name(struct intel_uncore_pmu *pmu)
>> {
>> struct intel_uncore_type *type = pmu->type;
>> @@ -855,12 +867,7 @@ static void uncore_get_pmu_name(struct intel_uncore_pmu *pmu)
>> * Use uncore_type_&typeid_&boxid as name.
>> */
>> if (!type->name) {
>> - if (type->num_boxes == 1)
>> - sprintf(pmu->name, "uncore_type_%u", type->type_id);
>> - else {
>> - sprintf(pmu->name, "uncore_type_%u_%d",
>> - type->type_id, type->box_ids[pmu->pmu_idx]);
>> - }
>> + uncore_get_alias_name(pmu->name, pmu);
>> return;
>> }
>>
>> diff --git a/arch/x86/events/intel/uncore.h b/arch/x86/events/intel/uncore.h
>> index 6d44b7e..f65fb73 100644
>> --- a/arch/x86/events/intel/uncore.h
>> +++ b/arch/x86/events/intel/uncore.h
>> @@ -560,6 +560,7 @@ struct event_constraint *
>> uncore_get_constraint(struct intel_uncore_box *box, struct perf_event *event);
>> void uncore_put_constraint(struct intel_uncore_box *box, struct perf_event *event);
>> u64 uncore_shared_reg_config(struct intel_uncore_box *box, int idx);
>> +void uncore_get_alias_name(char *pmu_name, struct intel_uncore_pmu *pmu);
>>
>> extern struct intel_uncore_type *empty_uncore[];
>> extern struct intel_uncore_type **uncore_msr_uncores;
>> diff --git a/arch/x86/events/intel/uncore_snbep.c b/arch/x86/events/intel/uncore_snbep.c
>> index 44f2469..fc09ca4 100644
>> --- a/arch/x86/events/intel/uncore_snbep.c
>> +++ b/arch/x86/events/intel/uncore_snbep.c
>> @@ -5429,6 +5429,33 @@ static const struct attribute_group spr_uncore_chabox_format_group = {
>> .attrs = spr_uncore_cha_formats_attr,
>> };
>>
>> +static ssize_t alias_show(struct device *dev,
>> + struct device_attribute *attr,
>> + char *buf)
>> +{
>> + struct intel_uncore_pmu *pmu = dev_to_uncore_pmu(dev);
>> + char pmu_name[UNCORE_PMU_NAME_LEN];
>> +
>> + uncore_get_alias_name(pmu_name, pmu);
>> + return snprintf(buf, PAGE_SIZE, "%s\n", pmu_name);
>
> Please use sysfs_emit() for new sysfs attributes.
>
Sure.
- return snprintf(buf, PAGE_SIZE, "%s\n", pmu_name);
+ return sysfs_emit(buf, "%s\n", pmu_name);
>> +}
>> +
>> +static DEVICE_ATTR_RO(alias);
>> +
>> +static struct attribute *uncore_alias_name_attrs[] = {
>> + &dev_attr_alias.attr,
>> + NULL
>> +};
>> +
>> +static struct attribute_group uncore_alias_name = {
>> + .attrs = uncore_alias_name_attrs,
>> +};
>> +
>> +static const struct attribute_group *spr_uncore_attr_update[] = {
>> + &uncore_alias_name,
>> + NULL,
>> +};
>
> ATTRIBUTE_GROUPS()?
>
Yes, I think we can use it.
I will change it in V3.
Thanks,
Kan
Powered by blists - more mailing lists