Re: [PATCH] kernel: trace: preemptirq_delay_test: add MODULE_DESCRIPTION()

2024-05-18 Thread Google
On Sat, 18 May 2024 15:54:49 -0700 Jeff Johnson wrote: > Fix the 'make W=1' warning: > > WARNING: modpost: missing MODULE_DESCRIPTION() in > kernel/trace/preemptirq_delay_test.o > Looks good to me. Acked-by: Masami Hiramatsu (Google) Fixes: f96e8577da10 ("lib: Ad

Re: [PATCHv5 bpf-next 6/8] x86/shstk: Add return uprobe support

2024-05-13 Thread Google
r debugging. But I think that needs another series of patches. We also need to discuss when it should be prohibited and how (e.g. audit interface? SELinux?). But I think this series is just optimizing currently available uprobes with a new syscall. I don't think it changes such security concerning. Thank you, > > jirka -- Masami Hiramatsu (Google)

Re: [PATCHv5 bpf-next 7/8] selftests/x86: Add return uprobe shadow stack test

2024-05-13 Thread Google
> + uretprobe_trigger(); > + > + printf("[OK]\tUretprobe test\n"); > + err = 0; > + > +out: > + ARCH_PRCTL(ARCH_SHSTK_DISABLE, ARCH_SHSTK_SHSTK); > + signal(SIGSEGV, SIG_DFL); > + if (fd) > + close(fd); > + return err; > +} > + > void segv_handler_ptrace(int signum, siginfo_t *si, void *uc) > { > /* The SSP adjustment caused a segfault. */ > @@ -867,6 +1003,12 @@ int main(int argc, char *argv[]) > goto out; > } > > + if (test_uretprobe()) { > + ret = 1; > + printf("[FAIL]\turetprobe test\n"); > + goto out; > + } > + > return ret; > > out: > -- > 2.44.0 > -- Masami Hiramatsu (Google)

Re: [PATCH RESEND v8 07/16] mm/execmem, arch: convert simple overrides of module_alloc to execmem

2024-05-07 Thread Google
e does > not implement execmem_arch_setup(), execmem_alloc() will fall back to > module_alloc(). > Looks good to me. Reviewed-by: Masami Hiramatsu (Google) Thanks, > Signed-off-by: Mike Rapoport (IBM) > Acked-by: Song Liu > --- > arch/loongarch/kernel/module.c | 19 ++

Re: [PATCH RESEND v8 05/16] module: make module_memory_{alloc,free} more self-contained

2024-05-07 Thread Google
On Sun, 5 May 2024 19:06:17 +0300 Mike Rapoport wrote: > From: "Mike Rapoport (IBM)" > > Move the logic related to the memory allocation and freeing into > module_memory_alloc() and module_memory_free(). > Looks good to me. Reviewed-by: Masami Hiramatsu (Google

[PATCH v10 36/36] fgraph: Skip recording calltime/rettime if it is not nneeded

2024-05-07 Thread Masami Hiramatsu (Google)
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

[PATCH v10 35/36] Documentation: probes: Update fprobe on function-graph tracer

2024-05-07 Thread Masami Hiramatsu (Google)
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

[PATCH v10 17/36] function_graph: Move graph notrace bit to shadow stack global var

2024-05-07 Thread Masami Hiramatsu (Google)
are) Signed-off-by: Masami Hiramatsu (Google) --- Changes in v2: - Make description lines shorter than 76 chars. --- include/linux/trace_recursion.h |7 --- kernel/trace/trace.h |9 + kernel/trace/trace_functions_graph.c | 10 ++ 3 files chan

[PATCH v10 34/36] selftests/ftrace: Add a test case for repeating register/unregister fprobe

2024-05-07 Thread Masami Hiramatsu (Google)
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

[PATCH v10 33/36] selftests: ftrace: Remove obsolate maxactive syntax check

2024-05-07 Thread Masami Hiramatsu (Google)
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

[PATCH v10 32/36] tracing/fprobe: Remove nr_maxactive from fprobe

2024-05-07 Thread Masami Hiramatsu (Google)
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

[PATCH v10 31/36] fprobe: Rewrite fprobe on function-graph tracer

2024-05-07 Thread Masami Hiramatsu (Google)
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_FREGS. So

[PATCH v10 30/36] ftrace: Add CONFIG_HAVE_FTRACE_GRAPH_FUNC

2024-05-07 Thread Masami Hiramatsu (Google)
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

[PATCH v10 29/36] bpf: Enable kprobe_multi feature if CONFIG_FPROBE is enabled

2024-05-07 Thread Masami Hiramatsu (Google)
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

[PATCH v10 28/36] tracing/fprobe: Enable fprobe events with CONFIG_DYNAMIC_FTRACE_WITH_ARGS

