Re: [PATCH v2 2/3] perf x86: Add compaction function for uncore attributes
From: Sudarikov, Roman
Date: Wed Dec 11 2019 - 09:21:55 EST
On 10.12.2019 13:37, Peter Zijlstra wrote:
On Tue, Dec 10, 2019 at 12:14:50PM +0300, roman.sudarikov@xxxxxxxxxxxxxxx wrote:
From: Roman Sudarikov <roman.sudarikov@xxxxxxxxxxxxxxx>
In current design, there is an implicit assumption that array of pointers
to uncore type attributes is NULL terminated. However, not all attributes
are mandatory for each Uncore unit type, e.g. "events" is required for
IMC but doesn't exist for CHA. That approach correctly supports only one
optional attribute which also must be the last in the row.
The patch removes limitation by safely removing embedded NULL elements.
Co-developed-by: Alexander Antonov <alexander.antonov@xxxxxxxxx>
Signed-off-by: Alexander Antonov <alexander.antonov@xxxxxxxxx>
Signed-off-by: Roman Sudarikov <roman.sudarikov@xxxxxxxxxxxxxxx>
---
arch/x86/events/intel/uncore.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/arch/x86/events/intel/uncore.c b/arch/x86/events/intel/uncore.c
index 24e120289018..a05352c4fc01 100644
--- a/arch/x86/events/intel/uncore.c
+++ b/arch/x86/events/intel/uncore.c
@@ -923,6 +923,22 @@ static void uncore_types_exit(struct intel_uncore_type **types)
uncore_type_exit(*types);
}
+static void uncore_type_attrs_compaction(struct intel_uncore_type *type)
+{
+ int i, j;
+ int size = ARRAY_SIZE(type->attr_groups);
+
+ for (i = 0, j = 0; i < size; i++) {
+ if (!type->attr_groups[i])
+ continue;
+ if (i > j) {
+ type->attr_groups[j] = type->attr_groups[i];
+ type->attr_groups[i] = NULL;
+ }
+ j++;
+ }
+}
GregKH had objections to us playing silly games like that and made us
use is_visible() for the regular PMU driver. Also see commit:
baa0c83363c7 ("perf/x86: Use the new pmu::update_attrs attribute group")
Removed theuncore_type_attrs_compaction() function and implemented Kan's
suggestion to replace NULL events_group by the empty attributes group:
diff --git a/arch/x86/events/intel/uncore.c b/arch/x86/events/intel/uncore.c
index e8532923bd45..110b3603f56f 100644
--- a/arch/x86/events/intel/uncore.c
+++ b/arch/x86/events/intel/uncore.c
@@ -923,6 +923,14 @@ static void uncore_types_exit(struct
intel_uncore_type **types)
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ uncore_type_exit(*types);
Â}
+static struct attribute *empty_attrs[] = {
+ÂÂÂÂÂÂ NULL,
+};
+
+static const struct attribute_group empty_group = {
+ÂÂÂÂÂÂ .attrs = empty_attrs,
+};
+
Âstatic int __init uncore_type_init(struct intel_uncore_type *type,
bool setid)
Â{
ÂÂÂÂÂÂÂ struct intel_uncore_pmu *pmus;
@@ -968,7 +976,8 @@ static int __init uncore_type_init(struct
intel_uncore_type *type, bool setid)
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ attr_group->attrs[j] =
&type->event_descs[j].attr.attr;
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ type->events_group = &attr_group->group;
-ÂÂÂÂÂÂ }
+ÂÂÂÂÂÂ } else
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ type->events_group = &empty_group;
ÂÂÂÂÂÂÂ type->pmu_group = &uncore_pmu_attr_group;