2023-02-22 11:55:32

by Tiezhu Yang

[permalink] [raw]
Subject: [PATCH bpf-next 0/3] Fix some build errors for bpf selftest on LoongArch

Tiezhu Yang (3):
libbpf: Use struct user_pt_regs to define __PT_REGS_CAST() for
LoongArch
selftests/bpf: Check __TARGET_ARCH_loongarch if target is bpf for
LoongArch
selftests/bpf: Check __ARCH_WANT_SET_GET_RLIMIT before
syscall(__NR_getrlimit)

tools/include/uapi/asm/bitsperlong.h | 2 +-
tools/lib/bpf/bpf_tracing.h | 2 ++
tools/testing/selftests/bpf/prog_tests/user_ringbuf.c | 2 ++
3 files changed, 5 insertions(+), 1 deletion(-)

--
2.1.0



2023-02-22 11:55:34

by Tiezhu Yang

[permalink] [raw]
Subject: [PATCH bpf-next 3/3] selftests/bpf: Check __ARCH_WANT_SET_GET_RLIMIT before syscall(__NR_getrlimit)

__NR_getrlimit is defined only if __ARCH_WANT_SET_GET_RLIMIT is defined:

#ifdef __ARCH_WANT_SET_GET_RLIMIT
/* getrlimit and setrlimit are superseded with prlimit64 */
#define __NR_getrlimit 163
...
#endif

Some archs do not define __ARCH_WANT_SET_GET_RLIMIT, it should check
__ARCH_WANT_SET_GET_RLIMIT before syscall(__NR_getrlimit) to fix the
following build error:

TEST-OBJ [test_progs] user_ringbuf.test.o
tools/testing/selftests/bpf/prog_tests/user_ringbuf.c: In function 'kick_kernel_cb':
tools/testing/selftests/bpf/prog_tests/user_ringbuf.c:593:17: error: '__NR_getrlimit' undeclared (first use in this function)
593 | syscall(__NR_getrlimit);
| ^~~~~~~~~~~~~~
tools/testing/selftests/bpf/prog_tests/user_ringbuf.c:593:17: note: each undeclared identifier is reported only once for each function it appears in
make: *** [Makefile:573: tools/testing/selftests/bpf/user_ringbuf.test.o] Error 1
make: Leaving directory 'tools/testing/selftests/bpf'

Signed-off-by: Tiezhu Yang <[email protected]>
---
tools/testing/selftests/bpf/prog_tests/user_ringbuf.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/user_ringbuf.c b/tools/testing/selftests/bpf/prog_tests/user_ringbuf.c
index 3a13e10..0550307 100644
--- a/tools/testing/selftests/bpf/prog_tests/user_ringbuf.c
+++ b/tools/testing/selftests/bpf/prog_tests/user_ringbuf.c
@@ -590,7 +590,9 @@ static void *kick_kernel_cb(void *arg)
/* Kick the kernel, causing it to drain the ring buffer and then wake
* up the test thread waiting on epoll.
*/
+#ifdef __ARCH_WANT_SET_GET_RLIMIT
syscall(__NR_getrlimit);
+#endif

return NULL;
}
--
2.1.0


2023-02-22 18:07:16

by Alexei Starovoitov

[permalink] [raw]
Subject: Re: [PATCH bpf-next 3/3] selftests/bpf: Check __ARCH_WANT_SET_GET_RLIMIT before syscall(__NR_getrlimit)

On Wed, Feb 22, 2023 at 3:55 AM Tiezhu Yang <[email protected]> wrote:
>
> __NR_getrlimit is defined only if __ARCH_WANT_SET_GET_RLIMIT is defined:
>
> #ifdef __ARCH_WANT_SET_GET_RLIMIT
> /* getrlimit and setrlimit are superseded with prlimit64 */
> #define __NR_getrlimit 163
> ...
> #endif
>
> Some archs do not define __ARCH_WANT_SET_GET_RLIMIT, it should check
> __ARCH_WANT_SET_GET_RLIMIT before syscall(__NR_getrlimit) to fix the
> following build error:
>
> TEST-OBJ [test_progs] user_ringbuf.test.o
> tools/testing/selftests/bpf/prog_tests/user_ringbuf.c: In function 'kick_kernel_cb':
> tools/testing/selftests/bpf/prog_tests/user_ringbuf.c:593:17: error: '__NR_getrlimit' undeclared (first use in this function)
> 593 | syscall(__NR_getrlimit);
> | ^~~~~~~~~~~~~~
> tools/testing/selftests/bpf/prog_tests/user_ringbuf.c:593:17: note: each undeclared identifier is reported only once for each function it appears in
> make: *** [Makefile:573: tools/testing/selftests/bpf/user_ringbuf.test.o] Error 1
> make: Leaving directory 'tools/testing/selftests/bpf'
>
> Signed-off-by: Tiezhu Yang <[email protected]>
> ---
> tools/testing/selftests/bpf/prog_tests/user_ringbuf.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/user_ringbuf.c b/tools/testing/selftests/bpf/prog_tests/user_ringbuf.c
> index 3a13e10..0550307 100644
> --- a/tools/testing/selftests/bpf/prog_tests/user_ringbuf.c
> +++ b/tools/testing/selftests/bpf/prog_tests/user_ringbuf.c
> @@ -590,7 +590,9 @@ static void *kick_kernel_cb(void *arg)
> /* Kick the kernel, causing it to drain the ring buffer and then wake
> * up the test thread waiting on epoll.
> */
> +#ifdef __ARCH_WANT_SET_GET_RLIMIT
> syscall(__NR_getrlimit);
> +#endif