2024-05-07 Thread Masami Hiramatsu (Google)
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

[PATCH v10 27/36] tracing: Add ftrace_fill_perf_regs() for perf event

2024-05-07 Thread Masami Hiramatsu (Google)
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-by: Masami

[PATCH v10 26/36] tracing: Add ftrace_partial_regs() for converting ftrace_regs to pt_regs

2024-05-07 Thread Masami Hiramatsu (Google)
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

[PATCH v10 25/36] fprobe: Use ftrace_regs in fprobe exit handler

2024-05-07 Thread Masami Hiramatsu (Google)
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 a new

[PATCH v10 24/36] fprobe: Use ftrace_regs in fprobe entry handler

2024-05-07 Thread Masami Hiramatsu (Google)
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

[PATCH v10 23/36] function_graph: Pass ftrace_regs to retfunc

2024-05-07 Thread Masami Hiramatsu (Google)
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

[PATCH v10 22/36] function_graph: Replace fgraph_ret_regs with ftrace_regs

2024-05-07 Thread Masami Hiramatsu (Google)
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

[PATCH v10 21/36] function_graph: Pass ftrace_regs to entryfunc

2024-05-07 Thread Masami Hiramatsu (Google)
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

[PATCH v10 20/36] ftrace: Add multiple fgraph storage selftest

2024-05-07 Thread Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google) Add a selftest for multiple function graph tracer with storage on a same function. In this case, the shadow stack entry will be shared among those fgraph with different data storage. So this will ensure the fgraph will not mixed those storage data. Signed-off

[PATCH v10 19/36] function_graph: Add selftest for passing local variables

2024-05-07 Thread Masami Hiramatsu (Google)
From: Steven Rostedt (VMware) Add boot up selftest that passes variables from a function entry to a function exit, and make sure that they do get passed around. Signed-off-by: Steven Rostedt (VMware) Signed-off-by: Masami Hiramatsu (Google) --- Changes in v2: - Add reserved size test

[PATCH v10 04/36] function_graph: Convert ret_stack to a series of longs

2024-05-07 Thread Masami Hiramatsu (Google)
for the return side of the functions. Signed-off-by: Steven Rostedt (VMware) Signed-off-by: Masami Hiramatsu (Google) --- include/linux/sched.h |2 - kernel/trace/fgraph.c | 124 - 2 files changed, 71 insertions(+), 55 deletions(-) diff --git

[PATCH v10 18/36] function_graph: Implement fgraph_reserve_data() and fgraph_retrieve_data()

2024-05-07 Thread Masami Hiramatsu (Google)
shadow ret_stack and this then can be retrieved by fgraph_retrieve_data() called by the corresponding retfunc(). Signed-off-by: Steven Rostedt (VMware) Signed-off-by: Masami Hiramatsu (Google) --- Changes in v10: - Fix to support data size up to 32 words (previously it only support up

[PATCH v10 16/36] function_graph: Move graph depth stored data to shadow stack global var

2024-05-07 Thread Masami Hiramatsu (Google)
are) Signed-off-by: Masami Hiramatsu (Google) --- include/linux/trace_recursion.h | 29 - kernel/trace/trace.h| 34 -- 2 files changed, 32 insertions(+), 31 deletions(-) diff --git a/include/linux/trace_recursion.h b/incl

[PATCH v10 15/36] function_graph: Move set_graph_function tests to shadow stack global var

2024-05-07 Thread Masami Hiramatsu (Google)
are) Signed-off-by: Masami Hiramatsu (Google) --- include/linux/trace_recursion.h |5 + kernel/trace/trace.h | 32 +--- kernel/trace/trace_functions_graph.c |6 +++--- kernel/trace/trace_irqsoff.c |4 ++-- kernel/tr

[PATCH v10 07/36] function_graph: Allow multiple users to attach to function graph

2024-05-07 Thread Masami Hiramatsu (Google)
are called is not completely handled yet, but that shouldn't be too hard to manage. Signed-off-by: Steven Rostedt (VMware) Signed-off-by: Masami Hiramatsu (Google) --- Changes in v10: - Rephrase all "index" on shadow stack to "offset" and some "ret_stack" to &

[PATCH v10 14/36] function_graph: Add "task variables" per task for fgraph_ops

2024-05-07 Thread Masami Hiramatsu (Google)
s on a per task basis having a way to maintain state for each task. Signed-off-by: Steven Rostedt (VMware) Signed-off-by: Masami Hiramatsu (Google) --- Changes in v10: - Explain where the task vars is placed in shadow stack. Changes in v3: - Move fgraph_ops::idx to previous patch in

