diff mbox series

[bpf-next,3/3] selftests/bpf: Check __ARCH_WANT_SET_GET_RLIMIT before syscall(__NR_getrlimit)

Message ID 1677066908-15224-4-git-send-email-yangtiezhu@loongson.cn (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series Fix some build errors for bpf selftest on LoongArch | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for bpf-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 14 maintainers not CCed: linux-kselftest@vger.kernel.org john.fastabend@gmail.com sdf@google.com shuah@kernel.org jolsa@kernel.org memxor@gmail.com song@kernel.org mykolal@fb.com martin.lau@linux.dev haoluo@google.com void@manifault.com joannelkoong@gmail.com yhs@fb.com kpsingh@kernel.org
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 9 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-2 success Logs for build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-3 success Logs for build for aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-5 success Logs for build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-6 success Logs for build for x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-7 success Logs for llvm-toolchain
bpf/vmtest-bpf-next-VM_Test-8 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-4 success Logs for build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-9 success Logs for test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-10 success Logs for test_maps on aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-12 success Logs for test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-13 success Logs for test_maps on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-14 success Logs for test_progs on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-15 success Logs for test_progs on aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-17 fail Logs for test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-18 fail Logs for test_progs on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-19 success Logs for test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-20 success Logs for test_progs_no_alu32 on aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-22 fail Logs for test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-23 fail Logs for test_progs_no_alu32 on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-24 success Logs for test_progs_no_alu32_parallel on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-25 success Logs for test_progs_no_alu32_parallel on aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-27 success Logs for test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-28 success Logs for test_progs_no_alu32_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-29 success Logs for test_progs_parallel on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-30 success Logs for test_progs_parallel on aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-32 success Logs for test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-33 success Logs for test_progs_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-34 success Logs for test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-35 success Logs for test_verifier on aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-36 success Logs for test_verifier on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-37 success Logs for test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-38 success Logs for test_verifier on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-16 success Logs for test_progs on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-21 success Logs for test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-26 success Logs for test_progs_no_alu32_parallel on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-31 success Logs for test_progs_parallel on s390x with gcc
bpf/vmtest-bpf-next-PR fail PR summary
bpf/vmtest-bpf-next-VM_Test-11 success Logs for test_maps on s390x with gcc

Commit Message

Tiezhu Yang Feb. 22, 2023, 11:55 a.m. UTC 
__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 <yangtiezhu@loongson.cn>
---
 tools/testing/selftests/bpf/prog_tests/user_ringbuf.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Alexei Starovoitov Feb. 22, 2023, 6:06 p.m. UTC | #1 
On Wed, Feb 22, 2023 at 3:55 AM Tiezhu Yang <yangtiezhu@loongson.cn> 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 <yangtiezhu@loongson.cn>
> ---
>  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.
Mykola Lysenko Feb. 22, 2023, 8:03 p.m. UTC | #2 
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 <alexei.starovoitov@gmail.com> wrote:
> 
> !-------------------------------------------------------------------|
>  This Message Is From an External Sender
> 
> |-------------------------------------------------------------------!
> 
> On Wed, Feb 22, 2023 at 3:55 AM Tiezhu Yang <yangtiezhu@loongson.cn> 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 <yangtiezhu@loongson.cn>
>> ---
>> 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.
Tiezhu Yang Feb. 23, 2023, 2:48 a.m. UTC | #3 
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 <alexei.starovoitov@gmail.com> wrote:
>>
>> !-------------------------------------------------------------------|
>>  This Message Is From an External Sender
>>
>> |-------------------------------------------------------------------!
>>
>> On Wed, Feb 22, 2023 at 3:55 AM Tiezhu Yang <yangtiezhu@loongson.cn> 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 <yangtiezhu@loongson.cn>
>>> ---
>>> 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.
diff mbox series

Patch

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;
 }