[PATCH 2/5] perf/arm_cspmu: Simplify attribute groups
Ilkka Koskinen
ilkka at os.amperecomputing.com
Wed Dec 6 18:47:01 PST 2023
On Tue, 5 Dec 2023, Robin Murphy wrote:
> The attribute group array itself is always the same, so there's no
> need to allocate it separately. Storing it directly in our instance
> data saves memory and gives us one less point of failure.
>
> Signed-off-by: Robin Murphy <robin.murphy at arm.com>
Reviewed-by: Ilkka Koskinen <ilkka at os.amperecomputing.com>
Cheers, Ilkka
> ---
> drivers/perf/arm_cspmu/arm_cspmu.c | 26 +++++++++-----------------
> drivers/perf/arm_cspmu/arm_cspmu.h | 1 +
> 2 files changed, 10 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
> index a3347b1287e6..f7aa2ac5fd88 100644
> --- a/drivers/perf/arm_cspmu/arm_cspmu.c
> +++ b/drivers/perf/arm_cspmu/arm_cspmu.c
> @@ -501,23 +501,16 @@ arm_cspmu_alloc_format_attr_group(struct arm_cspmu *cspmu)
> return format_group;
> }
>
> -static struct attribute_group **
> -arm_cspmu_alloc_attr_group(struct arm_cspmu *cspmu)
> +static int arm_cspmu_alloc_attr_groups(struct arm_cspmu *cspmu)
> {
> - struct attribute_group **attr_groups = NULL;
> - struct device *dev = cspmu->dev;
> + const struct attribute_group **attr_groups = cspmu->attr_groups;
> const struct arm_cspmu_impl_ops *impl_ops = &cspmu->impl.ops;
>
> cspmu->identifier = impl_ops->get_identifier(cspmu);
> cspmu->name = impl_ops->get_name(cspmu);
>
> if (!cspmu->identifier || !cspmu->name)
> - return NULL;
> -
> - attr_groups = devm_kcalloc(dev, 5, sizeof(struct attribute_group *),
> - GFP_KERNEL);
> - if (!attr_groups)
> - return NULL;
> + return -ENOMEM;
>
> attr_groups[0] = arm_cspmu_alloc_event_attr_group(cspmu);
> attr_groups[1] = arm_cspmu_alloc_format_attr_group(cspmu);
> @@ -525,9 +518,9 @@ arm_cspmu_alloc_attr_group(struct arm_cspmu *cspmu)
> attr_groups[3] = &arm_cspmu_cpumask_attr_group;
>
> if (!attr_groups[0] || !attr_groups[1])
> - return NULL;
> + return -ENOMEM;
>
> - return attr_groups;
> + return 0;
> }
>
> static inline void arm_cspmu_reset_counters(struct arm_cspmu *cspmu)
> @@ -1166,11 +1159,10 @@ static int arm_cspmu_get_cpus(struct arm_cspmu *cspmu)
> static int arm_cspmu_register_pmu(struct arm_cspmu *cspmu)
> {
> int ret, capabilities;
> - struct attribute_group **attr_groups;
>
> - attr_groups = arm_cspmu_alloc_attr_group(cspmu);
> - if (!attr_groups)
> - return -ENOMEM;
> + ret = arm_cspmu_alloc_attr_groups(cspmu);
> + if (ret)
> + return ret;
>
> ret = cpuhp_state_add_instance(arm_cspmu_cpuhp_state,
> &cspmu->cpuhp_node);
> @@ -1192,7 +1184,7 @@ static int arm_cspmu_register_pmu(struct arm_cspmu *cspmu)
> .start = arm_cspmu_start,
> .stop = arm_cspmu_stop,
> .read = arm_cspmu_read,
> - .attr_groups = (const struct attribute_group **)attr_groups,
> + .attr_groups = cspmu->attr_groups,
> .capabilities = capabilities,
> };
>
> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.h b/drivers/perf/arm_cspmu/arm_cspmu.h
> index 2fe723555a6b..c9163acfe810 100644
> --- a/drivers/perf/arm_cspmu/arm_cspmu.h
> +++ b/drivers/perf/arm_cspmu/arm_cspmu.h
> @@ -157,6 +157,7 @@ struct arm_cspmu {
> int cycle_counter_logical_idx;
>
> struct arm_cspmu_hw_events hw_events;
> + const struct attribute_group *attr_groups[5];
>
> struct arm_cspmu_impl impl;
> };
> --
> 2.39.2.101.g768bb238c484.dirty
>
>
More information about the linux-arm-kernel
mailing list