[PATCH v10 13/36] function_graph: Use a simple LRU for fgraph_array index number

2024-05-07 Thread Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google) Since the fgraph_array index is used for the bitmap on the shadow stack, it may leave some entries after a function_graph instance is removed. Thus if another instance reuses the fgraph_array index soon after releasing it, the fgraph may confuse to call the newer

[PATCH v10 12/36] function_graph: Have the instances use their own ftrace_ops for filtering

2024-05-07 Thread Masami Hiramatsu (Google)
function_graph_enter_ops() instead of function_graph_enter() so that it avoid pushing on shadow stack multiple times on the same function. Signed-off-by: Steven Rostedt (VMware) Signed-off-by: Masami Hiramatsu (Google) --- Changes in v10: - Use "offset" for shadow stack instead of "i

[PATCH v10 11/36] ftrace: Allow ftrace startup flags exist without dynamic ftrace

2024-05-07 Thread Masami Hiramatsu (Google)
to compile because dynamic ftrace is disabled. This change is needed to move some of the logic of what is passed to ftrace_startup() out of the parameters of ftrace_startup(). Signed-off-by: Steven Rostedt (VMware) Signed-off-by: Masami Hiramatsu (Google) --- include/linux/ftrace.h | 18

[PATCH v10 10/36] ftrace: Allow function_graph tracer to be enabled in instances

2024-05-07 Thread Masami Hiramatsu (Google)
-by: Steven Rostedt (VMware) Signed-off-by: Masami Hiramatsu (Google) --- Changes in v2: - Fix to remove set_graph_array() completely. --- include/linux/ftrace.h |1 + kernel/trace/ftrace.c|1 + kernel/trace/trace.h | 13 ++- kernel

[PATCH v10 09/36] ftrace/function_graph: Pass fgraph_ops to function graph callbacks

2024-05-07 Thread Masami Hiramatsu (Google)
to the function graph tracer. Signed-off-by: Steven Rostedt (VMware) Signed-off-by: Masami Hiramatsu (Google) --- Changes in v2: - cleanup to set argument name on function prototype. --- include/linux/ftrace.h | 10 +++--- kernel/trace/fgraph.c| 16

[PATCH v10 08/36] function_graph: Remove logic around ftrace_graph_entry and return

2024-05-07 Thread Masami Hiramatsu (Google)
into the function_graph tracer. Signed-off-by: Steven Rostedt (VMware) Signed-off-by: Masami Hiramatsu (Google) --- Changes in v2: - Fix typo and make lines shorter than 76 chars in the description. - Remove unneeded return from return_run() function. --- kernel/trace/fgraph.c | 67

[PATCH v10 06/36] function_graph: Add an array structure that will allow multiple callbacks

2024-05-07 Thread Masami Hiramatsu (Google)
on the shadow stack. We need to only save the index, because this will allow the fgraph_ops to be freed before the function returns (which may happen if the function call schedule for a long time). Signed-off-by: Steven Rostedt (VMware) Signed-off-by: Masami Hiramatsu (Google) --- Changes in v2: - Remove

[PATCH v10 05/36] fgraph: Use BUILD_BUG_ON() to make sure we have structures divisible by long

2024-05-07 Thread Masami Hiramatsu (Google)
dt (VMware) Signed-off-by: Masami Hiramatsu (Google) --- Changes in v7: - Use DIV_ROUND_UP() to calculate FGRAPH_RET_INDEX --- kernel/trace/fgraph.c |9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c index 30

[PATCH v10 02/36] tracing: Rename ftrace_regs_return_value to ftrace_regs_get_return_value

2024-05-07 Thread Masami Hiramatsu (Google)
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

[PATCH v10 03/36] x86: tracing: Add ftrace_regs definition in the header

2024-05-07 Thread Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google) Add ftrace_regs definition for x86_64 in the ftrace header to clarify what register will be accessible from ftrace_regs. Signed-off-by: Masami Hiramatsu (Google) --- Changes in v3: - Add rip to be saved. Changes in v2: - Newly added. --- arch/x86/include

[PATCH v10 01/36] tracing: Add a comment about ftrace_regs definition

2024-05-07 Thread Masami Hiramatsu (Google)
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

[PATCH v10 00/36] tracing: fprobe: function_graph: Multi-function graph and fprobe on fgraph

2024-05-07 Thread Masami Hiramatsu (Google)
ck. This series can be applied against the probes/for-next branch, which is based on v6.9-rc6. This series can also be found below branch. https://git.kernel.org/pub/scm/linux/kernel/git/mhiramat/linux.git/log/?h=topic/fprobe-on-fgraph Thank you, --- Masami Hiramatsu (Google) (21): tracing:

