Hi all,
After merging the net-next tree, today's linux-next build (native perf)
failed like this:
In file included from builtin-trace.c:907:
trace/beauty/msg_flags.c: In function 'syscall_arg__scnprintf_msg_flags':
trace/beauty/msg_flags.c:28:21: error: 'MSG_SPLICE_PAGES' undeclared (first use in this function)
28 | if (flags & MSG_##n) { \
| ^~~~
trace/beauty/msg_flags.c:50:9: note: in expansion of macro 'P_MSG_FLAG'
50 | P_MSG_FLAG(SPLICE_PAGES);
| ^~~~~~~~~~
trace/beauty/msg_flags.c:28:21: note: each undeclared identifier is reported only once for each function it appears in
28 | if (flags & MSG_##n) { \
| ^~~~
trace/beauty/msg_flags.c:50:9: note: in expansion of macro 'P_MSG_FLAG'
50 | P_MSG_FLAG(SPLICE_PAGES);
| ^~~~~~~~~~
Caused by commit
b848b26c6672 ("net: Kill MSG_SENDPAGE_NOTLAST")
There is no MSG_SPLICE_PAGES in tools/perf/trace/beauty/include/linux/socket.h
I have reverted that commit for today.
--
Cheers,
Stephen Rothwell
Stephen Rothwell <[email protected]> wrote:
> After merging the net-next tree, today's linux-next build (native perf)
> failed like this:
>
> In file included from builtin-trace.c:907:
> trace/beauty/msg_flags.c: In function 'syscall_arg__scnprintf_msg_flags':
> trace/beauty/msg_flags.c:28:21: error: 'MSG_SPLICE_PAGES' undeclared (first use in this function)
I tried applying the attached patch, but it doesn't make any difference.
Any idea what I've missed? Also, why do we have duplicates of all the kernel
headers in the tools/ directory?
David
---
commit 878ff45f5f746f6773224ff952c490b5812462f2
Author: David Howells <[email protected]>
Date: Mon Jun 26 08:08:12 2023 +0100
tools: Fix MSG_SPLICE_PAGES build error in trace tools
Fix the following error:
In file included from builtin-trace.c:907:
trace/beauty/msg_flags.c: In function 'syscall_arg__scnprintf_msg_flags':
trace/beauty/msg_flags.c:28:21: error: 'MSG_SPLICE_PAGES' undeclared (first use in this function)
28 | if (flags & MSG_##n) { \
| ^~~~
trace/beauty/msg_flags.c:50:9: note: in expansion of macro 'P_MSG_FLAG'
50 | P_MSG_FLAG(SPLICE_PAGES);
| ^~~~~~~~~~
trace/beauty/msg_flags.c:28:21: note: each undeclared identifier is reported only once for each function it appears in
28 | if (flags & MSG_##n) { \
| ^~~~
trace/beauty/msg_flags.c:50:9: note: in expansion of macro 'P_MSG_FLAG'
50 | P_MSG_FLAG(SPLICE_PAGES);
| ^~~~~~~~~~
There is no MSG_SPLICE_PAGES in tools/perf/trace/beauty/include/linux/socket.h
Fixes: b848b26c6672 ("net: Kill MSG_SENDPAGE_NOTLAST")
Reported-by: Stephen Rothwell <[email protected]>
Link: https://lore.kernel.org/r/[email protected]/
Signed-off-by: David Howells <[email protected]>
cc: "David S. Miller" <[email protected]>
cc: Eric Dumazet <[email protected]>
cc: Jakub Kicinski <[email protected]>
cc: Paolo Abeni <[email protected]>
cc: Jens Axboe <[email protected]>
cc: Matthew Wilcox <[email protected]>
cc: [email protected]
cc: [email protected]
cc: [email protected]
cc: [email protected]
cc: [email protected]
cc: [email protected]
cc: [email protected]
cc: [email protected]
cc: [email protected]
cc: [email protected]
cc: [email protected]
cc: [email protected]
cc: [email protected]
cc: [email protected]
cc: [email protected]
cc: [email protected]
cc: [email protected]
cc: [email protected]
diff --git a/tools/perf/trace/beauty/include/linux/socket.h b/tools/perf/trace/beauty/include/linux/socket.h
index 3bef212a24d7..77cb707a566a 100644
--- a/tools/perf/trace/beauty/include/linux/socket.h
+++ b/tools/perf/trace/beauty/include/linux/socket.h
@@ -326,6 +326,7 @@ struct ucred {
*/
#define MSG_ZEROCOPY 0x4000000 /* Use user data in kernel path */
+#define MSG_SPLICE_PAGES 0x8000000 /* Splice the pages from the iterator in sendmsg() */
#define MSG_FASTOPEN 0x20000000 /* Send data in TCP SYN */
#define MSG_CMSG_CLOEXEC 0x40000000 /* Set close_on_exec for file
descriptor received through
Hi David,
On Mon, 26 Jun 2023 08:48:26 +0100 David Howells <[email protected]> wrote:
>
> Stephen Rothwell <[email protected]> wrote:
>
> > After merging the net-next tree, today's linux-next build (native perf)
> > failed like this:
> >
> > In file included from builtin-trace.c:907:
> > trace/beauty/msg_flags.c: In function 'syscall_arg__scnprintf_msg_flags':
> > trace/beauty/msg_flags.c:28:21: error: 'MSG_SPLICE_PAGES' undeclared (first use in this function)
>
> I tried applying the attached patch, but it doesn't make any difference.
I wonder if it is using the system headers? Or depends on "make headers-install"?
--
Cheers,
Stephen Rothwell
Our MPTCP CI and Stephen got this error:
In file included from builtin-trace.c:907:
trace/beauty/msg_flags.c: In function 'syscall_arg__scnprintf_msg_flags':
trace/beauty/msg_flags.c:28:21: error: 'MSG_SPLICE_PAGES' undeclared (first use in this function)
28 | if (flags & MSG_##n) { | ^~~~
trace/beauty/msg_flags.c:50:9: note: in expansion of macro 'P_MSG_FLAG'
50 | P_MSG_FLAG(SPLICE_PAGES);
| ^~~~~~~~~~
trace/beauty/msg_flags.c:28:21: note: each undeclared identifier is reported only once for each function it appears in
28 | if (flags & MSG_##n) { | ^~~~
trace/beauty/msg_flags.c:50:9: note: in expansion of macro 'P_MSG_FLAG'
50 | P_MSG_FLAG(SPLICE_PAGES);
| ^~~~~~~~~~
The fix is similar to what was done with MSG_FASTOPEN: the new macro is
defined if it is not defined in the system headers.
Fixes: b848b26c6672 ("net: Kill MSG_SENDPAGE_NOTLAST")
Reported-by: Stephen Rothwell <[email protected]>
Closes: https://lore.kernel.org/r/[email protected]/
Signed-off-by: Matthieu Baerts <[email protected]>
---
Notes:
@David: I solved it like that in MPTCP tree. Does it work for you too?
I guess tools/perf/trace/beauty/include/linux/socket.h file still needs
to be updated, not just to add MSG_SPLICE_PAGES but also other
modifications done in this file. Maybe best to sync with Arnaldo because
he might do it soon during the coming merge window I guess.
Cc: David Howells <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
tools/perf/trace/beauty/msg_flags.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/perf/trace/beauty/msg_flags.c b/tools/perf/trace/beauty/msg_flags.c
index 5cdebd7ece7e..aa9934020232 100644
--- a/tools/perf/trace/beauty/msg_flags.c
+++ b/tools/perf/trace/beauty/msg_flags.c
@@ -8,6 +8,9 @@
#ifndef MSG_WAITFORONE
#define MSG_WAITFORONE 0x10000
#endif
+#ifndef MSG_SPLICE_PAGES
+#define MSG_SPLICE_PAGES 0x8000000
+#endif
#ifndef MSG_FASTOPEN
#define MSG_FASTOPEN 0x20000000
#endif
base-commit: 9ae440b8fdd6772b6c007fa3d3766530a09c9045
--
2.40.1
Hello,
On Mon, Jun 26, 2023 at 2:27 PM Jakub Kicinski <[email protected]> wrote:
>
> On Mon, 26 Jun 2023 11:02:39 +0200 Matthieu Baerts wrote:
> > Our MPTCP CI and Stephen got this error:
> >
> > In file included from builtin-trace.c:907:
> > trace/beauty/msg_flags.c: In function 'syscall_arg__scnprintf_msg_flags':
> > trace/beauty/msg_flags.c:28:21: error: 'MSG_SPLICE_PAGES' undeclared (first use in this function)
> > 28 | if (flags & MSG_##n) { | ^~~~
> > trace/beauty/msg_flags.c:50:9: note: in expansion of macro 'P_MSG_FLAG'
> > 50 | P_MSG_FLAG(SPLICE_PAGES);
> > | ^~~~~~~~~~
> > trace/beauty/msg_flags.c:28:21: note: each undeclared identifier is reported only once for each function it appears in
> > 28 | if (flags & MSG_##n) { | ^~~~
> > trace/beauty/msg_flags.c:50:9: note: in expansion of macro 'P_MSG_FLAG'
> > 50 | P_MSG_FLAG(SPLICE_PAGES);
> > | ^~~~~~~~~~
> >
> > The fix is similar to what was done with MSG_FASTOPEN: the new macro is
> > defined if it is not defined in the system headers.
> >
> > Fixes: b848b26c6672 ("net: Kill MSG_SENDPAGE_NOTLAST")
> > Reported-by: Stephen Rothwell <[email protected]>
> > Closes: https://lore.kernel.org/r/[email protected]/
> > Signed-off-by: Matthieu Baerts <[email protected]>
> > ---
> >
> > Notes:
> > @David: I solved it like that in MPTCP tree. Does it work for you too?
> >
> > I guess tools/perf/trace/beauty/include/linux/socket.h file still needs
> > to be updated, not just to add MSG_SPLICE_PAGES but also other
> > modifications done in this file. Maybe best to sync with Arnaldo because
> > he might do it soon during the coming merge window I guess.
> >
> > Cc: David Howells <[email protected]>
> > Cc: Arnaldo Carvalho de Melo <[email protected]>
>
> Hi Arnaldo, are you okay with us taking this into the networking tree?
> Or do you prefer to sync the header after everything lands in Linus's
> tree?
Arnaldo is on vacation now, and I'm taking care of the patches
on behalf of him.
As it's introduced in the networking tree, it should be fine to
carry the fix together. I'll sync the header later.
But in general you don't need to change the copy of the tools
headers together. It also needs to support old & new kernels
so different care should be taken. Please separate tooling
changes and let us handle them.
Thanks,
Namhyung
On Mon, 26 Jun 2023 11:02:39 +0200 Matthieu Baerts wrote:
> Our MPTCP CI and Stephen got this error:
>
> In file included from builtin-trace.c:907:
> trace/beauty/msg_flags.c: In function 'syscall_arg__scnprintf_msg_flags':
> trace/beauty/msg_flags.c:28:21: error: 'MSG_SPLICE_PAGES' undeclared (first use in this function)
> 28 | if (flags & MSG_##n) { | ^~~~
> trace/beauty/msg_flags.c:50:9: note: in expansion of macro 'P_MSG_FLAG'
> 50 | P_MSG_FLAG(SPLICE_PAGES);
> | ^~~~~~~~~~
> trace/beauty/msg_flags.c:28:21: note: each undeclared identifier is reported only once for each function it appears in
> 28 | if (flags & MSG_##n) { | ^~~~
> trace/beauty/msg_flags.c:50:9: note: in expansion of macro 'P_MSG_FLAG'
> 50 | P_MSG_FLAG(SPLICE_PAGES);
> | ^~~~~~~~~~
>
> The fix is similar to what was done with MSG_FASTOPEN: the new macro is
> defined if it is not defined in the system headers.
>
> Fixes: b848b26c6672 ("net: Kill MSG_SENDPAGE_NOTLAST")
> Reported-by: Stephen Rothwell <[email protected]>
> Closes: https://lore.kernel.org/r/[email protected]/
> Signed-off-by: Matthieu Baerts <[email protected]>
> ---
>
> Notes:
> @David: I solved it like that in MPTCP tree. Does it work for you too?
>
> I guess tools/perf/trace/beauty/include/linux/socket.h file still needs
> to be updated, not just to add MSG_SPLICE_PAGES but also other
> modifications done in this file. Maybe best to sync with Arnaldo because
> he might do it soon during the coming merge window I guess.
>
> Cc: David Howells <[email protected]>
> Cc: Arnaldo Carvalho de Melo <[email protected]>
Hi Arnaldo, are you okay with us taking this into the networking tree?
Or do you prefer to sync the header after everything lands in Linus's
tree?
On Mon, 26 Jun 2023 14:41:56 -0700 Namhyung Kim wrote:
> > Hi Arnaldo, are you okay with us taking this into the networking tree?
> > Or do you prefer to sync the header after everything lands in Linus's
> > tree?
>
> Arnaldo is on vacation now, and I'm taking care of the patches
> on behalf of him.
>
> As it's introduced in the networking tree, it should be fine to
> carry the fix together. I'll sync the header later.
Will do, thanks!
> But in general you don't need to change the copy of the tools
> headers together. It also needs to support old & new kernels
> so different care should be taken. Please separate tooling
> changes and let us handle them.
Ack, I'm not sure what makes this a special case, from Stephen's
original report:
https://lore.kernel.org/all/[email protected]/
it sounded like perf won't build without the fix.
Hi Jakub, Namhyung,
@Namhyung: thank you for the explanations about the header files in the other thread!
26 Jun 2023 23:49:36 Jakub Kicinski <[email protected]>:
> On Mon, 26 Jun 2023 14:41:56 -0700 Namhyung Kim wrote:
>
>> But in general you don't need to change the copy of the tools
>> headers together. It also needs to support old & new kernels
>> so different care should be taken. Please separate tooling
>> changes and let us handle them.
>
> Ack, I'm not sure what makes this a special case, from Stephen's
> original report:
>
> https://lore.kernel.org/all/[email protected]/
> > it sounded like perf won't build without the fix.
The copy of the header file is confusing :)
If I understood properly, here, we need the modification in msg_flags.c from Perf code because this file has been recently modified by David's series in net-next to support a new flag (not present in the system header files yet). Without this patch, Perf doesn't build.
Now regarding the "internal" copy of header files: we don't need to modify it because it is just used by Perf maintainers to monitor the modifications in the original file. If I'm not mistaken, it might even be better to revert the line that has been removed in David's original series in net-next in this header file. But now that Namhyung is aware of that modification, I guess the revert is probably not needed. But in short, it is then better not to modify this header file in the networking tree :-)
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
http://www.tessares.net
Hello:
This patch was applied to netdev/net-next.git (main)
by Paolo Abeni <[email protected]>:
On Mon, 26 Jun 2023 11:02:39 +0200 you wrote:
> Our MPTCP CI and Stephen got this error:
>
> In file included from builtin-trace.c:907:
> trace/beauty/msg_flags.c: In function 'syscall_arg__scnprintf_msg_flags':
> trace/beauty/msg_flags.c:28:21: error: 'MSG_SPLICE_PAGES' undeclared (first use in this function)
> 28 | if (flags & MSG_##n) { | ^~~~
> trace/beauty/msg_flags.c:50:9: note: in expansion of macro 'P_MSG_FLAG'
> 50 | P_MSG_FLAG(SPLICE_PAGES);
> | ^~~~~~~~~~
> trace/beauty/msg_flags.c:28:21: note: each undeclared identifier is reported only once for each function it appears in
> 28 | if (flags & MSG_##n) { | ^~~~
> trace/beauty/msg_flags.c:50:9: note: in expansion of macro 'P_MSG_FLAG'
> 50 | P_MSG_FLAG(SPLICE_PAGES);
> | ^~~~~~~~~~
>
> [...]
Here is the summary with links:
- [net-next] perf trace: fix MSG_SPLICE_PAGES build error
https://git.kernel.org/netdev/net-next/c/2553a5270d6c
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html