This is clearly breaks user_ringbuf test on x86:
https://github.com/kernel-patches/bpf/actions/runs/4242660318/jobs/7374845859

Please do not send patches that make selftest compile on your favorite arch.
Make sure the patches work correctly on other archs too.

2023-02-22 20:03:57

by Mykola Lysenko

[permalink] [raw]
Subject: Re: [PATCH bpf-next 3/3] selftests/bpf: Check __ARCH_WANT_SET_GET_RLIMIT before syscall(__NR_getrlimit)

Hi Tiezhu,

You can run BPF CI tests on your patch before sending it out by following these instructions:
https://docs.kernel.org/bpf/bpf_devel_QA.html#q-how-do-i-run-bpf-ci-on-my-changes-before-sending-them-out-for-review

Thanks,
Mykola

> On Feb 22, 2023, at 10:06 AM, Alexei Starovoitov <[email protected]> wrote:
>
> !-------------------------------------------------------------------|
> This Message Is From an External Sender
>
> |-------------------------------------------------------------------!
>
> On Wed, Feb 22, 2023 at 3:55 AM Tiezhu Yang <[email protected]> wrote:
>>
>> __NR_getrlimit is defined only if __ARCH_WANT_SET_GET_RLIMIT is defined:
>>
>> #ifdef __ARCH_WANT_SET_GET_RLIMIT
>> /* getrlimit and setrlimit are superseded with prlimit64 */
>> #define __NR_getrlimit 163
>> ...
>> #endif
>>
>> Some archs do not define __ARCH_WANT_SET_GET_RLIMIT, it should check
>> __ARCH_WANT_SET_GET_RLIMIT before syscall(__NR_getrlimit) to fix the
>> following build error:
>>
>> TEST-OBJ [test_progs] user_ringbuf.test.o
>> tools/testing/selftests/bpf/prog_tests/user_ringbuf.c: In function 'kick_kernel_cb':
>> tools/testing/selftests/bpf/prog_tests/user_ringbuf.c:593:17: error: '__NR_getrlimit' undeclared (first use in this function)
>> 593 | syscall(__NR_getrlimit);
>> | ^~~~~~~~~~~~~~
>> tools/testing/selftests/bpf/prog_tests/user_ringbuf.c:593:17: note: each undeclared identifier is reported only once for each function it appears in
>> make: *** [Makefile:573: tools/testing/selftests/bpf/user_ringbuf.test.o] Error 1
>> make: Leaving directory 'tools/testing/selftests/bpf'
>>
>> Signed-off-by: Tiezhu Yang <[email protected]>
>> ---
>> tools/testing/selftests/bpf/prog_tests/user_ringbuf.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/tools/testing/selftests/bpf/prog_tests/user_ringbuf.c b/tools/testing/selftests/bpf/prog_tests/user_ringbuf.c
>> index 3a13e10..0550307 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/user_ringbuf.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/user_ringbuf.c
>> @@ -590,7 +590,9 @@ static void *kick_kernel_cb(void *arg)
>> /* Kick the kernel, causing it to drain the ring buffer and then wake
>> * up the test thread waiting on epoll.
>> */
>> +#ifdef __ARCH_WANT_SET_GET_RLIMIT
>> syscall(__NR_getrlimit);
>> +#endif
>
> This is clearly breaks user_ringbuf test on x86:
> https://github.com/kernel-patches/bpf/actions/runs/4242660318/jobs/7374845859
>
> Please do not send patches that make selftest compile on your favorite arch.
> Make sure the patches work correctly on other archs too.


2023-02-23 02:49:17

by Tiezhu Yang

[permalink] [raw]
Subject: Re: [PATCH bpf-next 3/3] selftests/bpf: Check __ARCH_WANT_SET_GET_RLIMIT before syscall(__NR_getrlimit)



