Re: [PATCH bpf-next] selftests/bpf: Skip test when perf_event_open returns EOPNOTSUPP

[Yonghong Song <yonghong.song@linux.dev>]

On 4/1/24 7:22 PM, Pu Lehui wrote:
>
>
> On 2024/4/1 23:29, Yonghong Song wrote:
>>
>> On 4/1/24 5:33 AM, Pu Lehui wrote:
>>> From: Pu Lehui <pulehui@huawei.com>
>>>
>>> For the bpf selftest, the semantics of perf_event_open returning ENOENT
>>> and EOPNOTSUPP imply that continuing the test is meaningless. Let’s 
>>> skip
>>> test when perf_event_open returns EOPNOTSUPP, which has already been
>>> skipped in other test cases.
>>
>> Could you explain when EOPNOTSUPP is returned for these two tests?
>> Is this riscv specific? If the EOPNOTSUPP is returned due to missing
>> config in tools/testing/selftests/bpf/config, we should add that to
>> ensure the test can execute properly.
>>
>
> Hello Yonghong, I tested on riscv but it is not unique to riscv. When 
> the perf driver does not support sampling events, perf_event_open will 
> return EOPNOTSUPP, that is, PERF_PMU_CAP_NO_INTERRUPT is set to the 
> pmu capabilities. This is no problem with riscv with sscofpmf 
> extension and sbi pmu driver. I found that PERF_PMU_CAP_NO_INTERRUPT 
> is set not only in the riscv-related perf driver. At the same time, it 
> is possible to return EOPNOTSUPP about PERF_PMU_CAP_AUX_OUTPUT on the 
> path of perf_event_open. I think it resonable to skip such use cases 
> for functions not supported by non-bpf modules, what do you think?

Thanks for explanation. I checked kernel/events/core.c and arch/x86/.... I agree that if hardware has capabilities PERF_PMU_CAP_NO_INTERRUPT
or PERF_PMU_CAP_AUX_OUTPUT, EOPNOTSUPP could be returned, although I think this should be extremely rare for x86 cpu's (probably only really old ones).
Several other arch's also have hardware which has capability PERF_PMU_CAP_NO_INTERRUPT, so may indeed hit this issue.

In bpf-next/arch/riscv, I didn't find usage of PERF_PMU_CAP_NO_INTERRUPT though.

Your patch looks good to me. Since you hit the issue with riscv so please describe how EOPNOTSUPP could be returned in your test. It would be a good
justification for your patch.

>
>>>
>>> Signed-off-by: Pu Lehui <pulehui@huawei.com>
>>> ---
>>> tools/testing/selftests/bpf/prog_tests/send_signal.c | 2 +-
>>> .../testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c | 2 +-
>>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/bpf/prog_tests/send_signal.c 
>>> b/tools/testing/selftests/bpf/prog_tests/send_signal.c
>>> index b15b343ebb6b..920aee41bd58 100644
>>> --- a/tools/testing/selftests/bpf/prog_tests/send_signal.c
>>> +++ b/tools/testing/selftests/bpf/prog_tests/send_signal.c
>>> @@ -179,7 +179,7 @@ static void test_send_signal_nmi(bool 
>>> signal_thread)
>>>       pmu_fd = syscall(__NR_perf_event_open, &attr, 0 /* pid */,
>>>                -1 /* cpu */, -1 /* group_fd */, 0 /* flags */);
>>>       if (pmu_fd == -1) {
>>> -        if (errno == ENOENT) {
>>> +        if (errno == ENOENT || errno == EOPNOTSUPP) {
>>>               printf("%s:SKIP:no PERF_COUNT_HW_CPU_CYCLES\n",
>>>                      __func__);
>>>               test__skip();
>>> diff --git 
>>> a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c 
>>> b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
>>> index 5db9eec24b5b..0832fd787457 100644
>>> --- a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
>>> +++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
>>> @@ -35,7 +35,7 @@ void test_stacktrace_build_id_nmi(void)
>>>       pmu_fd = syscall(__NR_perf_event_open, &attr, -1 /* pid */,
>>>                0 /* cpu 0 */, -1 /* group id */,
>>>                0 /* flags */);
>>> -    if (pmu_fd < 0 && errno == ENOENT) {
>>> +    if (pmu_fd < 0 && (errno == ENOENT || errno == EOPNOTSUPP)) {
>>>           printf("%s:SKIP:no PERF_COUNT_HW_CPU_CYCLES\n", __func__);
>>>           test__skip();
>>>           goto cleanup;
>