linux-kernel - Re: [PATCH] driver: perf: arm_pmu: Drop some unused arguments from armv8_pmu

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <896fc51e-2c74-29f7-2c7e-f14f29c401a4@arm.com>
Date:   Mon, 9 Oct 2023 10:17:59 +0100
From:   James Clark <james.clark@....com>
To:     Anshuman Khandual <anshuman.khandual@....com>,
        linux-arm-kernel@...ts.infradead.org,
        Robin Murphy <Robin.Murphy@....com>
Cc:     Will Deacon <will@...nel.org>, Mark Rutland <mark.rutland@....com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] driver: perf: arm_pmu: Drop some unused arguments from
 armv8_pmu_init()



On 09/10/2023 04:56, Anshuman Khandual wrote:
> There is just a single call site remaining for armv8_pmu_init(), passing on
> NULL pointers for all custom 'struct attribute_group'. These arguments are
> not really getting used and hence can just be dropped off, thus simplifying
> the code further.
> 
> Cc: Will Deacon <will@...nel.org>
> Cc: Mark Rutland <mark.rutland@....com>
> Cc: linux-arm-kernel@...ts.infradead.org
> Cc: linux-kernel@...r.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@....com>
> ---
> This applies on v6.6-rc5.
> 
>  drivers/perf/arm_pmuv3.c | 17 +++++------------
>  1 file changed, 5 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/perf/arm_pmuv3.c b/drivers/perf/arm_pmuv3.c
> index 8fcaa26f0f8a..fe4db1831662 100644
> --- a/drivers/perf/arm_pmuv3.c
> +++ b/drivers/perf/arm_pmuv3.c
> @@ -1187,10 +1187,7 @@ static void armv8_pmu_register_sysctl_table(void)
>  }
>  
>  static int armv8_pmu_init(struct arm_pmu *cpu_pmu, char *name,
> -			  int (*map_event)(struct perf_event *event),
> -			  const struct attribute_group *events,
> -			  const struct attribute_group *format,
> -			  const struct attribute_group *caps)
> +			  int (*map_event)(struct perf_event *event))
>  {
>  	int ret = armv8pmu_probe_pmu(cpu_pmu);
>  	if (ret)
> @@ -1212,13 +1209,9 @@ static int armv8_pmu_init(struct arm_pmu *cpu_pmu, char *name,
>  
>  	cpu_pmu->name			= name;
>  	cpu_pmu->map_event		= map_event;
> -	cpu_pmu->attr_groups[ARMPMU_ATTR_GROUP_EVENTS] = events ?
> -			events : &armv8_pmuv3_events_attr_group;
> -	cpu_pmu->attr_groups[ARMPMU_ATTR_GROUP_FORMATS] = format ?
> -			format : &armv8_pmuv3_format_attr_group;
> -	cpu_pmu->attr_groups[ARMPMU_ATTR_GROUP_CAPS] = caps ?
> -			caps : &armv8_pmuv3_caps_attr_group;
> -
> +	cpu_pmu->attr_groups[ARMPMU_ATTR_GROUP_EVENTS] = &armv8_pmuv3_events_attr_group;
> +	cpu_pmu->attr_groups[ARMPMU_ATTR_GROUP_FORMATS] = &armv8_pmuv3_format_attr_group;
> +	cpu_pmu->attr_groups[ARMPMU_ATTR_GROUP_CAPS] = &armv8_pmuv3_caps_attr_group;
>  	armv8_pmu_register_sysctl_table();
>  	return 0;
>  }
> @@ -1226,7 +1219,7 @@ static int armv8_pmu_init(struct arm_pmu *cpu_pmu, char *name,
>  static int armv8_pmu_init_nogroups(struct arm_pmu *cpu_pmu, char *name,
>  				   int (*map_event)(struct perf_event *event))
>  {
> -	return armv8_pmu_init(cpu_pmu, name, map_event, NULL, NULL, NULL);
> +	return armv8_pmu_init(cpu_pmu, name, map_event);

I think the whole point of the nogroups wrapper was to add the NULLs. If
you remove them, then you can remove the nogroups function too and just
call armv8_pmu_init() directly instead.

And as it wasn't clear why they were there in the first place, I went to
look and found this (e424b17) :

  Although nobody uses non-default sysfs attributes today, there's
  minimal impact to preserving the notion that maybe, some day, somebody
  might, so we may as well keep up appearances.

It might be worth mentioning that the decision has now been made in the
other way.


>  }
>  
>  #define PMUV3_INIT_SIMPLE(name)						\