[PATCH] s390/pai: Simplify pai_init() and always return -ENODEV on failure

[PATCH] s390/pai: Simplify pai_init() and always return -ENODEV on failure

[Thomas Richter]

Code simplification and remove variable state.
Return -ENODEV in all failures to indicate the PAI PMUs are not
available.

Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
---
 arch/s390/kernel/perf_pai.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/arch/s390/kernel/perf_pai.c b/arch/s390/kernel/perf_pai.c
index 013c3dae21ec..b6c9dca09510 100644
--- a/arch/s390/kernel/perf_pai.c
+++ b/arch/s390/kernel/perf_pai.c
@@ -1304,7 +1304,7 @@ static int pai_offline_cpu(unsigned int cpu)
 
 static int __init pai_init(void)
 {
-	int state, rc;
+	int rc;
 
 	/* Setup s390dbf facility */
 	paidbg = debug_register("pai", 1, 1, 128);
@@ -1315,23 +1315,21 @@ static int __init pai_init(void)
 	debug_register_view(paidbg, &debug_sprintf_view);
 
 	/* CPUHP_BP_PREPARE_DYN --> before CPU is brought online */
-	state = cpuhp_setup_state(CPUHP_BP_PREPARE_DYN, "perf/pai:prepare",
-				  pai_online_cpu, pai_offline_cpu);
-	rc = state < 0 ? state : 0;
+	rc = cpuhp_setup_state(CPUHP_BP_PREPARE_DYN, "perf/pai:prepare",
+			       pai_online_cpu, pai_offline_cpu);
 	if (rc < 0)
 		goto out_debug;
 
-	rc = -ENODEV;
 	if (!paipmu_setup())
 		goto out_cpuhp;
 	return 0;
 
 out_cpuhp:
-	cpuhp_remove_state(state);
+	cpuhp_remove_state(rc);
 out_debug:
 	debug_unregister_view(paidbg, &debug_sprintf_view);
 	debug_unregister(paidbg);
-	return rc;
+	return -ENODEV;
 }
 
 device_initcall(pai_init);
-- 
2.55.0

Re: [PATCH] s390/pai: Simplify pai_init() and always return -ENODEV on failure

[Sumanth Korikkar]

On Thu, Sep 03, 2026 at 10:15:57AM +0200, Thomas Richter wrote:
> Code simplification and remove variable state.
> Return -ENODEV in all failures to indicate the PAI PMUs are not
> available.
> 
> Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>

Looks good to me.

Reviewed-by: Sumanth Korikkar <sumanthk@linux.ibm.com>

Re: [PATCH] s390/pai: Simplify pai_init() and always return -ENODEV on failure

[Heiko Carstens]

On Thu, Sep 03, 2026 at 10:15:57AM +0200, Thomas Richter wrote:
> Code simplification and remove variable state.
> Return -ENODEV in all failures to indicate the PAI PMUs are not
> available.

Why do you want to hide different error / return codes? That makes
debugging in case of an error more difficult.

Re: [PATCH] s390/pai: Simplify pai_init() and always return -ENODEV on failure

[Thomas Richter]

On 9/14/26 14:14, Heiko Carstens wrote:
> On Thu, Sep 03, 2026 at 10:15:57AM +0200, Thomas Richter wrote:
>> Code simplification and remove variable state.
>> Return -ENODEV in all failures to indicate the PAI PMUs are not
>> available.
> 
> Why do you want to hide different error / return codes? That makes
> debugging in case of an error more difficult.

Well, hiding was not the intent.

The kernel common code does a lot of checks at event creation,
then calls the PMU event_init() call back to install the event.
When this fails the return code is checked and further action
depends on this return code. See

  perf_init_event()
  +--> perf_try_event_init()
       +--> PMU->event_init()

See include/linux/perf_event.h for return code meanings.

When we return the value from cpuhp_setup_state(), it must no be
-ENOENT and some other values. Because those are used for postprocessing.

Thats why I always return -ENODEV, meaning event is valid for PMU, but
PMU not operational. I thouhgt this might fit best.
  
-- 
Thomas Richter, Dept 3303, IBM s390 Linux Development, Boeblingen, Germany
--
IBM Deutschland Research & Development GmbH

Vorsitzender des Aufsichtsrats: Wolfgang Wendt

Geschäftsführung: David Faller

Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294

Re: [PATCH] s390/pai: Simplify pai_init() and always return -ENODEV on failure

[Heiko Carstens]

On Mon, Sep 21, 2026 at 01:40:21PM +0200, Thomas Richter wrote:
> On 9/14/26 14:14, Heiko Carstens wrote:
> > On Thu, Sep 03, 2026 at 10:15:57AM +0200, Thomas Richter wrote:
> >> Code simplification and remove variable state.
> >> Return -ENODEV in all failures to indicate the PAI PMUs are not
> >> available.
> > 
> > Why do you want to hide different error / return codes? That makes
> > debugging in case of an error more difficult.
> 
> Well, hiding was not the intent.
> 
> The kernel common code does a lot of checks at event creation,
> then calls the PMU event_init() call back to install the event.
> When this fails the return code is checked and further action
> depends on this return code. See
> 
>   perf_init_event()
>   +--> perf_try_event_init()
>        +--> PMU->event_init()
> 
> See include/linux/perf_event.h for return code meanings.
> 
> When we return the value from cpuhp_setup_state(), it must no be
> -ENOENT and some other values. Because those are used for postprocessing.
> 
> Thats why I always return -ENODEV, meaning event is valid for PMU, but
> PMU not operational. I thouhgt this might fit best.

But what does have pai_init() to do with anything of the above?
pai_init() is called via init_call() mechanism.

Re: [PATCH] s390/pai: Simplify pai_init() and always return -ENODEV on failure

[Thomas Richter]

On 9/21/26 15:01, Heiko Carstens wrote:
> On Mon, Sep 21, 2026 at 01:40:21PM +0200, Thomas Richter wrote:
>> On 9/14/26 14:14, Heiko Carstens wrote:
>>> On Thu, Sep 03, 2026 at 10:15:57AM +0200, Thomas Richter wrote:
>>>> Code simplification and remove variable state.
>>>> Return -ENODEV in all failures to indicate the PAI PMUs are not
>>>> available.
>>>
>>> Why do you want to hide different error / return codes? That makes
>>> debugging in case of an error more difficult.
>>
>> Well, hiding was not the intent.
>>
>> The kernel common code does a lot of checks at event creation,
>> then calls the PMU event_init() call back to install the event.
>> When this fails the return code is checked and further action
>> depends on this return code. See
>>
>>   perf_init_event()
>>   +--> perf_try_event_init()
>>        +--> PMU->event_init()
>>
>> See include/linux/perf_event.h for return code meanings.
>>
>> When we return the value from cpuhp_setup_state(), it must no be
>> -ENOENT and some other values. Because those are used for postprocessing.
>>
>> Thats why I always return -ENODEV, meaning event is valid for PMU, but
>> PMU not operational. I thouhgt this might fit best.
> 
> But what does have pai_init() to do with anything of the above?
> pai_init() is called via init_call() mechanism.


Ahh well, that is correct... lets drop it. Sorry for the noise.

-- 
Thomas Richter, Dept 3303, IBM s390 Linux Development, Boeblingen, Germany
--
IBM Deutschland Research & Development GmbH

Vorsitzender des Aufsichtsrats: Wolfgang Wendt

Geschäftsführung: David Faller

Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294