* [PATCH] perf/smmuv3: Remove the leftover put_cpu() in error path @ 2019-12-10 10:46 Hanjun Guo 2019-12-10 11:04 ` Robin Murphy 2019-12-10 13:24 ` Will Deacon 0 siblings, 2 replies; 6+ messages in thread From: Hanjun Guo @ 2019-12-10 10:46 UTC (permalink / raw) To: Mark Rutland, Will Deacon Cc: Robin Murphy, Shameer Kolothum, linux-arm-kernel, linux-kernel, Hanjun Guo In smmu_pmu_probe(), there is put_cpu() in the error path, which is wrong because we use raw_smp_processor_id() to get the cpu ID, not get_cpu(), remove it. Signed-off-by: Hanjun Guo <guohanjun@huawei.com> --- drivers/perf/arm_smmuv3_pmu.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c index 773128f..fd1d46a 100644 --- a/drivers/perf/arm_smmuv3_pmu.c +++ b/drivers/perf/arm_smmuv3_pmu.c @@ -834,7 +834,6 @@ static int smmu_pmu_probe(struct platform_device *pdev) out_unregister: cpuhp_state_remove_instance_nocalls(cpuhp_state_num, &smmu_pmu->node); out_cpuhp_err: - put_cpu(); return err; } -- 1.7.12.4 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] perf/smmuv3: Remove the leftover put_cpu() in error path 2019-12-10 10:46 [PATCH] perf/smmuv3: Remove the leftover put_cpu() in error path Hanjun Guo @ 2019-12-10 11:04 ` Robin Murphy 2019-12-10 13:24 ` Will Deacon 1 sibling, 0 replies; 6+ messages in thread From: Robin Murphy @ 2019-12-10 11:04 UTC (permalink / raw) To: Hanjun Guo, Mark Rutland, Will Deacon Cc: Shameer Kolothum, linux-arm-kernel, linux-kernel On 10/12/2019 10:46 am, Hanjun Guo wrote: > In smmu_pmu_probe(), there is put_cpu() in the error path, > which is wrong because we use raw_smp_processor_id() to > get the cpu ID, not get_cpu(), remove it. Bah, somehow that slipped through the last round of review :) Acked-by: Robin Murphy <robin.murphy@arm.com> > Signed-off-by: Hanjun Guo <guohanjun@huawei.com> > --- > drivers/perf/arm_smmuv3_pmu.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c > index 773128f..fd1d46a 100644 > --- a/drivers/perf/arm_smmuv3_pmu.c > +++ b/drivers/perf/arm_smmuv3_pmu.c > @@ -834,7 +834,6 @@ static int smmu_pmu_probe(struct platform_device *pdev) > out_unregister: > cpuhp_state_remove_instance_nocalls(cpuhp_state_num, &smmu_pmu->node); > out_cpuhp_err: > - put_cpu(); > return err; > } > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] perf/smmuv3: Remove the leftover put_cpu() in error path 2019-12-10 10:46 [PATCH] perf/smmuv3: Remove the leftover put_cpu() in error path Hanjun Guo 2019-12-10 11:04 ` Robin Murphy @ 2019-12-10 13:24 ` Will Deacon 2019-12-10 13:55 ` Hanjun Guo 1 sibling, 1 reply; 6+ messages in thread From: Will Deacon @ 2019-12-10 13:24 UTC (permalink / raw) To: Hanjun Guo Cc: Mark Rutland, Robin Murphy, Shameer Kolothum, linux-arm-kernel, linux-kernel On Tue, Dec 10, 2019 at 06:46:24PM +0800, Hanjun Guo wrote: > In smmu_pmu_probe(), there is put_cpu() in the error path, > which is wrong because we use raw_smp_processor_id() to > get the cpu ID, not get_cpu(), remove it. > > Signed-off-by: Hanjun Guo <guohanjun@huawei.com> > --- > drivers/perf/arm_smmuv3_pmu.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c > index 773128f..fd1d46a 100644 > --- a/drivers/perf/arm_smmuv3_pmu.c > +++ b/drivers/perf/arm_smmuv3_pmu.c > @@ -834,7 +834,6 @@ static int smmu_pmu_probe(struct platform_device *pdev) > out_unregister: > cpuhp_state_remove_instance_nocalls(cpuhp_state_num, &smmu_pmu->node); > out_cpuhp_err: > - put_cpu(); > return err; Can we kill 'out_cpuhp_err' altogether then and just return err if we fail to add the hotplug instance? Will ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] perf/smmuv3: Remove the leftover put_cpu() in error path 2019-12-10 13:24 ` Will Deacon @ 2019-12-10 13:55 ` Hanjun Guo 2019-12-10 14:10 ` Will Deacon 0 siblings, 1 reply; 6+ messages in thread From: Hanjun Guo @ 2019-12-10 13:55 UTC (permalink / raw) To: Will Deacon Cc: Mark Rutland, Robin Murphy, Shameer Kolothum, linux-arm-kernel, linux-kernel On 2019/12/10 21:24, Will Deacon wrote: > On Tue, Dec 10, 2019 at 06:46:24PM +0800, Hanjun Guo wrote: >> In smmu_pmu_probe(), there is put_cpu() in the error path, >> which is wrong because we use raw_smp_processor_id() to >> get the cpu ID, not get_cpu(), remove it. >> >> Signed-off-by: Hanjun Guo <guohanjun@huawei.com> >> --- >> drivers/perf/arm_smmuv3_pmu.c | 1 - >> 1 file changed, 1 deletion(-) >> >> diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c >> index 773128f..fd1d46a 100644 >> --- a/drivers/perf/arm_smmuv3_pmu.c >> +++ b/drivers/perf/arm_smmuv3_pmu.c >> @@ -834,7 +834,6 @@ static int smmu_pmu_probe(struct platform_device *pdev) >> out_unregister: >> cpuhp_state_remove_instance_nocalls(cpuhp_state_num, &smmu_pmu->node); >> out_cpuhp_err: >> - put_cpu(); >> return err; > > Can we kill 'out_cpuhp_err' altogether then and just return err if we fail > to add the hotplug instance? Makes sense, but I think we can go further to kill both 'out_cpuhp_err' and 'out_register' as below [1], what do you think? Thanks Hanjun [1]: diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c index fd1d46a..a5adaba 100644 --- a/drivers/perf/arm_smmuv3_pmu.c +++ b/drivers/perf/arm_smmuv3_pmu.c @@ -814,14 +814,15 @@ static int smmu_pmu_probe(struct platform_device *pdev) if (err) { dev_err(dev, "Error %d registering hotplug, PMU @%pa\n", err, &res_0->start); - goto out_cpuhp_err; + return err; } err = perf_pmu_register(&smmu_pmu->pmu, name, -1); if (err) { + cpuhp_state_remove_instance_nocalls(cpuhp_state_num, &smmu_pmu->node); dev_err(dev, "Error %d registering PMU @%pa\n", err, &res_0->start); - goto out_unregister; + return err; } dev_info(dev, "Registered PMU @ %pa using %d counters with %s filter settings\n", @@ -830,11 +831,6 @@ static int smmu_pmu_probe(struct platform_device *pdev) "Individual"); return 0; - -out_unregister: - cpuhp_state_remove_instance_nocalls(cpuhp_state_num, &smmu_pmu->node); -out_cpuhp_err: - return err; } static int smmu_pmu_remove(struct platform_device *pdev) ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] perf/smmuv3: Remove the leftover put_cpu() in error path 2019-12-10 13:55 ` Hanjun Guo @ 2019-12-10 14:10 ` Will Deacon 2019-12-11 6:22 ` Hanjun Guo 0 siblings, 1 reply; 6+ messages in thread From: Will Deacon @ 2019-12-10 14:10 UTC (permalink / raw) To: Hanjun Guo Cc: Mark Rutland, Robin Murphy, Shameer Kolothum, linux-arm-kernel, linux-kernel On Tue, Dec 10, 2019 at 09:55:28PM +0800, Hanjun Guo wrote: > On 2019/12/10 21:24, Will Deacon wrote: > > On Tue, Dec 10, 2019 at 06:46:24PM +0800, Hanjun Guo wrote: > >> In smmu_pmu_probe(), there is put_cpu() in the error path, > >> which is wrong because we use raw_smp_processor_id() to > >> get the cpu ID, not get_cpu(), remove it. > >> > >> Signed-off-by: Hanjun Guo <guohanjun@huawei.com> > >> --- > >> drivers/perf/arm_smmuv3_pmu.c | 1 - > >> 1 file changed, 1 deletion(-) > >> > >> diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c > >> index 773128f..fd1d46a 100644 > >> --- a/drivers/perf/arm_smmuv3_pmu.c > >> +++ b/drivers/perf/arm_smmuv3_pmu.c > >> @@ -834,7 +834,6 @@ static int smmu_pmu_probe(struct platform_device *pdev) > >> out_unregister: > >> cpuhp_state_remove_instance_nocalls(cpuhp_state_num, &smmu_pmu->node); > >> out_cpuhp_err: > >> - put_cpu(); > >> return err; > > > > Can we kill 'out_cpuhp_err' altogether then and just return err if we fail > > to add the hotplug instance? > > Makes sense, but I think we can go further to kill both 'out_cpuhp_err' and > 'out_register' as below [1], what do you think? Although that's functionally correct, I'd prefer to keep out_unregister(), since it acts as good reminder to anybody extending this function in future that they need to unregister the hotplug instance on failure. Will ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] perf/smmuv3: Remove the leftover put_cpu() in error path 2019-12-10 14:10 ` Will Deacon @ 2019-12-11 6:22 ` Hanjun Guo 0 siblings, 0 replies; 6+ messages in thread From: Hanjun Guo @ 2019-12-11 6:22 UTC (permalink / raw) To: Will Deacon Cc: Mark Rutland, Robin Murphy, Shameer Kolothum, linux-arm-kernel, linux-kernel On 2019/12/10 22:10, Will Deacon wrote: > On Tue, Dec 10, 2019 at 09:55:28PM +0800, Hanjun Guo wrote: >> On 2019/12/10 21:24, Will Deacon wrote: >>> On Tue, Dec 10, 2019 at 06:46:24PM +0800, Hanjun Guo wrote: >>>> In smmu_pmu_probe(), there is put_cpu() in the error path, >>>> which is wrong because we use raw_smp_processor_id() to >>>> get the cpu ID, not get_cpu(), remove it. >>>> >>>> Signed-off-by: Hanjun Guo <guohanjun@huawei.com> >>>> --- >>>> drivers/perf/arm_smmuv3_pmu.c | 1 - >>>> 1 file changed, 1 deletion(-) >>>> >>>> diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c >>>> index 773128f..fd1d46a 100644 >>>> --- a/drivers/perf/arm_smmuv3_pmu.c >>>> +++ b/drivers/perf/arm_smmuv3_pmu.c >>>> @@ -834,7 +834,6 @@ static int smmu_pmu_probe(struct platform_device *pdev) >>>> out_unregister: >>>> cpuhp_state_remove_instance_nocalls(cpuhp_state_num, &smmu_pmu->node); >>>> out_cpuhp_err: >>>> - put_cpu(); >>>> return err; >>> >>> Can we kill 'out_cpuhp_err' altogether then and just return err if we fail >>> to add the hotplug instance? >> >> Makes sense, but I think we can go further to kill both 'out_cpuhp_err' and >> 'out_register' as below [1], what do you think? > > Although that's functionally correct, I'd prefer to keep out_unregister(), > since it acts as good reminder to anybody extending this function in future > that they need to unregister the hotplug instance on failure. OK, I will add Robin's ACK and resend. Thanks Hanjun ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-12-11 6:22 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-12-10 10:46 [PATCH] perf/smmuv3: Remove the leftover put_cpu() in error path Hanjun Guo 2019-12-10 11:04 ` Robin Murphy 2019-12-10 13:24 ` Will Deacon 2019-12-10 13:55 ` Hanjun Guo 2019-12-10 14:10 ` Will Deacon 2019-12-11 6:22 ` Hanjun Guo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®