Re: [PATCH resend ftrace] Asynchronous grace period for register_ftrace_direct()

2024-05-01 Thread Google
hen adding a new direct trampoline. This looks good to me. Reviewed-by: Masami Hiramatsu (Google) Thank you, > [1] https://lore.kernel.org/all/cover.1710877680.git@cloudflare.com/ > > Reported-by: Jakub Kicinski > Reported-by: Alexei Starovoitov > Reported-by: Chris Mason > S

Re: [v3] tracing/probes: Fix memory leak in traceprobe_parse_probe_arg_body()

2024-05-01 Thread Google
ke to read any improved (patch) version descriptions (or > changelogs)? Thanks, but those are nitpicks and I don't mind it. Thank you, > > Regards, > Markus -- Masami Hiramatsu (Google)

Re: [PATCH] eventfs/tracing: Add callback for release of an eventfs_inode

2024-05-01 Thread Google
On Tue, 30 Apr 2024 14:23:27 -0400 Steven Rostedt wrote: > From: "Steven Rostedt (Google)" > > Synthetic events create and destroy tracefs files when they are created > and removed. The tracing subsystem has its own file descriptor > representing the state of the event

Re: [PATCH v9 00/36] tracing: fprobe: function_graph: Multi-function graph and fprobe on fgraph

2024-04-30 Thread Google
changes) > > > > > > > > On the other hand, multi-kretprobes got significantly faster (+24%!). > > > Again, I don't know if it is expected or not, but it's a nice > > > improvement. > > > > Thanks! > > > > > > > > If you have

Re: [PATCH v9 29/36] bpf: Enable kprobe_multi feature if CONFIG_FPROBE is enabled

2024-04-29 Thread Google
On Thu, 25 Apr 2024 13:09:32 -0700 Andrii Nakryiko wrote: > On Mon, Apr 15, 2024 at 6:22 AM Masami Hiramatsu (Google) > wrote: > > > > From: Masami Hiramatsu (Google) > > > > Enable kprobe_multi feature if CONFIG_FPROBE is enabled. The pt_regs is

Re: [PATCH v9 36/36] fgraph: Skip recording calltime/rettime if it is not nneeded

2024-04-29 Thread Google
On Thu, 25 Apr 2024 13:15:08 -0700 Andrii Nakryiko wrote: > On Mon, Apr 15, 2024 at 6:25 AM Masami Hiramatsu (Google) > wrote: > > > > From: Masami Hiramatsu (Google) > > > > Skip recording calltime and rettime if the fgraph_ops does not need it. > > This i

Re: [PATCH v9 00/36] tracing: fprobe: function_graph: Multi-function graph and fprobe on fgraph

2024-04-29 Thread Google
iles changed, 2325 insertions(+), 882 deletions(-) > > create mode 100644 > > tools/testing/selftests/ftrace/test.d/dynevent/add_remove_fprobe_repeat.tc > > > > -- > > Masami Hiramatsu (Google) > > -- Masami Hiramatsu (Google)

Re: [PATCH v3] tracing/probes: Fix memory leak in traceprobe_parse_probe_arg_body()

2024-04-29 Thread Google
freed in this case and leaks its memory. > > Thus jump to the label 'fail' in that error case. > Looks good to me. Acked-by: Masami Hiramatsu (Google) Thank you! > Fixes: 032330abd08b ("tracing/probes: Cleanup probe argument parser") > Signed-off-by: LuMingYin > ---

Re: [PATCH] kernel/trace/trace_probe:Fixed memory leak issues in trace_probe.c.

2024-04-26 Thread Google
goto fail; > } > snprintf(parg->fmt, len, "%s[%d]", parg->type->fmttype, >parg->count); > -- > 2.25.1 > -- Masami Hiramatsu (Google)

Re: [PATCH 0/2] Objpool performance improvements

2024-04-26 Thread Google
> > include/linux/objpool.h | 105 +++-- > lib/objpool.c | 112 +++- > 2 files changed, 107 insertions(+), 110 deletions(-) > > -- > 2.43.0 > -- Masami Hiramatsu (Google)

Re: [PATCH v9 00/36] tracing: fprobe: function_graph: Multi-function graph and fprobe on fgraph

2024-04-25 Thread Google
is quite a refactoring :) > > On Mon, Apr 15, 2024 at 2:49 PM Masami Hiramatsu (Google) > wrote: > > > > Hi, > > > > Here is the 9th version of the series to re-implement the fprobe on > > function-graph tracer. The previous version is; > > > >

Re: [PATCH v9 01/36] tracing: Add a comment about ftrace_regs definition

