[PATCH] arch/s390/perf: Register cpumf_pmu with type = PERF_TYPE_RAW

[PATCH] arch/s390/perf: Register cpumf_pmu with type = PERF_TYPE_RAW

[JiaLong.Yang]

The struct pmu cpumf_pmu has handled generic events. So it need some
flags to tell core this thing.

Signed-off-by: JiaLong.Yang <jialong.yang@shingroup.cn>
---
 arch/s390/kernel/perf_cpum_cf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/s390/kernel/perf_cpum_cf.c b/arch/s390/kernel/perf_cpum_cf.c
index 41ed6e0f0a2a..6ba36cf50091 100644
--- a/arch/s390/kernel/perf_cpum_cf.c
+++ b/arch/s390/kernel/perf_cpum_cf.c
@@ -1213,7 +1213,7 @@ static int __init cpumf_pmu_init(void)
 	debug_register_view(cf_dbg, &debug_sprintf_view);
 
 	cpumf_pmu.attr_groups = cpumf_cf_event_group();
-	rc = perf_pmu_register(&cpumf_pmu, "cpum_cf", -1);
+	rc = perf_pmu_register(&cpumf_pmu, "cpum_cf", PERF_TYPE_RAW);
 	if (rc) {
 		pr_err("Registering the cpum_cf PMU failed with rc=%i\n", rc);
 		goto out2;
-- 
2.25.1

Re: [PATCH] arch/s390/perf: Register cpumf_pmu with type = PERF_TYPE_RAW

[Mark Rutland]

On Mon, Mar 04, 2024 at 10:27:01AM +0800, JiaLong.Yang wrote:
> The struct pmu cpumf_pmu has handled generic events. So it need some
> flags to tell core this thing.

It's not necessary to register as PERF_TYPE_RAW in order to handle raw events,
and PERF_TYPE_RAW is not a flag.

Have you encountered a functional problem, or was this found by inspection?

Mark.

> Signed-off-by: JiaLong.Yang <jialong.yang@shingroup.cn>
> ---
>  arch/s390/kernel/perf_cpum_cf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/s390/kernel/perf_cpum_cf.c b/arch/s390/kernel/perf_cpum_cf.c
> index 41ed6e0f0a2a..6ba36cf50091 100644
> --- a/arch/s390/kernel/perf_cpum_cf.c
> +++ b/arch/s390/kernel/perf_cpum_cf.c
> @@ -1213,7 +1213,7 @@ static int __init cpumf_pmu_init(void)
>  	debug_register_view(cf_dbg, &debug_sprintf_view);
>  
>  	cpumf_pmu.attr_groups = cpumf_cf_event_group();
> -	rc = perf_pmu_register(&cpumf_pmu, "cpum_cf", -1);
> +	rc = perf_pmu_register(&cpumf_pmu, "cpum_cf", PERF_TYPE_RAW);
>  	if (rc) {
>  		pr_err("Registering the cpum_cf PMU failed with rc=%i\n", rc);
>  		goto out2;
> -- 
> 2.25.1
> 
> 

Re: [PATCH] arch/s390/perf: Register cpumf_pmu with type = PERF_TYPE_RAW

[Yang Jialong 杨佳龙]

在 2024/3/4 17:34, Mark Rutland 写道:
> On Mon, Mar 04, 2024 at 10:27:01AM +0800, JiaLong.Yang wrote:
>> The struct pmu cpumf_pmu has handled generic events. So it need some
>> flags to tell core this thing.
> 
> It's not necessary to register as PERF_TYPE_RAW in order to handle raw events,
> and PERF_TYPE_RAW is not a flag.
> 
> Have you encountered a functional problem, or was this found by inspection?
> 

As you expected, I'm trying to confirm which one pmu has the capability 
to handle generic events in registering pmus instead of test generic 
events in each pmus when opening.
We can confirm that before using. We have pay more in handling them when 
opening.
So most driver developers use PERF_TYPE_RAW. x86 and arm use 
PERF_PMU_CAP_EXTENDED_HW_TYPE. Others use struct pmu::task_ctx_nr = 
perf_hw_context.
I think PERF_TYPE_RAW will be a easily accepted way. So ...

How do you think about this?

> Mark.
> 
>> Signed-off-by: JiaLong.Yang <jialong.yang@shingroup.cn>
>> ---
>>   arch/s390/kernel/perf_cpum_cf.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/s390/kernel/perf_cpum_cf.c b/arch/s390/kernel/perf_cpum_cf.c
>> index 41ed6e0f0a2a..6ba36cf50091 100644
>> --- a/arch/s390/kernel/perf_cpum_cf.c
>> +++ b/arch/s390/kernel/perf_cpum_cf.c
>> @@ -1213,7 +1213,7 @@ static int __init cpumf_pmu_init(void)
>>   	debug_register_view(cf_dbg, &debug_sprintf_view);
>>   
>>   	cpumf_pmu.attr_groups = cpumf_cf_event_group();
>> -	rc = perf_pmu_register(&cpumf_pmu, "cpum_cf", -1);
>> +	rc = perf_pmu_register(&cpumf_pmu, "cpum_cf", PERF_TYPE_RAW);
>>   	if (rc) {
>>   		pr_err("Registering the cpum_cf PMU failed with rc=%i\n", rc);
>>   		goto out2;
>> -- 
>> 2.25.1
>>
>>
> 

Re: [PATCH] arch/s390/perf: Register cpumf_pmu with type = PERF_TYPE_RAW

[Mark Rutland]

On Tue, Mar 05, 2024 at 09:43:16AM +0800, Yang Jialong 杨佳龙 wrote:
> 在 2024/3/4 17:34, Mark Rutland 写道:
> > On Mon, Mar 04, 2024 at 10:27:01AM +0800, JiaLong.Yang wrote:
> > > The struct pmu cpumf_pmu has handled generic events. So it need some
> > > flags to tell core this thing.
> > 
> > It's not necessary to register as PERF_TYPE_RAW in order to handle raw events,
> > and PERF_TYPE_RAW is not a flag.
> > 
> > Have you encountered a functional problem, or was this found by inspection?
> 
> As you expected, I'm trying to confirm which one pmu has the capability to
> handle generic events in registering pmus instead of test generic events in
> each pmus when opening.

If we want to do that, then we need a new flag on struct pmu to restrict which
events we try to open on a PMU.

If you want to do that, you need to Cc the perf maintainers and discuss that
rather than point-hacking individual drivers.

> We can confirm that before using. We have pay more in handling them when
> opening.
> So most driver developers use PERF_TYPE_RAW. x86 and arm use
> PERF_PMU_CAP_EXTENDED_HW_TYPE. Others use struct pmu::task_ctx_nr =
> perf_hw_context.
> I think PERF_TYPE_RAW will be a easily accepted way. So ...

No, this is a hack, and it doesn't solve the problem you describe above.

If we want to remove the need for most PMUs to look at perf_event_attr::type,
then we should have a new PERF_PMU_CAP_ flag on the PMU to say "this PMU
supports generic events" (or separate flags for the generic RAW/HW/CACHE
types), and update all relevant PMUs accordingly.

Please do not try to overload pmu::type with additional semantics; it's messy
enough as-is.

Mark. 

Re: [PATCH] arch/s390/perf: Register cpumf_pmu with type = PERF_TYPE_RAW

[Jialong Yang]

在 2024/3/5 20:40, Mark Rutland 写道:
> On Tue, Mar 05, 2024 at 09:43:16AM +0800, Yang Jialong 杨佳龙 wrote:
>> 在 2024/3/4 17:34, Mark Rutland 写道:
>>> On Mon, Mar 04, 2024 at 10:27:01AM +0800, JiaLong.Yang wrote:
>>>> The struct pmu cpumf_pmu has handled generic events. So it need some
>>>> flags to tell core this thing.
>>> It's not necessary to register as PERF_TYPE_RAW in order to handle raw events,
>>> and PERF_TYPE_RAW is not a flag.
>>>
>>> Have you encountered a functional problem, or was this found by inspection?
>> As you expected, I'm trying to confirm which one pmu has the capability to
>> handle generic events in registering pmus instead of test generic events in
>> each pmus when opening.
> If we want to do that, then we need a new flag on struct pmu to restrict which
> events we try to open on a PMU.
>
> If you want to do that, you need to Cc the perf maintainers and discuss that
> rather than point-hacking individual drivers.
>
>> We can confirm that before using. We have pay more in handling them when
>> opening.
>> So most driver developers use PERF_TYPE_RAW. x86 and arm use
>> PERF_PMU_CAP_EXTENDED_HW_TYPE. Others use struct pmu::task_ctx_nr =
>> perf_hw_context.
>> I think PERF_TYPE_RAW will be a easily accepted way. So ...
> No, this is a hack, and it doesn't solve the problem you describe above.
>
> If we want to remove the need for most PMUs to look at perf_event_attr::type,
> then we should have a new PERF_PMU_CAP_ flag on the PMU to say "this PMU
> supports generic events" (or separate flags for the generic RAW/HW/CACHE
> types), and update all relevant PMUs accordingly.
>
> Please do not try to overload pmu::type with additional semantics; it's messy
> enough as-is.

OK. Thanks for guiding.


>
> Mark.
>