Messages in this thread | | | Subject | Re: [PATCH] tracing/boottime: Fix kprobe multiple events | From | Maximilian Werner <> | Date | Wed, 17 Jun 2020 17:57:13 +0200 |
| |
We are a group of students from Leibniz University Hannover and this patch is part of a project of ours. That's why both of us signed this off.
Should we have added Masami to Cc? He didn't appear in the get_maintainer script.
-- Maximilian & Sascha
On 17.06.20 17:06, Steven Rostedt wrote: > On Wed, 17 Jun 2020 11:05:21 -0400 > Steven Rostedt <rostedt@goodmis.org> wrote: > >> On Wed, 17 Jun 2020 16:08:17 +0200 >> Sascha Ortmann <sascha.ortmann@stud.uni-hannover.de> wrote: >> >>> Fix boottime kprobe events to add multiple events even if one fails >>> and report probe generation failures. >>> >>> As an example, when we try to set multiprobe kprobe events in >>> bootconfig like this: >>> >>> ftrace.event.kprobes.vfsevents { >>> probes = "vfs_read $arg1 $arg2,, >>> !error! not reported;?", // leads to error >>> "vfs_write $arg1 $arg2" >>> } >>> >>> this will not work like expected. After commit >>> da0f1f4167e3af69e1d8b32d6d65195ddd2bfb64 ("tracing/boottime: >>> Fix kprobe event API usage"), the function >>> trace_boot_add_kprobe_event will not produce any error message, >>> aborting the function and stopping subsequent probes from getting >>> installed when adding a probe fails at kprobe_event_gen_cmd_start. >>> Furthermore, probes continue when kprobe_event_gen_cmd_end fails >>> (and kprobe_event_gen_cmd_start did not fail). In this case the >>> function even returns successfully when the last call to >>> kprobe_event_gen_cmd_end is successful. >>> >>> The behaviour of reporting and aborting after failures is not >>> consistent. >>> >>> The function trace_boot_add_kprobe_event now continues even when >>> one of the multiple events fails. Each failure is now reported >>> individually. Since the function can only return one result to the >>> caller, the function returns now the last failure (or none, if >>> nothing fails). >>> >>> Cc: linux-kernel@i4.cs.fau.de >>> Signed-off-by: Maximilian Werner <maximilian.werner96@gmail.com> >>> Signed-off-by: Sascha Ortmann <sascha.ortmann@stud.uni-hannover.de> >> >> Why the double signed off by? >> >> Masami, I'm fine with this, but needs your review. > > [ It appears that Masami wasn't in the Cc ] > > >> >> -- Steve >> >>> --- >>> kernel/trace/trace_boot.c | 16 +++++++++++----- >>> 1 file changed, 11 insertions(+), 5 deletions(-) >>> >>> diff --git a/kernel/trace/trace_boot.c b/kernel/trace/trace_boot.c >>> index 9de29bb45a27..dbb50184e060 100644 >>> --- a/kernel/trace/trace_boot.c >>> +++ b/kernel/trace/trace_boot.c >>> @@ -95,18 +95,24 @@ trace_boot_add_kprobe_event(struct xbc_node *node, const char *event) >>> struct xbc_node *anode; >>> char buf[MAX_BUF_LEN]; >>> const char *val; >>> + int error = 0; >>> int ret = 0; >>> >>> xbc_node_for_each_array_value(node, "probes", anode, val) { >>> kprobe_event_cmd_init(&cmd, buf, MAX_BUF_LEN); >>> >>> - ret = kprobe_event_gen_cmd_start(&cmd, event, val); >>> - if (ret) >>> - break; >>> + error = kprobe_event_gen_cmd_start(&cmd, event, val); >>> + if (error) { >>> + pr_err("Failed to generate probe: %s\n", buf); >>> + ret = error; >>> + continue; >>> + } >>> >>> - ret = kprobe_event_gen_cmd_end(&cmd); >>> - if (ret) >>> + error = kprobe_event_gen_cmd_end(&cmd); >>> + if (error) { >>> pr_err("Failed to add probe: %s\n", buf); >>> + ret = error; >>> + } >>> } >>> >>> return ret; >> >
| |