Message ID | 3065880.1687785614@warthog.procyon.org.uk |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:994d:0:b0:3d9:f83d:47d9 with SMTP id k13csp7496662vqr; Mon, 26 Jun 2023 06:49:33 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ45h0AU/6XfJ/ppv08M9CgIQgIfiaqReJhGbuvGiffaP/ok69t9aclngdcmo5UaxKXWp0SI X-Received: by 2002:a17:907:724c:b0:991:f427:2fdf with SMTP id ds12-20020a170907724c00b00991f4272fdfmr594525ejc.76.1687787373677; Mon, 26 Jun 2023 06:49:33 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1687787373; cv=none; d=google.com; s=arc-20160816; b=yxMJQSxNwZkOVlWpWUhzKebkf9nN0VqycSk9Ye0dwX02arw7LZWN3ihzNRgYa7NpDM +2s8cZoRIFBgyUlLcV0gTj3zeDEoqoEE29DnF3GNmVMJ2AYgkADVp7MGb8eTSZ+2HQrx bFdwC4XsZXnTf800zxSRJs3O0FGtAkJmedfNWyHKN+Qydu6hA5f0wFeowc0wkmCI2I8B 5XKij51v2BwBOiwiNCLKAZgS4M4XJUex6pjop11i/uvZ/NcxCBJoM6tIxDOjnycci+ms sOvy/Y121iYTPOx2USW4ZhJRRWE7u91Qa53Dn+ZCMvt5p89LGJlY9jdHWcyWkcWhYQmC gAVw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:message-id:date:content-transfer-encoding :content-id:mime-version:subject:cc:to:from:organization :dkim-signature; bh=p/Z1KtTHNDEz/UvhK/uC5AXS1uWtp7gEorowOqjT+2k=; fh=IxPptaTe4Qovi4syYGJbfGJ5qduupw7zn8CYhX1pL+A=; b=ocoEidhsBXh8usufdsLGn+dTxKfgn0HJgdXnmRugkf1uS6bFhtpsQAQr2tm13TvzQy 9GXhvyBhf8ueY2qt6Es09KhK7TgY/xUomYm4lsQ7o27aYspkPJ/SlXXx2V3XvuHbNM3o FwoC1UppjzR5MRqkFGNPoI6Gyb449bCVl5KMQi8VLkMGEXwWxhy0aqBTlnu0m9mvXoOB +5PFeBquTLM+h0tT8Pzq0A/mj2Aww/h7eMXWi3vhId6DJU+0KUHe29ZgsomekXa+xvmL 6Xszxgv7lS9lVVDZITs6S1updLwbMcbUH7zDurxv/Aq0G0Xk5I9TOWrpIUhT8ZWyJaYO DwOA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=HlqHyZSu; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id pj25-20020a170906d79900b00991cc9224c1si946965ejb.379.2023.06.26.06.49.08; Mon, 26 Jun 2023 06:49:33 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=HlqHyZSu; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229937AbjFZNVM (ORCPT <rfc822;filip.gregor98@gmail.com> + 99 others); Mon, 26 Jun 2023 09:21:12 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34944 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229889AbjFZNVJ (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Mon, 26 Jun 2023 09:21:09 -0400 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 29510B9 for <linux-kernel@vger.kernel.org>; Mon, 26 Jun 2023 06:20:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1687785622; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=p/Z1KtTHNDEz/UvhK/uC5AXS1uWtp7gEorowOqjT+2k=; b=HlqHyZSuLOfLUnj11cWrTO5hMEOmeHo+x2zxMniqQ2Ho4PDv37JSMuWoZ5SfZSvMXjhCA4 7rJmheuMzLOGCZgtWATFuu1Mz3Wzb/6ZT1IH3Mnv7F0lkMqs43j4wao2nRPTezCnimJhzP MA2Xl23OCct+qVwAF9CTHrVDFPK/J6s= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-654-CPSOP-6FMqeapgcO3dkupw-1; Mon, 26 Jun 2023 09:20:19 -0400 X-MC-Unique: CPSOP-6FMqeapgcO3dkupw-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.rdu2.redhat.com [10.11.54.8]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id E48B2858EED; Mon, 26 Jun 2023 13:20:17 +0000 (UTC) Received: from warthog.procyon.org.uk (unknown [10.42.28.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 3B361C478C6; Mon, 26 Jun 2023 13:20:15 +0000 (UTC) Organization: Red Hat UK Ltd. Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SI4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 3798903 From: David Howells <dhowells@redhat.com> To: netdev@vger.kernel.org cc: dhowells@redhat.com, Matthieu Baerts <matthieu.baerts@tessares.net>, Arnaldo Carvalho de Melo <acme@redhat.com>, David Miller <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, Jens Axboe <axboe@kernel.dk>, Matthew Wilcox <willy@infradead.org>, Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@redhat.com>, Arnaldo Carvalho de Melo <acme@kernel.org>, Mark Rutland <mark.rutland@arm.com>, Alexander Shishkin <alexander.shishkin@linux.intel.com>, Jiri Olsa <jolsa@kernel.org>, Namhyung Kim <namhyung@kernel.org>, Ian Rogers <irogers@google.com>, Adrian Hunter <adrian.hunter@intel.com>, linux-perf-users@vger.kernel.org, bpf@vger.kernel.org, linux-next@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH net-next] tools: Fix MSG_SPLICE_PAGES build error in trace tools MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-ID: <3065879.1687785614.1@warthog.procyon.org.uk> Content-Transfer-Encoding: quoted-printable Date: Mon, 26 Jun 2023 14:20:14 +0100 Message-ID: <3065880.1687785614@warthog.procyon.org.uk> X-Scanned-By: MIMEDefang 3.1 on 10.11.54.8 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1769773333060511042?= X-GMAIL-MSGID: =?utf-8?q?1769773333060511042?= |
Series |
[net-next] tools: Fix MSG_SPLICE_PAGES build error in trace tools
|
|
Commit Message
David Howells
June 26, 2023, 1:20 p.m. UTC
The following error is being seen the perf tools because they have their
own copies of a lot of kernel headers:
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);
| ^~~~~~~~~~
Fix this by (1) adding MSG_SPLICE_PAGES to
tools/perf/trace/beauty/include/linux/socket.h - which looks like it ought
to work, but doesn't, and (2) defining it conditionally in the file on
which the error occurs (suggested by Matthieu Baerts - this is also done
for some other flags).
Fixes: b848b26c6672 ("net: Kill MSG_SENDPAGE_NOTLAST")
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Link: https://lore.kernel.org/r/20230626112847.2ef3d422@canb.auug.org.au/
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Matthieu Baerts <matthieu.baerts@tessares.net>
cc: Arnaldo Carvalho de Melo <acme@redhat.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Eric Dumazet <edumazet@google.com>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Jens Axboe <axboe@kernel.dk>
cc: Matthew Wilcox <willy@infradead.org>
cc: bpf@vger.kernel.org
cc: dccp@vger.kernel.org
cc: linux-crypto@vger.kernel.org
cc: mptcp@lists.linux.dev
cc: netdev@vger.kernel.org
cc: tipc-discussion@lists.sourceforge.net
cc: virtualization@lists.linux-foundation.org
---
include/linux/socket.h | 1 +
msg_flags.c | 3 +++
2 files changed, 4 insertions(+)
Comments
Hi David, Thank you for the patch! On 26/06/2023 15:20, David Howells wrote: > The following error is being seen the perf tools because they have their > own copies of a lot of kernel headers: > > 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); > | ^~~~~~~~~~ > > Fix this by (1) adding MSG_SPLICE_PAGES to > tools/perf/trace/beauty/include/linux/socket.h - which looks like it ought > to work, but doesn't Commit 58277f502f42 ("perf trace beauty: Add script to autogenerate socket families table") seems to suggest this file is only used by tools/perf/trace/beauty/socket.sh script (and later by sockaddr.sh) but not by msg_flags.c. If I'm not mistaken, it is then not required to modify this file to fix the issue you mention. But it is still needed to modify it to avoid another warning: Warning: Kernel ABI header at 'tools/perf/trace/beauty/include/linux/socket.h' differs from latest version at 'include/linux/socket.h' diff -u tools/perf/trace/beauty/include/linux/socket.h include/linux/socket.h So another issue. When checking the differences between the two files, I see there are still also other modifications to import, e.g. it looks like you also added MSG_INTERNAL_SENDMSG_FLAGS macro in socket.h. Do you plan to fix that too? It might be clearer to have all these modifications of tools/perf/trace/beauty/include/linux/socket.h in a separated commit as it is fixing a different issue but that's a detail. As long as we can have perf compiling without issues when using the net-next tree :) Cheers, Matt
Matthieu Baerts <matthieu.baerts@tessares.net> wrote: > So another issue. When checking the differences between the two files, I > see there are still also other modifications to import, e.g. it looks > like you also added MSG_INTERNAL_SENDMSG_FLAGS macro in socket.h. Do you > plan to fix that too? That's just a list of other flags that we need to prevent userspace passing in - it's not a flag in its own right. David
On 26/06/2023 15:50, David Howells wrote: > Matthieu Baerts <matthieu.baerts@tessares.net> wrote: > >> So another issue. When checking the differences between the two files, I >> see there are still also other modifications to import, e.g. it looks >> like you also added MSG_INTERNAL_SENDMSG_FLAGS macro in socket.h. Do you >> plan to fix that too? > > That's just a list of other flags that we need to prevent userspace passing > in - it's not a flag in its own right. I agree. This file is not even used by C files, only by scripts parsing it if I'm not mistaken. But if there are differences with include/linux/socket.h, the warning I mentioned will be visible when compiling Perf. Cheers, Matt
Hello, Sorry I missed the conversation and the original change. On Mon, Jun 26, 2023 at 6:56 AM Matthieu Baerts <matthieu.baerts@tessares.net> wrote: > > On 26/06/2023 15:50, David Howells wrote: > > Matthieu Baerts <matthieu.baerts@tessares.net> wrote: > > > >> So another issue. When checking the differences between the two files, I > >> see there are still also other modifications to import, e.g. it looks > >> like you also added MSG_INTERNAL_SENDMSG_FLAGS macro in socket.h. Do you > >> plan to fix that too? > > > > That's just a list of other flags that we need to prevent userspace passing > > in - it's not a flag in its own right. > > I agree. This file is not even used by C files, only by scripts parsing > it if I'm not mistaken. > > But if there are differences with include/linux/socket.h, the warning I > mentioned will be visible when compiling Perf. Right, we want to keep the headers files in the tools in sync with the kernel ones. And they are used to generate some tables. Full explanation from Arnaldo below. So I'm ok for the msg_flags.c changes, but please refrain from changing the header directly. We will update it later. With that, Acked-by: Namhyung Kim <namhyung@kernel.org> Also I wonder if the tool needs to keep the original flag (SENDPAGE_NOTLAST) for the older kernels. In Arnaldo's explanation: There used to be no copies, with tools/ code using kernel headers directly. From time to time tools/perf/ broke due to legitimate kernel hacking. At some point Linus complained about such direct usage. Then we adopted the current model. The way these headers are used in perf are not restricted to just including them to compile something. They are sometimes used in scripts that convert defines into string tables, etc, so some change may break one of these scripts, or new MSRs may use some different #define pattern, etc. E.g.: $ ls -1 tools/perf/trace/beauty/*.sh | head -5 tools/perf/trace/beauty/arch_errno_names.sh tools/perf/trace/beauty/drm_ioctl.sh tools/perf/trace/beauty/fadvise.sh tools/perf/trace/beauty/fsconfig.sh tools/perf/trace/beauty/fsmount.sh $ $ tools/perf/trace/beauty/fadvise.sh static const char *fadvise_advices[] = { [0] = "NORMAL", [1] = "RANDOM", [2] = "SEQUENTIAL", [3] = "WILLNEED", [4] = "DONTNEED", [5] = "NOREUSE", }; $ The tools/perf/check-headers.sh script, part of the tools/ build process, points out changes in the original files. So its important not to touch the copies in tools/ when doing changes in the original kernel headers, that will be done later, when check-headers.sh inform about the change to the perf tools hackers.
On Mon, 26 Jun 2023 14:31:39 -0700 Namhyung Kim wrote: > Right, we want to keep the headers files in the tools in sync with > the kernel ones. And they are used to generate some tables. > Full explanation from Arnaldo below. > > So I'm ok for the msg_flags.c changes, but please refrain from > changing the header directly. We will update it later. > > With that, > Acked-by: Namhyung Kim <namhyung@kernel.org> Ah, missed this email, sounds like this is preferred to Matthieu's fix, we'll take this one. > Also I wonder if the tool needs to keep the original flag > (SENDPAGE_NOTLAST) for the older kernels. That's a bit unclear, because it's just a kernel-internal flag. Future kernels may well use that bit for something else. Better long term solution would be to use an enum so that the values are included in debuginfo and perf can read them at runtime :(
On Mon, Jun 26, 2023 at 2:53 PM Jakub Kicinski <kuba@kernel.org> wrote: > > On Mon, 26 Jun 2023 14:31:39 -0700 Namhyung Kim wrote: > > Right, we want to keep the headers files in the tools in sync with > > the kernel ones. And they are used to generate some tables. > > Full explanation from Arnaldo below. > > > > So I'm ok for the msg_flags.c changes, but please refrain from > > changing the header directly. We will update it later. > > > > With that, > > Acked-by: Namhyung Kim <namhyung@kernel.org> > > Ah, missed this email, sounds like this is preferred to Matthieu's > fix, we'll take this one. Hmm.. I believe they are the same when it comes to the changes in the .c file. > > > Also I wonder if the tool needs to keep the original flag > > (SENDPAGE_NOTLAST) for the older kernels. > > That's a bit unclear, because it's just a kernel-internal flag. > Future kernels may well use that bit for something else. Ah, ok then. I thought it's a user-visible flag. > > Better long term solution would be to use an enum so that the values > are included in debuginfo and perf can read them at runtime :( Right, we also consider BTF to read the values and hopefully get rid of the business of copying kernel headers. Thanks, Namhyung
On Mon, 2023-06-26 at 14:59 -0700, Namhyung Kim wrote: > On Mon, Jun 26, 2023 at 2:53 PM Jakub Kicinski <kuba@kernel.org> wrote: > > > > On Mon, 26 Jun 2023 14:31:39 -0700 Namhyung Kim wrote: > > > Right, we want to keep the headers files in the tools in sync with > > > the kernel ones. And they are used to generate some tables. > > > Full explanation from Arnaldo below. > > > > > > So I'm ok for the msg_flags.c changes, but please refrain from > > > changing the header directly. We will update it later. > > > > > > With that, > > > Acked-by: Namhyung Kim <namhyung@kernel.org> > > > > Ah, missed this email, sounds like this is preferred to Matthieu's > > fix, we'll take this one. > > Hmm.. I believe they are the same when it comes to the > changes in the .c file. > > > > > > Also I wonder if the tool needs to keep the original flag > > > (SENDPAGE_NOTLAST) for the older kernels. > > > > That's a bit unclear, because it's just a kernel-internal flag. > > Future kernels may well use that bit for something else. > > Ah, ok then. I thought it's a user-visible flag. > > > > > Better long term solution would be to use an enum so that the values > > are included in debuginfo and perf can read them at runtime :( > > Right, we also consider BTF to read the values and hopefully > get rid of the business of copying kernel headers. I read all the above as the preferred solution is the one provided by Mat's patch (not touching socket.h, same changes as here in msg_flags.c. As such, I'll restore Mat's patch in PW and will obsolete this one. Please raise a flag if I'm wrong ;) Cheers, Paolo
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 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