* Re: [PATCH v2] riscv: Only consider swbp/ss handlers for correct privileged mode
From: Puranjay Mohan @ 2023-08-28 15:35 UTC (permalink / raw)
To: bjorn
Cc: aou, bjorn, guoren, linux-kernel, linux-riscv, linux-trace-kernel,
lkp, namcaov, palmer, paul.walmsley, Puranjay Mohan
In-Reply-To: <20230827114003.224958-1-bjorn@kernel.org>
Tested-by: Puranjay Mohan <puranjay12@gmail.com>
^ permalink raw reply
* Re: [PATCH v4] tracepoint: add new `tcp:tcp_ca_event` trace event
From: Jakub Kicinski @ 2023-08-28 19:45 UTC (permalink / raw)
To: Zheao Li
Cc: edumazet, mhiramat, rostedt, davem, dsahern, pabeni, netdev,
linux-kernel, linux-trace-kernel, bpf
In-Reply-To: <20230825133246.344364-1-me@manjusaka.me>
On Fri, 25 Aug 2023 13:32:47 +0000 Zheao Li wrote:
> This the 4th version of the patch, the previous discusstion is here
>
> https://lore.kernel.org/linux-trace-kernel/20230807183308.9015-1-me@manjusaka.me/
>
> In this version of the code, here's some different:
>
> 1. The event name has been changed from `tcp_ca_event_set` to
> `tcp_ca_event`
>
> 2. Output the current protocol family in TP_printk
>
> 3. Show the ca_event symbol instead of the original number
>
> But the `./scripts/checkpatch.pl` has been failed to check this patch,
> because we sill have some code error in ./scripts/checkpatch.pl(in
> another world, the test would be failed when we use the
> scripts/checkpatch.pl to check the events/tcp.h
>
> Feel free to ask me if you have have any issues and ideas.
## Form letter - net-next-closed
The merge window for v6.6 has begun and therefore net-next is closed
for new drivers, features, code refactoring and optimizations.
We are currently accepting bug fixes only.
Please repost when net-next reopens after Sept 11th.
RFC patches sent for review only are obviously welcome at any time.
See: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#development-cycle
--
pw-bot: defer
^ permalink raw reply
* Re: [PATCH v7 bpf-next 09/10] bpftool: Add perf event names
From: Quentin Monnet @ 2023-08-29 9:20 UTC (permalink / raw)
To: Yafang Shao, ast, daniel, john.fastabend, andrii, martin.lau,
song, yhs, kpsingh, sdf, haoluo, jolsa, rostedt, mhiramat
Cc: bpf, linux-trace-kernel, Jiri Olsa
In-Reply-To: <20230709025630.3735-10-laoar.shao@gmail.com>
On 09/07/2023 03:56, Yafang Shao wrote:
> Add new functions and macros to get perf event names. These names except
> the perf_type_name are all copied from
> tool/perf/util/{parse-events,evsel}.c, so that in the future we will
> have a good chance to use the same code.
>
> Suggested-by: Jiri Olsa <olsajiri@gmail.com>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> Reviewed-by: Quentin Monnet <quentin@isovalent.com>
> Acked-by: Jiri Olsa <jolsa@kernel.org>
> ---
> tools/bpf/bpftool/link.c | 67 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 67 insertions(+)
>
> diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
> index a4f5a436777f..8e4d9176a6e8 100644
> --- a/tools/bpf/bpftool/link.c
> +++ b/tools/bpf/bpftool/link.c
[...]
> +#define perf_event_name(array, id) ({ \
> + const char *event_str = NULL; \
> + \
> + if ((id) >= 0 && (id) < ARRAY_SIZE(array)) \
Hi Yafang,
I'm observing build warnings when building bpftool after you series:
link.c: In function ‘perf_config_hw_cache_str’:
link.c:86:18: warning: comparison of unsigned expression in ‘>= 0’ is always true [-Wtype-limits]
86 | if ((id) >= 0 && (id) < ARRAY_SIZE(array)) \
| ^~
link.c:320:20: note: in expansion of macro ‘perf_event_name’
320 | hw_cache = perf_event_name(evsel__hw_cache, config & 0xff);
| ^~~~~~~~~~~~~~~
[... more of the same for the other calls to perf_event_name ...]
(using GCC 11.4.0)
Could you please send a follow-up to suppress them? We're always passing
unsigned, so it should be safe to drop the check on (id) >= 0.
Thanks,
Quentin
^ permalink raw reply
* Re: [PATCH v7 bpf-next 09/10] bpftool: Add perf event names
From: Yafang Shao @ 2023-08-29 14:35 UTC (permalink / raw)
To: Quentin Monnet
Cc: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
kpsingh, sdf, haoluo, jolsa, rostedt, mhiramat, bpf,
linux-trace-kernel, Jiri Olsa
In-Reply-To: <a35d9a2d-54a0-49ec-9ed1-8fcf1369d3cc@isovalent.com>
On Tue, Aug 29, 2023 at 5:20 PM Quentin Monnet <quentin@isovalent.com> wrote:
>
> On 09/07/2023 03:56, Yafang Shao wrote:
> > Add new functions and macros to get perf event names. These names except
> > the perf_type_name are all copied from
> > tool/perf/util/{parse-events,evsel}.c, so that in the future we will
> > have a good chance to use the same code.
> >
> > Suggested-by: Jiri Olsa <olsajiri@gmail.com>
> > Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> > Reviewed-by: Quentin Monnet <quentin@isovalent.com>
> > Acked-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> > tools/bpf/bpftool/link.c | 67 ++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 67 insertions(+)
> >
> > diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
> > index a4f5a436777f..8e4d9176a6e8 100644
> > --- a/tools/bpf/bpftool/link.c
> > +++ b/tools/bpf/bpftool/link.c
>
> [...]
>
> > +#define perf_event_name(array, id) ({ \
> > + const char *event_str = NULL; \
> > + \
> > + if ((id) >= 0 && (id) < ARRAY_SIZE(array)) \
>
> Hi Yafang,
>
> I'm observing build warnings when building bpftool after you series:
>
> link.c: In function ‘perf_config_hw_cache_str’:
> link.c:86:18: warning: comparison of unsigned expression in ‘>= 0’ is always true [-Wtype-limits]
> 86 | if ((id) >= 0 && (id) < ARRAY_SIZE(array)) \
> | ^~
> link.c:320:20: note: in expansion of macro ‘perf_event_name’
> 320 | hw_cache = perf_event_name(evsel__hw_cache, config & 0xff);
> | ^~~~~~~~~~~~~~~
> [... more of the same for the other calls to perf_event_name ...]
>
> (using GCC 11.4.0)
>
> Could you please send a follow-up to suppress them? We're always passing
> unsigned, so it should be safe to drop the check on (id) >= 0.
>
Thanks for your report.
will send a fix soon.
--
Regards
Yafang
^ permalink raw reply
* Re: [PATCH v3] scripts/link-vmlinux.sh: Add alias to duplicate symbols for kallsyms
From: Francis Laniel @ 2023-08-29 14:51 UTC (permalink / raw)
To: Alessandro Carminati (Red Hat)
Cc: Masami Hiramatsu, Steven Rostedt, Daniel Bristot de Oliveira,
Josh Poimboeuf, Masahiro Yamada, Luis Chamberlain,
Nathan Chancellor, Nick Desaulniers, Nicolas Schier,
Alexander Lobakin, Nick Alcock, Kris Van Hees, Eugene Loh,
Viktor Malik, Alessandro Carminati, linux-kbuild, linux-kernel,
linux-trace-kernel, live-patching
In-Reply-To: <20230828080423.3539686-1-alessandro.carminati@gmail.com>
Hi.
Le lundi 28 août 2023, 10:04:23 CEST Alessandro Carminati (Red Hat) a écrit :
> From: Alessandro Carminati <alessandro.carminati@gmail.com>
>
> It is not uncommon for drivers or modules related to similar peripherals
> to have symbols with the exact same name.
> While this is not a problem for the kernel's binary itself, it becomes an
> issue when attempting to trace or probe specific functions using
> infrastructure like ftrace or kprobe.
Thank you for the patch!
I tested it and it works fine:
root@vm-amd64:~# grep -m 6 ' name_show' /proc/kallsyms
ffffffff81108330 t name_show
ffffffff81108330 t name_show@_kernel_irq_irqdesc_c_264
ffffffff816d2dd0 t name_show
ffffffff816d2dd0 t name_show@_drivers_pnp_card_c_186
ffffffff81814990 t name_show
ffffffff81814990 t name_show@_drivers_gpu_drm_i915_gt_sysfs_engines_c_26
On the paper, it combines well with my other patch forbidding tracing non
unique symbols.
I will nonetheless need to try yours on top of mine and I will let you know
the result, even though everything should go well.
Regarding the code itself, from my review your implementation is good.
I was just wondering if we cannot avoid the first sort BY_NAME by modifying
find_duplicates() and the returned structured, but the index you are using
later should reduce the loopthrough time and compensate the first sort.
I have some specific comments but they are mainly nits:
> The tracing subsystem relies on the `nm -n vmlinux` output, which provides
> symbol information from the kernel's ELF binary. However, when multiple
> symbols share the same name, the standard nm output does not differentiate
> between them. This can lead to confusion and difficulty when trying to
> probe the intended symbol.
>
> ~ # cat /proc/kallsyms | grep " name_show"
> ffffffff8c4f76d0 t name_show
> ffffffff8c9cccb0 t name_show
> ffffffff8cb0ac20 t name_show
> ffffffff8cc728c0 t name_show
> ffffffff8ce0efd0 t name_show
> ffffffff8ce126c0 t name_show
> ffffffff8ce1dd20 t name_show
> ffffffff8ce24e70 t name_show
> ffffffff8d1104c0 t name_show
> ffffffff8d1fe480 t name_show
>
> **kas_alias** addresses this challenge by extending the symbol names with
> unique suffixes during the kernel build process.
> The newly created aliases for these duplicated symbols are unique names
> that can be fed to the ftracefs interface. By doing so, it enables
> previously unreachable symbols to be probed.
>
> ~ # cat /proc/kallsyms | grep " name_show"
> ffffffff974f76d0 t name_show
> ffffffff974f76d0 t name_show__alias__6340
This output is outdated.
This is not a big problem but it would be better with the updated one as the
"@_file_line" is better.
> ffffffff979cccb0 t name_show
> ffffffff979cccb0 t name_show__alias__6341
> ffffffff97b0ac20 t name_show
> ffffffff97b0ac20 t name_show__alias__6342
> ffffffff97c728c0 t name_show
> ffffffff97c728c0 t name_show__alias__6343
> ffffffff97e0efd0 t name_show
> ffffffff97e0efd0 t name_show__alias__6344
> ffffffff97e126c0 t name_show
> ffffffff97e126c0 t name_show__alias__6345
> ffffffff97e1dd20 t name_show
> ffffffff97e1dd20 t name_show__alias__6346
> ffffffff97e24e70 t name_show
> ffffffff97e24e70 t name_show__alias__6347
> ffffffff981104c0 t name_show
> ffffffff981104c0 t name_show__alias__6348
> ffffffff981fe480 t name_show
> ffffffff981fe480 t name_show__alias__6349
> ~ # echo "p:kprobes/evnt1 name_show__alias__6349" \
>
> > >/sys/kernel/tracing/kprobe_events
>
> ~ # cat /sys/kernel/tracing/kprobe_events
> p:kprobes/evnt1 name_show__alias__6349
>
> Changes from v1:
> - Integrated changes requested by Masami to exclude symbols with prefixes
> "_cfi" and "_pfx".
> - Introduced a small framework to handle patterns that need to be excluded
> from the alias production.
> - Excluded other symbols using the framework.
> - Introduced the ability to discriminate between text and data symbols.
> - Added two new config symbols in this version: CONFIG_KALLSYMS_ALIAS_DATA,
> which allows data for data, and CONFIG_KALLSYMS_ALIAS_DATA_ALL, which
> excludes all filters and provides an alias for each duplicated symbol.
>
> https://lore.kernel.org/all/20230711151925.1092080-1-alessandro.carminati@gm
> ail.com/
>
> Changes from v2:
> - Alias tags are created by querying DWARF information from the vmlinux.
> - The filename + line number is normalized and appended to the original
> name. - The tag begins with '@' to indicate the symbol source.
> - Not a change, but worth mentioning, since the alias is added to the
> existing list, the old duplicated name is preserved, and the livepatch way
> of dealing with duplicates is maintained.
> - Acknowledging the existence of scenarios where inlined functions declared
> in header files may result in multiple copies due to compiler behavior,
> though it is not actionable as it does not pose an operational issue.
> - Highlighting a single exception where the same name refers to different
> functions: the case of "compat_binfmt_elf.c," which directly includes
> "binfmt_elf.c" producing identical function copies in two separate
> modules.
>
> sample from new v3
>
> ~ # cat /proc/kallsyms | grep gic_mask_irq
> ffffd0b03c04dae4 t gic_mask_irq
> ffffd0b03c04dae4 t gic_mask_irq@_drivers_irqchip_irq-gic_c_167
> ffffd0b03c050960 t gic_mask_irq
> ffffd0b03c050960 t gic_mask_irq@_drivers_irqchip_irq-gic-v3_c_404
> ~ #
>
> https://lore.kernel.org/all/20230714150326.1152359-1-alessandro.carminati@gm
> ail.com/
>
> Signed-off-by: Alessandro Carminati (Red Hat)
> <alessandro.carminati@gmail.com> ---
> init/Kconfig | 36 ++++
> scripts/Makefile | 4 +
> scripts/kas_alias/Makefile | 4 +
> scripts/kas_alias/a2l.c | 268 ++++++++++++++++++++++++++++
> scripts/kas_alias/a2l.h | 32 ++++
> scripts/kas_alias/duplicates_list.c | 70 ++++++++
> scripts/kas_alias/duplicates_list.h | 15 ++
> scripts/kas_alias/item_list.c | 230 ++++++++++++++++++++++++
> scripts/kas_alias/item_list.h | 26 +++
> scripts/kas_alias/kas_alias.c | 217 ++++++++++++++++++++++
> scripts/link-vmlinux.sh | 11 +-
> 11 files changed, 910 insertions(+), 3 deletions(-)
> create mode 100644 scripts/kas_alias/Makefile
> create mode 100644 scripts/kas_alias/a2l.c
> create mode 100644 scripts/kas_alias/a2l.h
> create mode 100644 scripts/kas_alias/duplicates_list.c
> create mode 100644 scripts/kas_alias/duplicates_list.h
> create mode 100644 scripts/kas_alias/item_list.c
> create mode 100644 scripts/kas_alias/item_list.h
> create mode 100644 scripts/kas_alias/kas_alias.c
>
> diff --git a/init/Kconfig b/init/Kconfig
> index f7f65af4ee12..bc69fcd9cbc8 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1737,6 +1737,42 @@ config KALLSYMS_BASE_RELATIVE
> time constants, and no relocation pass is required at runtime to fix
> up the entries based on the runtime load address of the kernel.
>
> +config KALLSYMS_ALIAS
> + bool "Produces alias for duplicated symbols" if EXPERT
> + depends on KALLSYMS && (DEBUG_INFO_DWARF4 || DEBUG_INFO_DWARF5)
> + help
> + It is not uncommon for drivers or modules related to similar
> + peripherals to have symbols with the exact same name.
> + While this is not a problem for the kernel's binary itself, it
> + becomes an issue when attempting to trace or probe specific
> + functions using infrastructure like ftrace or kprobe.
> +
> + This option addresses this challenge by extending the symbol names
> + with unique suffixes during the kernel build process.
> + The newly created aliases for these duplicated symbols are unique
> + names that can be fed to the ftrace sysfs interface. By doing so, it
> + enables previously unreachable symbols to be probed.
> +
> +config CONFIG_KALLSYMS_ALIAS_DATA
> + bool "Produces alias also for data"
> + depends on KALLSYMS_ALIAS
> + help
> + Sometimes it can be useful to refer to data. In live patch
scenarios,
> + you may find yourself needing to use symbols that are shared with
> + other functions. Since symbols face the same issue as functions,
this
> + option allows you to create aliases for data as well.
> +
> +config CONFIG_KALLSYMS_ALIAS_DATA_ALL
> + bool "Removes all filter when producing data alias"
> + depends on CONFIG_KALLSYMS_ALIAS_DATA
> + help
> + When selecting data aliases, not all symbols are included in the set
> + This is because many symbols are unlikely to be used. If you choose
> + to have an alias for all data symbols, be aware that it will
> + significantly increase the size.
> +
> + If unsure, say N.
> +
> # end of the "standard kernel features (expert users)" menu
>
> # syscall, maps, verifier
> diff --git a/scripts/Makefile b/scripts/Makefile
> index 32b6ba722728..65fafe17cfe5 100644
> --- a/scripts/Makefile
> +++ b/scripts/Makefile
> @@ -49,3 +49,7 @@ subdir-$(CONFIG_SECURITY_SELINUX) += selinux
>
> # Let clean descend into subdirs
> subdir- += basic dtc gdb kconfig mod
> +
> +# KALLSyms alias
> +subdir-$(CONFIG_KALLSYMS_ALIAS) += kas_alias
> +
> diff --git a/scripts/kas_alias/Makefile b/scripts/kas_alias/Makefile
> new file mode 100644
> index 000000000000..e1fde69232b4
> --- /dev/null
> +++ b/scripts/kas_alias/Makefile
> @@ -0,0 +1,4 @@
> +# SPDX-License-Identifier: GPL-2.0
> +hostprogs-always-$(CONFIG_KALLSYMS_ALIAS) += kas_alias
> +
> +kas_alias-objs := duplicates_list.o item_list.o kas_alias.o a2l.o
> diff --git a/scripts/kas_alias/a2l.c b/scripts/kas_alias/a2l.c
> new file mode 100644
> index 000000000000..a9692ac30180
> --- /dev/null
> +++ b/scripts/kas_alias/a2l.c
> @@ -0,0 +1,268 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <sys/types.h>
> +#include <sys/wait.h>
> +#include <string.h>
> +#include <stdint.h>
> +#include <stdbool.h>
> +
> +#include "a2l.h"
> +
> +int addr2line_pid = -1;
> +int a2l_in[2];
> +int a2l_out[2];
> +char line[MAX_BUF];
> +char vmlinux_path[MAX_BUF];
> +char addr2line_cmd[MAX_CMD_LEN];
> +FILE *a2l_stdin, *a2l_stdout;
> +
> +static char *normalize_path(const char *input_path, char *output_path)
> +{
> + char *prev_token = NULL;
> + char *delimiter = "/";
> + char inbuf[MAX_BUF];
Rather than using memset below you can initialize it like this above:
char inbuf[MAX_BUF] = {0};
> + char *token;
> + char *pos;
> +
> + memset(inbuf, 0, MAX_BUF);
> + *output_path = '\0';
> + strncpy(inbuf, input_path, MAX_BUF);
> + if (!input_path || !output_path || strlen(input_path) == 0)
> + return NULL;
> +
> + token = strtok(inbuf, delimiter);
> + while (token) {
> + if (strcmp(token, "..") == 0 && prev_token) {
> + pos = strrchr(output_path, '/');
> + if (pos)
> + *pos = '\0';
> +
> + } else if (strcmp(token, ".") != 0) {
> + strcat(output_path, "/");
> + strcat(output_path, token);
> + }
> +
> + prev_token = token;
> + token = strtok(NULL, delimiter);
> + }
> +
> + return output_path;
> +}
> +
> +static void path_of(const char *full_path, char *path)
> +{
> + const char *last_slash = strrchr(full_path, '/');
> + size_t path_length;
> + char cwd[MAX_BUF];
> +
> + if (!last_slash) {
> + if (getcwd(cwd, sizeof(cwd)))
> + strcpy(path, cwd);
> + else
> + strcpy(path, ".");
> + } else {
> + path_length = last_slash - full_path;
> + strncpy(path, full_path, path_length);
> + path[path_length] = '\0';
> + }
> +}
> +
> +static bool file_exists(const char *file_path)
> +{
> + FILE *file;
> +
> + file = fopen(file_path, "r");
> + if (file) {
> + fclose(file);
> + return true;
> + }
> + return false;
> +}
> +
> +int addr2line_init(const char *cmd, const char *vmlinux)
> +{
> + if ((!file_exists(cmd)) || (!file_exists(vmlinux))) {
> + printf("file not found\n");
> + return 0;
> + }
> +
> + path_of(vmlinux, vmlinux_path);
> + if (pipe(a2l_in) == -1) {
> + printf("Failed to create pipe\n");
> + return 0;
> + }
> +
> + if (pipe(a2l_out) == -1) {
> + printf("Failed to create pipe\n");
> + return 0;
> + }
> +
> + addr2line_pid = fork();
> + if (addr2line_pid == -1) {
> + printf("Failed to fork process\n");
> + close(a2l_in[P_READ]);
> + close(a2l_in[P_WRITE]);
> + close(a2l_out[P_READ]);
> + close(a2l_out[P_WRITE]);
> + return 0;
> + }
> +
> + if (addr2line_pid == 0) {
> + dup2(a2l_in[P_READ], 0);
> + dup2(a2l_out[P_WRITE], 1);
> + close(a2l_in[P_WRITE]);
> + close(a2l_out[P_READ]);
> +
> + execlp(cmd, cmd, ADDR2LINE_ARGS, vmlinux, NULL);
> +
> + printf("Failed to execute addr2line command\n");
> + exit(1);
> + } else {
> + close(a2l_in[P_READ]);
> + close(a2l_out[P_WRITE]);
> + }
> +
> + a2l_stdin = fdopen(a2l_in[P_WRITE], "w");
> + if (!a2l_stdin) {
> + printf("Failed to open pipe a2l_in\n");
> + return 0;
> + }
> +
> + a2l_stdout = fdopen(a2l_out[P_READ], "r");
> + if (!a2l_stdout) {
> + printf("Failed to open pipe a2l_out\n");
> + fclose(a2l_stdin);
> + return 0;
> + }
> +
> + return 1;
> +}
> +
> +const char *remove_subdir(const char *home, const char *f_path)
> +{
> + int i = 0;
> +
> + while (*(home + i) == *(f_path + i))
Using home[i] and f_path[i] should do the trick too.
> + i++;
> +
> + return (strlen(home) != i) ? NULL : f_path + i;
> +}
> +
> +char *addr2line_get_lines(uint64_t address)
> +{
> + char buf[MAX_BUF];
> +
> + fprintf(a2l_stdin, "%08lx\n", address);
> + fflush(a2l_stdin);
> +
> + if (!fgets(line, sizeof(line), a2l_stdout)) {
> + printf("Failed to read lines from addr2line\n");
> + return NULL;
> + }
> +
> + if (!fgets(line, sizeof(line), a2l_stdout)) {
> + printf("Failed to read lines from addr2line\n");
> + return NULL;
> + }
> + line[strcspn(line, "\n")] = '\0';
> + strncpy(buf, line, MAX_BUF);
> + return normalize_path(buf, line);
> +}
> +
> +int addr2line_cleanup(void)
> +{
> + int status;
> +
> + if (addr2line_pid != -1) {
> + kill(addr2line_pid, SIGKILL);
> + waitpid(addr2line_pid, &status, 0);
> + fclose(a2l_stdin);
> + fclose(a2l_stdout);
> + addr2line_pid = -1;
> + }
> +
> + return 1;
> +}
> +
> +static char *find_executable(const char *command)
Nice function, I would rather have tried to execve() the command.
> +{
> + char *path_env = getenv("PATH");
> + char *executable_path;
> + char *path_copy;
> + char *path;
> + int n;
> +
> + if (!path_env)
> + return NULL;
> +
> + path_copy = strdup(path_env);
> + if (!path_copy)
> + return NULL;
> +
> + path = strtok(path_copy, ":");
> + while (path) {
> + n = snprintf(0, 0, "%s/%s", path, command);
> + executable_path = (char *)malloc(n + 1);
> + snprintf(executable_path, n + 1, "%s/%s", path, command);
> + if (access(executable_path, X_OK) == 0) {
> + free(path_copy);
> + return executable_path;
> + }
> +
> + path = strtok(NULL, ":");
> + free(executable_path);
> + executable_path = NULL;
> + }
> +
> + free(path_copy);
> + if (executable_path)
> + free(executable_path);
If you arrive here, executable_path should be NULL, right?
> + return NULL;
> +}
> +
> +const char *get_addr2line(int mode)
> +{
> + char *buf = "";
> +
> + switch (mode) {
> + case A2L_CROSS:
> + buf = getenv("CROSS_COMPILE");
> + memcpy(addr2line_cmd, buf, strlen(buf));
> + case A2L_DEFAULT:
> + memcpy(addr2line_cmd + strlen(buf), ADDR2LINE,
strlen(ADDR2LINE));
> + buf = find_executable(addr2line_cmd);
> + if (buf) {
> + memcpy(addr2line_cmd, buf, strlen(buf));
> + free(buf);
> + }
> + return addr2line_cmd;
> + case A2L_LLVM:
> + default:
> + return NULL;
> + }
> +}
> +
> +char *get_vmlinux(char *input)
> +{
> + const char *match_string1 = ".syms";
> + const char *match_string2 = ".tmp_vmlinux.kallsyms";
> + char *result = NULL;
> + char *match_pos;
> +
> + match_pos = strstr(input, match_string1);
> + if (!match_pos)
> + return NULL;
> +
> + match_pos = strstr(input, match_string2);
> + if (!match_pos)
> + return NULL;
> +
> + result = strdup(input);
> + match_pos = strstr(result, match_string1);
> + *match_pos = '\0';
> + return result;
> +}
> diff --git a/scripts/kas_alias/a2l.h b/scripts/kas_alias/a2l.h
> new file mode 100644
> index 000000000000..ca6419229dde
> --- /dev/null
> +++ b/scripts/kas_alias/a2l.h
> @@ -0,0 +1,32 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +#ifndef A2L_H
> +#define A2L_H
> +#include <stdint.h>
> +
> +#define ADDR2LINE "addr2line"
> +#define ADDR2LINE_ARGS "-fe"
> +//#define VMLINUX "vmlinux"
> +#define MAX_BUF 4096
> +#define MAX_CMD_LEN 256
> +#define P_READ 0
> +#define P_WRITE 1
> +#define A2L_DEFAULT 1
> +#define A2L_CROSS 2
> +#define A2L_LLVM 3
> +#define A2L_MAKE_VALUE 2
> +
> +extern int addr2line_pid;
> +extern int a2l_in[2];
> +extern int a2l_out[2];
> +extern char line[MAX_BUF];
> +extern char vmlinux_path[MAX_BUF];
> +extern char addr2line_cmd[MAX_CMD_LEN];
> +
> +int addr2line_init(const char *cmd, const char *vmlinux);
> +char *addr2line_get_lines(uint64_t address);
> +int addr2line_cleanup(void);
> +const char *remove_subdir(const char *home, const char *f_path);
> +const char *get_addr2line(int mode);
> +char *get_vmlinux(char *input);
> +
> +#endif
> diff --git a/scripts/kas_alias/duplicates_list.c
> b/scripts/kas_alias/duplicates_list.c new file mode 100644
> index 000000000000..e7a3d2917937
> --- /dev/null
> +++ b/scripts/kas_alias/duplicates_list.c
> @@ -0,0 +1,70 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <stdlib.h>
> +#include <stdbool.h>
> +
> +#include "item_list.h"
> +#include "duplicates_list.h"
> +
> +struct duplicate_item *find_duplicates(struct item *list)
> +{
> + struct duplicate_item *current_duplicate = NULL;
> + struct duplicate_item *duplicates = NULL;
> + struct duplicate_item *new_duplicate;
> + struct item *current_item = list;
> + bool prev_was_duplicate = false;
> + struct item *prev_item = NULL;
> +
> + while (current_item) {
> + if ((prev_item && (strcmp(current_item->symb_name, prev_item-
>symb_name)
> == 0)) || + prev_was_duplicate) {
> + if (!duplicates) {
> + duplicates = malloc(sizeof(struct
duplicate_item));
> + if (!duplicates)
> + return NULL;
> +
> + duplicates->original_item = prev_item;
> + duplicates->next = NULL;
> + current_duplicate = duplicates;
> + } else {
> + new_duplicate = malloc(sizeof(struct
duplicate_item));
> + if (!new_duplicate) {
> + free_duplicates(&duplicates);
> + return NULL;
> + }
> +
> + new_duplicate->original_item = prev_item;
> + new_duplicate->next = NULL;
> + current_duplicate->next = new_duplicate;
> + current_duplicate = new_duplicate;
> +
> + if ((strcmp(current_item->symb_name, prev_item-
>symb_name) != 0) &&
> + (prev_was_duplicate))
> + prev_was_duplicate = false;
> + else
> + prev_was_duplicate = true;
You can remove the branch and set prev_was_duplicate to:
prev_was_duplicate = strcmp(current_item->symb_name, prev_item->symb_name) !=
0 && prev_was_duplicate;
> + }
> + }
> +
> + prev_item = current_item;
> + current_item = current_item->next;
> + }
> +
> + return duplicates;
> +}
> +
> +void free_duplicates(struct duplicate_item **duplicates)
> +{
> + struct duplicate_item *duplicates_iterator = *duplicates;
> + struct duplicate_item *app;
> +
> + while (duplicates_iterator) {
> + app = duplicates_iterator;
> + duplicates_iterator = duplicates_iterator->next;
> + free(app);
> + }
> +
> + *duplicates = NULL;
> +}
> diff --git a/scripts/kas_alias/duplicates_list.h
> b/scripts/kas_alias/duplicates_list.h new file mode 100644
> index 000000000000..76aa73e584bc
> --- /dev/null
> +++ b/scripts/kas_alias/duplicates_list.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +#ifndef DUPLICATES_LIST_H
> +#define DUPLICATES_LIST_H
> +
> +#include "item_list.h"
> +
> +struct duplicate_item {
> + struct item *original_item;
> + struct duplicate_item *next;
> +};
> +
> +struct duplicate_item *find_duplicates(struct item *list);
> +void free_duplicates(struct duplicate_item **duplicates);
> +
> +#endif
> diff --git a/scripts/kas_alias/item_list.c b/scripts/kas_alias/item_list.c
> new file mode 100644
> index 000000000000..48f2e525592a
> --- /dev/null
> +++ b/scripts/kas_alias/item_list.c
> @@ -0,0 +1,230 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <stdint.h>
> +#include <string.h>
> +#include <stdbool.h>
> +#include <assert.h>
> +#include "item_list.h"
> +
> +#define CHECK_ORDER_BY_ADDRESS(sort_by, current, temp, op) \
> + ((sort_by) == BY_ADDRESS && (current)->addr op (temp)->addr)
> +#define CHECK_ORDER_BY_NAME(sort_by, current, temp, op) \
> + ((sort_by) == BY_NAME && strcmp((current)->symb_name, (temp)-
>symb_name)
> op 0) +
> +struct item *list_index[96] = {0};
> +
> +void build_index(struct item *list)
> +{
> + char current_first_letter = ' ';
> + struct item *current = list;
> +
> + while (current) {
> + if (current->symb_name[0] != current_first_letter) {
> + current_first_letter = current->symb_name[0];
> + list_index[current_first_letter - 32] = current;
> + }
> + current = current->next;
> + }
> +}
> +
> +struct item *add_item(struct item **list, const char *name, char stype,
> uint64_t addr) +{
> + struct item *new_item;
> + struct item *current;
> +
> + new_item = malloc(sizeof(struct item));
> + if (!new_item)
> + return NULL;
> +
> + strncpy(new_item->symb_name, name, MAX_NAME_SIZE);
> + new_item->symb_name[MAX_NAME_SIZE - 1] = '\0';
> + new_item->addr = addr;
> + new_item->stype = stype;
> + new_item->next = NULL;
> +
> + if (!(*list)) {
> + *list = new_item;
> + } else {
> + current = *list;
> + while (current->next)
> + current = current->next;
> +
> + current->next = new_item;
> + }
> + return new_item;
> +}
> +
> +void sort_list(struct item **list, int sort_by)
> +{
> + struct item *current = *list;
> + struct item *sorted = NULL;
> + struct item *next_item;
> + struct item *temp;
> +
> + if (!(*list) || !((*list)->next))
> + return;
> +
> + while (current) {
> + next_item = current->next;
> + if (!sorted ||
> + (CHECK_ORDER_BY_ADDRESS(sort_by, current, sorted, <) ||
> + CHECK_ORDER_BY_NAME(sort_by, current, sorted, >=))) {
> + current->next = sorted;
> + sorted = current;
> + } else {
> + temp = sorted;
> + while (temp->next &&
> + (CHECK_ORDER_BY_ADDRESS(sort_by, current, temp-
>next, >=) ||
> + CHECK_ORDER_BY_NAME(sort_by, current, temp->next,
>=)))
> + temp = temp->next;
> +
> + current->next = temp->next;
> + temp->next = current;
> + }
> + current = next_item;
> + }
> +
> + *list = sorted;
> +}
> +
> +struct item *merge(struct item *left, struct item *right, int sort_by)
> +{
> + struct item *current = NULL;
> + struct item *result = NULL;
> +
> + if (!left)
> + return right;
> + if (!right)
> + return left;
> +
> + if (sort_by == BY_NAME) {
> + if (strcmp(left->symb_name, right->symb_name) <= 0) {
> + result = left;
> + left = left->next;
> + } else {
> + result = right;
> + right = right->next;
> + }
> + } else {
> + if (sort_by == BY_ADDRESS) {
> + if (left->addr <= right->addr) {
> + result = left;
> + left = left->next;
> + } else {
> + result = right;
> + right = right->next;
> + }
> + }
> + }
> +
> + current = result;
> +
> + while (left && right) {
> + if (sort_by == BY_NAME) {
> + if (strcmp(left->symb_name, right->symb_name) <= 0) {
> + current->next = left;
> + left = left->next;
> + } else {
> + current->next = right;
> + right = right->next;
> + }
> + } else {
> + if (sort_by == BY_ADDRESS) {
> + if (left->addr <= right->addr) {
> + current->next = left;
> + left = left->next;
> + } else {
> + current->next = right;
> + right = right->next;
> + }
> + }
> + }
> +
> + current = current->next;
> + }
> +
> + if (left) {
> + current->next = left;
> + } else {
> + if (right)
> + current->next = right;
> + }
> +
> + return result;
> +}
> +
> +struct item *merge_sort(struct item *head, int sort_by)
> +{
> + struct item *right;
> + struct item *slow;
> + struct item *fast;
> + struct item *left;
> +
> + if (!head || !head->next)
> + return head;
> +
> + slow = head;
> + fast = head->next;
> +
> + while (fast && fast->next) {
> + slow = slow->next;
> + fast = fast->next->next;
> + }
> +
> + left = head;
> + right = slow->next;
> + slow->next = NULL;
> +
> + left = merge_sort(left, sort_by);
> + right = merge_sort(right, sort_by);
> +
> + return merge(left, right, sort_by);
> +}
> +
> +void sort_list_m(struct item **head, int sort_by)
> +{
> + if (!(*head) || !((*head)->next))
> + return;
> +
> + *head = merge_sort(*head, sort_by);
> +}
> +
> +int insert_after(struct item *list, const uint64_t search_addr,
> + const char *name, uint64_t addr, char stype)
> +{
> + struct item *new_item;
> + struct item *current;
> + int ret = 0;
> +
> + current = (list_index[name[0] - 32]) ? list_index[name[0] - 32] :
list;
> + while (current) {
> + if (current->addr == search_addr) {
> + new_item = malloc(sizeof(struct item));
> + if (!new_item)
> + return ret;
> + strncpy(new_item->symb_name, name, MAX_NAME_SIZE);
> + new_item->symb_name[MAX_NAME_SIZE - 1] = '\0';
> + new_item->addr = addr;
> + new_item->stype = stype;
> + new_item->next = current->next;
> + current->next = new_item;
> + ret = 1;
> + break;
> + }
> + current = current->next;
> + }
> + return ret;
> +}
> +
> +void free_items(struct item **head)
> +{
> + struct item *app, *item_iterator = *head;
> +
> + while (item_iterator) {
> + app = item_iterator;
> + item_iterator = item_iterator->next;
> + free(app);
> + }
> + *head = NULL;
> +}
> diff --git a/scripts/kas_alias/item_list.h b/scripts/kas_alias/item_list.h
> new file mode 100644
> index 000000000000..b4891cb088ee
> --- /dev/null
> +++ b/scripts/kas_alias/item_list.h
> @@ -0,0 +1,26 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +#ifndef ITEM_LIST_H
> +#define ITEM_LIST_H
> +#include <stdint.h>
> +
> +#define MAX_NAME_SIZE 256
> +#define BY_ADDRESS 1
> +#define BY_NAME 2
> +
> +struct item {
> + char symb_name[MAX_NAME_SIZE];
> + uint64_t addr;
> + char stype;
> + struct item *next;
> +};
> +
> +void build_index(struct item *list);
> +struct item *add_item(struct item **list, const char *name, char stype,
> uint64_t addr); +void sort_list(struct item **list, int sort_by);
> +struct item *merge(struct item *left, struct item *right, int sort_by);
> +struct item *merge_sort(struct item *head, int sort_by);
> +void sort_list_m(struct item **head, int sort_by);
> +int insert_after(struct item *list, const uint64_t search_addr,
> + const char *name, uint64_t addr, char stype);
> +void free_items(struct item **head);
> +#endif
> diff --git a/scripts/kas_alias/kas_alias.c b/scripts/kas_alias/kas_alias.c
> new file mode 100644
> index 000000000000..532aeb39f851
> --- /dev/null
> +++ b/scripts/kas_alias/kas_alias.c
> @@ -0,0 +1,217 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <stdint.h>
> +#include <unistd.h>
> +#include <string.h>
> +#include <stdbool.h>
> +#include <stdarg.h>
> +#include <regex.h>
> +
> +#include "item_list.h"
> +#include "duplicates_list.h"
> +#include "a2l.h"
> +
> +#define SYMB_IS_TEXT(s) ((((s)->stype) == 't') || (((s)->stype) == 'T'))
> +#define SYMB_IS_DATA(s) ((((s)->stype) == 'b') || (((s)->stype) == 'B') ||
> \ + (((s)->stype) == 'd') || (((s)->stype) == 'D') || \
> + (((s)->stype) == 'r') || (((s)->stype) == 'R'))
> +#ifdef CONFIG_KALLSYMS_ALIAS_DATA
> +#define SYMB_NEEDS_ALIAS(s) (SYMB_IS_TEXT(s) || SYMB_IS_DATA(s))
> +#else
> +#define SYMB_NEEDS_ALIAS(s) SYMB_IS_TEXT(s)
> +#endif
> +#define FNOMATCH 0
> +#define FMATCH 1
> +#define EREGEX 2
> +
> +const char *ignore_regex[] = {
> + "^__cfi_.*$", // __cfi_ preamble
> +#ifndef CONFIG_KALLSYMS_ALIAS_DATA_ALL
> + "^_*TRACE_SYSTEM.*$",
> + "^__already_done\\.[0-9]+$", // Call a function once data
> + "^___tp_str\\.[0-9]+$",
> + "^___done\\.[0-9]+$",
> + "^__print_once\\.[0-9]+$",
> + "^_rs\\.[0-9]+$",
> + "^__compound_literal\\.[0-9]+$",
> + "^___once_key\\.[0-9]+$",
> + "^__func__\\.[0-9]+$",
> + "^__msg\\.[0-9]+$",
> + "^CSWTCH\\.[0-9]+$",
> + "^__flags\\.[0-9]+$",
> + "^__wkey.*$",
> + "^__mkey.*$",
> + "^__key.*$",
> +#endif
> + "^__pfx_.*$" // NOP-padding
> +};
> +
> +int suffix_serial;
> +
> +static inline void verbose_msg(bool verbose, const char *fmt, ...)
> +{
> + va_list args;
> +
> + va_start(args, fmt);
> + if (verbose)
> + printf(fmt, args);
> +
> + va_end(args);
> +}
> +
> +static void create_suffix(const char *name, char *output_suffix)
> +{
> + sprintf(output_suffix, "%s__alias__%d", name, suffix_serial++);
> +}
> +
> +static void create_file_suffix(const char *name, uint64_t address, char
> *output_suffix, char *cwd) +{
> + const char *f_path;
> + char *buf;
> + int i = 0;
> +
> + buf = addr2line_get_lines(address);
> + f_path = remove_subdir(cwd, buf);
> + if (f_path) {
> + sprintf(output_suffix, "%s@%s", name, f_path);
> + while (*(output_suffix + i) != '\0') {
> + switch (*(output_suffix + i)) {
> + case '/':
> + case ':':
> + case '.':
> + *(output_suffix + i) = '_';
> + break;
> + default:
> + }
> + i++;
> + }
> + } else {
> + create_suffix(name, output_suffix);
> + }
> +}
> +
> +static int filter_symbols(char *symbol, const char **ignore_list, int
> regex_no) +{
> + regex_t regex;
> + int res, i;
> +
> + for (i = 0; i < regex_no; i++) {
> + res = regcomp(®ex, ignore_list[i], REG_EXTENDED);
> + if (res)
> + return -EREGEX;
> +
> + res = regexec(®ex, symbol, 0, NULL, 0);
> + regfree(®ex);
> + switch (res) {
> + case 0:
> + return FMATCH;
> + case REG_NOMATCH:
> + break;
> + default:
> + return -EREGEX;
> + }
> + }
> +
> + return FNOMATCH;
> +}
> +
> +int main(int argc, char *argv[])
> +{
> + char t, sym_name[MAX_NAME_SIZE], new_name[MAX_NAME_SIZE + 15];
> + struct duplicate_item *duplicate_iterator;
> + struct duplicate_item *duplicate;
> + struct item *head = {NULL};
> + bool need_2_process = true;
> + struct item *last = {NULL};
> + struct item *current;
> + int verbose_mode = 0;
> + uint64_t address;
> + FILE *fp;
> + int res;
> +
> + if (argc < 2 || argc > 3) {
> + printf("Usage: %s <nmfile> [-verbose]\n", argv[0]);
> + return 1;
> + }
> +
> + if (argc == 3 && strcmp(argv[2], "-verbose") == 0)
> + verbose_mode = 1;
> +
> + verbose_msg(verbose_mode, "Scanning nm data(%s)\n", argv[1]);
> +
> + fp = fopen(argv[1], "r");
> + if (!fp) {
> + printf("Can't open input file.\n");
> + return 1;
> + }
> +
> + if (!addr2line_init(get_addr2line(A2L_DEFAULT), get_vmlinux(argv[1])))
> + return 1;
> +
> + while (fscanf(fp, "%lx %c %99s\n", &address, &t, sym_name) == 3) {
> + if (strstr(sym_name, "@_")) {
> + if (verbose_mode && need_2_process)
> + printf("Already processed\n");
> + need_2_process = false;
> + }
> + last = add_item(&last, sym_name, t, address);
> + if (!last) {
> + printf("Error in allocate memory\n");
> + free_items(&head);
> + return 1;
> + }
> +
> + if (!head)
> + head = last;
> + }
> +
> + fclose(fp);
> +
> + if (need_2_process) {
> + verbose_msg(verbose_mode, "Sorting nm data\n");
> + sort_list_m(&head, BY_NAME);
> + verbose_msg(verbose_mode, "Scanning nm data for duplicates\n");
> + duplicate = find_duplicates(head);
> + if (!duplicate) {
> + printf("Error in duplicates list\n");
> + return 1;
> + }
> +
> + verbose_msg(verbose_mode, "Applying suffixes\n");
> + build_index(head);
> + duplicate_iterator = duplicate;
> + while (duplicate_iterator) {
> + res = filter_symbols(duplicate_iterator->original_item-
>symb_name,
> + ignore_regex, sizeof(ignore_regex) /
> + sizeof(ignore_regex[0]));
> + if (res != FMATCH &&
> + SYMB_NEEDS_ALIAS(duplicate_iterator->original_item))
{
> + if (res < 0)
> + return 1;
> +
> + create_file_suffix(duplicate_iterator-
>original_item->symb_name,
> + duplicate_iterator-
>original_item->addr,
> + new_name, vmlinux_path);
> + if (!insert_after(head, duplicate_iterator-
>original_item->addr,
> + new_name, duplicate_iterator-
>original_item->addr,
> + duplicate_iterator-
>original_item->stype))
> + return 1;
> + }
> +
> + duplicate_iterator = duplicate_iterator->next;
> + }
> +
> + sort_list_m(&head, BY_ADDRESS);
> + }
> + current = head;
> + while (current) {
> + printf("%08lx %c %s\n", current->addr, current->stype,
> current->symb_name); + current = current->next;
> + }
> +
> + free_items(&head);
> + free_duplicates(&duplicate);
> + addr2line_cleanup();
> + return 0;
> +}
> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> index a432b171be82..cacf60b597ce 100755
> --- a/scripts/link-vmlinux.sh
> +++ b/scripts/link-vmlinux.sh
> @@ -89,8 +89,9 @@ vmlinux_link()
>
> ldflags="${ldflags} ${wl}--script=${objtree}/${KBUILD_LDS}"
>
> - # The kallsyms linking does not need debug symbols included.
> - if [ "$output" != "${output#.tmp_vmlinux.kallsyms}" ] ; then
> + # The kallsyms linking does not need debug symbols included, unless
the
> KALLSYMS_ALIAS. + if [ ! is_enabled CONFIG_KALLSYMS_ALIAS ] && \
> + [ "$output" != "${output#.tmp_vmlinux.kallsyms}" ] ; then
> ldflags="${ldflags} ${wl}--strip-debug"
> fi
>
> @@ -161,7 +162,11 @@ kallsyms()
> fi
>
> info KSYMS ${2}
> - scripts/kallsyms ${kallsymopt} ${1} > ${2}
> + if is_enabled CONFIG_KALLSYMS_ALIAS; then
> + ALIAS=".alias"
> + scripts/kas_alias/kas_alias ${1} >${1}${ALIAS}
> + fi
> + scripts/kallsyms ${kallsymopt} ${1}${ALIAS} > ${2}
> }
>
> # Perform one step in kallsyms generation, including temporary linking of
Best regards.
^ permalink raw reply
* [PATCH] visl: use canonical ftrace path
From: Ross Zwisler @ 2023-08-29 20:46 UTC (permalink / raw)
To: linux-kernel
Cc: Ross Zwisler, Mark Rutland, Masami Hiramatsu,
Mauro Carvalho Chehab, Steven Rostedt, linux-media,
linux-trace-kernel, Daniel Almeida, Hans Verkuil
From: Ross Zwisler <zwisler@google.com>
The canonical location for the tracefs filesystem is at /sys/kernel/tracing.
But, from Documentation/trace/ftrace.rst:
Before 4.1, all ftrace tracing control files were within the debugfs
file system, which is typically located at /sys/kernel/debug/tracing.
For backward compatibility, when mounting the debugfs file system,
the tracefs file system will be automatically mounted at:
/sys/kernel/debug/tracing
Update the visl decoder driver documentation to use this tracefs path.
Signed-off-by: Ross Zwisler <zwisler@google.com>
---
Documentation/admin-guide/media/visl.rst | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/admin-guide/media/visl.rst b/Documentation/admin-guide/media/visl.rst
index 7d2dc78341c9..4328c6c72d30 100644
--- a/Documentation/admin-guide/media/visl.rst
+++ b/Documentation/admin-guide/media/visl.rst
@@ -78,7 +78,7 @@ The trace events are defined on a per-codec basis, e.g.:
.. code-block:: bash
- $ ls /sys/kernel/debug/tracing/events/ | grep visl
+ $ ls /sys/kernel/tracing/events/ | grep visl
visl_fwht_controls
visl_h264_controls
visl_hevc_controls
@@ -90,13 +90,13 @@ For example, in order to dump HEVC SPS data:
.. code-block:: bash
- $ echo 1 > /sys/kernel/debug/tracing/events/visl_hevc_controls/v4l2_ctrl_hevc_sps/enable
+ $ echo 1 > /sys/kernel/tracing/events/visl_hevc_controls/v4l2_ctrl_hevc_sps/enable
The SPS data will be dumped to the trace buffer, i.e.:
.. code-block:: bash
- $ cat /sys/kernel/debug/tracing/trace
+ $ cat /sys/kernel/tracing/trace
video_parameter_set_id 0
seq_parameter_set_id 0
pic_width_in_luma_samples 1920
--
2.42.0.rc2.253.gd59a3bf2b4-goog
^ permalink raw reply related
* Re: [RFC PATCH] mm/damon/core: add a tracepoint for damos apply target regions
From: Steven Rostedt @ 2023-08-29 23:16 UTC (permalink / raw)
To: SeongJae Park
Cc: damon, Andrew Morton, linux-mm, linux-trace-kernel, linux-kernel
In-Reply-To: <20230827004045.49516-1-sj@kernel.org>
On Sun, 27 Aug 2023 00:40:45 +0000
SeongJae Park <sj@kernel.org> wrote:
> + TP_STRUCT__entry(
> + __field(unsigned int, context_idx)
4 bytes
> + __field(unsigned int, scheme_idx)
4 bytes
> + __field(unsigned long, target_idx)
8 bytes
> + __field(unsigned int, nr_regions)
4 bytes
> + __field(unsigned long, start)
This is going to cause a 4 byte hole. I would move nr_regions after end
so that it stays properly aligned.
-- Steve
> + __field(unsigned long, end)
> + __field(unsigned int, nr_accesses)
> + __field(unsigned int, age)
> + ),
> +
^ permalink raw reply
* Re: [PATCH v3 1/1] tracing/kprobes: Return EADDRNOTAVAIL when func matches several symbols
From: Steven Rostedt @ 2023-08-29 23:57 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: Francis Laniel, linux-kernel, linux-trace-kernel
In-Reply-To: <20230825221321.faaa33e03afc48abc345c24f@kernel.org>
On Fri, 25 Aug 2023 22:13:21 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> > Excellent catch! Thank you, I will apply this patch and send v4 right after.
> > Regarding test, do you think I can add a test for the EADDRNOTAVAIL case?
>
> Hmm, in that case, you need to change something in tracefs/README so that
> we can identify the kernel has different behavior. Or we have to change
> this is a "Fix" for backporting.
I prefer this to be a Fix and backported.
Thanks,
-- Steve
^ permalink raw reply
* Re: [PATCH v2 08/92] fs: new helper: simple_rename_timestamp
From: Al Viro @ 2023-08-30 0:19 UTC (permalink / raw)
To: Jeff Layton
Cc: jk, arnd, mpe, npiggin, christophe.leroy, hca, gor, agordeev,
borntraeger, svens, gregkh, arve, tkjos, maco, joel, brauner,
cmllamas, surenb, dennis.dalessandro, jgg, leon, bwarrum,
rituagar, ericvh, lucho, asmadeus, linux_oss, dsterba, dhowells,
marc.dionne, raven, luisbg, salah.triki, aivazian.tigran,
ebiederm, keescook, clm, josef, xiubli, idryomov, jaharkes, coda,
jlbec, hch, nico, rafael, code, ardb, xiang, chao, huyue2,
jefflexu, linkinjeon, sj1557.seo, jack, tytso, adilger.kernel,
jaegeuk, hirofumi, miklos, rpeterso, agruenba, richard,
anton.ivanov, johannes, mikulas, mike.kravetz, muchun.song, dwmw2,
shaggy, tj, trond.myklebust, anna, chuck.lever, neilb, kolga,
Dai.Ngo, tom, konishi.ryusuke, anton, almaz.alexandrovich, mark,
joseph.qi, me, hubcap, martin, amir73il, mcgrof, yzaikin,
tony.luck, gpiccoli, al, sfrench, pc, lsahlber, sprasad,
senozhatsky, phillip, rostedt, mhiramat, dushistov, hdegoede,
djwong, dlemoal, naohiro.aota, jth, ast, daniel, andrii,
martin.lau, song, yhs, john.fastabend, kpsingh, sdf, haoluo,
jolsa, hughd, akpm, davem, edumazet, kuba, pabeni, john.johansen,
paul, jmorris, serge, stephen.smalley.work, eparis, jgross, stern,
lrh2000, sebastian.reichel, wsa+renesas, quic_ugoswami,
quic_linyyuan, john, error27, quic_uaggarwa, hayama, jomajm,
axboe, dhavale, dchinner, hannes, zhangpeng362, slava,
gargaditya08, penguin-kernel, yifeliu, madkar, ezk, yuzhe, willy,
okanatov, jeffxu, linux, mirimmad17, yijiangshan, yang.yang29,
xu.xin16, chengzhihao1, shr, Liam.Howlett, adobriyan, chi.minghao,
roberto.sassu, linuszeng, bvanassche, zohar, yi.zhang, trix,
fmdefrancesco, ebiggers, princekumarmaurya06, chenzhongjin, riel,
shaozhengchao, jingyuwang_vip, linuxppc-dev, linux-kernel,
linux-s390, linux-rdma, linux-usb, v9fs, linux-fsdevel, linux-afs,
autofs, linux-mm, linux-btrfs, ceph-devel, codalist, ecryptfs,
linux-efi, linux-erofs, linux-ext4, linux-f2fs-devel,
cluster-devel, linux-um, linux-mtd, jfs-discussion, linux-nfs,
linux-nilfs, linux-ntfs-dev, ntfs3, ocfs2-devel,
linux-karma-devel, devel, linux-unionfs, linux-hardening,
reiserfs-devel, linux-cifs, samba-technical, linux-trace-kernel,
linux-xfs, bpf, netdev, apparmor, linux-security-module, selinux
In-Reply-To: <20230705185812.579118-3-jlayton@kernel.org>
On Wed, Jul 05, 2023 at 02:58:11PM -0400, Jeff Layton wrote:
> + * POSIX mandates that the old and new parent directories have their ctime and
> + * mtime updated, and that inodes of @old_dentry and @new_dentry (if any), have
> + * their ctime updated.
APPLICATION USAGE
Some implementations mark for update the last file status change timestamp
of renamed files and some do not. Applications which make use of the
last file status change timestamp may behave differently with respect
to renamed files unless they are designed to allow for either behavior.
So for children POSIX permits rather than mandates. Doesn't really matter;
Linux behaviour had been to touch ctime on children since way back, if
not since the very beginning.
^ permalink raw reply
* Re: [PATCH v2 08/92] fs: new helper: simple_rename_timestamp
From: Jeff Layton @ 2023-08-30 0:48 UTC (permalink / raw)
To: Al Viro
Cc: jk, arnd, mpe, npiggin, christophe.leroy, hca, gor, agordeev,
borntraeger, svens, gregkh, arve, tkjos, maco, joel, brauner,
cmllamas, surenb, dennis.dalessandro, jgg, leon, bwarrum,
rituagar, ericvh, lucho, asmadeus, linux_oss, dsterba, dhowells,
marc.dionne, raven, luisbg, salah.triki, aivazian.tigran,
ebiederm, keescook, clm, josef, xiubli, idryomov, jaharkes, coda,
jlbec, hch, nico, rafael, code, ardb, xiang, chao, huyue2,
jefflexu, linkinjeon, sj1557.seo, jack, tytso, adilger.kernel,
jaegeuk, hirofumi, miklos, rpeterso, agruenba, richard,
anton.ivanov, johannes, mikulas, mike.kravetz, muchun.song, dwmw2,
shaggy, tj, trond.myklebust, anna, chuck.lever, neilb, kolga,
Dai.Ngo, tom, konishi.ryusuke, anton, almaz.alexandrovich, mark,
joseph.qi, me, hubcap, martin, amir73il, mcgrof, yzaikin,
tony.luck, gpiccoli, al, sfrench, pc, lsahlber, sprasad,
senozhatsky, phillip, rostedt, mhiramat, dushistov, hdegoede,
djwong, dlemoal, naohiro.aota, jth, ast, daniel, andrii,
martin.lau, song, yhs, john.fastabend, kpsingh, sdf, haoluo,
jolsa, hughd, akpm, davem, edumazet, kuba, pabeni, john.johansen,
paul, jmorris, serge, stephen.smalley.work, eparis, jgross, stern,
lrh2000, sebastian.reichel, wsa+renesas, quic_ugoswami,
quic_linyyuan, john, error27, quic_uaggarwa, hayama, jomajm,
axboe, dhavale, dchinner, hannes, zhangpeng362, slava,
gargaditya08, penguin-kernel, yifeliu, madkar, ezk, yuzhe, willy,
okanatov, jeffxu, linux, mirimmad17, yijiangshan, yang.yang29,
xu.xin16, chengzhihao1, shr, Liam.Howlett, adobriyan, chi.minghao,
roberto.sassu, linuszeng, bvanassche, zohar, yi.zhang, trix,
fmdefrancesco, ebiggers, princekumarmaurya06, chenzhongjin, riel,
shaozhengchao, jingyuwang_vip, linuxppc-dev, linux-kernel,
linux-s390, linux-rdma, linux-usb, v9fs, linux-fsdevel, linux-afs,
autofs, linux-mm, linux-btrfs, ceph-devel, codalist, ecryptfs,
linux-efi, linux-erofs, linux-ext4, linux-f2fs-devel,
cluster-devel, linux-um, linux-mtd, jfs-discussion, linux-nfs,
linux-nilfs, linux-ntfs-dev, ntfs3, ocfs2-devel,
linux-karma-devel, devel, linux-unionfs, linux-hardening,
reiserfs-devel, linux-cifs, samba-technical, linux-trace-kernel,
linux-xfs, bpf, netdev, apparmor, linux-security-module, selinux
In-Reply-To: <20230830001917.GC461907@ZenIV>
On Wed, 2023-08-30 at 01:19 +0100, Al Viro wrote:
> On Wed, Jul 05, 2023 at 02:58:11PM -0400, Jeff Layton wrote:
>
> > + * POSIX mandates that the old and new parent directories have their ctime and
> > + * mtime updated, and that inodes of @old_dentry and @new_dentry (if any), have
> > + * their ctime updated.
>
> APPLICATION USAGE
> Some implementations mark for update the last file status change timestamp
> of renamed files and some do not. Applications which make use of the
> last file status change timestamp may behave differently with respect
> to renamed files unless they are designed to allow for either behavior.
>
> So for children POSIX permits rather than mandates. Doesn't really matter;
> Linux behaviour had been to touch ctime on children since way back, if
> not since the very beginning.
Mea culpa. You're quite correct. I'll plan to roll a small patch to
update the comment over this function.
Thanks!
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: [RFC PATCH] mm/damon/core: add a tracepoint for damos apply target regions
From: SeongJae Park @ 2023-08-30 1:36 UTC (permalink / raw)
To: Steven Rostedt
Cc: SeongJae Park, damon, Andrew Morton, linux-mm, linux-trace-kernel,
linux-kernel
In-Reply-To: <20230829191629.25c5f0e2@rorschach.local.home>
Hi Steven,
On Tue, 29 Aug 2023 19:16:29 -0400 Steven Rostedt <rostedt@goodmis.org> wrote:
> On Sun, 27 Aug 2023 00:40:45 +0000
> SeongJae Park <sj@kernel.org> wrote:
>
> > + TP_STRUCT__entry(
> > + __field(unsigned int, context_idx)
>
> 4 bytes
>
> > + __field(unsigned int, scheme_idx)
>
> 4 bytes
>
> > + __field(unsigned long, target_idx)
>
> 8 bytes
>
> > + __field(unsigned int, nr_regions)
>
> 4 bytes
>
> > + __field(unsigned long, start)
>
> This is going to cause a 4 byte hole. I would move nr_regions after end
> so that it stays properly aligned.
That makes sense, thank you for letting me know this! I will do so in the next
spin.
Thanks,
SJ
>
> -- Steve
>
>
>
> > + __field(unsigned long, end)
> > + __field(unsigned int, nr_accesses)
> > + __field(unsigned int, age)
> > + ),
> > +
>
^ permalink raw reply
* Re: [PATCH v3] scripts/link-vmlinux.sh: Add alias to duplicate symbols for kallsyms
From: Masami Hiramatsu @ 2023-08-30 6:00 UTC (permalink / raw)
To: Alessandro Carminati (Red Hat)
Cc: Masami Hiramatsu, Steven Rostedt, Daniel Bristot de Oliveira,
Josh Poimboeuf, Masahiro Yamada, Luis Chamberlain,
Nathan Chancellor, Nick Desaulniers, Nicolas Schier,
Alexander Lobakin, Nick Alcock, Kris Van Hees, Eugene Loh,
Francis Laniel, Viktor Malik, linux-kbuild, linux-kernel,
linux-trace-kernel, live-patching
In-Reply-To: <20230828080423.3539686-1-alessandro.carminati@gmail.com>
Hi Alessandro,
Thanks for your update!
On Mon, 28 Aug 2023 08:04:23 +0000
"Alessandro Carminati (Red Hat)" <alessandro.carminati@gmail.com> wrote:
> From: Alessandro Carminati <alessandro.carminati@gmail.com>
>
> It is not uncommon for drivers or modules related to similar peripherals
> to have symbols with the exact same name.
> While this is not a problem for the kernel's binary itself, it becomes an
> issue when attempting to trace or probe specific functions using
> infrastructure like ftrace or kprobe.
>
> The tracing subsystem relies on the `nm -n vmlinux` output, which provides
> symbol information from the kernel's ELF binary. However, when multiple
> symbols share the same name, the standard nm output does not differentiate
> between them. This can lead to confusion and difficulty when trying to
> probe the intended symbol.
>
> ~ # cat /proc/kallsyms | grep " name_show"
> ffffffff8c4f76d0 t name_show
> ffffffff8c9cccb0 t name_show
> ffffffff8cb0ac20 t name_show
> ffffffff8cc728c0 t name_show
> ffffffff8ce0efd0 t name_show
> ffffffff8ce126c0 t name_show
> ffffffff8ce1dd20 t name_show
> ffffffff8ce24e70 t name_show
> ffffffff8d1104c0 t name_show
> ffffffff8d1fe480 t name_show
>
> **kas_alias** addresses this challenge by extending the symbol names with
> unique suffixes during the kernel build process.
> The newly created aliases for these duplicated symbols are unique names
> that can be fed to the ftracefs interface. By doing so, it enables
> previously unreachable symbols to be probed.
>
> ~ # cat /proc/kallsyms | grep " name_show"
> ffffffff974f76d0 t name_show
> ffffffff974f76d0 t name_show__alias__6340
> ffffffff979cccb0 t name_show
> ffffffff979cccb0 t name_show__alias__6341
> ffffffff97b0ac20 t name_show
> ffffffff97b0ac20 t name_show__alias__6342
> ffffffff97c728c0 t name_show
> ffffffff97c728c0 t name_show__alias__6343
> ffffffff97e0efd0 t name_show
> ffffffff97e0efd0 t name_show__alias__6344
> ffffffff97e126c0 t name_show
> ffffffff97e126c0 t name_show__alias__6345
> ffffffff97e1dd20 t name_show
> ffffffff97e1dd20 t name_show__alias__6346
> ffffffff97e24e70 t name_show
> ffffffff97e24e70 t name_show__alias__6347
> ffffffff981104c0 t name_show
> ffffffff981104c0 t name_show__alias__6348
> ffffffff981fe480 t name_show
> ffffffff981fe480 t name_show__alias__6349
> ~ # echo "p:kprobes/evnt1 name_show__alias__6349" \
> > >/sys/kernel/tracing/kprobe_events
> ~ # cat /sys/kernel/tracing/kprobe_events
> p:kprobes/evnt1 name_show__alias__6349
>
> Changes from v1:
> - Integrated changes requested by Masami to exclude symbols with prefixes
> "_cfi" and "_pfx".
> - Introduced a small framework to handle patterns that need to be excluded
> from the alias production.
> - Excluded other symbols using the framework.
> - Introduced the ability to discriminate between text and data symbols.
> - Added two new config symbols in this version: CONFIG_KALLSYMS_ALIAS_DATA,
> which allows data for data, and CONFIG_KALLSYMS_ALIAS_DATA_ALL, which
> excludes all filters and provides an alias for each duplicated symbol.
>
> https://lore.kernel.org/all/20230711151925.1092080-1-alessandro.carminati@gmail.com/
>
> Changes from v2:
> - Alias tags are created by querying DWARF information from the vmlinux.
> - The filename + line number is normalized and appended to the original name.
> - The tag begins with '@' to indicate the symbol source.
> - Not a change, but worth mentioning, since the alias is added to the existing
> list, the old duplicated name is preserved, and the livepatch way of dealing
> with duplicates is maintained.
> - Acknowledging the existence of scenarios where inlined functions declared in
> header files may result in multiple copies due to compiler behavior, though
> it is not actionable as it does not pose an operational issue.
> - Highlighting a single exception where the same name refers to different
> functions: the case of "compat_binfmt_elf.c," which directly includes
> "binfmt_elf.c" producing identical function copies in two separate
> modules.
>
> sample from new v3
>
> ~ # cat /proc/kallsyms | grep gic_mask_irq
> ffffd0b03c04dae4 t gic_mask_irq
> ffffd0b03c04dae4 t gic_mask_irq@_drivers_irqchip_irq-gic_c_167
> ffffd0b03c050960 t gic_mask_irq
> ffffd0b03c050960 t gic_mask_irq@_drivers_irqchip_irq-gic-v3_c_404
Could you also update the sample output in the patch description?
And I can not see this line information but a serial number aliases,
is there any way to enable this file-name aliases? (LLVM is not supported?)
I think we'd better to have a new CONFIG_KALLSYMS_ALIAS_SRCLINE and
clarify what is required.
> ~ #
>
> https://lore.kernel.org/all/20230714150326.1152359-1-alessandro.carminati@gmail.com/
>
> Signed-off-by: Alessandro Carminati (Red Hat) <alessandro.carminati@gmail.com>
> ---
> init/Kconfig | 36 ++++
> scripts/Makefile | 4 +
> scripts/kas_alias/Makefile | 4 +
> scripts/kas_alias/a2l.c | 268 ++++++++++++++++++++++++++++
> scripts/kas_alias/a2l.h | 32 ++++
> scripts/kas_alias/duplicates_list.c | 70 ++++++++
> scripts/kas_alias/duplicates_list.h | 15 ++
> scripts/kas_alias/item_list.c | 230 ++++++++++++++++++++++++
> scripts/kas_alias/item_list.h | 26 +++
> scripts/kas_alias/kas_alias.c | 217 ++++++++++++++++++++++
> scripts/link-vmlinux.sh | 11 +-
> 11 files changed, 910 insertions(+), 3 deletions(-)
> create mode 100644 scripts/kas_alias/Makefile
> create mode 100644 scripts/kas_alias/a2l.c
> create mode 100644 scripts/kas_alias/a2l.h
> create mode 100644 scripts/kas_alias/duplicates_list.c
> create mode 100644 scripts/kas_alias/duplicates_list.h
> create mode 100644 scripts/kas_alias/item_list.c
> create mode 100644 scripts/kas_alias/item_list.h
> create mode 100644 scripts/kas_alias/kas_alias.c
>
> diff --git a/init/Kconfig b/init/Kconfig
> index f7f65af4ee12..bc69fcd9cbc8 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1737,6 +1737,42 @@ config KALLSYMS_BASE_RELATIVE
> time constants, and no relocation pass is required at runtime to fix
> up the entries based on the runtime load address of the kernel.
>
> +config KALLSYMS_ALIAS
> + bool "Produces alias for duplicated symbols" if EXPERT
> + depends on KALLSYMS && (DEBUG_INFO_DWARF4 || DEBUG_INFO_DWARF5)
> + help
> + It is not uncommon for drivers or modules related to similar
> + peripherals to have symbols with the exact same name.
> + While this is not a problem for the kernel's binary itself, it
> + becomes an issue when attempting to trace or probe specific
> + functions using infrastructure like ftrace or kprobe.
> +
> + This option addresses this challenge by extending the symbol names
> + with unique suffixes during the kernel build process.
> + The newly created aliases for these duplicated symbols are unique
> + names that can be fed to the ftrace sysfs interface. By doing so, it
> + enables previously unreachable symbols to be probed.
> +
> +config CONFIG_KALLSYMS_ALIAS_DATA
You don't need 'CONFIG_' prefix here.
> + bool "Produces alias also for data"
> + depends on KALLSYMS_ALIAS
> + help
> + Sometimes it can be useful to refer to data. In live patch scenarios,
> + you may find yourself needing to use symbols that are shared with
> + other functions. Since symbols face the same issue as functions, this
> + option allows you to create aliases for data as well.
> +
> +config CONFIG_KALLSYMS_ALIAS_DATA_ALL
> + bool "Removes all filter when producing data alias"
> + depends on CONFIG_KALLSYMS_ALIAS_DATA
> + help
> + When selecting data aliases, not all symbols are included in the set
> + This is because many symbols are unlikely to be used. If you choose
> + to have an alias for all data symbols, be aware that it will
> + significantly increase the size.
> +
> + If unsure, say N.
> +
> # end of the "standard kernel features (expert users)" menu
>
> # syscall, maps, verifier
> diff --git a/scripts/Makefile b/scripts/Makefile
> index 32b6ba722728..65fafe17cfe5 100644
> --- a/scripts/Makefile
> +++ b/scripts/Makefile
> @@ -49,3 +49,7 @@ subdir-$(CONFIG_SECURITY_SELINUX) += selinux
>
> # Let clean descend into subdirs
> subdir- += basic dtc gdb kconfig mod
> +
> +# KALLSyms alias
> +subdir-$(CONFIG_KALLSYMS_ALIAS) += kas_alias
> +
> diff --git a/scripts/kas_alias/Makefile b/scripts/kas_alias/Makefile
> new file mode 100644
> index 000000000000..e1fde69232b4
> --- /dev/null
> +++ b/scripts/kas_alias/Makefile
> @@ -0,0 +1,4 @@
> +# SPDX-License-Identifier: GPL-2.0
> +hostprogs-always-$(CONFIG_KALLSYMS_ALIAS) += kas_alias
> +
> +kas_alias-objs := duplicates_list.o item_list.o kas_alias.o a2l.o
> diff --git a/scripts/kas_alias/a2l.c b/scripts/kas_alias/a2l.c
> new file mode 100644
> index 000000000000..a9692ac30180
> --- /dev/null
> +++ b/scripts/kas_alias/a2l.c
> @@ -0,0 +1,268 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <sys/types.h>
> +#include <sys/wait.h>
> +#include <string.h>
> +#include <stdint.h>
> +#include <stdbool.h>
> +
> +#include "a2l.h"
> +
> +int addr2line_pid = -1;
> +int a2l_in[2];
> +int a2l_out[2];
> +char line[MAX_BUF];
> +char vmlinux_path[MAX_BUF];
> +char addr2line_cmd[MAX_CMD_LEN];
> +FILE *a2l_stdin, *a2l_stdout;
> +
> +static char *normalize_path(const char *input_path, char *output_path)
> +{
> + char *prev_token = NULL;
> + char *delimiter = "/";
> + char inbuf[MAX_BUF];
> + char *token;
> + char *pos;
> +
> + memset(inbuf, 0, MAX_BUF);
> + *output_path = '\0';
> + strncpy(inbuf, input_path, MAX_BUF);
> + if (!input_path || !output_path || strlen(input_path) == 0)
> + return NULL;
> +
> + token = strtok(inbuf, delimiter);
> + while (token) {
> + if (strcmp(token, "..") == 0 && prev_token) {
> + pos = strrchr(output_path, '/');
> + if (pos)
> + *pos = '\0';
> +
> + } else if (strcmp(token, ".") != 0) {
> + strcat(output_path, "/");
> + strcat(output_path, token);
> + }
> +
> + prev_token = token;
> + token = strtok(NULL, delimiter);
> + }
> +
> + return output_path;
> +}
> +
> +static void path_of(const char *full_path, char *path)
> +{
> + const char *last_slash = strrchr(full_path, '/');
> + size_t path_length;
> + char cwd[MAX_BUF];
> +
> + if (!last_slash) {
> + if (getcwd(cwd, sizeof(cwd)))
> + strcpy(path, cwd);
> + else
> + strcpy(path, ".");
> + } else {
> + path_length = last_slash - full_path;
> + strncpy(path, full_path, path_length);
> + path[path_length] = '\0';
> + }
> +}
> +
> +static bool file_exists(const char *file_path)
> +{
> + FILE *file;
> +
> + file = fopen(file_path, "r");
> + if (file) {
> + fclose(file);
> + return true;
> + }
> + return false;
> +}
> +
> +int addr2line_init(const char *cmd, const char *vmlinux)
> +{
> + if ((!file_exists(cmd)) || (!file_exists(vmlinux))) {
> + printf("file not found\n");
> + return 0;
> + }
nit: mis-indentation?
> +
> + path_of(vmlinux, vmlinux_path);
> + if (pipe(a2l_in) == -1) {
> + printf("Failed to create pipe\n");
> + return 0;
> + }
> +
> + if (pipe(a2l_out) == -1) {
> + printf("Failed to create pipe\n");
> + return 0;
> + }
> +
> + addr2line_pid = fork();
> + if (addr2line_pid == -1) {
> + printf("Failed to fork process\n");
> + close(a2l_in[P_READ]);
> + close(a2l_in[P_WRITE]);
> + close(a2l_out[P_READ]);
> + close(a2l_out[P_WRITE]);
> + return 0;
> + }
> +
> + if (addr2line_pid == 0) {
> + dup2(a2l_in[P_READ], 0);
> + dup2(a2l_out[P_WRITE], 1);
> + close(a2l_in[P_WRITE]);
> + close(a2l_out[P_READ]);
> +
> + execlp(cmd, cmd, ADDR2LINE_ARGS, vmlinux, NULL);
> +
> + printf("Failed to execute addr2line command\n");
> + exit(1);
> + } else {
> + close(a2l_in[P_READ]);
> + close(a2l_out[P_WRITE]);
> + }
> +
> + a2l_stdin = fdopen(a2l_in[P_WRITE], "w");
> + if (!a2l_stdin) {
> + printf("Failed to open pipe a2l_in\n");
> + return 0;
> + }
> +
> + a2l_stdout = fdopen(a2l_out[P_READ], "r");
> + if (!a2l_stdout) {
> + printf("Failed to open pipe a2l_out\n");
> + fclose(a2l_stdin);
> + return 0;
> + }
> +
> + return 1;
> +}
> +
> +const char *remove_subdir(const char *home, const char *f_path)
> +{
> + int i = 0;
> +
> + while (*(home + i) == *(f_path + i))
> + i++;
> +
> + return (strlen(home) != i) ? NULL : f_path + i;
> +}
> +
> +char *addr2line_get_lines(uint64_t address)
> +{
> + char buf[MAX_BUF];
> +
> + fprintf(a2l_stdin, "%08lx\n", address);
> + fflush(a2l_stdin);
> +
> + if (!fgets(line, sizeof(line), a2l_stdout)) {
> + printf("Failed to read lines from addr2line\n");
> + return NULL;
> + }
> +
> + if (!fgets(line, sizeof(line), a2l_stdout)) {
> + printf("Failed to read lines from addr2line\n");
> + return NULL;
> + }
> +
> + line[strcspn(line, "\n")] = '\0';
> + strncpy(buf, line, MAX_BUF);
> + return normalize_path(buf, line);
> +}
> +
> +int addr2line_cleanup(void)
> +{
> + int status;
> +
> + if (addr2line_pid != -1) {
> + kill(addr2line_pid, SIGKILL);
> + waitpid(addr2line_pid, &status, 0);
> + fclose(a2l_stdin);
> + fclose(a2l_stdout);
> + addr2line_pid = -1;
> + }
> +
> + return 1;
> +}
> +
> +static char *find_executable(const char *command)
> +{
> + char *path_env = getenv("PATH");
> + char *executable_path;
> + char *path_copy;
> + char *path;
> + int n;
> +
> + if (!path_env)
> + return NULL;
> +
> + path_copy = strdup(path_env);
> + if (!path_copy)
> + return NULL;
> +
> + path = strtok(path_copy, ":");
> + while (path) {
> + n = snprintf(0, 0, "%s/%s", path, command);
> + executable_path = (char *)malloc(n + 1);
> + snprintf(executable_path, n + 1, "%s/%s", path, command);
> + if (access(executable_path, X_OK) == 0) {
> + free(path_copy);
> + return executable_path;
> + }
> +
> + path = strtok(NULL, ":");
> + free(executable_path);
> + executable_path = NULL;
nit: mis-indentation?
> + }
> +
> + free(path_copy);
> + if (executable_path)
> + free(executable_path);
> + return NULL;
> +}
> +
> +const char *get_addr2line(int mode)
> +{
> + char *buf = "";
> +
> + switch (mode) {
> + case A2L_CROSS:
> + buf = getenv("CROSS_COMPILE");
> + memcpy(addr2line_cmd, buf, strlen(buf));
> + case A2L_DEFAULT:
> + memcpy(addr2line_cmd + strlen(buf), ADDR2LINE, strlen(ADDR2LINE));
> + buf = find_executable(addr2line_cmd);
> + if (buf) {
> + memcpy(addr2line_cmd, buf, strlen(buf));
> + free(buf);
> + }
> + return addr2line_cmd;
> + case A2L_LLVM:
> + default:
> + return NULL;
> + }
> +}
> +
> +char *get_vmlinux(char *input)
> +{
> + const char *match_string1 = ".syms";
> + const char *match_string2 = ".tmp_vmlinux.kallsyms";
> + char *result = NULL;
> + char *match_pos;
> +
> + match_pos = strstr(input, match_string1);
> + if (!match_pos)
> + return NULL;
> +
> + match_pos = strstr(input, match_string2);
> + if (!match_pos)
> + return NULL;
> +
> + result = strdup(input);
> + match_pos = strstr(result, match_string1);
> + *match_pos = '\0';
> + return result;
> +}
> diff --git a/scripts/kas_alias/a2l.h b/scripts/kas_alias/a2l.h
> new file mode 100644
> index 000000000000..ca6419229dde
> --- /dev/null
> +++ b/scripts/kas_alias/a2l.h
> @@ -0,0 +1,32 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +#ifndef A2L_H
> +#define A2L_H
> +#include <stdint.h>
> +
> +#define ADDR2LINE "addr2line"
> +#define ADDR2LINE_ARGS "-fe"
> +//#define VMLINUX "vmlinux"
Nit: this should be removed if it is not used.
> +#define MAX_BUF 4096
> +#define MAX_CMD_LEN 256
> +#define P_READ 0
> +#define P_WRITE 1
> +#define A2L_DEFAULT 1
> +#define A2L_CROSS 2
> +#define A2L_LLVM 3
> +#define A2L_MAKE_VALUE 2
> +
> +extern int addr2line_pid;
> +extern int a2l_in[2];
> +extern int a2l_out[2];
> +extern char line[MAX_BUF];
> +extern char vmlinux_path[MAX_BUF];
> +extern char addr2line_cmd[MAX_CMD_LEN];
> +
> +int addr2line_init(const char *cmd, const char *vmlinux);
> +char *addr2line_get_lines(uint64_t address);
> +int addr2line_cleanup(void);
> +const char *remove_subdir(const char *home, const char *f_path);
> +const char *get_addr2line(int mode);
> +char *get_vmlinux(char *input);
> +
> +#endif
> diff --git a/scripts/kas_alias/duplicates_list.c b/scripts/kas_alias/duplicates_list.c
> new file mode 100644
> index 000000000000..e7a3d2917937
> --- /dev/null
> +++ b/scripts/kas_alias/duplicates_list.c
> @@ -0,0 +1,70 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <stdlib.h>
> +#include <stdbool.h>
> +
> +#include "item_list.h"
> +#include "duplicates_list.h"
> +
/* The item list must be sorted. */
I think we can find the duplicated symbol without sorting the list,
but such implementation level optimization will be done later.
> +struct duplicate_item *find_duplicates(struct item *list)
> +{
> + struct duplicate_item *current_duplicate = NULL;
> + struct duplicate_item *duplicates = NULL;
> + struct duplicate_item *new_duplicate;
> + struct item *current_item = list;
> + bool prev_was_duplicate = false;
> + struct item *prev_item = NULL;
> +
> + while (current_item) {
> + if ((prev_item && (strcmp(current_item->symb_name, prev_item->symb_name) == 0)) ||
> + prev_was_duplicate) {
> + if (!duplicates) {
> + duplicates = malloc(sizeof(struct duplicate_item));
> + if (!duplicates)
> + return NULL;
> +
> + duplicates->original_item = prev_item;
> + duplicates->next = NULL;
> + current_duplicate = duplicates;
> + } else {
> + new_duplicate = malloc(sizeof(struct duplicate_item));
> + if (!new_duplicate) {
> + free_duplicates(&duplicates);
> + return NULL;
> + }
> +
> + new_duplicate->original_item = prev_item;
> + new_duplicate->next = NULL;
> + current_duplicate->next = new_duplicate;
> + current_duplicate = new_duplicate;
> +
> + if ((strcmp(current_item->symb_name, prev_item->symb_name) != 0) &&
> + (prev_was_duplicate))
> + prev_was_duplicate = false;
> + else
> + prev_was_duplicate = true;
> + }
> + }
> +
> + prev_item = current_item;
> + current_item = current_item->next;
> + }
> +
> + return duplicates;
> +}
> +
> +void free_duplicates(struct duplicate_item **duplicates)
> +{
> + struct duplicate_item *duplicates_iterator = *duplicates;
> + struct duplicate_item *app;
> +
> + while (duplicates_iterator) {
> + app = duplicates_iterator;
> + duplicates_iterator = duplicates_iterator->next;
> + free(app);
> + }
> +
> + *duplicates = NULL;
> +}
> diff --git a/scripts/kas_alias/duplicates_list.h b/scripts/kas_alias/duplicates_list.h
> new file mode 100644
> index 000000000000..76aa73e584bc
> --- /dev/null
> +++ b/scripts/kas_alias/duplicates_list.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +#ifndef DUPLICATES_LIST_H
> +#define DUPLICATES_LIST_H
> +
> +#include "item_list.h"
> +
> +struct duplicate_item {
> + struct item *original_item;
> + struct duplicate_item *next;
> +};
> +
> +struct duplicate_item *find_duplicates(struct item *list);
> +void free_duplicates(struct duplicate_item **duplicates);
> +
> +#endif
> diff --git a/scripts/kas_alias/item_list.c b/scripts/kas_alias/item_list.c
> new file mode 100644
> index 000000000000..48f2e525592a
> --- /dev/null
> +++ b/scripts/kas_alias/item_list.c
> @@ -0,0 +1,230 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <stdint.h>
> +#include <string.h>
> +#include <stdbool.h>
> +#include <assert.h>
> +#include "item_list.h"
> +
> +#define CHECK_ORDER_BY_ADDRESS(sort_by, current, temp, op) \
> + ((sort_by) == BY_ADDRESS && (current)->addr op (temp)->addr)
> +#define CHECK_ORDER_BY_NAME(sort_by, current, temp, op) \
> + ((sort_by) == BY_NAME && strcmp((current)->symb_name, (temp)->symb_name) op 0)
> +
> +struct item *list_index[96] = {0};
> +
> +void build_index(struct item *list)
> +{
> + char current_first_letter = ' ';
> + struct item *current = list;
> +
> + while (current) {
> + if (current->symb_name[0] != current_first_letter) {
> + current_first_letter = current->symb_name[0];
> + list_index[current_first_letter - 32] = current;
> + }
> + current = current->next;
> + }
> +}
> +
> +struct item *add_item(struct item **list, const char *name, char stype, uint64_t addr)
> +{
> + struct item *new_item;
> + struct item *current;
> +
> + new_item = malloc(sizeof(struct item));
> + if (!new_item)
> + return NULL;
> +
> + strncpy(new_item->symb_name, name, MAX_NAME_SIZE);
> + new_item->symb_name[MAX_NAME_SIZE - 1] = '\0';
> + new_item->addr = addr;
> + new_item->stype = stype;
> + new_item->next = NULL;
> +
> + if (!(*list)) {
> + *list = new_item;
> + } else {
> + current = *list;
> + while (current->next)
> + current = current->next;
> +
> + current->next = new_item;
> + }
> + return new_item;
> +}
> +
> +void sort_list(struct item **list, int sort_by)
> +{
> + struct item *current = *list;
> + struct item *sorted = NULL;
> + struct item *next_item;
> + struct item *temp;
> +
> + if (!(*list) || !((*list)->next))
> + return;
> +
> + while (current) {
> + next_item = current->next;
> + if (!sorted ||
> + (CHECK_ORDER_BY_ADDRESS(sort_by, current, sorted, <) ||
> + CHECK_ORDER_BY_NAME(sort_by, current, sorted, >=))) {
> + current->next = sorted;
> + sorted = current;
> + } else {
> + temp = sorted;
> + while (temp->next &&
> + (CHECK_ORDER_BY_ADDRESS(sort_by, current, temp->next, >=) ||
> + CHECK_ORDER_BY_NAME(sort_by, current, temp->next, >=)))
> + temp = temp->next;
> +
> + current->next = temp->next;
> + temp->next = current;
> + }
> + current = next_item;
> + }
> +
> + *list = sorted;
> +}
> +
> +struct item *merge(struct item *left, struct item *right, int sort_by)
> +{
> + struct item *current = NULL;
> + struct item *result = NULL;
> +
> + if (!left)
> + return right;
> + if (!right)
> + return left;
> +
> + if (sort_by == BY_NAME) {
> + if (strcmp(left->symb_name, right->symb_name) <= 0) {
> + result = left;
> + left = left->next;
> + } else {
> + result = right;
> + right = right->next;
> + }
> + } else {
> + if (sort_by == BY_ADDRESS) {
> + if (left->addr <= right->addr) {
> + result = left;
> + left = left->next;
> + } else {
> + result = right;
> + right = right->next;
> + }
> + }
> + }
> +
> + current = result;
> +
> + while (left && right) {
> + if (sort_by == BY_NAME) {
> + if (strcmp(left->symb_name, right->symb_name) <= 0) {
> + current->next = left;
> + left = left->next;
> + } else {
> + current->next = right;
> + right = right->next;
> + }
> + } else {
> + if (sort_by == BY_ADDRESS) {
> + if (left->addr <= right->addr) {
> + current->next = left;
> + left = left->next;
> + } else {
> + current->next = right;
> + right = right->next;
> + }
> + }
> + }
> +
> + current = current->next;
> + }
> +
> + if (left) {
> + current->next = left;
> + } else {
> + if (right)
> + current->next = right;
> + }
> +
> + return result;
> +}
> +
> +struct item *merge_sort(struct item *head, int sort_by)
> +{
> + struct item *right;
> + struct item *slow;
> + struct item *fast;
> + struct item *left;
> +
> + if (!head || !head->next)
> + return head;
> +
> + slow = head;
> + fast = head->next;
> +
> + while (fast && fast->next) {
> + slow = slow->next;
> + fast = fast->next->next;
> + }
> +
> + left = head;
> + right = slow->next;
> + slow->next = NULL;
> +
> + left = merge_sort(left, sort_by);
> + right = merge_sort(right, sort_by);
> +
> + return merge(left, right, sort_by);
> +}
> +
> +void sort_list_m(struct item **head, int sort_by)
> +{
> + if (!(*head) || !((*head)->next))
> + return;
> +
> + *head = merge_sort(*head, sort_by);
> +}
> +
> +int insert_after(struct item *list, const uint64_t search_addr,
> + const char *name, uint64_t addr, char stype)
> +{
> + struct item *new_item;
> + struct item *current;
> + int ret = 0;
> +
> + current = (list_index[name[0] - 32]) ? list_index[name[0] - 32] : list;
> + while (current) {
> + if (current->addr == search_addr) {
> + new_item = malloc(sizeof(struct item));
> + if (!new_item)
> + return ret;
> + strncpy(new_item->symb_name, name, MAX_NAME_SIZE);
> + new_item->symb_name[MAX_NAME_SIZE - 1] = '\0';
> + new_item->addr = addr;
> + new_item->stype = stype;
> + new_item->next = current->next;
> + current->next = new_item;
> + ret = 1;
> + break;
> + }
> + current = current->next;
> + }
> + return ret;
> +}
> +
> +void free_items(struct item **head)
> +{
> + struct item *app, *item_iterator = *head;
> +
> + while (item_iterator) {
> + app = item_iterator;
> + item_iterator = item_iterator->next;
> + free(app);
> + }
> + *head = NULL;
> +}
> diff --git a/scripts/kas_alias/item_list.h b/scripts/kas_alias/item_list.h
> new file mode 100644
> index 000000000000..b4891cb088ee
> --- /dev/null
> +++ b/scripts/kas_alias/item_list.h
> @@ -0,0 +1,26 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +#ifndef ITEM_LIST_H
> +#define ITEM_LIST_H
> +#include <stdint.h>
> +
> +#define MAX_NAME_SIZE 256
> +#define BY_ADDRESS 1
> +#define BY_NAME 2
> +
> +struct item {
> + char symb_name[MAX_NAME_SIZE];
> + uint64_t addr;
> + char stype;
> + struct item *next;
> +};
> +
> +void build_index(struct item *list);
> +struct item *add_item(struct item **list, const char *name, char stype, uint64_t addr);
> +void sort_list(struct item **list, int sort_by);
> +struct item *merge(struct item *left, struct item *right, int sort_by);
> +struct item *merge_sort(struct item *head, int sort_by);
> +void sort_list_m(struct item **head, int sort_by);
> +int insert_after(struct item *list, const uint64_t search_addr,
> + const char *name, uint64_t addr, char stype);
> +void free_items(struct item **head);
> +#endif
> diff --git a/scripts/kas_alias/kas_alias.c b/scripts/kas_alias/kas_alias.c
> new file mode 100644
> index 000000000000..532aeb39f851
> --- /dev/null
> +++ b/scripts/kas_alias/kas_alias.c
> @@ -0,0 +1,217 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <stdint.h>
> +#include <unistd.h>
> +#include <string.h>
> +#include <stdbool.h>
> +#include <stdarg.h>
> +#include <regex.h>
> +
> +#include "item_list.h"
> +#include "duplicates_list.h"
> +#include "a2l.h"
> +
> +#define SYMB_IS_TEXT(s) ((((s)->stype) == 't') || (((s)->stype) == 'T'))
> +#define SYMB_IS_DATA(s) ((((s)->stype) == 'b') || (((s)->stype) == 'B') || \
> + (((s)->stype) == 'd') || (((s)->stype) == 'D') || \
> + (((s)->stype) == 'r') || (((s)->stype) == 'R'))
> +#ifdef CONFIG_KALLSYMS_ALIAS_DATA
> +#define SYMB_NEEDS_ALIAS(s) (SYMB_IS_TEXT(s) || SYMB_IS_DATA(s))
> +#else
> +#define SYMB_NEEDS_ALIAS(s) SYMB_IS_TEXT(s)
> +#endif
> +#define FNOMATCH 0
> +#define FMATCH 1
> +#define EREGEX 2
> +
> +const char *ignore_regex[] = {
> + "^__cfi_.*$", // __cfi_ preamble
> +#ifndef CONFIG_KALLSYMS_ALIAS_DATA_ALL
> + "^_*TRACE_SYSTEM.*$",
> + "^__already_done\\.[0-9]+$", // Call a function once data
> + "^___tp_str\\.[0-9]+$",
> + "^___done\\.[0-9]+$",
> + "^__print_once\\.[0-9]+$",
> + "^_rs\\.[0-9]+$",
> + "^__compound_literal\\.[0-9]+$",
> + "^___once_key\\.[0-9]+$",
> + "^__func__\\.[0-9]+$",
> + "^__msg\\.[0-9]+$",
> + "^CSWTCH\\.[0-9]+$",
> + "^__flags\\.[0-9]+$",
> + "^__wkey.*$",
> + "^__mkey.*$",
> + "^__key.*$",
> +#endif
> + "^__pfx_.*$" // NOP-padding
> +};
> +
> +int suffix_serial;
> +
> +static inline void verbose_msg(bool verbose, const char *fmt, ...)
> +{
> + va_list args;
> +
> + va_start(args, fmt);
> + if (verbose)
> + printf(fmt, args);
> +
> + va_end(args);
> +}
> +
> +static void create_suffix(const char *name, char *output_suffix)
> +{
> + sprintf(output_suffix, "%s__alias__%d", name, suffix_serial++);
> +}
> +
> +static void create_file_suffix(const char *name, uint64_t address, char *output_suffix, char *cwd)
> +{
> + const char *f_path;
> + char *buf;
> + int i = 0;
> +
> + buf = addr2line_get_lines(address);
> + f_path = remove_subdir(cwd, buf);
> + if (f_path) {
> + sprintf(output_suffix, "%s@%s", name, f_path);
> + while (*(output_suffix + i) != '\0') {
> + switch (*(output_suffix + i)) {
> + case '/':
> + case ':':
> + case '.':
This is not enough because file path will accept more than that.
E.g. '+', '&', '-'.
So I think we should check it by '!isalnum()'. (allow-list method)
> + *(output_suffix + i) = '_';
> + break;
> + default:
> + }
> + i++;
> + }
> + } else {
> + create_suffix(name, output_suffix);
> + }
> +}
> +
> +static int filter_symbols(char *symbol, const char **ignore_list, int regex_no)
> +{
> + regex_t regex;
> + int res, i;
> +
> + for (i = 0; i < regex_no; i++) {
> + res = regcomp(®ex, ignore_list[i], REG_EXTENDED);
> + if (res)
> + return -EREGEX;
> +
> + res = regexec(®ex, symbol, 0, NULL, 0);
> + regfree(®ex);
> + switch (res) {
> + case 0:
> + return FMATCH;
> + case REG_NOMATCH:
> + break;
> + default:
> + return -EREGEX;
> + }
> + }
> +
> + return FNOMATCH;
> +}
> +
> +int main(int argc, char *argv[])
> +{
> + char t, sym_name[MAX_NAME_SIZE], new_name[MAX_NAME_SIZE + 15];
> + struct duplicate_item *duplicate_iterator;
> + struct duplicate_item *duplicate;
> + struct item *head = {NULL};
> + bool need_2_process = true;
> + struct item *last = {NULL};
> + struct item *current;
> + int verbose_mode = 0;
> + uint64_t address;
> + FILE *fp;
> + int res;
> +
> + if (argc < 2 || argc > 3) {
> + printf("Usage: %s <nmfile> [-verbose]\n", argv[0]);
> + return 1;
> + }
> +
> + if (argc == 3 && strcmp(argv[2], "-verbose") == 0)
> + verbose_mode = 1;
> +
> + verbose_msg(verbose_mode, "Scanning nm data(%s)\n", argv[1]);
> +
> + fp = fopen(argv[1], "r");
> + if (!fp) {
> + printf("Can't open input file.\n");
> + return 1;
> + }
> +
> + if (!addr2line_init(get_addr2line(A2L_DEFAULT), get_vmlinux(argv[1])))
> + return 1;
> +
> + while (fscanf(fp, "%lx %c %99s\n", &address, &t, sym_name) == 3) {
> + if (strstr(sym_name, "@_")) {
> + if (verbose_mode && need_2_process)
> + printf("Already processed\n");
> + need_2_process = false;
> + }
> + last = add_item(&last, sym_name, t, address);
> + if (!last) {
> + printf("Error in allocate memory\n");
> + free_items(&head);
> + return 1;
> + }
> +
> + if (!head)
> + head = last;
> + }
> +
> + fclose(fp);
> +
> + if (need_2_process) {
> + verbose_msg(verbose_mode, "Sorting nm data\n");
> + sort_list_m(&head, BY_NAME);
> + verbose_msg(verbose_mode, "Scanning nm data for duplicates\n");
> + duplicate = find_duplicates(head);
> + if (!duplicate) {
> + printf("Error in duplicates list\n");
> + return 1;
> + }
> +
> + verbose_msg(verbose_mode, "Applying suffixes\n");
> + build_index(head);
> + duplicate_iterator = duplicate;
> + while (duplicate_iterator) {
> + res = filter_symbols(duplicate_iterator->original_item->symb_name,
> + ignore_regex, sizeof(ignore_regex) /
> + sizeof(ignore_regex[0]));
> + if (res != FMATCH &&
> + SYMB_NEEDS_ALIAS(duplicate_iterator->original_item)) {
> + if (res < 0)
> + return 1;
> +
> + create_file_suffix(duplicate_iterator->original_item->symb_name,
> + duplicate_iterator->original_item->addr,
> + new_name, vmlinux_path);
> + if (!insert_after(head, duplicate_iterator->original_item->addr,
> + new_name, duplicate_iterator->original_item->addr,
> + duplicate_iterator->original_item->stype))
> + return 1;
> + }
> +
> + duplicate_iterator = duplicate_iterator->next;
> + }
> +
> + sort_list_m(&head, BY_ADDRESS);
> + }
> + current = head;
> + while (current) {
> + printf("%08lx %c %s\n", current->addr, current->stype, current->symb_name);
> + current = current->next;
> + }
> +
> + free_items(&head);
> + free_duplicates(&duplicate);
> + addr2line_cleanup();
> + return 0;
> +}
> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> index a432b171be82..cacf60b597ce 100755
> --- a/scripts/link-vmlinux.sh
> +++ b/scripts/link-vmlinux.sh
> @@ -89,8 +89,9 @@ vmlinux_link()
>
> ldflags="${ldflags} ${wl}--script=${objtree}/${KBUILD_LDS}"
>
> - # The kallsyms linking does not need debug symbols included.
> - if [ "$output" != "${output#.tmp_vmlinux.kallsyms}" ] ; then
> + # The kallsyms linking does not need debug symbols included, unless the KALLSYMS_ALIAS.
> + if [ ! is_enabled CONFIG_KALLSYMS_ALIAS ] && \
> + [ "$output" != "${output#.tmp_vmlinux.kallsyms}" ] ; then
> ldflags="${ldflags} ${wl}--strip-debug"
> fi
>
> @@ -161,7 +162,11 @@ kallsyms()
> fi
>
> info KSYMS ${2}
> - scripts/kallsyms ${kallsymopt} ${1} > ${2}
> + if is_enabled CONFIG_KALLSYMS_ALIAS; then
> + ALIAS=".alias"
> + scripts/kas_alias/kas_alias ${1} >${1}${ALIAS}
> + fi
Nit: wrong indentation.
And ALIAS is not defined if CONFIG_KALLSYMS_ALIAS=n.
> + scripts/kallsyms ${kallsymopt} ${1}${ALIAS} > ${2}
> }
>
> # Perform one step in kallsyms generation, including temporary linking of
> --
> 2.34.1
>
Thank you,
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH V11 0/5] riscv: Optimize function trace
From: Björn Töpel @ 2023-08-30 15:28 UTC (permalink / raw)
To: Palmer Dabbelt
Cc: suagrfillet, Paul Walmsley, aou, rostedt, mhiramat, Mark Rutland,
guoren, suagrfillet, Bjorn Topel, jszhang, Conor Dooley, pulehui,
linux-riscv, linux-kernel, linux-trace-kernel, songshuaishuai,
bpf
In-Reply-To: <87zg2hse7m.fsf@all.your.base.are.belong.to.us>
Björn Töpel <bjorn@kernel.org> writes:
> Palmer Dabbelt <palmer@rivosinc.com> writes:
>
>> On Wed, 12 Jul 2023 11:11:08 PDT (-0700), bjorn@kernel.org wrote:
>>> Song Shuai <suagrfillet@gmail.com> writes:
>>>
>>> [...]
>>>
>>>> Add WITH_DIRECT_CALLS support [3] (patch 3, 4)
>>>> ==============================================
>>>
>>> We've had some offlist discussions, so here's some input for a wider
>>> audience! Most importantly, this is for Palmer, so that this series is
>>> not merged until a proper BPF trampoline fix is in place.
>>>
>>> Note that what's currently usable from BPF trampoline *works*. It's
>>> when this series is added that it breaks.
>>>
>>> TL;DR This series adds DYNAMIC_FTRACE_WITH_DIRECT_CALLS, which enables
>>> fentry/fexit BPF trampoline support. Unfortunately the
>>> fexit/BPF_TRAMP_F_SKIP_FRAME parts of the RV BPF trampoline breaks
>>> with this addition, and need to be addressed *prior* merging this
>>> series. An easy way to reproduce, is just calling any of the kselftest
>>> tests that uses fexit patching.
>>>
>>> The issue is around the nop seld, and how a call is done; The nop sled
>>> (patchable-function-entry) size changed from 16B to 8B in commit
>>> 6724a76cff85 ("riscv: ftrace: Reduce the detour code size to half"), but
>>> BPF code still uses the old 16B. So it'll work for BPF programs, but not
>>> for regular kernel functions.
>>>
>>> An example:
>>>
>>> | ffffffff80fa4150 <bpf_fentry_test1>:
>>> | ffffffff80fa4150: 0001 nop
>>> | ffffffff80fa4152: 0001 nop
>>> | ffffffff80fa4154: 0001 nop
>>> | ffffffff80fa4156: 0001 nop
>>> | ffffffff80fa4158: 1141 add sp,sp,-16
>>> | ffffffff80fa415a: e422 sd s0,8(sp)
>>> | ffffffff80fa415c: 0800 add s0,sp,16
>>> | ffffffff80fa415e: 6422 ld s0,8(sp)
>>> | ffffffff80fa4160: 2505 addw a0,a0,1
>>> | ffffffff80fa4162: 0141 add sp,sp,16
>>> | ffffffff80fa4164: 8082 ret
>>>
>>> is patched to:
>>>
>>> | ffffffff80fa4150: f70c0297 auipc t0,-150208512
>>> | ffffffff80fa4154: eb0282e7 jalr t0,t0,-336
>>>
>>> The return address to bpf_fentry_test1 is stored in t0 at BPF
>>> trampoline entry. Return to the *parent* is in ra. The trampline has
>>> to deal with this.
>>>
>>> For BPF_TRAMP_F_SKIP_FRAME/CALL_ORIG, the BPF trampoline will skip too
>>> many bytes, and not correctly handle parent calls.
>>>
>>> Further; The BPF trampoline currently has a different way of patching
>>> the nops for BPF programs, than what ftrace does. That should be changed
>>> to match what ftrace does (auipc/jalr t0).
>>>
>>> To summarize:
>>> * Align BPF nop sled with patchable-function-entry: 8B.
>>> * Adapt BPF trampoline for 8B nop sleds.
>>> * Adapt BPF trampoline t0 return, ra parent scheme.
>>
>> Thanks for digging into this one, I agree we need to sort out the BPF
>> breakages before we merge this. Sounds like there's a rabbit hole here,
>> but hopefully we can get it sorted out.
>>
>> I've dropped this from patchwork and such, as we'll need at least
>> another spin.
>
> Palmer,
>
> The needed BPF patch is upstream in the bpf-next tree, and has been for
> a couple of weeks.
>
> I think this series is a candidate for RISC-V -next! It would help
> RISC-V BPF a lot in terms of completeness.
Palmer,
The needed fix for BPF is now in Linus' tree, commit 25ad10658dc1
("riscv, bpf: Adapt bpf trampoline to optimized riscv ftrace
framework"). IOW, this ftrace series can be merged now.
Björn
^ permalink raw reply
* Re: [PATCH v4 6/9] tracing/fprobe: Enable fprobe events with CONFIG_DYNAMIC_FTRACE_WITH_ARGS
From: Masami Hiramatsu @ 2023-08-30 7:20 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: Alexei Starovoitov, Steven Rostedt, Florent Revest,
linux-trace-kernel, LKML, Martin KaFai Lau, bpf, Sven Schnelle,
Alexei Starovoitov, Jiri Olsa, Arnaldo Carvalho de Melo,
Daniel Borkmann, Alan Maguire, Mark Rutland, Peter Zijlstra,
Thomas Gleixner
In-Reply-To: <169280379741.282662.12221517584561036597.stgit@devnote2>
On Thu, 24 Aug 2023 00:16:37 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> +#if !defined(CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS) || \
> + defined(CONFIG_HAVE_PT_REGS_TO_FTRACE_REGS_CAST)
> +
> +static __always_inline
> +struct pt_regs *perf_fprobe_partial_regs(struct ftrace_regs *fregs)
> +{
> + /* See include/linux/ftrace.h, this returns &fregs->regs */
> + return ftrace_partial_regs(fregs, NULL);
> +}
> +
> +#define perf_fprobe_return_regs(regs) do {} while (0)
> +
> +#else /* CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS && !CONFIG_HAVE_PT_REGS_TO_FTRACE_REGS_CAST */
> +
> +/* Since fprobe handlers can be nested, pt_regs buffer need to be a stack */
> +#define PERF_FPROBE_REGS_MAX 4
> +
> +struct pt_regs_stack {
> + struct pt_regs regs[PERF_FPROBE_REGS_MAX];
> + int idx;
> +};
> +
> +static DEFINE_PER_CPU(struct pt_regs_stack, perf_fprobe_regs);
> +
> +static __always_inline
> +struct pt_regs *perf_fprobe_partial_regs(struct ftrace_regs *fregs)
> +{
> + struct pt_regs_stack *stack = this_cpu_ptr(&perf_fprobe_regs);
> + struct pt_regs *regs;
> +
> + if (stack->idx < PERF_FPROBE_REGS_MAX) {
> + regs = stack->regs[stack->idx++];
> + return ftrace_partial_regs(fregs, regs);
> + }
> + return NULL;
> +}
> +
> +static __always_inline void perf_fprobe_return_regs(struct pt_regs *regs)
> +{
> + struct pt_regs_stack *stack = this_cpu_ptr(&perf_fprobe_regs);
> +
> + if (WARN_ON_ONCE(regs != stack->regs[stack->idx]))
> + return;
> +
> + --stack->idx;
> +}
Ah, I found that the perf_trace_buf_alloc() does the same thing. So
perf_trace_buf_alloc(size, &pt_regs, &rctx);
will give us the pt_regs at that point. Trace event does that so I think
it is OK to do that here.
Thank you,
> +
> +#endif /* !CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS || CONFIG_HAVE_PT_REGS_TO_FTRACE_REGS_CAST */
> +
> static int fentry_perf_func(struct trace_fprobe *tf, unsigned long entry_ip,
> - struct pt_regs *regs)
> + struct ftrace_regs *fregs)
> {
> struct trace_event_call *call = trace_probe_event_call(&tf->tp);
> struct fentry_trace_entry_head *entry;
> struct hlist_head *head;
> int size, __size, dsize;
> + struct pt_regs *regs;
> int rctx;
>
> + regs = perf_fprobe_partial_regs(fregs);
> + if (!regs)
> + return -EINVAL;
> +
> head = this_cpu_ptr(call->perf_events);
> if (hlist_empty(head))
> - return 0;
> + goto out;
>
> - dsize = __get_data_size(&tf->tp, regs);
> + dsize = __get_data_size(&tf->tp, fregs);
> __size = sizeof(*entry) + tf->tp.size + dsize;
> size = ALIGN(__size + sizeof(u32), sizeof(u64));
> size -= sizeof(u32);
>
> entry = perf_trace_buf_alloc(size, NULL, &rctx);
Here, we can borrow the pt_regs.
> if (!entry)
> - return 0;
> + goto out;
>
> entry->ip = entry_ip;
> memset(&entry[1], 0, dsize);
> - store_trace_args(&entry[1], &tf->tp, regs, sizeof(*entry), dsize);
> + store_trace_args(&entry[1], &tf->tp, fregs, sizeof(*entry), dsize);
> perf_trace_buf_submit(entry, size, rctx, call->event.type, 1, regs,
> head, NULL);
> +out:
> + perf_fprobe_return_regs(regs);
> return 0;
> }
> NOKPROBE_SYMBOL(fentry_perf_func);
>
> static void
> fexit_perf_func(struct trace_fprobe *tf, unsigned long entry_ip,
> - unsigned long ret_ip, struct pt_regs *regs)
> + unsigned long ret_ip, struct ftrace_regs *fregs)
> {
> struct trace_event_call *call = trace_probe_event_call(&tf->tp);
> struct fexit_trace_entry_head *entry;
> struct hlist_head *head;
> int size, __size, dsize;
> + struct pt_regs *regs;
> int rctx;
>
> + regs = perf_fprobe_partial_regs(fregs);
> + if (!regs)
> + return;
> +
> head = this_cpu_ptr(call->perf_events);
> if (hlist_empty(head))
> - return;
> + goto out;
>
> - dsize = __get_data_size(&tf->tp, regs);
> + dsize = __get_data_size(&tf->tp, fregs);
> __size = sizeof(*entry) + tf->tp.size + dsize;
> size = ALIGN(__size + sizeof(u32), sizeof(u64));
> size -= sizeof(u32);
>
> entry = perf_trace_buf_alloc(size, NULL, &rctx);
Ditto.
Thanks,
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH v1 1/1] bitops: Share BYTES_TO_BITS() for everyone
From: Masami Hiramatsu @ 2023-08-30 7:16 UTC (permalink / raw)
To: Alexander Lobakin
Cc: Andy Shevchenko, Yury Norov, Uros Bizjak,
Masami Hiramatsu (Google), linux-kernel, linux-trace-kernel,
Steven Rostedt
In-Reply-To: <a5574786-0a49-454d-67e3-571983e6a6e8@intel.com>
On Fri, 25 Aug 2023 16:49:07 +0200
Alexander Lobakin <aleksander.lobakin@intel.com> wrote:
> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Date: Thu, 24 Aug 2023 15:37:28 +0300
>
> > It may be new callers for the same macro, share it.
> >
> > Note, it's unknown why it's represented in the current form instead of
> > simple multiplication and commit 1ff511e35ed8 ("tracing/kprobes: Add
> > bitfield type") doesn't explain that neither. Let leave it as is and
> > we may improve it in the future.
>
> Maybe symmetrical change in tools/ like I did[0] an aeon ago?
Hm, it looks the same. The reason why I used the macro was I didn't
know the BITS_PER_TYPE() at that point. Now it is OK to replace it
with 'bytes * BITS_PER_TYPE(char)'.
Thank you,
>
> >
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> > include/linux/bitops.h | 2 ++
> > kernel/trace/trace_probe.c | 3 +--
> > 2 files changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/linux/bitops.h b/include/linux/bitops.h
> > index 2ba557e067fe..66dc091e0c28 100644
> > --- a/include/linux/bitops.h
> > +++ b/include/linux/bitops.h
> > @@ -21,6 +21,8 @@
> > #define BITS_TO_U32(nr) __KERNEL_DIV_ROUND_UP(nr, BITS_PER_TYPE(u32))
> > #define BITS_TO_BYTES(nr) __KERNEL_DIV_ROUND_UP(nr, BITS_PER_TYPE(char))
> >
> > +#define BYTES_TO_BITS(nb) ((BITS_PER_LONG * (nb)) / sizeof(long))
> > +
> > extern unsigned int __sw_hweight8(unsigned int w);
> > extern unsigned int __sw_hweight16(unsigned int w);
> > extern unsigned int __sw_hweight32(unsigned int w);
> > diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> > index c68a72707852..da6297d24d61 100644
> > --- a/kernel/trace/trace_probe.c
> > +++ b/kernel/trace/trace_probe.c
> > @@ -11,6 +11,7 @@
> > */
> > #define pr_fmt(fmt) "trace_probe: " fmt
> >
> > +#include <linux/bitops.h>
> > #include <linux/bpf.h>
> >
> > #include "trace_probe.h"
> > @@ -830,8 +831,6 @@ parse_probe_arg(char *arg, const struct fetch_type *type,
> > return ret;
> > }
> >
> > -#define BYTES_TO_BITS(nb) ((BITS_PER_LONG * (nb)) / sizeof(long))
> > -
> > /* Bitfield type needs to be parsed into a fetch function */
> > static int __parse_bitfield_probe_arg(const char *bf,
> > const struct fetch_type *t,
>
> [0]
> https://github.com/alobakin/linux/commit/fd308001fe6d38837fe820427209a6a99e4850a8
>
> Thanks,
> Olek
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH] visl: use canonical ftrace path
From: Masami Hiramatsu @ 2023-08-31 0:12 UTC (permalink / raw)
To: Ross Zwisler
Cc: linux-kernel, Ross Zwisler, Mark Rutland, Masami Hiramatsu,
Mauro Carvalho Chehab, Steven Rostedt, linux-media,
linux-trace-kernel, Daniel Almeida, Hans Verkuil
In-Reply-To: <20230829204600.3210276-2-zwisler@kernel.org>
On Tue, 29 Aug 2023 14:46:01 -0600
Ross Zwisler <zwisler@kernel.org> wrote:
> From: Ross Zwisler <zwisler@google.com>
>
> The canonical location for the tracefs filesystem is at /sys/kernel/tracing.
>
> But, from Documentation/trace/ftrace.rst:
>
> Before 4.1, all ftrace tracing control files were within the debugfs
> file system, which is typically located at /sys/kernel/debug/tracing.
> For backward compatibility, when mounting the debugfs file system,
> the tracefs file system will be automatically mounted at:
>
> /sys/kernel/debug/tracing
>
> Update the visl decoder driver documentation to use this tracefs path.
>
> Signed-off-by: Ross Zwisler <zwisler@google.com>
Looks good to me.
Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Thanks!
> ---
> Documentation/admin-guide/media/visl.rst | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/admin-guide/media/visl.rst b/Documentation/admin-guide/media/visl.rst
> index 7d2dc78341c9..4328c6c72d30 100644
> --- a/Documentation/admin-guide/media/visl.rst
> +++ b/Documentation/admin-guide/media/visl.rst
> @@ -78,7 +78,7 @@ The trace events are defined on a per-codec basis, e.g.:
>
> .. code-block:: bash
>
> - $ ls /sys/kernel/debug/tracing/events/ | grep visl
> + $ ls /sys/kernel/tracing/events/ | grep visl
> visl_fwht_controls
> visl_h264_controls
> visl_hevc_controls
> @@ -90,13 +90,13 @@ For example, in order to dump HEVC SPS data:
>
> .. code-block:: bash
>
> - $ echo 1 > /sys/kernel/debug/tracing/events/visl_hevc_controls/v4l2_ctrl_hevc_sps/enable
> + $ echo 1 > /sys/kernel/tracing/events/visl_hevc_controls/v4l2_ctrl_hevc_sps/enable
>
> The SPS data will be dumped to the trace buffer, i.e.:
>
> .. code-block:: bash
>
> - $ cat /sys/kernel/debug/tracing/trace
> + $ cat /sys/kernel/tracing/trace
> video_parameter_set_id 0
> seq_parameter_set_id 0
> pic_width_in_luma_samples 1920
> --
> 2.42.0.rc2.253.gd59a3bf2b4-goog
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* [linus:master] [tracing/synthetic] ddeea494a1: BUG:unable_to_handle_page_fault_for_address
From: kernel test robot @ 2023-08-31 2:26 UTC (permalink / raw)
To: Sven Schnelle
Cc: oe-lkp, lkp, linux-kernel, Steven Rostedt, Masami Hiramatsu,
linux-trace-kernel, oliver.sang
Hello,
kernel test robot noticed "BUG:unable_to_handle_page_fault_for_address" on:
commit: ddeea494a16f32522bce16ee65f191d05d4b8282 ("tracing/synthetic: Use union instead of casts")
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git master
in testcase: boot
compiler: clang-16
test machine: qemu-system-i386 -enable-kvm -cpu SandyBridge -smp 2 -m 4G
(please refer to attached dmesg/kmsg for entire log/backtrace)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202308311026.b4bc6948-oliver.sang@intel.com
[ 390.645593][ T41] BUG: unable to handle page fault for address: 001c24ca
[ 390.646174][ T41] #PF: supervisor read access in kernel mode
[ 390.646669][ T41] #PF: error_code(0x0000) - not-present page
[ 390.647170][ T41] *pdpt = 000000002e192001 *pde = 0000000000000000
[ 390.647717][ T41] Oops: 0000 [#1] PREEMPT SMP
[ 390.648101][ T41] CPU: 1 PID: 41 Comm: rcu_scale_write Tainted: G T 6.5.0-rc6-00003-gddeea494a16f #1 402c7a624341eb9320bc4b1c80c0d4ec9184aa32
[ 390.649312][ T41] EIP: string (lib/vsprintf.c:644)
[ 390.649695][ T41] Code: 73 43 89 c1 8b 44 24 10 89 04 24 89 5c 24 04 c1 fb 10 0f 84 ab 00 00 00 31 c0 89 ce eb 09 40 39 c3 0f 84 a2 00 00 00 8d 0c 07 <0f> b6 14 06 84 d2 0f 84 99 00 00 00 3b 4c 24 0c 73 e2 88 11 eb de
All code
========
0: 73 43 jae 0x45
2: 89 c1 mov %eax,%ecx
4: 8b 44 24 10 mov 0x10(%rsp),%eax
8: 89 04 24 mov %eax,(%rsp)
b: 89 5c 24 04 mov %ebx,0x4(%rsp)
f: c1 fb 10 sar $0x10,%ebx
12: 0f 84 ab 00 00 00 je 0xc3
18: 31 c0 xor %eax,%eax
1a: 89 ce mov %ecx,%esi
1c: eb 09 jmp 0x27
1e: 40 39 c3 rex cmp %eax,%ebx
21: 0f 84 a2 00 00 00 je 0xc9
27: 8d 0c 07 lea (%rdi,%rax,1),%ecx
2a:* 0f b6 14 06 movzbl (%rsi,%rax,1),%edx <-- trapping instruction
2e: 84 d2 test %dl,%dl
30: 0f 84 99 00 00 00 je 0xcf
36: 3b 4c 24 0c cmp 0xc(%rsp),%ecx
3a: 73 e2 jae 0x1e
3c: 88 11 mov %dl,(%rcx)
3e: eb de jmp 0x1e
Code starting with the faulting instruction
===========================================
0: 0f b6 14 06 movzbl (%rsi,%rax,1),%edx
4: 84 d2 test %dl,%dl
6: 0f 84 99 00 00 00 je 0xa5
c: 3b 4c 24 0c cmp 0xc(%rsp),%ecx
10: 73 e2 jae 0xfffffffffffffff4
12: 88 11 mov %dl,(%rcx)
14: eb de jmp 0xfffffffffffffff4
[ 390.651345][ T41] EAX: 00000000 EBX: ffffffff ECX: c31b291f EDX: 00000000
[ 390.651928][ T41] ESI: 001c24ca EDI: c31b291f EBP: c428fcc4 ESP: c428fca0
[ 390.652517][ T41] DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068 EFLAGS: 00010046
[ 390.653162][ T41] CR0: 80050033 CR2: 001c24ca CR3: 2e2a62a0 CR4: 000406f0
[ 390.653800][ T41] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
[ 390.654421][ T41] DR6: fffe0ff0 DR7: 00000400
[ 390.654809][ T41] Call Trace:
[ 390.655097][ T41] ? __die_body (arch/x86/kernel/dumpstack.c:478 arch/x86/kernel/dumpstack.c:420)
[ 390.655473][ T41] ? __die (arch/x86/kernel/dumpstack.c:434)
[ 390.655807][ T41] ? page_fault_oops (arch/x86/mm/fault.c:703)
[ 390.656237][ T41] ? pvclock_clocksource_read_nowd (arch/x86/include/asm/pvclock.h:36 arch/x86/kernel/pvclock.c:79 arch/x86/kernel/pvclock.c:120)
[ 390.656769][ T41] ? kernelmode_fixup_or_oops (arch/x86/mm/fault.c:761)
[ 390.657284][ T41] ? __bad_area_nosemaphore (arch/x86/mm/fault.c:817)
[ 390.657763][ T41] ? pvclock_clocksource_read_nowd (arch/x86/include/asm/pvclock.h:36 arch/x86/kernel/pvclock.c:79 arch/x86/kernel/pvclock.c:120)
[ 390.658279][ T41] ? bad_area_nosemaphore (arch/x86/mm/fault.c:866)
[ 390.658710][ T41] ? do_user_addr_fault (arch/x86/mm/fault.c:?)
[ 390.659137][ T41] ? exc_page_fault (arch/x86/include/asm/irqflags.h:19 arch/x86/include/asm/irqflags.h:67 arch/x86/include/asm/irqflags.h:127 arch/x86/mm/fault.c:1494 arch/x86/mm/fault.c:1542)
[ 390.659538][ T41] ? local_clock_noinstr (kernel/sched/build_utility.c:301)
[ 390.659974][ T41] ? pvclock_clocksource_read_nowd (arch/x86/mm/fault.c:1499)
[ 390.660529][ T41] ? handle_exception (arch/x86/entry/entry_32.S:1049)
[ 390.660982][ T41] ? pvclock_clocksource_read_nowd (arch/x86/mm/fault.c:1499)
[ 390.661525][ T41] ? string (lib/vsprintf.c:644)
[ 390.661882][ T41] ? pvclock_clocksource_read_nowd (arch/x86/mm/fault.c:1499)
[ 390.662443][ T41] ? string (lib/vsprintf.c:644)
[ 390.662796][ T41] vsnprintf (lib/vsprintf.c:2817)
[ 390.663174][ T41] seq_buf_vprintf (lib/seq_buf.c:64)
[ 390.663579][ T41] trace_seq_printf (include/linux/seq_buf.h:47 kernel/trace/trace_seq.c:96)
[ 390.663979][ T41] print_synth_event (kernel/trace/trace_events_synth.c:? kernel/trace/trace_events_synth.c:409)
[ 390.664446][ T41] print_trace_fmt (kernel/trace/trace.c:?)
[ 390.664878][ T41] print_trace_line (kernel/trace/trace.c:?)
[ 390.665321][ T41] ftrace_dump (kernel/trace/trace.c:10167)
[ 390.665711][ T41] rcu_scale_writer (kernel/rcu/rcuscale.c:486)
[ 390.666140][ T41] kthread (kernel/kthread.c:391)
[ 390.666516][ T41] ? rcu_scale_reader (kernel/rcu/rcuscale.c:410)
[ 390.666972][ T41] ? kthread_unuse_mm (kernel/kthread.c:342)
[ 390.667422][ T41] ? kthread_unuse_mm (kernel/kthread.c:342)
[ 390.667879][ T41] ret_from_fork (arch/x86/kernel/process.c:151)
[ 390.668271][ T41] ret_from_fork_asm (arch/x86/entry/entry_32.S:741)
[ 390.668691][ T41] entry_INT80_32 (arch/x86/entry/entry_32.S:947)
[ 390.669124][ T41] Modules linked in:
[ 390.669435][ T41] CR2: 00000000001c24ca
[ 390.669806][ T41] ---[ end trace 0000000000000000 ]---
[ 390.670258][ T41] EIP: string (lib/vsprintf.c:644)
[ 390.670623][ T41] Code: 73 43 89 c1 8b 44 24 10 89 04 24 89 5c 24 04 c1 fb 10 0f 84 ab 00 00 00 31 c0 89 ce eb 09 40 39 c3 0f 84 a2 00 00 00 8d 0c 07 <0f> b6 14 06 84 d2 0f 84 99 00 00 00 3b 4c 24 0c 73 e2 88 11 eb de
All code
========
0: 73 43 jae 0x45
2: 89 c1 mov %eax,%ecx
4: 8b 44 24 10 mov 0x10(%rsp),%eax
8: 89 04 24 mov %eax,(%rsp)
b: 89 5c 24 04 mov %ebx,0x4(%rsp)
f: c1 fb 10 sar $0x10,%ebx
12: 0f 84 ab 00 00 00 je 0xc3
18: 31 c0 xor %eax,%eax
1a: 89 ce mov %ecx,%esi
1c: eb 09 jmp 0x27
1e: 40 39 c3 rex cmp %eax,%ebx
21: 0f 84 a2 00 00 00 je 0xc9
27: 8d 0c 07 lea (%rdi,%rax,1),%ecx
2a:* 0f b6 14 06 movzbl (%rsi,%rax,1),%edx <-- trapping instruction
2e: 84 d2 test %dl,%dl
30: 0f 84 99 00 00 00 je 0xcf
36: 3b 4c 24 0c cmp 0xc(%rsp),%ecx
3a: 73 e2 jae 0x1e
3c: 88 11 mov %dl,(%rcx)
3e: eb de jmp 0x1e
Code starting with the faulting instruction
===========================================
0: 0f b6 14 06 movzbl (%rsi,%rax,1),%edx
4: 84 d2 test %dl,%dl
6: 0f 84 99 00 00 00 je 0xa5
c: 3b 4c 24 0c cmp 0xc(%rsp),%ecx
10: 73 e2 jae 0xfffffffffffffff4
12: 88 11 mov %dl,(%rcx)
14: eb de jmp 0xfffffffffffffff4
The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20230831/202308311026.b4bc6948-oliver.sang@intel.com
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* [RFC PATCH 1/1] smp: Change function signatures to use call_single_data_t
From: Leonardo Bras @ 2023-08-31 6:31 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Peter Zijlstra, Josh Poimboeuf,
Guo Ren, Valentin Schneider, Leonardo Bras, Paul E. McKenney,
Juergen Gross, Yury Norov, Imran Khan
Cc: linux-kernel, linux-trace-kernel
call_single_data_t is a size-aligned typedef of struct __call_single_data.
This alignment is desirable in order to have smp_call_function*() avoid
bouncing an extra cacheline in case of an unaligned csd, given this
would hurt performance.
Since the removal of struct request->csd in commit 660e802c76c8
("blk-mq: use percpu csd to remote complete instead of per-rq csd") there
are no current users of smp_call_function*() with unaligned csd.
Change every 'struct __call_single_data' function parameter to
'call_single_data_t', so we have warnings if any new code tries to
introduce an smp_call_function*() call with unaligned csd.
Signed-off-by: Leonardo Bras <leobras@redhat.com>
---
include/linux/smp.h | 2 +-
include/trace/events/csd.h | 8 ++++----
kernel/smp.c | 26 +++++++++++++-------------
kernel/up.c | 2 +-
4 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/include/linux/smp.h b/include/linux/smp.h
index 91ea4a67f8ca..e87520dc2959 100644
--- a/include/linux/smp.h
+++ b/include/linux/smp.h
@@ -53,7 +53,7 @@ int smp_call_function_single(int cpuid, smp_call_func_t func, void *info,
void on_each_cpu_cond_mask(smp_cond_func_t cond_func, smp_call_func_t func,
void *info, bool wait, const struct cpumask *mask);
-int smp_call_function_single_async(int cpu, struct __call_single_data *csd);
+int smp_call_function_single_async(int cpu, call_single_data_t *csd);
/*
* Cpus stopping functions in panic. All have default weak definitions.
diff --git a/include/trace/events/csd.h b/include/trace/events/csd.h
index 67e9d01f80c2..58cc83b99c34 100644
--- a/include/trace/events/csd.h
+++ b/include/trace/events/csd.h
@@ -12,7 +12,7 @@ TRACE_EVENT(csd_queue_cpu,
TP_PROTO(const unsigned int cpu,
unsigned long callsite,
smp_call_func_t func,
- struct __call_single_data *csd),
+ call_single_data_t *csd),
TP_ARGS(cpu, callsite, func, csd),
@@ -39,7 +39,7 @@ TRACE_EVENT(csd_queue_cpu,
*/
DECLARE_EVENT_CLASS(csd_function,
- TP_PROTO(smp_call_func_t func, struct __call_single_data *csd),
+ TP_PROTO(smp_call_func_t func, call_single_data_t *csd),
TP_ARGS(func, csd),
@@ -57,12 +57,12 @@ DECLARE_EVENT_CLASS(csd_function,
);
DEFINE_EVENT(csd_function, csd_function_entry,
- TP_PROTO(smp_call_func_t func, struct __call_single_data *csd),
+ TP_PROTO(smp_call_func_t func, call_single_data_t *csd),
TP_ARGS(func, csd)
);
DEFINE_EVENT(csd_function, csd_function_exit,
- TP_PROTO(smp_call_func_t func, struct __call_single_data *csd),
+ TP_PROTO(smp_call_func_t func, call_single_data_t *csd),
TP_ARGS(func, csd)
);
diff --git a/kernel/smp.c b/kernel/smp.c
index 8455a53465af..8c714583786b 100644
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -127,7 +127,7 @@ send_call_function_ipi_mask(struct cpumask *mask)
}
static __always_inline void
-csd_do_func(smp_call_func_t func, void *info, struct __call_single_data *csd)
+csd_do_func(smp_call_func_t func, void *info, call_single_data_t *csd)
{
trace_csd_function_entry(func, csd);
func(info);
@@ -174,7 +174,7 @@ module_param(csd_lock_timeout, ulong, 0444);
static atomic_t csd_bug_count = ATOMIC_INIT(0);
/* Record current CSD work for current CPU, NULL to erase. */
-static void __csd_lock_record(struct __call_single_data *csd)
+static void __csd_lock_record(call_single_data_t *csd)
{
if (!csd) {
smp_mb(); /* NULL cur_csd after unlock. */
@@ -189,13 +189,13 @@ static void __csd_lock_record(struct __call_single_data *csd)
/* Or before unlock, as the case may be. */
}
-static __always_inline void csd_lock_record(struct __call_single_data *csd)
+static __always_inline void csd_lock_record(call_single_data_t *csd)
{
if (static_branch_unlikely(&csdlock_debug_enabled))
__csd_lock_record(csd);
}
-static int csd_lock_wait_getcpu(struct __call_single_data *csd)
+static int csd_lock_wait_getcpu(call_single_data_t *csd)
{
unsigned int csd_type;
@@ -210,7 +210,7 @@ static int csd_lock_wait_getcpu(struct __call_single_data *csd)
* the CSD_TYPE_SYNC/ASYNC types provide the destination CPU,
* so waiting on other types gets much less information.
*/
-static bool csd_lock_wait_toolong(struct __call_single_data *csd, u64 ts0, u64 *ts1, int *bug_id)
+static bool csd_lock_wait_toolong(call_single_data_t *csd, u64 ts0, u64 *ts1, int *bug_id)
{
int cpu = -1;
int cpux;
@@ -276,7 +276,7 @@ static bool csd_lock_wait_toolong(struct __call_single_data *csd, u64 ts0, u64 *
* previous function call. For multi-cpu calls its even more interesting
* as we'll have to ensure no other cpu is observing our csd.
*/
-static void __csd_lock_wait(struct __call_single_data *csd)
+static void __csd_lock_wait(call_single_data_t *csd)
{
int bug_id = 0;
u64 ts0, ts1;
@@ -290,7 +290,7 @@ static void __csd_lock_wait(struct __call_single_data *csd)
smp_acquire__after_ctrl_dep();
}
-static __always_inline void csd_lock_wait(struct __call_single_data *csd)
+static __always_inline void csd_lock_wait(call_single_data_t *csd)
{
if (static_branch_unlikely(&csdlock_debug_enabled)) {
__csd_lock_wait(csd);
@@ -300,17 +300,17 @@ static __always_inline void csd_lock_wait(struct __call_single_data *csd)
smp_cond_load_acquire(&csd->node.u_flags, !(VAL & CSD_FLAG_LOCK));
}
#else
-static void csd_lock_record(struct __call_single_data *csd)
+static void csd_lock_record(call_single_data_t *csd)
{
}
-static __always_inline void csd_lock_wait(struct __call_single_data *csd)
+static __always_inline void csd_lock_wait(call_single_data_t *csd)
{
smp_cond_load_acquire(&csd->node.u_flags, !(VAL & CSD_FLAG_LOCK));
}
#endif
-static __always_inline void csd_lock(struct __call_single_data *csd)
+static __always_inline void csd_lock(call_single_data_t *csd)
{
csd_lock_wait(csd);
csd->node.u_flags |= CSD_FLAG_LOCK;
@@ -323,7 +323,7 @@ static __always_inline void csd_lock(struct __call_single_data *csd)
smp_wmb();
}
-static __always_inline void csd_unlock(struct __call_single_data *csd)
+static __always_inline void csd_unlock(call_single_data_t *csd)
{
WARN_ON(!(csd->node.u_flags & CSD_FLAG_LOCK));
@@ -376,7 +376,7 @@ void __smp_call_single_queue(int cpu, struct llist_node *node)
* for execution on the given CPU. data must already have
* ->func, ->info, and ->flags set.
*/
-static int generic_exec_single(int cpu, struct __call_single_data *csd)
+static int generic_exec_single(int cpu, call_single_data_t *csd)
{
if (cpu == smp_processor_id()) {
smp_call_func_t func = csd->func;
@@ -667,7 +667,7 @@ EXPORT_SYMBOL(smp_call_function_single);
*
* Return: %0 on success or negative errno value on error
*/
-int smp_call_function_single_async(int cpu, struct __call_single_data *csd)
+int smp_call_function_single_async(int cpu, call_single_data_t *csd)
{
int err = 0;
diff --git a/kernel/up.c b/kernel/up.c
index a38b8b095251..df50828cc2f0 100644
--- a/kernel/up.c
+++ b/kernel/up.c
@@ -25,7 +25,7 @@ int smp_call_function_single(int cpu, void (*func) (void *info), void *info,
}
EXPORT_SYMBOL(smp_call_function_single);
-int smp_call_function_single_async(int cpu, struct __call_single_data *csd)
+int smp_call_function_single_async(int cpu, call_single_data_t *csd)
{
unsigned long flags;
--
2.42.0
^ permalink raw reply related
* Re: [RFC PATCH 1/1] smp: Change function signatures to use call_single_data_t
From: Guo Ren @ 2023-08-31 6:58 UTC (permalink / raw)
To: Leonardo Bras
Cc: Steven Rostedt, Masami Hiramatsu, Peter Zijlstra, Josh Poimboeuf,
Valentin Schneider, Paul E. McKenney, Juergen Gross, Yury Norov,
Imran Khan, linux-kernel, linux-trace-kernel
In-Reply-To: <20230831063129.335425-1-leobras@redhat.com>
On Thu, Aug 31, 2023 at 2:31 PM Leonardo Bras <leobras@redhat.com> wrote:
>
> call_single_data_t is a size-aligned typedef of struct __call_single_data.
>
> This alignment is desirable in order to have smp_call_function*() avoid
> bouncing an extra cacheline in case of an unaligned csd, given this
> would hurt performance.
>
> Since the removal of struct request->csd in commit 660e802c76c8
> ("blk-mq: use percpu csd to remote complete instead of per-rq csd") there
> are no current users of smp_call_function*() with unaligned csd.
>
> Change every 'struct __call_single_data' function parameter to
> 'call_single_data_t', so we have warnings if any new code tries to
> introduce an smp_call_function*() call with unaligned csd.
I agree to prevent __call_single_data usage.
Reviewed-by: Guo Ren <guoren@kernel.org>
/*
* structure shares (partial) layout with struct irq_work
*/
struct __call_single_data {
struct __call_single_node node;
smp_call_func_t func;
void *info;
};
#define CSD_INIT(_func, _info) \
(struct __call_single_data){ .func = (_func), .info = (_info), }
/* Use __aligned() to avoid to use 2 cache lines for 1 csd */
typedef struct __call_single_data call_single_data_t
__aligned(sizeof(struct __call_single_data));
>
> Signed-off-by: Leonardo Bras <leobras@redhat.com>
> ---
> include/linux/smp.h | 2 +-
> include/trace/events/csd.h | 8 ++++----
> kernel/smp.c | 26 +++++++++++++-------------
> kernel/up.c | 2 +-
> 4 files changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/include/linux/smp.h b/include/linux/smp.h
> index 91ea4a67f8ca..e87520dc2959 100644
> --- a/include/linux/smp.h
> +++ b/include/linux/smp.h
> @@ -53,7 +53,7 @@ int smp_call_function_single(int cpuid, smp_call_func_t func, void *info,
> void on_each_cpu_cond_mask(smp_cond_func_t cond_func, smp_call_func_t func,
> void *info, bool wait, const struct cpumask *mask);
>
> -int smp_call_function_single_async(int cpu, struct __call_single_data *csd);
> +int smp_call_function_single_async(int cpu, call_single_data_t *csd);
>
> /*
> * Cpus stopping functions in panic. All have default weak definitions.
> diff --git a/include/trace/events/csd.h b/include/trace/events/csd.h
> index 67e9d01f80c2..58cc83b99c34 100644
> --- a/include/trace/events/csd.h
> +++ b/include/trace/events/csd.h
> @@ -12,7 +12,7 @@ TRACE_EVENT(csd_queue_cpu,
> TP_PROTO(const unsigned int cpu,
> unsigned long callsite,
> smp_call_func_t func,
> - struct __call_single_data *csd),
> + call_single_data_t *csd),
>
> TP_ARGS(cpu, callsite, func, csd),
>
> @@ -39,7 +39,7 @@ TRACE_EVENT(csd_queue_cpu,
> */
> DECLARE_EVENT_CLASS(csd_function,
>
> - TP_PROTO(smp_call_func_t func, struct __call_single_data *csd),
> + TP_PROTO(smp_call_func_t func, call_single_data_t *csd),
>
> TP_ARGS(func, csd),
>
> @@ -57,12 +57,12 @@ DECLARE_EVENT_CLASS(csd_function,
> );
>
> DEFINE_EVENT(csd_function, csd_function_entry,
> - TP_PROTO(smp_call_func_t func, struct __call_single_data *csd),
> + TP_PROTO(smp_call_func_t func, call_single_data_t *csd),
> TP_ARGS(func, csd)
> );
>
> DEFINE_EVENT(csd_function, csd_function_exit,
> - TP_PROTO(smp_call_func_t func, struct __call_single_data *csd),
> + TP_PROTO(smp_call_func_t func, call_single_data_t *csd),
> TP_ARGS(func, csd)
> );
>
> diff --git a/kernel/smp.c b/kernel/smp.c
> index 8455a53465af..8c714583786b 100644
> --- a/kernel/smp.c
> +++ b/kernel/smp.c
> @@ -127,7 +127,7 @@ send_call_function_ipi_mask(struct cpumask *mask)
> }
>
> static __always_inline void
> -csd_do_func(smp_call_func_t func, void *info, struct __call_single_data *csd)
> +csd_do_func(smp_call_func_t func, void *info, call_single_data_t *csd)
> {
> trace_csd_function_entry(func, csd);
> func(info);
> @@ -174,7 +174,7 @@ module_param(csd_lock_timeout, ulong, 0444);
> static atomic_t csd_bug_count = ATOMIC_INIT(0);
>
> /* Record current CSD work for current CPU, NULL to erase. */
> -static void __csd_lock_record(struct __call_single_data *csd)
> +static void __csd_lock_record(call_single_data_t *csd)
> {
> if (!csd) {
> smp_mb(); /* NULL cur_csd after unlock. */
> @@ -189,13 +189,13 @@ static void __csd_lock_record(struct __call_single_data *csd)
> /* Or before unlock, as the case may be. */
> }
>
> -static __always_inline void csd_lock_record(struct __call_single_data *csd)
> +static __always_inline void csd_lock_record(call_single_data_t *csd)
> {
> if (static_branch_unlikely(&csdlock_debug_enabled))
> __csd_lock_record(csd);
> }
>
> -static int csd_lock_wait_getcpu(struct __call_single_data *csd)
> +static int csd_lock_wait_getcpu(call_single_data_t *csd)
> {
> unsigned int csd_type;
>
> @@ -210,7 +210,7 @@ static int csd_lock_wait_getcpu(struct __call_single_data *csd)
> * the CSD_TYPE_SYNC/ASYNC types provide the destination CPU,
> * so waiting on other types gets much less information.
> */
> -static bool csd_lock_wait_toolong(struct __call_single_data *csd, u64 ts0, u64 *ts1, int *bug_id)
> +static bool csd_lock_wait_toolong(call_single_data_t *csd, u64 ts0, u64 *ts1, int *bug_id)
> {
> int cpu = -1;
> int cpux;
> @@ -276,7 +276,7 @@ static bool csd_lock_wait_toolong(struct __call_single_data *csd, u64 ts0, u64 *
> * previous function call. For multi-cpu calls its even more interesting
> * as we'll have to ensure no other cpu is observing our csd.
> */
> -static void __csd_lock_wait(struct __call_single_data *csd)
> +static void __csd_lock_wait(call_single_data_t *csd)
> {
> int bug_id = 0;
> u64 ts0, ts1;
> @@ -290,7 +290,7 @@ static void __csd_lock_wait(struct __call_single_data *csd)
> smp_acquire__after_ctrl_dep();
> }
>
> -static __always_inline void csd_lock_wait(struct __call_single_data *csd)
> +static __always_inline void csd_lock_wait(call_single_data_t *csd)
> {
> if (static_branch_unlikely(&csdlock_debug_enabled)) {
> __csd_lock_wait(csd);
> @@ -300,17 +300,17 @@ static __always_inline void csd_lock_wait(struct __call_single_data *csd)
> smp_cond_load_acquire(&csd->node.u_flags, !(VAL & CSD_FLAG_LOCK));
> }
> #else
> -static void csd_lock_record(struct __call_single_data *csd)
> +static void csd_lock_record(call_single_data_t *csd)
> {
> }
>
> -static __always_inline void csd_lock_wait(struct __call_single_data *csd)
> +static __always_inline void csd_lock_wait(call_single_data_t *csd)
> {
> smp_cond_load_acquire(&csd->node.u_flags, !(VAL & CSD_FLAG_LOCK));
> }
> #endif
>
> -static __always_inline void csd_lock(struct __call_single_data *csd)
> +static __always_inline void csd_lock(call_single_data_t *csd)
> {
> csd_lock_wait(csd);
> csd->node.u_flags |= CSD_FLAG_LOCK;
> @@ -323,7 +323,7 @@ static __always_inline void csd_lock(struct __call_single_data *csd)
> smp_wmb();
> }
>
> -static __always_inline void csd_unlock(struct __call_single_data *csd)
> +static __always_inline void csd_unlock(call_single_data_t *csd)
> {
> WARN_ON(!(csd->node.u_flags & CSD_FLAG_LOCK));
>
> @@ -376,7 +376,7 @@ void __smp_call_single_queue(int cpu, struct llist_node *node)
> * for execution on the given CPU. data must already have
> * ->func, ->info, and ->flags set.
> */
> -static int generic_exec_single(int cpu, struct __call_single_data *csd)
> +static int generic_exec_single(int cpu, call_single_data_t *csd)
> {
> if (cpu == smp_processor_id()) {
> smp_call_func_t func = csd->func;
> @@ -667,7 +667,7 @@ EXPORT_SYMBOL(smp_call_function_single);
> *
> * Return: %0 on success or negative errno value on error
> */
> -int smp_call_function_single_async(int cpu, struct __call_single_data *csd)
> +int smp_call_function_single_async(int cpu, call_single_data_t *csd)
> {
> int err = 0;
>
> diff --git a/kernel/up.c b/kernel/up.c
> index a38b8b095251..df50828cc2f0 100644
> --- a/kernel/up.c
> +++ b/kernel/up.c
> @@ -25,7 +25,7 @@ int smp_call_function_single(int cpu, void (*func) (void *info), void *info,
> }
> EXPORT_SYMBOL(smp_call_function_single);
>
> -int smp_call_function_single_async(int cpu, struct __call_single_data *csd)
> +int smp_call_function_single_async(int cpu, call_single_data_t *csd)
> {
> unsigned long flags;
>
> --
> 2.42.0
>
--
Best Regards
Guo Ren
^ permalink raw reply
* Re: [PATCH v3 1/1] tracing/kprobes: Return EADDRNOTAVAIL when func matches several symbols
From: Francis Laniel @ 2023-08-31 7:14 UTC (permalink / raw)
To: Masami Hiramatsu (Google), Steven Rostedt
Cc: linux-kernel, linux-trace-kernel
In-Reply-To: <20230829195719.0eaf9035@rorschach.local.home>
Hi.
Le mercredi 30 août 2023, 01:57:19 CEST Steven Rostedt a écrit :
> On Fri, 25 Aug 2023 22:13:21 +0900
>
> Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> > > Excellent catch! Thank you, I will apply this patch and send v4 right
> > > after. Regarding test, do you think I can add a test for the
> > > EADDRNOTAVAIL case?>
> > Hmm, in that case, you need to change something in tracefs/README so that
> > we can identify the kernel has different behavior. Or we have to change
> > this is a "Fix" for backporting.
>
> I prefer this to be a Fix and backported.
This makes sense, I will send v5 to stable mailing list too!
> Thanks,
>
> -- Steve
Best regards.
^ permalink raw reply
* linux-6.5/tools/tracing/rtla/src/utils.c:548:invalidScanfFormatWidth
From: David Binderman @ 2023-08-31 10:53 UTC (permalink / raw)
To: bristot@kernel.org, rostedt@goodmis.org, paul.walmsley@sifive.com,
palmer@dabbelt.com, aou@eecs.berkeley.edu,
linux-trace-kernel@vger.kernel.org, Linux Kernel Mailing List,
linux-riscv@lists.infradead.org
Hello there,
I just tried the static analyser cppcheck over the source code of linux-6.5
and got this:
linux-6.5/tools/tracing/rtla/src/utils.c:548:9: error: Width 1024 given in format string (no. 1) is larger than destination buffer 'mount_point[1024]', use %1023s to prevent overflowing it. [invalidScanfFormatWidth]
Source code is
while (fscanf(fp, "%*s %" STR(MAX_PATH) "s %99s %*s %*d %*d\n", mount_point, type) == 2) {
Maybe better code:
while (fscanf(fp, "%*s %" STR(MAX_PATH - 1) "s %99s %*s %*d %*d\n", mount_point, type) == 2) {
Regards
David Binderman
^ permalink raw reply
* [PATCH] tracing: zero the pipe cpumask on alloc to avoid spurious -EBUSY
From: Brian Foster @ 2023-08-31 12:55 UTC (permalink / raw)
To: linux-trace-kernel; +Cc: linux-kernel, rostedt, zhengyejian1
The pipe cpumask used to serialize opens between the main and percpu
trace pipes is not zeroed or initialized. This can result in
spurious -EBUSY returns if underlying memory is not fully zeroed.
This has been observed by immediate failure to read the main
trace_pipe file on an otherwise newly booted and idle system:
# cat /sys/kernel/debug/tracing/trace_pipe
cat: /sys/kernel/debug/tracing/trace_pipe: Device or resource busy
Zero the allocation of pipe_cpumask to avoid the problem.
Fixes: c2489bb7e6be ("tracing: Introduce pipe_cpumask to avoid race on trace_pipes")
Signed-off-by: Brian Foster <bfoster@redhat.com>
---
Hi,
I ran into this problem just recently on one of my test VMs immediately
after updating to a v6.5 base. A revert of the aforementioned commit
addressed the problem. I'm not terribly familiar with the tracing code,
but on further inspection I noticed the cpumask doesn't appear to be
initialized anywhere. I suppose this could alternatively do a
cpumask_clear() or whatever after allocation, but either way this
addresses the problem for me.
Please CC on replies as I'm not subscribed to the list. Thanks.
Brian
kernel/trace/trace.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 8e64aaad5361..2656ca3b9b39 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -9486,7 +9486,7 @@ static struct trace_array *trace_array_create(const char *name)
if (!alloc_cpumask_var(&tr->tracing_cpumask, GFP_KERNEL))
goto out_free_tr;
- if (!alloc_cpumask_var(&tr->pipe_cpumask, GFP_KERNEL))
+ if (!zalloc_cpumask_var(&tr->pipe_cpumask, GFP_KERNEL))
goto out_free_tr;
tr->trace_flags = global_trace.trace_flags & ~ZEROED_TRACE_FLAGS;
@@ -10431,7 +10431,7 @@ __init static int tracer_alloc_buffers(void)
if (trace_create_savedcmd() < 0)
goto out_free_temp_buffer;
- if (!alloc_cpumask_var(&global_trace.pipe_cpumask, GFP_KERNEL))
+ if (!zalloc_cpumask_var(&global_trace.pipe_cpumask, GFP_KERNEL))
goto out_free_savedcmd;
/* TODO: make the number of buffers hot pluggable with CPUS */
--
2.41.0
^ permalink raw reply related
* Re: [PATCH v1 1/1] bitops: Share BYTES_TO_BITS() for everyone
From: Andy Shevchenko @ 2023-08-31 13:21 UTC (permalink / raw)
To: Alexander Lobakin
Cc: Yury Norov, Uros Bizjak, Masami Hiramatsu (Google), linux-kernel,
linux-trace-kernel, Steven Rostedt
In-Reply-To: <a5574786-0a49-454d-67e3-571983e6a6e8@intel.com>
On Fri, Aug 25, 2023 at 04:49:07PM +0200, Alexander Lobakin wrote:
> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Date: Thu, 24 Aug 2023 15:37:28 +0300
>
> > It may be new callers for the same macro, share it.
> >
> > Note, it's unknown why it's represented in the current form instead of
> > simple multiplication and commit 1ff511e35ed8 ("tracing/kprobes: Add
> > bitfield type") doesn't explain that neither. Let leave it as is and
> > we may improve it in the future.
>
> Maybe symmetrical change in tools/ like I did[0] an aeon ago?
Hmm... Why can't you simply upstream your version? It seems better than mine.
> [0]
> https://github.com/alobakin/linux/commit/fd308001fe6d38837fe820427209a6a99e4850a8
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH] tracing: Fix race issue between cpu buffer write and swap
From: Zheng Yejian @ 2023-08-31 13:27 UTC (permalink / raw)
To: rostedt, mhiramat
Cc: linux-kernel, linux-trace-kernel, xukuohai, zhengyejian1
Warning happened in rb_end_commit() at code:
if (RB_WARN_ON(cpu_buffer, !local_read(&cpu_buffer->committing)))
WARNING: CPU: 0 PID: 139 at kernel/trace/ring_buffer.c:3142
rb_commit+0x402/0x4a0
Call Trace:
ring_buffer_unlock_commit+0x42/0x250
trace_buffer_unlock_commit_regs+0x3b/0x250
trace_event_buffer_commit+0xe5/0x440
trace_event_buffer_reserve+0x11c/0x150
trace_event_raw_event_sched_switch+0x23c/0x2c0
__traceiter_sched_switch+0x59/0x80
__schedule+0x72b/0x1580
schedule+0x92/0x120
worker_thread+0xa0/0x6f0
It is because the race between writing event into cpu buffer and swapping
cpu buffer through file per_cpu/cpu0/snapshot:
Write on CPU 0 Swap buffer by per_cpu/cpu0/snapshot on CPU 1
-------- --------
tracing_snapshot_write()
[...]
ring_buffer_lock_reserve()
cpu_buffer = buffer->buffers[cpu]; // 1. Suppose find 'cpu_buffer_a';
[...]
rb_reserve_next_event()
[...]
ring_buffer_swap_cpu()
if (local_read(&cpu_buffer_a->committing))
goto out_dec;
if (local_read(&cpu_buffer_b->committing))
goto out_dec;
buffer_a->buffers[cpu] = cpu_buffer_b;
buffer_b->buffers[cpu] = cpu_buffer_a;
// 2. cpu_buffer has swapped here.
rb_start_commit(cpu_buffer);
if (unlikely(READ_ONCE(cpu_buffer->buffer)
!= buffer)) { // 3. This check passed due to 'cpu_buffer->buffer'
[...] // has not changed here.
return NULL;
}
cpu_buffer_b->buffer = buffer_a;
cpu_buffer_a->buffer = buffer_b;
[...]
// 4. Reserve event from 'cpu_buffer_a'.
ring_buffer_unlock_commit()
[...]
cpu_buffer = buffer->buffers[cpu]; // 5. Now find 'cpu_buffer_b' !!!
rb_commit(cpu_buffer)
rb_end_commit() // 6. WARN for the wrong 'committing' state !!!
Based on above analysis, we can easily reproduce by following testcase:
``` bash
#!/bin/bash
dmesg -n 7
sysctl -w kernel.panic_on_warn=1
TR=/sys/kernel/tracing
echo 7 > ${TR}/buffer_size_kb
echo "sched:sched_switch" > ${TR}/set_event
while [ true ]; do
echo 1 > ${TR}/per_cpu/cpu0/snapshot
done &
while [ true ]; do
echo 1 > ${TR}/per_cpu/cpu0/snapshot
done &
while [ true ]; do
echo 1 > ${TR}/per_cpu/cpu0/snapshot
done &
```
To fix it, IIUC, we can use smp_call_function_single() to do the swap on
the target cpu where the buffer is located, so that above race would be
avoided.
Fixes: f1affcaaa861 ("tracing: Add snapshot in the per_cpu trace directories")
Signed-off-by: Zheng Yejian <zhengyejian1@huawei.com>
---
kernel/trace/trace.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 8e64aaad5361..55bb08aaf392 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -7618,6 +7618,11 @@ static int tracing_snapshot_open(struct inode *inode, struct file *file)
return ret;
}
+static void tracing_swap_cpu_buffer(void *tr)
+{
+ update_max_tr_single((struct trace_array *)tr, current, smp_processor_id());
+}
+
static ssize_t
tracing_snapshot_write(struct file *filp, const char __user *ubuf, size_t cnt,
loff_t *ppos)
@@ -7676,13 +7681,15 @@ tracing_snapshot_write(struct file *filp, const char __user *ubuf, size_t cnt,
ret = tracing_alloc_snapshot_instance(tr);
if (ret < 0)
break;
- local_irq_disable();
/* Now, we're going to swap */
- if (iter->cpu_file == RING_BUFFER_ALL_CPUS)
+ if (iter->cpu_file == RING_BUFFER_ALL_CPUS) {
+ local_irq_disable();
update_max_tr(tr, current, smp_processor_id(), NULL);
- else
- update_max_tr_single(tr, current, iter->cpu_file);
- local_irq_enable();
+ local_irq_enable();
+ } else {
+ smp_call_function_single(iter->cpu_file, tracing_swap_cpu_buffer,
+ (void *)tr, 1);
+ }
break;
default:
if (tr->allocated_snapshot) {
--
2.25.1
^ permalink raw reply related
* Re: [PATCH] visl: use canonical ftrace path
From: Steven Rostedt @ 2023-08-31 13:39 UTC (permalink / raw)
To: Ross Zwisler
Cc: linux-kernel, Ross Zwisler, Mark Rutland, Masami Hiramatsu,
Mauro Carvalho Chehab, linux-media, linux-trace-kernel,
Daniel Almeida, Hans Verkuil
In-Reply-To: <20230829204600.3210276-2-zwisler@kernel.org>
On Tue, 29 Aug 2023 14:46:01 -0600
Ross Zwisler <zwisler@kernel.org> wrote:
> From: Ross Zwisler <zwisler@google.com>
>
> The canonical location for the tracefs filesystem is at /sys/kernel/tracing.
>
> But, from Documentation/trace/ftrace.rst:
>
> Before 4.1, all ftrace tracing control files were within the debugfs
> file system, which is typically located at /sys/kernel/debug/tracing.
> For backward compatibility, when mounting the debugfs file system,
> the tracefs file system will be automatically mounted at:
>
> /sys/kernel/debug/tracing
>
> Update the visl decoder driver documentation to use this tracefs path.
>
> Signed-off-by: Ross Zwisler <zwisler@google.com>
Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Thanks Ross!
-- Steve
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox