diff mbox series

[2/5] perf/arm_cspmu: Simplify attribute groups

Message ID f75cacb440011df35fc476cf4be37484fd044b92.1701793996.git.robin.murphy@arm.com (mailing list archive)
State New, archived
Headers show
Series perf/arm_cspmu: Add devicetree support | expand

Commit Message

Robin Murphy Dec. 5, 2023, 4:51 p.m. UTC 
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@arm.com>
---
 drivers/perf/arm_cspmu/arm_cspmu.c | 26 +++++++++-----------------
 drivers/perf/arm_cspmu/arm_cspmu.h |  1 +
 2 files changed, 10 insertions(+), 17 deletions(-)

Comments

Ilkka Koskinen Dec. 7, 2023, 2:47 a.m. UTC | #1 
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@arm.com>

Reviewed-by: Ilkka Koskinen <ilkka@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
>
>
diff mbox series

Patch

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;
 };