From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Howard Chu <howardchu95@gmail.com>
Cc: adrian.hunter@intel.com, irogers@google.com, jolsa@kernel.org,
kan.liang@linux.intel.com, namhyung@kernel.org,
linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
kernel test robot <lkp@intel.com>,
Arnaldo Carvalho de Melo <acme@redhat.com>
Subject: Re: [PATCH v4 5/8] perf test: Add landlock workload
Date: Thu, 4 Jul 2024 16:32:00 -0300 [thread overview]
Message-ID: <Zob4sO8-D-hepgD2@x1> (raw)
In-Reply-To: <20240704124354.904540-6-howardchu95@gmail.com>
On Thu, Jul 04, 2024 at 08:43:51PM +0800, Howard Chu wrote:
> We'll use it to add a regression test for the BTF augmentation of enum
> arguments for tracepoints in 'perf trace':
>
> root@x1:~# perf trace -e landlock_add_rule perf test -w landlock
> 0.000 ( 0.009 ms): perf/747160 landlock_add_rule(ruleset_fd: 11, rule_type: LANDLOCK_RULE_PATH_BENEATH, rule_attr: 0x7ffd8e258594, flags: 45) = -1 EINVAL (Invalid argument)
> 0.011 ( 0.002 ms): perf/747160 landlock_add_rule(ruleset_fd: 11, rule_type: LANDLOCK_RULE_NET_PORT, rule_attr: 0x7ffd8e2585a0, flags: 45) = -1 EINVAL (Invalid argument)
> root@x1:~#
>
> Committer notes:
>
> It was agreed on the discussion (see Link1 below) to shorten then name of
> the workload from 'landlock_add_rule' to 'landlock', and I moved it to a
> separate patch.
>
> Howard fixed the build error found by Namhyung (see Link2 below),
> changing the landlock.h header to the one in source tree, and including
> syscall.h for the '__NR_landlock_add_rule' syscall number.
>
> However, there is another problem. Because of this line in Makefile.config:
> INC_FLAGS += -I$(srctree)/tools/arch/$(SRCARCH)/include/uapi ,
> we'll include 'tools/arch/x86/include/uapi/asm/unistd_64.h' in the source
> tree. But what we want is '/usr/include/asm/unistd_64.h'.
> This hardcoded unistd_64.h in the source tree is not cool for the landlock
> workload because it is a simplified list of syscall numbers for particular
> use cases, we need to discard this search path if we want the
> __NR_landlock_add_rule macro. To solve this problem, Howard added a
> CFLAGS_REMOVE_landlock.o to remove the flag of
> -I$(srctree)/tools/arch/$(SRCARCH)/include/uapi.
>
> The problem above will not occur in some arch, say arm64
> and riscv because they include asm-generic/unistd.h instead. The arch
> that it really affects is the ones that include asm/unistd_32.h and
> asm/unistd_64.h.
>
> linux $ find . -regex './arch/.*/include/uapi/asm/unistd.h' | xargs grep -H --color=auto 'include <asm'
> ./arch/x86/include/uapi/asm/unistd.h:# include <asm/unistd_32.h>
> ./arch/x86/include/uapi/asm/unistd.h:# include <asm/unistd_x32.h>
> ./arch/x86/include/uapi/asm/unistd.h:# include <asm/unistd_64.h>
> ./arch/parisc/include/uapi/asm/unistd.h:#include <asm/unistd_64.h>
> ./arch/parisc/include/uapi/asm/unistd.h:#include <asm/unistd_32.h>
> ./arch/nios2/include/uapi/asm/unistd.h:#include <asm-generic/unistd.h>
> ./arch/mips/include/uapi/asm/unistd.h:#include <asm/sgidefs.h>
> ./arch/mips/include/uapi/asm/unistd.h:#include <asm/unistd_o32.h>
> ./arch/mips/include/uapi/asm/unistd.h:#include <asm/unistd_n64.h>
> ./arch/mips/include/uapi/asm/unistd.h:#include <asm/unistd_n32.h>
> ./arch/s390/include/uapi/asm/unistd.h:#include <asm/unistd_64.h>
> ./arch/s390/include/uapi/asm/unistd.h:#include <asm/unistd_32.h>
> ./arch/arm64/include/uapi/asm/unistd.h:#include <asm-generic/unistd.h>
> ./arch/riscv/include/uapi/asm/unistd.h:#include <asm-generic/unistd.h>
> ./arch/sparc/include/uapi/asm/unistd.h:#include <asm/unistd_64.h>
> ./arch/sparc/include/uapi/asm/unistd.h:#include <asm/unistd_32.h>
> ./arch/xtensa/include/uapi/asm/unistd.h:#include <asm/unistd_32.h>
> ./arch/hexagon/include/uapi/asm/unistd.h:#include <asm-generic/unistd.h>
> ./arch/openrisc/include/uapi/asm/unistd.h:#include <asm-generic/unistd.h>
> ./arch/arm/include/uapi/asm/unistd.h:#include <asm/unistd-eabi.h>
> ./arch/arm/include/uapi/asm/unistd.h:#include <asm/unistd-oabi.h>
> ./arch/alpha/include/uapi/asm/unistd.h:#include <asm/unistd_32.h>
> ./arch/sh/include/uapi/asm/unistd.h:#include <asm/unistd_32.h>
> ./arch/m68k/include/uapi/asm/unistd.h:#include <asm/unistd_32.h>
> ./arch/microblaze/include/uapi/asm/unistd.h:#include <asm/unistd_32.h>
> ./arch/arc/include/uapi/asm/unistd.h:#include <asm-generic/unistd.h>
> ./arch/powerpc/include/uapi/asm/unistd.h:#include <asm/unistd_32.h>
> ./arch/powerpc/include/uapi/asm/unistd.h:#include <asm/unistd_64.h>
> ./arch/csky/include/uapi/asm/unistd.h:#include <asm-generic/unistd.h>
> ./arch/loongarch/include/uapi/asm/unistd.h:#include <asm-generic/unistd.h>
>
> Reported-by: Namhyung Kim <namhyung@kernel.org>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/r/202406250302.E4WaX9Ud-lkp@intel.com/
> Closes: https://lore.kernel.org/linux-perf-users/Zn8TfuQi0iq7bMVD@google.com/
> Suggested-by: Arnaldo Carvalho de Melo <acme@kernel.org>
> Signed-off-by: Howard Chu <howardchu95@gmail.com>
> Tested-by: Arnaldo Carvalho de Melo <acme@kernel.org>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: Ian Rogers <irogers@google.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Kan Liang <kan.liang@linux.intel.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Link1: https://lore.kernel.org/lkml/CAH0uvohaypdTV6Z7O5QSK+va_qnhZ6BP6oSJ89s1c1E0CjgxDA@mail.gmail.com
> Link2: https://lore.kernel.org/linux-perf-users/Zn8TfuQi0iq7bMVD@google.com/
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
> tools/perf/tests/builtin-test.c | 1 +
> tools/perf/tests/tests.h | 1 +
> tools/perf/tests/workloads/Build | 2 ++
> tools/perf/tests/workloads/landlock.c | 38 +++++++++++++++++++++++++++
> 4 files changed, 42 insertions(+)
> create mode 100644 tools/perf/tests/workloads/landlock.c
>
> diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> index c3d84b67ca8e..470a9709427d 100644
> --- a/tools/perf/tests/builtin-test.c
> +++ b/tools/perf/tests/builtin-test.c
> @@ -152,6 +152,7 @@ static struct test_workload *workloads[] = {
> &workload__sqrtloop,
> &workload__brstack,
> &workload__datasym,
> + &workload__landlock,
> };
>
> static int num_subtests(const struct test_suite *t)
> diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
> index 3aa7701ee0e9..6ea2be86b7bf 100644
> --- a/tools/perf/tests/tests.h
> +++ b/tools/perf/tests/tests.h
> @@ -205,6 +205,7 @@ DECLARE_WORKLOAD(leafloop);
> DECLARE_WORKLOAD(sqrtloop);
> DECLARE_WORKLOAD(brstack);
> DECLARE_WORKLOAD(datasym);
> +DECLARE_WORKLOAD(landlock);
>
> extern const char *dso_to_test;
> extern const char *test_objdump_path;
> diff --git a/tools/perf/tests/workloads/Build b/tools/perf/tests/workloads/Build
> index 48bf0d3b0f3d..e132d5d95983 100644
> --- a/tools/perf/tests/workloads/Build
> +++ b/tools/perf/tests/workloads/Build
> @@ -6,8 +6,10 @@ perf-test-y += leafloop.o
> perf-test-y += sqrtloop.o
> perf-test-y += brstack.o
> perf-test-y += datasym.o
> +perf-test-y += landlock.o
>
> CFLAGS_sqrtloop.o = -g -O0 -fno-inline -U_FORTIFY_SOURCE
> CFLAGS_leafloop.o = -g -O0 -fno-inline -fno-omit-frame-pointer -U_FORTIFY_SOURCE
> CFLAGS_brstack.o = -g -O0 -fno-inline -U_FORTIFY_SOURCE
> CFLAGS_datasym.o = -g -O0 -fno-inline -U_FORTIFY_SOURCE
> +CFLAGS_REMOVE_landlock.o = -I$(srctree)/tools/arch/$(SRCARCH)/include/uapi
> diff --git a/tools/perf/tests/workloads/landlock.c b/tools/perf/tests/workloads/landlock.c
> new file mode 100644
> index 000000000000..c4f29b17f2a7
> --- /dev/null
> +++ b/tools/perf/tests/workloads/landlock.c
> @@ -0,0 +1,38 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#include <sys/syscall.h> // for __NR_landlock_add_rule
> +#include <linux/compiler.h>
> +#include <unistd.h>
> +#include "../tests.h"
> +#ifdef __NR_landlock_add_rule
> +#include "../../../../include/uapi/linux/landlock.h"
We can't access files outside tools/
⬢[acme@toolbox linux]$ cd tools/perf/tests/workloads
⬢[acme@toolbox workloads]$ ls -la ../../../../include/uapi/linux/landlock.h
-rw-r--r--. 1 acme acme 9309 Dec 8 2023 ../../../../include/uapi/linux/landlock.h
⬢[acme@toolbox workloads]$ realpath ../../../../include/uapi/linux/landlock.h
/home/acme/git/linux/include/uapi/linux/landlock.h
⬢[acme@toolbox workloads]$
Take a look at the alternative patch I just sent, this was a real "mid
air collision" :-)
- Arnaldo
next prev parent reply other threads:[~2024-07-04 19:32 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-04 12:43 [PATCH v4 0/8] perf trace: Augment enum arguments with BTF Howard Chu
2024-07-04 12:43 ` [PATCH v4 1/8] perf trace: Fix iteration of syscall ids in syscalltbl->entries Howard Chu
2024-07-04 12:43 ` [PATCH v4 2/8] perf trace: BTF-based enum pretty printing for syscall args Howard Chu
2024-07-04 12:43 ` [PATCH v4 3/8] perf trace: Augment non-syscall tracepoints with enum arguments with BTF Howard Chu
2024-07-04 12:43 ` [PATCH v4 4/8] perf trace: Filter enum arguments with enum names Howard Chu
2024-07-04 12:43 ` [PATCH v4 5/8] perf test: Add landlock workload Howard Chu
2024-07-04 19:32 ` Arnaldo Carvalho de Melo [this message]
2024-07-05 9:14 ` Howard Chu
2024-07-04 12:43 ` [PATCH v4 6/8] perf test trace_btf_enum: Add regression test for the BTF augmentation of enums in 'perf trace' Howard Chu
2024-07-04 12:43 ` [PATCH v4 7/8] perf trace: Introduce trace__btf_scnprintf() Howard Chu
2024-07-04 12:43 ` [PATCH v4 8/8] perf trace: Remove arg_fmt->is_enum, we can get that from the BTF type Howard Chu
2024-07-04 19:52 ` [PATCH v4 0/8] perf trace: Augment enum arguments with BTF Arnaldo Carvalho de Melo
2024-07-05 9:10 ` Howard Chu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Zob4sO8-D-hepgD2@x1 \
--to=acme@kernel.org \
--cc=acme@redhat.com \
--cc=adrian.hunter@intel.com \
--cc=howardchu95@gmail.com \
--cc=irogers@google.com \
--cc=jolsa@kernel.org \
--cc=kan.liang@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=lkp@intel.com \
--cc=namhyung@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.