[bpf-next] selftests/bpf: fix uprobe_multi compilation error

Commit Message

Alan Maguire Sept. 26, 2024, 2:49 p.m. UTC

When building selftests, the following was seen:

uprobe_multi.c: In function ‘trigger_uprobe’:
uprobe_multi.c:108:40: error: ‘MADV_PAGEOUT’ undeclared (first use in this function)
  108 |                 madvise(addr, page_sz, MADV_PAGEOUT);
      |                                        ^~~~~~~~~~~~
uprobe_multi.c:108:40: note: each undeclared identifier is reported only once for each function it appears in
make: *** [Makefile:850: bpf-next/tools/testing/selftests/bpf/uprobe_multi] Error 1

...even with updated UAPI headers. It seems the above value is
defined in UAPI <linux/mman.h> but including that file triggers
other redefinition errors.  Simplest solution is to add a
guarded definition, as was done for MADV_POPULATE_READ.

Fixes: 3c217a182018 ("selftests/bpf: add build ID tests")
Signed-off-by: Alan Maguire <[e-mail]>
---
 tools/testing/selftests/bpf/uprobe_multi.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Eduard Zingerman Sept. 26, 2024, 11:44 p.m. UTC | #1

On Thu, 2024-09-26 at 15:49 +0100, Alan Maguire wrote:
> When building selftests, the following was seen:
> 
> uprobe_multi.c: In function ‘trigger_uprobe’:
> uprobe_multi.c:108:40: error: ‘MADV_PAGEOUT’ undeclared (first use in this function)
>   108 |                 madvise(addr, page_sz, MADV_PAGEOUT);
>       |                                        ^~~~~~~~~~~~
> uprobe_multi.c:108:40: note: each undeclared identifier is reported only once for each function it appears in
> make: *** [Makefile:850: bpf-next/tools/testing/selftests/bpf/uprobe_multi] Error 1
> 
> ...even with updated UAPI headers. It seems the above value is
> defined in UAPI <linux/mman.h> but including that file triggers
> other redefinition errors.  Simplest solution is to add a
> guarded definition, as was done for MADV_POPULATE_READ.
> 
> Fixes: 3c217a182018 ("selftests/bpf: add build ID tests")
> Signed-off-by: Alan Maguire <[e-mail]>
> ---

I was curious why this error is not triggered on my local machine or CI.
MADV_PAGEOUT is indeed defined in UAPI.
Selftests build picks it from host system header, which is
/usr/include/bits/mman-linux.h for my Fedora 40 setup.
The MADV_PAGEOUT was added by commit [1] back in 2019
(and should be available from Linux 5.4, I guess Alan uses a very old kernel).

I think that at some point in time we should adjust selftests to use
UAPI headers that come from the kernel being tested, not from the host.
Until that happens, I think this fix is fine.

[1] 1a4e58cce84e ("mm: introduce MADV_PAGEOUT")

Acked-by: Eduard Zingerman <[e-mail]>

(I want back to 2019...)

[...]

Alan Maguire Sept. 27, 2024, 10:26 a.m. UTC | #2

On 27/09/2024 00:44, Eduard Zingerman wrote:
> On Thu, 2024-09-26 at 15:49 +0100, Alan Maguire wrote:
>> When building selftests, the following was seen:
>>
>> uprobe_multi.c: In function ‘trigger_uprobe’:
>> uprobe_multi.c:108:40: error: ‘MADV_PAGEOUT’ undeclared (first use in this function)
>>   108 |                 madvise(addr, page_sz, MADV_PAGEOUT);
>>       |                                        ^~~~~~~~~~~~
>> uprobe_multi.c:108:40: note: each undeclared identifier is reported only once for each function it appears in
>> make: *** [Makefile:850: bpf-next/tools/testing/selftests/bpf/uprobe_multi] Error 1
>>
>> ...even with updated UAPI headers. It seems the above value is
>> defined in UAPI <linux/mman.h> but including that file triggers
>> other redefinition errors.  Simplest solution is to add a
>> guarded definition, as was done for MADV_POPULATE_READ.
>>
>> Fixes: 3c217a182018 ("selftests/bpf: add build ID tests")
>> Signed-off-by: Alan Maguire <[e-mail]>
>> ---
> 
> I was curious why this error is not triggered on my local machine or CI.
> MADV_PAGEOUT is indeed defined in UAPI.
> Selftests build picks it from host system header, which is
> /usr/include/bits/mman-linux.h for my Fedora 40 setup.
> The MADV_PAGEOUT was added by commit [1] back in 2019
> (and should be available from Linux 5.4, I guess Alan uses a very old kernel).
> 
> I think that at some point in time we should adjust selftests to use
> UAPI headers that come from the kernel being tested, not from the host.
> Until that happens, I think this fix is fine.
> 

From what I can see at my end, /usr/include/bits/mman.h is provided by
glibc-headers. On distros the glibc-headers can get quite out of date. I
do a "make headers_install" prior to running bpf selftests to get latest
UAPI headers from the kernel in /usr/include/linux, but that wasn't
enough in this case since /usr/include/sys/mman.h and
/usr/include/bits/mman.h were provided via glibc-headers.

When I tried using <linux/mman.h> in the program, I ended up getting
complaints about madvise() being undefined, and when I used both
<sys/mman.h> and <linux/mman.h> there were complaints about mutiple
definitions so the approach taken in the patch seemed like the most
straightforward.

Thanks for taking a look!

Alan

> [1] 1a4e58cce84e ("mm: introduce MADV_PAGEOUT")
> 
> Acked-by: Eduard Zingerman <[e-mail]>
> 
> (I want back to 2019...)
> 
> [...]
>

[e-mail] Sept. 27, 2024, 8:50 p.m. UTC | #3

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <[e-mail]>:

On Thu, 26 Sep 2024 15:49:48 +0100 you wrote:
> When building selftests, the following was seen:
> 
> uprobe_multi.c: In function ‘trigger_uprobe’:
> uprobe_multi.c:108:40: error: ‘MADV_PAGEOUT’ undeclared (first use in this function)
>   108 |                 madvise(addr, page_sz, MADV_PAGEOUT);
>       |                                        ^~~~~~~~~~~~
> uprobe_multi.c:108:40: note: each undeclared identifier is reported only once for each function it appears in
> make: *** [Makefile:850: bpf-next/tools/testing/selftests/bpf/uprobe_multi] Error 1
> 
> [...]

Here is the summary with links:
  - [bpf-next] selftests/bpf: fix uprobe_multi compilation error
    https://git.kernel.org/bpf/bpf-next/c/db38ed2cfa62

You are awesome, thank you!

Patch

diff --git a/tools/testing/selftests/bpf/uprobe_multi.c b/tools/testing/selftests/bpf/uprobe_multi.c
index c7828b13e5ff..dd38dc68f635 100644
--- a/tools/testing/selftests/bpf/uprobe_multi.c
+++ b/tools/testing/selftests/bpf/uprobe_multi.c
@@ -12,6 +12,10 @@ 
 #define MADV_POPULATE_READ 22
 #endif
 
+#ifndef MADV_PAGEOUT
+#define MADV_PAGEOUT 21
+#endif
+
 int __attribute__((weak)) uprobe(void)
 {
 	return 0;