2024-04-24 Thread Google
On Wed, 24 Apr 2024 15:19:24 +0200 Florent Revest wrote: > On Wed, Apr 24, 2024 at 2:23 PM Florent Revest wrote: > > > > On Mon, Apr 15, 2024 at 2:49 PM Masami Hiramatsu (Google) > > wrote: > > > > > > From: Masami Hiramatsu (Google) > > > >

Re: [PATCH 1/2] tracing/user_events: Fix non-spaced field matching

2024-04-22 Thread Google
r is copied from user > to kernel, so it cannot change (and no other threads access it). I also > checked trace_parse_run_command() which is the same. So at least in this > context the non-atomic part is OK. Oh, sorry if I scared you. I've seen bugs get introduced into loops like this many times (while updating the code), so I try to keep it simple. I'm sure that your code has no bugs. Thank you, -- Masami Hiramatsu (Google)

Re: [PATCH v2] uprobes: reduce contention on uprobes_tree access

2024-04-22 Thread Google
_list(struct inode *inode, > get_uprobe(u); > } > } > - spin_unlock(_treelock); > + read_unlock(_treelock); > } > > /* @vma contains reference counter, not the probed instruction. */ > @@ -1407,9 +1407,9 @@ vma_has_uprobes(struct vm_area_struct *vma, unsigned > long start, unsigned long e > min = vaddr_to_offset(vma, start); > max = min + (end - start) - 1; > > - spin_lock(_treelock); > + read_lock(_treelock); > n = find_node_in_range(inode, min, max); > - spin_unlock(_treelock); > + read_unlock(_treelock); > > return !!n; > } > -- > 2.43.0 > -- Masami Hiramatsu (Google)

Re: [PATCH v2] uprobes: reduce contention on uprobes_tree access

2024-04-22 Thread Google
d - start) - 1; > > > > - spin_lock(_treelock); > > + read_lock(_treelock); > > n = find_node_in_range(inode, min, max); > > if (n) { > > for (t = n; t; t = rb_prev(t)) { > > @@ -1316,7 +1316,7 @@ static void build_probe_list(struct inode *inode, > > get_uprobe(u); > > } > > } > > - spin_unlock(_treelock); > > + read_unlock(_treelock); > > } > > > > /* @vma contains reference counter, not the probed instruction. */ > > @@ -1407,9 +1407,9 @@ vma_has_uprobes(struct vm_area_struct *vma, unsigned > > long start, unsigned long e > > min = vaddr_to_offset(vma, start); > > max = min + (end - start) - 1; > > > > - spin_lock(_treelock); > > + read_lock(_treelock); > > n = find_node_in_range(inode, min, max); > > - spin_unlock(_treelock); > > + read_unlock(_treelock); > > > > return !!n; > > } > > -- > > 2.43.0 > > > -- Masami Hiramatsu (Google)

Re: [PATCHv3 bpf-next 0/7] uprobe: uretprobe speed up

2024-04-22 Thread Google
d! I reviewed the series and just except for the manpage, it looks good to me. Reviewed-by: Masami Hiramatsu (Google) for the series. If Linux API maintainers are OK, I can pick this in probes/for-next. (BTW, who will pick the manpage patch?) Thank you, > > v3 changes: > - adde

Re: [PATCH 7/7] man2: Add uretprobe syscall page

2024-04-22 Thread Google
NOTES > +.BR uretprobe() > +exists only to allow the invocation of return uprobe consumers. > +It should > +.B never > +be called directly. > +Details of the arguments (if any) passed to > +.BR uretprobe () > +and the return value are specific for given architecture. > -- > 2.44.0 > -- Masami Hiramatsu (Google)

Re: [PATCH v5 14/15] kprobes: remove dependency on CONFIG_MODULES

2024-04-22 Thread Google
n non-modular kernels. > > Add #ifdef CONFIG_MODULE guards for the code dealing with kprobes inside > modules, make CONFIG_KPROBES select CONFIG_EXECMEM and drop the > dependency of CONFIG_KPROBES on CONFIG_MODULES. Looks good to me. Acked-by: Masami Hiramatsu (Google) Thank y

Re: [PATCH v4 2/2] rethook: honor CONFIG_FTRACE_VALIDATE_RCU_IS_WATCHING in rethook_try_get()

2024-04-20 Thread Google
Thanks for update! This looks good to me. > > Thanks, Masami! Will you take it through your tree, or you'd like to > route it through bpf-next? OK, let me take it through linux-trace tree. Thank you! > > > > > Acked-by: Masami Hiramatsu (Google) > > &

Re: [PATCH 1/2] tracing/user_events: Fix non-spaced field matching

