> + }
> +
> seq_printf(m, " %s %-44s %15lu\n", tu->filename,
> - trace_probe_name(&tu->tp), tu->nhit);
> +trace_probe_name(&tu->tp), nhits);
> return 0;
> }
>
> @@ -1512,7 +1529,8 @@ static int uprobe_dispatcher(struct uprobe_consumer
> *con, struct pt_regs *regs)
> int ret = 0;
>
> tu = container_of(con, struct trace_uprobe, consumer);
> - tu->nhit++;
> +
> + this_cpu_inc(*tu->nhits);
>
> udd.tu = tu;
> udd.bp_addr = instruction_pointer(regs);
> --
> 2.43.5
>
--
Masami Hiramatsu (Google)
, " %s %-44s %15lu\n", tu->filename,
> - trace_probe_name(&tu->tp), tu->nhit);
> +trace_probe_name(&tu->tp), nhits);
> return 0;
> }
>
> @@ -1512,7 +1529,8 @@ static int uprobe_dispatcher(struct uprobe_consumer
> *con, struct pt_regs *regs)
> int ret = 0;
>
> tu = container_of(con, struct trace_uprobe, consumer);
> - tu->nhit++;
> +
> + this_cpu_inc(*tu->nhits);
>
> udd.tu = tu;
> udd.bp_addr = instruction_pointer(regs);
> --
> 2.43.5
>
--
Masami Hiramatsu (Google)
here? Another tracing_iter_reset() in
s_start() does not cause the soft lockups in the same situation?
Thank you,
> }
> } else {
> cpu = iter->cpu_file;
> --
> 2.25.1
>
>
--
Masami Hiramatsu (Google)
K, let me consider to rebase on tip/perf/core.
Thank you,
>
> Oleg.
>
--
Masami Hiramatsu (Google)
x it?
Interesting, this fixes the issue!
And it is great to find how to avoid the LTO removes function like this.
Tested-by: Masami Hiramatsu (Google)
Thank you,
>
> -- Steve
>
> diff --git a/kernel/trace/trace_selftest_dynamic.c
> b/kernel/trace/trace_selftest_dynamic.c
&g
On Wed, 21 Aug 2024 08:43:51 +0900
Masami Hiramatsu (Google) wrote:
> On Tue, 20 Aug 2024 18:11:09 -0400
> Steven Rostedt wrote:
>
> > On Wed, 21 Aug 2024 07:05:39 +0900
> > Masami Hiramatsu (Google) wrote:
> >
> >
> > > Does the noinline attrib
On Tue, 20 Aug 2024 19:49:14 -0400
Steven Rostedt wrote:
> On Wed, 21 Aug 2024 08:43:51 +0900
> Masami Hiramatsu (Google) wrote:
>
> > > Can you add the __used and see if it fixes it?
> >
> > Adding __used to DYN_FTRACE_TEST_NAME() and DYN_FTRACE_TEST_NAME2()
On Tue, 20 Aug 2024 18:11:09 -0400
Steven Rostedt wrote:
> On Wed, 21 Aug 2024 07:05:39 +0900
> Masami Hiramatsu (Google) wrote:
>
>
> > Does the noinline attribute prevent embedding callsite too? I mean
> >
> > extern callee()
> >
> > noinline ca
gt; >
> > > > On Tue, 20 Aug 2024 00:56:49 +0900
> > > > Masami Hiramatsu (Google) wrote:
> > > > >
> > > > > >
> > > > > > We may need to add "noinline" or something to make sure those
> > &
On Mon, 19 Aug 2024 12:02:44 -0400
Steven Rostedt wrote:
> On Tue, 20 Aug 2024 00:56:49 +0900
> Masami Hiramatsu (Google) wrote:
> >
> > >
> > > We may need to add "noinline" or something to make sure those functions
> > > don't get in
From: Masami Hiramatsu (Google)
With ftrace boot-time selftest, kmemleak reported some memory leaks in
the new test case for function graph storage for multiple tracers.
unreferenced object 0x888005060080 (size 32):
comm "swapper/0", pid 1, jiffies 4294676440
hex dump (firs
On Mon, 19 Aug 2024 12:02:44 -0400
Steven Rostedt wrote:
> On Tue, 20 Aug 2024 00:56:49 +0900
> Masami Hiramatsu (Google) wrote:
> >
> > >
> > > We may need to add "noinline" or something to make sure those functions
> > > don't get in
On Mon, 19 Aug 2024 11:29:02 -0400
Steven Rostedt wrote:
> On Mon, 19 Aug 2024 17:11:52 +0900
> Masami Hiramatsu (Google) wrote:
>
> > CONFIG_LTO=y
> > CONFIG_LTO_CLANG=y
>
> Hi Masami,
>
> Does it still fail if you disable the above?
No, I found that cau
From: Masami Hiramatsu (Google)
Skip recording calltime and rettime if the fgraph_ops does not need it.
This is a kind of performance optimization for fprobe. Since the fprobe
user does not use these entries, recording timestamp in fgraph is just
a overhead (e.g. eBPF, ftrace). So introduce the
From: Masami Hiramatsu (Google)
Update fprobe documentation for the new fprobe on function-graph
tracer. This includes some bahvior changes and pt_regs to
ftrace_regs interface change.
Signed-off-by: Masami Hiramatsu (Google)
---
Changes in v2:
- Update @fregs parameter explanation
From: Masami Hiramatsu (Google)
This test case repeats define and undefine the fprobe dynamic event to
ensure that the fprobe does not cause any issue with such operations.
Signed-off-by: Masami Hiramatsu (Google)
---
.../test.d/dynevent/add_remove_fprobe_repeat.tc| 19
From: Masami Hiramatsu (Google)
Since the fprobe event does not support maxactive anymore, stop
testing the maxactive syntax error checking.
Signed-off-by: Masami Hiramatsu (Google)
---
.../ftrace/test.d/dynevent/fprobe_syntax_errors.tc |4 +---
1 file changed, 1 insertion(+), 3 deletions
From: Masami Hiramatsu (Google)
Remove depercated fprobe::nr_maxactive. This involves fprobe events to
rejects the maxactive number.
Signed-off-by: Masami Hiramatsu (Google)
---
Changes in v2:
- Newly added.
---
include/linux/fprobe.h |2 --
kernel/trace/trace_fprobe.c | 44
From: Masami Hiramatsu (Google)
Since the new fgraph requires to initialize fgraph_ops.ops.func_hash before
calling register_ftrace_graph(), initialize it with default (tracing all
functions) parameter.
Signed-off-by: Masami Hiramatsu (Google)
---
kernel/trace/ftrace.c |4
1 file
From: Masami Hiramatsu (Google)
Rewrite fprobe implementation on function-graph tracer.
Major API changes are:
- 'nr_maxactive' field is deprecated.
- This depends on CONFIG_DYNAMIC_FTRACE_WITH_ARGS or
!CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS, and
CONFIG_HAVE_FUNCTION_GRAPH
From: Masami Hiramatsu (Google)
Add CONFIG_HAVE_FTRACE_GRAPH_FUNC kconfig in addition to ftrace_graph_func
macro check. This is for the other feature (e.g. FPROBE) which requires to
access ftrace_regs from fgraph_ops::entryfunc() can avoid compiling if
the fgraph can not pass the valid
From: Masami Hiramatsu (Google)
Enable kprobe_multi feature if CONFIG_FPROBE is enabled. The pt_regs is
converted from ftrace_regs by ftrace_partial_regs(), thus some registers
may always returns 0. But it should be enough for function entry (access
arguments) and exit (access return value
From: Masami Hiramatsu (Google)
Allow fprobe events to be enabled with CONFIG_DYNAMIC_FTRACE_WITH_ARGS.
With this change, fprobe events mostly use ftrace_regs instead of pt_regs.
Note that if the arch doesn't enable HAVE_PT_REGS_COMPAT_FTRACE_REGS,
fprobe events will not be able to be used
From: Masami Hiramatsu (Google)
Add ftrace_fill_perf_regs() which should be compatible with the
perf_fetch_caller_regs(). In other words, the pt_regs returned from the
ftrace_fill_perf_regs() must satisfy 'user_mode(regs) == false' and can be
used for stack tracing.
Signed-off-
From: Masami Hiramatsu (Google)
Add ftrace_partial_regs() which converts the ftrace_regs to pt_regs.
This is for the eBPF which needs this to keep the same pt_regs interface
to access registers.
Thus when replacing the pt_regs with ftrace_regs in fprobes (which is
used by kprobe_multi eBPF event
From: Masami Hiramatsu (Google)
Change the fprobe exit handler to use ftrace_regs structure instead of
pt_regs. This also introduce HAVE_PT_REGS_TO_FTRACE_REGS_CAST which means
the ftrace_regs's memory layout is equal to the pt_regs so that those are
able to cast. Fprobe introduces
From: Masami Hiramatsu (Google)
This allows fprobes to be available with CONFIG_DYNAMIC_FTRACE_WITH_ARGS
instead of CONFIG_DYNAMIC_FTRACE_WITH_REGS, then we can enable fprobe
on arm64.
Signed-off-by: Masami Hiramatsu (Google)
Acked-by: Florent Revest
---
Changes in v6:
- Keep using
From: Masami Hiramatsu (Google)
Pass ftrace_regs to the fgraph_ops::retfunc(). If ftrace_regs is not
available, it passes a NULL instead. User callback function can access
some registers (including return address) via this ftrace_regs.
Signed-off-by: Masami Hiramatsu (Google)
---
Changes in
From: Masami Hiramatsu (Google)
Use ftrace_regs instead of fgraph_ret_regs for tracing return value
on function_graph tracer because of simplifying the callback interface.
The CONFIG_HAVE_FUNCTION_GRAPH_RETVAL is also replaced by
CONFIG_HAVE_FUNCTION_GRAPH_FREGS.
Signed-off-by: Masami
From: Masami Hiramatsu (Google)
Pass ftrace_regs to the fgraph_ops::entryfunc(). If ftrace_regs is not
available, it passes a NULL instead. User callback function can access
some registers (including return address) via this ftrace_regs.
Signed-off-by: Masami Hiramatsu (Google)
---
Changes in
From: Masami Hiramatsu (Google)
Rename ftrace_regs_return_value to ftrace_regs_get_return_value as same as
other ftrace_regs_get/set_* APIs.
Signed-off-by: Masami Hiramatsu (Google)
Acked-by: Mark Rutland
---
Changes in v6:
- Moved to top of the series.
Changes in v3:
- Newly added
From: Masami Hiramatsu (Google)
To clarify what will be expected on ftrace_regs, add a comment to the
architecture independent definition of the ftrace_regs.
Signed-off-by: Masami Hiramatsu (Google)
Acked-by: Mark Rutland
---
Changes in v8:
- Update that the saved registers depends on the
From: Masami Hiramatsu (Google)
Since the register_ftrace_graph() assigns a new fgraph_ops to
fgraph_array before registring it by ftrace_startup_subops(), the new
fgraph_ops can be used in function_graph_enter().
In most cases, it is still OK because those fgraph_ops's hashtable is
al
e given
'fprobe' data structure pointer is still valid. Note that it is
possible to unregister fprobe before the return callback runs. Thus
the address validation must be done before using it in the return
callback.
Download
This series can be applied against the ftrace/for-next bra
From: Masami Hiramatsu (Google)
Add a test case for tracepoint events on modules. This checks if it can add
and remove the events correctly.
Signed-off-by: Masami Hiramatsu (Google)
---
Changes in v3:
- Add not-loaded module test.
---
tools/testing/selftests/ftrace/config
From: Masami Hiramatsu (Google)
Support raw tracepoint events on future loaded (unloaded) modules.
This allows user to create raw tracepoint events which can be used from
module's __init functions.
Note: since the kernel does not have any information about the tracepoints
in the unl
From: Masami Hiramatsu (Google)
Support raw tracepoint event on module by fprobe events.
Since it only uses for_each_kernel_tracepoint() to find a tracepoint,
the tracepoints on modules are not handled. Thus if user specified a
tracepoint on a module, it shows an error.
This adds new
From: Masami Hiramatsu (Google)
Add for_each_tracepoint_in_module() function to iterate tracepoints in
a module. This API is needed for handling tracepoints in a loading
module from tracepoint_module_notifier callback function.
This also update for_each_module_tracepoint() to pass the module to
From: Masami Hiramatsu (Google)
Add for_each_module_tracepoint() for iterating over tracepoints
on modules. This is similar to the for_each_kernel_tracepoint()
but only for the tracepoints on modules (not including kernel
built-in tracepoints).
Signed-off-by: Masami Hiramatsu (Google
r can specify any tracepoint name for tracepoint events. It will be
just ignored.
You can download this series from;
https://git.kernel.org/pub/scm/linux/kernel/git/mhiramat/linux.git
topic/tprobe-on-module
Thank you,
---
Masami Hiramatsu (Google) (5):
tracepoint: Support iterating
is considered a
> second-class citizen within the kernel, I would argue that tracing
> is a third-class citizen and should not needlessly modify the
> behavior of classes above it.
>
> Thanks,
>
> Mathieu
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> https://www.efficios.com
>
--
Masami Hiramatsu (Google)
trace_probe_name(&tu->tp), tu->nhit);
> +trace_probe_name(&tu->tp), nhits);
> return 0;
> }
>
> @@ -1507,7 +1524,8 @@ static int uprobe_dispatcher(struct uprobe_consumer
> *con, struct pt_regs *regs)
> int ret = 0;
>
> tu = container_of(con, struct trace_uprobe, consumer);
> - tu->nhit++;
> +
> + this_cpu_inc(*tu->nhits);
>
> udd.tu = tu;
> udd.bp_addr = instruction_pointer(regs);
> --
> 2.43.5
>
--
Masami Hiramatsu (Google)
probe_events
/sys/kernel/tracing # cat kprobe_events
p:kprobes/p_c_start_llvm_8011538628216713357_0 c_start.llvm.8011538628216713357
p:kprobes/p_c_start_0 c_start
And ftrace too.
/sys/kernel/tracing # grep ^c_start available_filter_functions
c_start.llvm.8011538628216713357
c_start
c_start.llvm.17132674095431275852
Tes
h.
With compiler suffixes, the source line aliases should remove those
suffixes and add new suffix like below.
1234 t name_show.llvm.12345678
1234 t name_show@kernel_irq_irqdesc_c_264
> I would allow to find the symbols with and without the suffix using
> a single API.
I wonder why we need it? for ftrace filter?
The same symbol name does not mean the same function prototype.
For the kprobes (and fprobes, ftraces too) user must specify which
actual symbol is what you want to probe.
Of course if user can say "hey kprobe, probe name_show", but that is
unclear (not unique symbol) so kprobe will reject it. If there is
.llvm suffix, user can specify one of them. But it is still unclear
to user where in the source code the symbol is actually defined.
So eventually, we need the debuginfo or this @suffix.
Thank you,
>
> Best Regards,
> Petr
--
Masami Hiramatsu (Google)
profile file is useful because it shows "missed"
counter. This tells user whether your trace data drops some events or not.
But if uprobes profile only shows the number of hit, we can use the
histogram trigger if needed.
Thank you,
>
> >
> > jirka
> >
> > > return 0;
> > > }
> > >
> > > @@ -1507,7 +1506,6 @@ static int uprobe_dispatcher(struct uprobe_consumer
> > > *con, struct pt_regs *regs)
> > > int ret = 0;
> > >
> > > tu = container_of(con, struct trace_uprobe, consumer);
> > > - tu->nhit++;
> > >
> > > udd.tu = tu;
> > > udd.bp_addr = instruction_pointer(regs);
> > > --
> > > 2.43.5
> > >
--
Masami Hiramatsu (Google)
bviously run into the same issue with
> just LTO.
Yeah, without 1/3 of this series, user can not specify llvm suffixed
symbols on kprobe events. However, as perf-probe does, users can use
the relative offset from a unique symbol too. (kprobe does not care
the function boundary.)
Thank you,
>
> Sami
--
Masami Hiramatsu (Google)
onfused because they can not find
> > which symbol is kprobed.)
> >
> > Sorry about the conclusion (so I NAK this), but this is a good discussion.
>
> Do you mean we do not want patch 3/3, but would like to keep 1/3 and part
> of 2/3 (remove the _without_suffix APIs)? If this is the case, we are
> undoing the change by Sami in [1], and thus may break some tracing tools.
BTW, I confirmed that the PATCH 1/3 and 2/3 fixes kprobes to probe on suffixed
symbols correctly. (because 1/3 allows to search suffixed symbols)
/sys/kernel/tracing # cat dynamic_events
p:kprobes/p_c_stop_llvm_17132674095431275852_0 c_stop.llvm.17132674095431275852
p:kprobes/p_c_stop_llvm_8011538628216713357_0 c_stop.llvm.8011538628216713357
p:kprobes/p_c_stop_0 c_stop
Thank you,
>
> Sami, could you please share your thoughts on this?
>
> If this works, I will send next version with 1/3 and part of 2/3.
>
> Thanks,
> Song
>
> [1]
> https://lore.kernel.org/all/20210408182843.1754385-8-samitolva...@google.com/
>
--
Masami Hiramatsu (Google)
get confused because they can not find
> > which symbol is kprobed.)
> >
> > Sorry about the conclusion (so I NAK this), but this is a good discussion.
>
> Do you mean we do not want patch 3/3, but would like to keep 1/3 and part
> of 2/3 (remove the _without_suffix APIs)? If this is the case, we are
> undoing the change by Sami in [1], and thus may break some tracing tools.
What tracing tools may be broke and why?
For this suffix problem, I would like to add another patch to allow probing on
suffixed symbols. (It seems suffixed symbols are not available at this point)
The problem is that the suffixed symbols maybe a "part" of the original
function,
thus user has to carefully use it.
>
> Sami, could you please share your thoughts on this?
Sami, I would like to know what problem you have on kprobes.
Thank you,
>
> If this works, I will send next version with 1/3 and part of 2/3.
>
> Thanks,
> Song
>
> [1]
> https://lore.kernel.org/all/20210408182843.1754385-8-samitolva...@google.com/
>
--
Masami Hiramatsu (Google)
t;, ".isra.0",
> and ".isra.0.cold". We didn't do anything about these before this set. So I
> think we are OK not handling them now. We sure can enable it for GCC built
> kernel in the future.
Hmm, I think it should be handled as it is. This means it should do as
livepatch does. Since I expected user will check kallsyms if gets error,
we should keep this as it is. (if a symbol has suffix, it should accept
symbol with suffix, or user will get confused because they can not find
which symbol is kprobed.)
Sorry about the conclusion (so I NAK this), but this is a good discussion.
Thanks,
>
> Thanks,
> Song
>
>
>
--
Masami Hiramatsu (Google)
M LTO observed:
> - * - foo.llvm.[0-9a-f]+
> + * character in an identifier in C, so we can just remove the
> + * suffix.
>*/
> - res = strstr(s, ".llvm.");
> + res = strstr(s, ".");
nit: "strchr(s, '.')" ?
Anyway,
Reviewed-by: Masami Hiramatsu (Google)
Thank you,
--
Masami Hiramatsu (Google)
really true.
>
> At the moment, I have no objections to keep the _without_suffix
> APIs. But for long term, I still think we need to set clear
> expectations for the users: tracing symbols from sources is not
> reliable.
OK, I understand this part. I agree the problem. Even if the symbol
is unique on kallsyms (without suffix), it may have a suffix and is
not correct function entry.
I think to solve this issue, we need a better DWARF, or add a symbol
suffix like;
https://lkml.org/lkml/2023/12/4/1535
Thank you,
>
> Thanks,
> Song
>
--
Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google)
Since str_has_prefix() takes the prefix as the 2nd argument and the string
as the first, is_cfi_preamble_symbol() always fails to check the prefix.
Fix the function parameter order so that it correctly check the prefix.
Fixes: de02f2ac5d8c ("kprobes: Pro
clearer.
> > kallsyms_lookup_static_name()
> > kallsyms_on_each_match_static_symbol()
But this is not correctly check the symbol is static or not. If you
check the symbol is really static, it is OK. (return NULL if the symbol
is global.)
Thank you,
>
> While these names are shorter, I think they are more confusing. Not all
> folks know that only static functions can have suffixes.
>
> Maybe we should not hide the "try match full name first first" in the
> API, and let the users handle it. Then, we can safely call the new APIs
> *_without_suffix(), as Masami suggested.
>
> If there is no objections, I will send v2 based on this direction.
>
> Thanks,
> Song
>
--
Masami Hiramatsu (Google)
-by: Masami Hiramatsu (Google)
Thanks,
> Oleg.
> ---
>
> kernel/events/uprobes.c | 47 ---
> 1 file changed, 24 insertions(+), 23 deletions(-)
>
--
Masami Hiramatsu (Google)
if (!tu->inode)
> + if (!tu->uprobe)
> continue;
>
> - uprobe_unregister(tu->inode, tu->offset, &tu->consumer);
> - tu->inode = NULL;
> + uprobe_unregister(tu->uprobe, &tu->consumer);
> + tu->uprobe = NULL;
> }
> }
>
> @@ -1305,7 +1305,7 @@ static int uprobe_perf_close(struct trace_event_call
> *call,
> return 0;
>
> list_for_each_entry(tu, trace_probe_probe_list(tp), tp.list) {
> - ret = uprobe_apply(tu->inode, tu->offset, &tu->consumer, false);
> + ret = uprobe_apply(tu->uprobe, &tu->consumer, false);
> if (ret)
> break;
> }
> @@ -1329,7 +1329,7 @@ static int uprobe_perf_open(struct trace_event_call
> *call,
> return 0;
>
> list_for_each_entry(tu, trace_probe_probe_list(tp), tp.list) {
> - err = uprobe_apply(tu->inode, tu->offset, &tu->consumer, true);
> + err = uprobe_apply(tu->uprobe, &tu->consumer, true);
> if (err) {
> uprobe_perf_close(call, event);
> break;
> --
> 2.25.1.362.g51ebf55
>
>
--
Masami Hiramatsu (Google)
> *tu, filter_func_t filter)
> tu->consumer.filter = filter;
> tu->inode = d_real_inode(tu->path.dentry);
>
> - if (tu->ref_ctr_offset)
> - ret = uprobe_register_refctr(tu->inode, tu->offset,
> - tu->ref_ctr_offset, &tu->consumer);
> - else
> - ret = uprobe_register(tu->inode, tu->offset, &tu->consumer);
> -
> + ret = uprobe_register(tu->inode, tu->offset, tu->ref_ctr_offset,
> &tu->consumer);
> if (ret)
> tu->inode = NULL;
>
> --
> 2.25.1.362.g51ebf55
>
--
Masami Hiramatsu (Google)
bol(count_symbols, func_name,
> &ctx.count);
> + if (IS_ENABLED(CONFIG_LTO_CLANG) && !ctx.count) {
> + kallsyms_on_each_match_symbol_or_prefix(
> + count_symbols, func_name, &ctx.count);
> + }
> + }
>
> module_kallsyms_on_each_symbol(mod, count_mod_symbols, &ctx);
>
> --
> 2.43.0
>
--
Masami Hiramatsu (Google)
n a text buffer. */
> diff --git a/kernel/kallsyms_selftest.c b/kernel/kallsyms_selftest.c
> index 2f84896a7bcb..873f7c445488 100644
> --- a/kernel/kallsyms_selftest.c
> +++ b/kernel/kallsyms_selftest.c
> @@ -187,31 +187,11 @@ static void test_perf_kallsyms_lookup_name(void)
> stat.min, stat.max, div_u64(stat.sum, stat.real_cnt));
> }
>
> -static bool match_cleanup_name(const char *s, const char *name)
> -{
> - char *p;
> - int len;
> -
> - if (!IS_ENABLED(CONFIG_LTO_CLANG))
> - return false;
> -
> - p = strstr(s, ".llvm.");
> - if (!p)
> - return false;
> -
> - len = strlen(name);
> - if (p - s != len)
> - return false;
> -
> - return !strncmp(s, name, len);
> -}
> -
> static int find_symbol(void *data, const char *name, unsigned long addr)
> {
> struct test_stat *stat = (struct test_stat *)data;
>
> - if (strcmp(name, stat->name) == 0 ||
> - (!stat->perf && match_cleanup_name(name, stat->name))) {
> + if (!strcmp(name, stat->name)) {
> stat->real_cnt++;
> stat->addr = addr;
>
> --
> 2.43.0
>
--
Masami Hiramatsu (Google)
On Mon, 29 Jul 2024 19:02:41 -0400
Steven Rostedt wrote:
> On Mon, 29 Jul 2024 23:49:24 +0900
> Masami Hiramatsu (Google) wrote:
>
> > On Fri, 26 Jul 2024 14:42:08 -0400
> > Steven Rostedt wrote:
> >
> > > From: Steven Rostedt
> > >
>
n if it detects an error
> (like refcount_inc() on '0').
>
> Signed-off-by: Steven Rostedt (Google)
Looks good to me.
Masami Hiramatsu (Google)
Thanks,
> ---
> include/linux/trace_events.h | 2 +-
> kernel/trace/trace_events.c | 8
> 2 files changed, 5
freed) or a crash as it could have
> incorrectly freed the event itself.
>
> Link:
> https://lore.kernel.org/all/20240719204701.1605950-1-mini...@grsecurity.net/
>
> Cc: sta...@vger.kernel.org
> Reported-by: Mathias Krause
> Fixes: b63db58e2fa5d ("eventfs/tracing: Add
that this code was reworked in v6.7 by commit 4bbd93455659
> ("kprobes: kretprobe scalability improvement") and the new objpool
> implementation doesn't have this problem.
>
Looks good to me.
Acked-by: Masami Hiramatsu (Google)
This should go directly into stable because there
x 9df3e2973626..9435185c10ef 100644
> > > > --- a/include/linux/trace_events.h
> > > > +++ b/include/linux/trace_events.h
> > > > @@ -880,7 +880,6 @@ do {
> > > > \
> > > > struct perf_event;
> > > >
> > > > DECLARE_PER_CPU(struct pt_regs, perf_trace_regs);
> > > > -DECLARE_PER_CPU(int, bpf_kprobe_override);
> > > >
> > > > extern int perf_trace_init(struct perf_event *event);
> > > > extern void perf_trace_destroy(struct perf_event *event);
> > > > --
> > > > 2.39.2
> > > >
> > > >
> >
> >
> > --
> > Masami Hiramatsu (Google)
--
Masami Hiramatsu (Google)
On Mon, 15 Jul 2024 19:21:52 +0200
Oleg Nesterov wrote:
> On 07/15, Masami Hiramatsu wrote:
> >
> > Hi Peter, Oleg,
> >
> > If this is OK for you, please give your Ack.
>
> Acked-by: Oleg Nesterov
>
Thanks for the Ack!
--
Masami Hiramatsu (Google)
On Mon, 15 Jul 2024 14:47:45 -0400
Steven Rostedt wrote:
> From: "Steven Rostedt (Google)"
>
> Gone but never forgotten.
>
> [ Also moved Daniel's name to be consistent with the alphabetical order ]
>
Reviewed-by: Masami Hiramatsu (Google)
Thank you,
Hi Peter, Oleg,
If this is OK for you, please give your Ack.
Thank you,
On Fri, 12 Jul 2024 09:26:17 +0900
"Masami Hiramatsu (Google)" wrote:
> From: Masami Hiramatsu (Google)
>
> Add uprobes entry to MAINTAINERS to clarify the maintainers.
>
> Suggested-by: Peter
On Thu, 11 Jul 2024 11:14:20 -0700
Jeff Johnson wrote:
> On 6/2/24 23:45, Masami Hiramatsu (Google) wrote:
> > On Mon, 3 Jun 2024 11:25:59 +0800
> > "wuqiang.matt" wrote:
> >
> >> On 2024/6/1 08:31, Jeff Johnson wrote:
> >>> make allmodco
On Thu, 11 Jul 2024 11:14:20 -0700
Jeff Johnson wrote:
> On 6/2/24 23:45, Masami Hiramatsu (Google) wrote:
> > On Mon, 3 Jun 2024 11:25:59 +0800
> > "wuqiang.matt" wrote:
> >
> >> On 2024/6/1 08:31, Jeff Johnson wrote:
> >>> make allmodco
> }
> >
> > #ifndef __NR_uretprobe
> > -#define __NR_uretprobe 463
> > +#define __NR_uretprobe 467
> > #endif
> >
> > __naked unsigned long uretprobe_syscall_call_1(void)
> > --
> > 2.45.2
> >
--
Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google)
Add uprobes entry to MAINTAINERS to clarify the maintainers.
Suggested-by: Peter Zijlstra
Signed-off-by: Masami Hiramatsu (Google)
---
MAINTAINERS | 13 +
1 file changed, 13 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index da5352dbd4f3
ools/bootconfig, because tools/bootconfig will
> - * run the parser sanity test.
> - * This does NOT mean lib/bootconfig.c is available in the user space.
> - * However, if you change this file, please make sure the tools/bootconfig
> - * has no issue on building and running.
> - */
> -#include
> #endif
>
> /*
> --
> 2.39.2
>
--
Masami Hiramatsu (Google)
iewed-by: Masami Hiramatsu (Google)
> ---
> kernel/events/uprobes.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> index 4950decebe5c..e5b7c6239970 100644
> --- a/kernel/events/uprobes.c
>
On Thu, 11 Jul 2024 13:02:39 +0200
Peter Zijlstra wrote:
> With handle_swbp() triggering concurrently on (all) CPUs, tree_lock
> becomes a bottleneck. Avoid treelock by doing RCU lookups of the
> uprobe.
>
Looks good to me.
Acked-by: Masami Hiramatsu (Google)
Thanks,
> Signe
he tools/bootconfig
> * has no issue on building and running.
> */
> -#include
> #endif
>
> /*
> --
> 2.45.2
>
--
Masami Hiramatsu (Google)
;
> >
> > extern int perf_trace_init(struct perf_event *event);
> > extern void perf_trace_destroy(struct perf_event *event);
> > --
> > 2.39.2
> >
> >
--
Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google)
The kernel test robot reported that the find_module() is not available
if CONFIG_MODULES=n.
Fix this error by hiding find_modules() in #ifdef CONFIG_MODULES with
related rcu locks as try_module_get_by_name().
Reported-by: kernel test robot
Closes:
https
From: Masami Hiramatsu (Google)
The kernel test robot reported that the find_module() is not available
if CONFIG_MODULES=n.
Fix this error by hiding find_modules() in #ifdef CONFIG_MODULES with
related rcu locks as try_module_get_by_name().
Reported-by: kernel test robot
Closes:
https
On Tue, 9 Jul 2024 09:04:36 +0900
"Masami Hiramatsu (Google)" wrote:
> From: Masami Hiramatsu (Google)
>
> Currently, kprobe event checks whether the target symbol name is unique
> or not, so that it does not put a probe on an unexpected place. But this
> skips the ch
;ll never get to
> the #BP.
Hmm, kprobes checks the instruction and reject if it is ENDBR.
Shouldn't uprobe also skip the ENDBR too?
Thank you,
>
> Also, we tried very hard to not have a literal encode ENDBR (I really
> should teach objtool about this one :/). If it somehow makes sense to
> keep this clause, please use: gen_endbr()
--
Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google)
Currently, kprobe event checks whether the target symbol name is unique
or not, so that it does not put a probe on an unexpected place. But this
skips the check if the target is on a module because the module may not
be loaded.
To fix this issue, this patch
GFP_KERNEL to GFP_NOWAIT to prevent sleep in
> irq_work.
>
> This change wouldn't impact functionality in practice because the worst-size
> is 2K.
>
Looks good to me.
Acked-by: Masami Hiramatsu (Google)
and
Fixes: 8d6e90983ade ("tracing: Create a sparse bitma
> > we could delay the allocation of 'return_instance' until the first
> > consumer returns 0, so there's no perf regression
> >
> > that way we could treat all consumers the same and we wouldn't need
> > the session flag..
> >
> > ok looks like good idea ;-) will try that
>
> Just please double check that we don't pass through 1 or 2 as a return
> result for BPF uprobes/multi-uprobes, so that we don't have any
> accidental changes of behavior.
Agreed. BTW, even if the uprobe is removed, the ret_handler should be called?
I think both 1 and 2 case, we should skip ret_handler.
> > > > >
> > > > > #ifdef CONFIG_UPROBES
> > > > > @@ -80,6 +83,12 @@ struct uprobe_task {
> > > > > unsigned intdepth;
> > > > > };
> > > > >
> > > > > +struct session_consumer {
> > > > > + __u64 cookie;
> > > >
> > > > And this cookie looks not scalable. If we can pass a data to handler, I
> > > > would like to
> > > > reuse it to pass the target function parameters to ret_handler as
> > > > kretprobe/fprobe does.
> > > >
> > > > int (*handler)(struct uprobe_consumer *self, struct pt_regs
> > > > *regs, void *data);
> > > >
> > > > uprobes can collect its uc's required sizes and allocate the memory
> > > > (shadow stack frame)
> > > > at handler_chain().
> > >
> > > The goal here is to keep this simple and fast. I'd prefer to keep it
> > > small and fixed size, if possible. I'm thinking about caching and
> > > reusing return_instance as one of the future optimizations, so if we
> > > can keep this more or less fixed (assuming there is typically not more
> > > than 1 or 2 consumers per uprobe, which seems realistic), this will
> > > provide a way to avoid excessive memory allocations.
Hmm, so you mean user will allocate another "data map" and use cookie as
a key to access the data? That is possible but sounds a bit redundant.
If such "data map" allocation is also provided, it is more useful.
Thank you,
--
Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google)
Currently, kprobe event checks whether the target symbol name is unique
or not, so that it does not put a probe on an unexpected place. But this
skips the check if the target is on a module because the module may not
be loaded.
To fix this issue, this patch
,7 @@ create_local_trace_kprobe(char *func, void *addr,
> unsigned long offs,
> int ret;
> char *event;
>
> - if (func) {
> + if (func && !strchr(func, ':')) {
> unsigned int count;
>
> count = number_of_same_symbols(func);
> --
> 2.34.1
>
--
Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google)
Skip push operation only when there is no fgraph_ops which sets retfunc.
This is for optimizing performance of fprobe on fgraph. Since the major
use case of fprobe is putting a probe on function entry and another
probe on exit. Since these probes are independent
From: Masami Hiramatsu (Google)
Skip recording calltime and rettime if the fgraph_ops does not need it.
This is a kind of performance optimization for fprobe. Since the fprobe
user does not use these entries, recording timestamp in fgraph is just
a overhead (e.g. eBPF, ftrace). So introduce the
From: Masami Hiramatsu (Google)
Update fprobe documentation for the new fprobe on function-graph
tracer. This includes some bahvior changes and pt_regs to
ftrace_regs interface change.
Signed-off-by: Masami Hiramatsu (Google)
---
Changes in v2:
- Update @fregs parameter explanation
From: Masami Hiramatsu (Google)
This test case repeats define and undefine the fprobe dynamic event to
ensure that the fprobe does not cause any issue with such operations.
Signed-off-by: Masami Hiramatsu (Google)
---
.../test.d/dynevent/add_remove_fprobe_repeat.tc| 19
From: Masami Hiramatsu (Google)
Since the fprobe event does not support maxactive anymore, stop
testing the maxactive syntax error checking.
Signed-off-by: Masami Hiramatsu (Google)
---
.../ftrace/test.d/dynevent/fprobe_syntax_errors.tc |4 +---
1 file changed, 1 insertion(+), 3 deletions
From: Masami Hiramatsu (Google)
Remove depercated fprobe::nr_maxactive. This involves fprobe events to
rejects the maxactive number.
Signed-off-by: Masami Hiramatsu (Google)
---
Changes in v2:
- Newly added.
---
include/linux/fprobe.h |2 --
kernel/trace/trace_fprobe.c | 44
From: Masami Hiramatsu (Google)
Rewrite fprobe implementation on function-graph tracer.
Major API changes are:
- 'nr_maxactive' field is deprecated.
- This depends on CONFIG_DYNAMIC_FTRACE_WITH_ARGS or
!CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS, and
CONFIG_HAVE_FUNCTION_GRAPH
From: Masami Hiramatsu (Google)
Add CONFIG_HAVE_FTRACE_GRAPH_FUNC kconfig in addition to ftrace_graph_func
macro check. This is for the other feature (e.g. FPROBE) which requires to
access ftrace_regs from fgraph_ops::entryfunc() can avoid compiling if
the fgraph can not pass the valid
From: Masami Hiramatsu (Google)
Enable kprobe_multi feature if CONFIG_FPROBE is enabled. The pt_regs is
converted from ftrace_regs by ftrace_partial_regs(), thus some registers
may always returns 0. But it should be enough for function entry (access
arguments) and exit (access return value
From: Masami Hiramatsu (Google)
Allow fprobe events to be enabled with CONFIG_DYNAMIC_FTRACE_WITH_ARGS.
With this change, fprobe events mostly use ftrace_regs instead of pt_regs.
Note that if the arch doesn't enable HAVE_PT_REGS_COMPAT_FTRACE_REGS,
fprobe events will not be able to be used
From: Masami Hiramatsu (Google)
Add ftrace_fill_perf_regs() which should be compatible with the
perf_fetch_caller_regs(). In other words, the pt_regs returned from the
ftrace_fill_perf_regs() must satisfy 'user_mode(regs) == false' and can be
used for stack tracing.
Signed-off-
From: Masami Hiramatsu (Google)
Add ftrace_partial_regs() which converts the ftrace_regs to pt_regs.
This is for the eBPF which needs this to keep the same pt_regs interface
to access registers.
Thus when replacing the pt_regs with ftrace_regs in fprobes (which is
used by kprobe_multi eBPF event
From: Masami Hiramatsu (Google)
Change the fprobe exit handler to use ftrace_regs structure instead of
pt_regs. This also introduce HAVE_PT_REGS_TO_FTRACE_REGS_CAST which means
the ftrace_regs's memory layout is equal to the pt_regs so that those are
able to cast. Fprobe introduces
From: Masami Hiramatsu (Google)
This allows fprobes to be available with CONFIG_DYNAMIC_FTRACE_WITH_ARGS
instead of CONFIG_DYNAMIC_FTRACE_WITH_REGS, then we can enable fprobe
on arm64.
Signed-off-by: Masami Hiramatsu (Google)
Acked-by: Florent Revest
---
Changes in v6:
- Keep using
From: Masami Hiramatsu (Google)
Pass ftrace_regs to the fgraph_ops::retfunc(). If ftrace_regs is not
available, it passes a NULL instead. User callback function can access
some registers (including return address) via this ftrace_regs.
Signed-off-by: Masami Hiramatsu (Google)
---
Changes in
From: Masami Hiramatsu (Google)
Use ftrace_regs instead of fgraph_ret_regs for tracing return value
on function_graph tracer because of simplifying the callback interface.
The CONFIG_HAVE_FUNCTION_GRAPH_RETVAL is also replaced by
CONFIG_HAVE_FUNCTION_GRAPH_FREGS.
Signed-off-by: Masami
1 - 100 of 1001 matches
Mail list logo
