From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6DFE6CD80B0 for ; Tue, 10 Oct 2023 11:59:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231538AbjJJL7g (ORCPT ); Tue, 10 Oct 2023 07:59:36 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43988 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230122AbjJJL7f (ORCPT ); Tue, 10 Oct 2023 07:59:35 -0400 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 66AB094 for ; Tue, 10 Oct 2023 04:59:33 -0700 (PDT) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id B17CA1FB; Tue, 10 Oct 2023 05:00:13 -0700 (PDT) Received: from [10.1.196.40] (e121345-lin.cambridge.arm.com [10.1.196.40]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 399393F7A6; Tue, 10 Oct 2023 04:59:32 -0700 (PDT) Message-ID: Date: Tue, 10 Oct 2023 12:59:30 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] driver: perf: arm_pmu: Drop some unused arguments from armv8_pmu_init() Content-Language: en-GB To: James Clark , Anshuman Khandual , linux-arm-kernel@lists.infradead.org Cc: Will Deacon , Mark Rutland , linux-kernel@vger.kernel.org References: <20231009035638.165270-1-anshuman.khandual@arm.com> <896fc51e-2c74-29f7-2c7e-f14f29c401a4@arm.com> From: Robin Murphy In-Reply-To: <896fc51e-2c74-29f7-2c7e-f14f29c401a4@arm.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 09/10/2023 10:17 am, James Clark wrote: > > > 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 >> Cc: Mark Rutland >> Cc: linux-arm-kernel@lists.infradead.org >> Cc: linux-kernel@vger.kernel.org >> Signed-off-by: Anshuman Khandual >> --- >> 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. Indeed the "nogroups" wrapper is entirely meaningless if the callee no longer accepts groups anyway. > 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. Right, the intent at the time was very much just a cosmetic cleanup to help readability and simplify adding new PMU names, and rather deliberately stepping around the question of making material changes to the interface itself. If we've reached the point where we're happy to agree that consistency with the PMUv2 code is no longer helpful to continuing development of the PMUv3 code, then *that* would be the fundamental point of this change. Thanks, Robin.