2024-04-20 Thread Google
Ah, nevermind. Synthetic event parses the field by strsep(';') first and argv_split(). So it does not have this issue. Thank you, > > Thanks, > -Beau > > > > + /* We must fixup 'field;field' to 'field; field' */ > > > + char *fixed = fix_semis_no_space(args, count); > > > + char **split; > > > + > > > + if (!fixed) > > > + return NULL; > > > + > > > + /* We do a normal split afterwards */ > > > + split = argv_split(GFP_KERNEL, fixed, argc); > > > + > > > + /* We can free since argv_split makes a copy */ > > > + kfree(fixed); > > > + > > > + return split; > > > + } > > > + > > > + /* No fixup is required */ > > > + return argv_split(GFP_KERNEL, args, argc); > > > +} > > > + > > > /* > > > * Parses the event name, arguments and flags then registers if > > > successful. > > > * The name buffer lifetime is owned by this method for success cases > > > only. > > > @@ -2012,7 +2098,7 @@ static int user_event_parse(struct user_event_group > > > *group, char *name, > > > return -EPERM; > > > > > > if (args) { > > > - argv = argv_split(GFP_KERNEL, args, ); > > > + argv = user_event_argv_split(args, ); > > > > > > if (!argv) > > > return -ENOMEM; > > > -- > > > 2.34.1 > > > > > > > > > -- > > Masami Hiramatsu (Google) -- Masami Hiramatsu (Google)

Re: [PATCH v4 14/15] kprobes: remove dependency on CONFIG_MODULES

2024-04-20 Thread Google
NG) { > ... > } > > so moving out 'enum module_state' won't be enough. Hmm, this part should be inline functions like; #ifdef CONFIG_MODULES static inline bool module_is_coming(struct module *mod) { return mod->state == MODULE_STATE_COMING; } #else #define module_is_coming(mod) (false) #endif Then we don't need the enum. Thank you, > > > > > > >> -- > > >> Masami Hiramatsu > > > > > -- > Sincerely yours, > Mike. > -- Masami Hiramatsu (Google)

Re: [PATCH v4 05/15] mm: introduce execmem_alloc() and execmem_free()

2024-04-20 Thread Google
module_alloc(). However, if an architecture only supports breakpoint/trap based kprobe, it does not need to consider whether the execmem is allocated. > > We can always share large ROX pages as long as they are within the correct > address space. The permissions for them are ROX and the alignment > differences are due to KASAN and this is handled during allocation of the > large page to refill the cache. __execmem_cache_alloc() only needs to limit > the search for the address space of the range. So I don't think EXECMEM_KPROBE always same as EXECMEM_MODULE_TEXT, it should be configured for each arch. Especially, if it is only used for searching parameter, it looks OK to me. Thank you, > > And regardless, they way we deal with sharing of the cache can be sorted > out later. > > > Thanks, > > Song > > -- > Sincerely yours, > Mike. > -- Masami Hiramatsu (Google)

Re: [PATCH v9 07/36] function_graph: Allow multiple users to attach to function graph

2024-04-20 Thread Google
On Fri, 19 Apr 2024 23:52:58 -0400 Steven Rostedt wrote: > On Mon, 15 Apr 2024 21:50:20 +0900 > "Masami Hiramatsu (Google)" wrote: > > > @@ -27,23 +28,157 @@ > > > > #define FGRAPH_RET_SIZE sizeof(struct ftrace_ret_stack) > > #define FGRAPH_RET

Re: [PATCH v9 00/36] tracing: fprobe: function_graph: Multi-function graph and fprobe on fgraph

2024-04-18 Thread Google
Hi Steve, Can you review this series? Especially, [07/36] and [12/36] has been changed a lot from your original patch. Thank you, On Mon, 15 Apr 2024 21:48:59 +0900 "Masami Hiramatsu (Google)" wrote: > Hi, > > Here is the 9th version of the series to re-implement the f

Re: [PATCH 1/2] tracing/user_events: Fix non-spaced field matching

2024-04-18 Thread Google
v_split(GFP_KERNEL, args, argc); > +} > + > /* > * Parses the event name, arguments and flags then registers if successful. > * The name buffer lifetime is owned by this method for success cases only. > @@ -2012,7 +2098,7 @@ static int user_event_parse(struct user_event_group > *group, char *name, > return -EPERM; > > if (args) { > - argv = argv_split(GFP_KERNEL, args, ); > + argv = user_event_argv_split(args, ); > > if (!argv) > return -ENOMEM; > -- > 2.34.1 > -- Masami Hiramatsu (Google)

Re: [PATCH v4 2/2] rethook: honor CONFIG_FTRACE_VALIDATE_RCU_IS_WATCHING in rethook_try_get()

2024-04-18 Thread Google
which rely on rethook) runtime throughput > by 2.3%, according to BPF benchmarks ([0]). > > [0] > https://lore.kernel.org/bpf/caef4bzauq2wkmjzdc9s0rbwa01bybgwhn6andxqshyia47p...@mail.gmail.com/ > > Signed-off-by: Andrii Nakryiko Thanks for update! This looks good to m

Re: [PATCH] uprobes: reduce contention on uprobes_tree access

2024-04-18 Thread Google
> > > > > > > - spin_lock(_treelock); > > > > > > > > > > > > + write_lock(_treelock); > > > > > > > > > > > > u = __insert_uprobe(uprobe); > > > > > > > > > > > > - spin_unlock(_tr

Re: [PATCH v4 05/15] mm: introduce execmem_alloc() and execmem_free()

2024-04-17 Thread Google
l be used to identify the calling subsystem and to > allow architectures define parameters for ranges suitable for that > subsystem. > This looks good to me for the kprobe part. Acked-by: Masami Hiramatsu (Google) Thank you, > Signed-off-by: Mike Rapoport (IBM) > --- > arc

Re: [PATCH for-next v2] tracing/kprobes: Add symbol counting check when module loads

2024-04-17 Thread Google
Sorry, this is actually v3. (miss-configured...) Thanks, On Thu, 18 Apr 2024 05:46:34 +0900 "Masami Hiramatsu (Google)" wrote: > From: Masami Hiramatsu (Google) > > Currently, kprobe event checks whether the target symbol name is unique > or not, so that i

[PATCH for-next v2] tracing/kprobes: Add symbol counting check when module loads

2024-04-17 Thread 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

Re: [PATCH for-next v2] tracing/kprobes: Add symbol counting check when module loads

2024-04-17 Thread Google
On Tue, 16 Apr 2024 00:47:26 -0400 Steven Rostedt wrote: > On Mon, 15 Apr 2024 18:40:23 +0900 > "Masami Hiramatsu (Google)" wrote: > > > Check the number of probe target symbols in the target module when > > the module is loaded. If the probe is not on the unique

[PATCH] bootconfig: Fix the kerneldoc of _xbc_exit()

2024-04-15 Thread Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google) Fix the kerneldoc of _xbc_exit() which is updated to have an @early argument and the function name is changed. Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-kbuild-all/202404150036.kpj3hefa-...@intel.com/ Signed-off-by: Masami Hiramatsu

[PATCH v9 36/36] fgraph: Skip recording calltime/rettime if it is not nneeded

2024-04-15 Thread Masami Hiramatsu (Google)
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

[PATCH v9 35/36] Documentation: probes: Update fprobe on function-graph tracer

2024-04-15 Thread Masami Hiramatsu (Google)
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

[PATCH v9 34/36] selftests/ftrace: Add a test case for repeating register/unregister fprobe

2024-04-15 Thread Masami Hiramatsu (Google)
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

[PATCH v9 33/36] selftests: ftrace: Remove obsolate maxactive syntax check

2024-04-15 Thread Masami Hiramatsu (Google)
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

[PATCH v9 32/36] tracing/fprobe: Remove nr_maxactive from fprobe

2024-04-15 Thread Masami Hiramatsu (Google)
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

[PATCH v9 31/36] fprobe: Rewrite fprobe on function-graph tracer

2024-04-15 Thread Masami Hiramatsu (Google)
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_FREGS. So

[PATCH v9 30/36] ftrace: Add CONFIG_HAVE_FTRACE_GRAPH_FUNC

2024-04-15 Thread Masami Hiramatsu (Google)
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

[PATCH v9 29/36] bpf: Enable kprobe_multi feature if CONFIG_FPROBE is enabled

2024-04-15 Thread Masami Hiramatsu (Google)
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

[PATCH v9 28/36] tracing/fprobe: Enable fprobe events with CONFIG_DYNAMIC_FTRACE_WITH_ARGS

2024-04-15 Thread Masami Hiramatsu (Google)
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

[PATCH v9 27/36] tracing: Add ftrace_fill_perf_regs() for perf event

2024-04-15 Thread Masami Hiramatsu (Google)
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-by: Masami

[PATCH v9 26/36] tracing: Add ftrace_partial_regs() for converting ftrace_regs to pt_regs

2024-04-15 Thread Masami Hiramatsu (Google)
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

[PATCH v9 25/36] fprobe: Use ftrace_regs in fprobe exit handler

2024-04-15 Thread Masami Hiramatsu (Google)
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 a new

[PATCH v9 24/36] fprobe: Use ftrace_regs in fprobe entry handler

2024-04-15 Thread Masami Hiramatsu (Google)
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

[PATCH v9 23/36] function_graph: Pass ftrace_regs to retfunc

2024-04-15 Thread Masami Hiramatsu (Google)
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

[PATCH v9 22/36] function_graph: Replace fgraph_ret_regs with ftrace_regs

2024-04-15 Thread Masami Hiramatsu (Google)
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

[PATCH v9 21/36] function_graph: Pass ftrace_regs to entryfunc

2024-04-15 Thread Masami Hiramatsu (Google)
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

[PATCH v9 20/36] ftrace: Add multiple fgraph storage selftest

2024-04-15 Thread Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google) Add a selftest for multiple function graph tracer with storage on a same function. In this case, the shadow stack entry will be shared among those fgraph with different data storage. So this will ensure the fgraph will not mixed those storage data. Signed-off

[PATCH v9 19/36] function_graph: Add selftest for passing local variables

2024-04-15 Thread Masami Hiramatsu (Google)
From: Steven Rostedt (VMware) Add boot up selftest that passes variables from a function entry to a function exit, and make sure that they do get passed around. Signed-off-by: Steven Rostedt (VMware) Signed-off-by: Masami Hiramatsu (Google) --- Changes in v2: - Add reserved size test

[PATCH v9 18/36] function_graph: Implement fgraph_reserve_data() and fgraph_retrieve_data()

2024-04-15 Thread Masami Hiramatsu (Google)
shadow ret_stack and this then can be retrieved by fgraph_retrieve_data() called by the corresponding retfunc(). Signed-off-by: Steven Rostedt (VMware) Signed-off-by: Masami Hiramatsu (Google) --- Changes in v8: - Avoid using DIV_ROUND_UP() in the hot path. Changes in v3: - Store

[PATCH v9 17/36] function_graph: Move graph notrace bit to shadow stack global var

2024-04-15 Thread Masami Hiramatsu (Google)
are) Signed-off-by: Masami Hiramatsu (Google) --- Changes in v2: - Make description lines shorter than 76 chars. --- include/linux/trace_recursion.h |7 --- kernel/trace/trace.h |9 + kernel/trace/trace_functions_graph.c | 10 ++ 3 files chan

[PATCH v9 16/36] function_graph: Move graph depth stored data to shadow stack global var

2024-04-15 Thread Masami Hiramatsu (Google)
are) Signed-off-by: Masami Hiramatsu (Google) --- include/linux/trace_recursion.h | 29 - kernel/trace/trace.h| 34 -- 2 files changed, 32 insertions(+), 31 deletions(-) diff --git a/include/linux/trace_recursion.h b/incl

[PATCH v9 15/36] function_graph: Move set_graph_function tests to shadow stack global var

2024-04-15 Thread Masami Hiramatsu (Google)
are) Signed-off-by: Masami Hiramatsu (Google) --- include/linux/trace_recursion.h |5 + kernel/trace/trace.h | 32 +--- kernel/trace/trace_functions_graph.c |6 +++--- kernel/trace/trace_irqsoff.c |4 ++-- kernel/tr

[PATCH v9 14/36] function_graph: Add "task variables" per task for fgraph_ops

2024-04-15 Thread Masami Hiramatsu (Google)
s on a per task basis having a way to maintain state for each task. Signed-off-by: Steven Rostedt (VMware) Signed-off-by: Masami Hiramatsu (Google) --- Changes in v3: - Move fgraph_ops::idx to previous patch in the series. Changes in v2: - Make description lines shorter than 76 chars. --

[PATCH v9 13/36] function_graph: Use a simple LRU for fgraph_array index number

2024-04-15 Thread Masami Hiramatsu (Google)
From: Masami Hiramatsu (Google) Since the fgraph_array index is used for the bitmap on the shadow stack, it may leave some entries after a function_graph instance is removed. Thus if another instance reuses the fgraph_array index soon after releasing it, the fgraph may confuse to call the newer

[PATCH v9 12/36] function_graph: Have the instances use their own ftrace_ops for filtering

2024-04-15 Thread Masami Hiramatsu (Google)
function_graph_enter_ops() instead of function_graph_enter() so that it avoid pushing on shadow stack multiple times on the same function. Signed-off-by: Steven Rostedt (VMware) Signed-off-by: Masami Hiramatsu (Google) --- Changes in v9: - Fix to clear fgraph_array correctly when ftrace_startup

[PATCH v9 11/36] ftrace: Allow ftrace startup flags exist without dynamic ftrace

2024-04-15 Thread Masami Hiramatsu (Google)
to compile because dynamic ftrace is disabled. This change is needed to move some of the logic of what is passed to ftrace_startup() out of the parameters of ftrace_startup(). Signed-off-by: Steven Rostedt (VMware) Signed-off-by: Masami Hiramatsu (Google) --- include/linux/ftrace.h | 18

  1   2   3   4   5   6   7   8   9   10   >