On 02/23/2023 04:03 AM, Mykola Lysenko wrote:
> Hi Tiezhu,
>
> You can run BPF CI tests on your patch before sending it out by following these instructions:
> https://docs.kernel.org/bpf/bpf_devel_QA.html#q-how-do-i-run-bpf-ci-on-my-changes-before-sending-them-out-for-review

OK, thank you.

After commit 80d7da1cac62 ("asm-generic: Drop getrlimit and setrlimit
syscalls from default list"), new architectures won't need to include
getrlimit and setrlimit, they are superseded with prlimit64.

In order to maintain compatibility for the new architectures, such as
LoongArch which does not define __NR_getrlimit, it is better to use
__NR_prlimit64 instead of __NR_getrlimit in user_ringbuf.c to fix the
build error.

diff --git a/tools/testing/selftests/bpf/prog_tests/user_ringbuf.c
b/tools/testing/selftests/bpf/prog_tests/user_ringbuf.c
index 3a13e10..e51721d 100644
--- a/tools/testing/selftests/bpf/prog_tests/user_ringbuf.c
+++ b/tools/testing/selftests/bpf/prog_tests/user_ringbuf.c
@@ -590,7 +590,7 @@ static void *kick_kernel_cb(void *arg)
/* Kick the kernel, causing it to drain the ring buffer and
then wake
* up the test thread waiting on epoll.
*/
- syscall(__NR_getrlimit);
+ syscall(__NR_prlimit64);

return NULL;
}

I will test it and then send v2. If you have more suggestions,
please let me know.

Thanks,
Tiezhu

>
> Thanks,
> Mykola
>
>> On Feb 22, 2023, at 10:06 AM, Alexei Starovoitov <[email protected]> wrote:
>>
>> !-------------------------------------------------------------------|
>> This Message Is From an External Sender
>>
>> |-------------------------------------------------------------------!
>>
>> On Wed, Feb 22, 2023 at 3:55 AM Tiezhu Yang <[email protected]> wrote:
>>>
>>> __NR_getrlimit is defined only if __ARCH_WANT_SET_GET_RLIMIT is defined:
>>>
>>> #ifdef __ARCH_WANT_SET_GET_RLIMIT
>>> /* getrlimit and setrlimit are superseded with prlimit64 */
>>> #define __NR_getrlimit 163
>>> ...
>>> #endif
>>>
>>> Some archs do not define __ARCH_WANT_SET_GET_RLIMIT, it should check
>>> __ARCH_WANT_SET_GET_RLIMIT before syscall(__NR_getrlimit) to fix the
>>> following build error:
>>>
>>> TEST-OBJ [test_progs] user_ringbuf.test.o
>>> tools/testing/selftests/bpf/prog_tests/user_ringbuf.c: In function 'kick_kernel_cb':
>>> tools/testing/selftests/bpf/prog_tests/user_ringbuf.c:593:17: error: '__NR_getrlimit' undeclared (first use in this function)
>>> 593 | syscall(__NR_getrlimit);
>>> | ^~~~~~~~~~~~~~
>>> tools/testing/selftests/bpf/prog_tests/user_ringbuf.c:593:17: note: each undeclared identifier is reported only once for each function it appears in
>>> make: *** [Makefile:573: tools/testing/selftests/bpf/user_ringbuf.test.o] Error 1
>>> make: Leaving directory 'tools/testing/selftests/bpf'
>>>
>>> Signed-off-by: Tiezhu Yang <[email protected]>
>>> ---
>>> tools/testing/selftests/bpf/prog_tests/user_ringbuf.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/tools/testing/selftests/bpf/prog_tests/user_ringbuf.c b/tools/testing/selftests/bpf/prog_tests/user_ringbuf.c
>>> index 3a13e10..0550307 100644
>>> --- a/tools/testing/selftests/bpf/prog_tests/user_ringbuf.c
>>> +++ b/tools/testing/selftests/bpf/prog_tests/user_ringbuf.c
>>> @@ -590,7 +590,9 @@ static void *kick_kernel_cb(void *arg)
>>> /* Kick the kernel, causing it to drain the ring buffer and then wake
>>> * up the test thread waiting on epoll.
>>> */
>>> +#ifdef __ARCH_WANT_SET_GET_RLIMIT
>>> syscall(__NR_getrlimit);
>>> +#endif
>>
>> This is clearly breaks user_ringbuf test on x86:
>> https://github.com/kernel-patches/bpf/actions/runs/4242660318/jobs/7374845859
>>
>> Please do not send patches that make selftest compile on your favorite arch.
>> Make sure the patches work correctly on other archs too.