Messages in this thread | | | Subject | Re: [PATCH v2 2/3] perf x86: Add compaction function for uncore attributes | From | "Sudarikov, Roman" <> | Date | Tue, 10 Dec 2019 21:32:24 +0300 |
| |
On 10.12.2019 13:37, Peter Zijlstra wrote: > On Tue, Dec 10, 2019 at 12:14:50PM +0300, roman.sudarikov@linux.intel.com wrote: >> From: Roman Sudarikov <roman.sudarikov@linux.intel.com> >> >> 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@intel.com> >> Signed-off-by: Alexander Antonov <alexander.antonov@intel.com> >> Signed-off-by: Roman Sudarikov <roman.sudarikov@linux.intel.com> >> --- >> 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")
Hi Peter,
if I understand correctly, that commit is intended for graceful handling of cases where we need to probe hardware first before making decision whether to show or not that particular event for that particular pmu. What I'm doing has slightly different context - I'm creating the mapping per pmu type. That mapping is not hardware dependent but implementation dependent meaning that if the mapping is not implemented for some pmu type, then the mapping attribute has no way to show up following current implementation, right? If the mapping is implemented then it shows up for all pmus of that type.
I can rework it following the approach implemented in the commit you mentioned but the benefit is not immediately obvious :-)
Thanks, Roman
| |