* [PATCH 0/4] ftrace: Fix a few issues
@ 2017-05-13 19:31 Naveen N. Rao
2017-05-13 19:31 ` [PATCH 1/4] ftrace: Simplify glob handling in unregister_ftrace_function_probe_func() Naveen N. Rao
` (3 more replies)
0 siblings, 4 replies; 18+ messages in thread
From: Naveen N. Rao @ 2017-05-13 19:31 UTC (permalink / raw)
To: Steven Rostedt, Shuah Khan
Cc: Michael Ellerman, linux-kernel, linux-kselftest
This series fixes a kernel oops when an ftrace instance is deleted while
there are still active event triggers. Patch 2 provides details on how
to reproduce as well as the kernel oops message.
This issue was reported by Michael Ellerman as a crash seen when trying
to run the ftrace test suite. In looking into it, I noticed that the
issue actually showed up due to a few bashisms in the ftrace tests when
run on Ubuntu. Those bashisms meant that the ftrace instance was being
deleted without removing the event triggers. Patch 3 includes a fix for
the bashisms.
Patch 4 adds a test case to explicitly catch this issue going forward.
- Naveen
Naveen N. Rao (4):
ftrace: Simplify glob handling in
unregister_ftrace_function_probe_func()
ftrace/instances: Clear function triggers when removing instances
selftests/ftrace: Fix bashisms
selftests/ftrace: Add test to remove instance with active event
triggers
kernel/trace/ftrace.c | 12 ++++++++++--
kernel/trace/trace.c | 1 +
kernel/trace/trace.h | 1 +
tools/testing/selftests/ftrace/ftracetest | 2 +-
.../selftests/ftrace/test.d/ftrace/func_event_triggers.tc | 2 +-
tools/testing/selftests/ftrace/test.d/functions | 4 ++--
.../selftests/ftrace/test.d/instances/instance-event.tc | 8 ++++++--
7 files changed, 22 insertions(+), 8 deletions(-)
--
2.12.2
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/4] ftrace: Simplify glob handling in unregister_ftrace_function_probe_func()
2017-05-13 19:31 [PATCH 0/4] ftrace: Fix a few issues Naveen N. Rao
@ 2017-05-13 19:31 ` Naveen N. Rao
2017-05-15 17:22 ` Steven Rostedt
2017-05-13 19:31 ` [PATCH 2/4] ftrace/instances: Clear function triggers when removing instances Naveen N. Rao
` (2 subsequent siblings)
3 siblings, 1 reply; 18+ messages in thread
From: Naveen N. Rao @ 2017-05-13 19:31 UTC (permalink / raw)
To: Steven Rostedt, Shuah Khan
Cc: Michael Ellerman, linux-kernel, linux-kselftest
Handle a NULL glob properly.
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
kernel/trace/ftrace.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 39dca4e86a94..28dc824ad072 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -4144,9 +4144,9 @@ unregister_ftrace_function_probe_func(char *glob, struct trace_array *tr,
int i, ret = -ENODEV;
int size;
- if (glob && (strcmp(glob, "*") == 0 || !strlen(glob)))
+ if (!glob || (glob && (strcmp(glob, "*") == 0 || !strlen(glob))))
func_g.search = NULL;
- else if (glob) {
+ else {
int not;
func_g.type = filter_parse_regex(glob, strlen(glob),
--
2.12.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/4] ftrace/instances: Clear function triggers when removing instances
2017-05-13 19:31 [PATCH 0/4] ftrace: Fix a few issues Naveen N. Rao
2017-05-13 19:31 ` [PATCH 1/4] ftrace: Simplify glob handling in unregister_ftrace_function_probe_func() Naveen N. Rao
@ 2017-05-13 19:31 ` Naveen N. Rao
2017-05-15 15:21 ` Steven Rostedt
2017-05-16 2:20 ` Steven Rostedt
2017-05-13 19:31 ` [PATCH 3/4] selftests/ftrace: Fix bashisms Naveen N. Rao
2017-05-13 19:31 ` [PATCH 4/4] selftests/ftrace: Add test to remove instance with active event triggers Naveen N. Rao
3 siblings, 2 replies; 18+ messages in thread
From: Naveen N. Rao @ 2017-05-13 19:31 UTC (permalink / raw)
To: Steven Rostedt, Shuah Khan
Cc: Michael Ellerman, linux-kernel, linux-kselftest
If instance directories are deleted while there are registered function
triggers:
# cd /sys/kernel/debug/tracing/instances
# mkdir test
# echo "schedule:enable_event:sched:sched_switch" > test/set_ftrace_filter
# rmdir test
Unable to handle kernel paging request for data at address 0x00000008
Unable to handle kernel paging request for data at address 0x00000008
Faulting instruction address: 0xc0000000021edde8
Oops: Kernel access of bad area, sig: 11 [#1]
SMP NR_CPUS=2048
NUMA
pSeries
Modules linked in: iptable_mangle ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat nf_nat_ipv4 nf_nat nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack ipt_REJECT nf_reject_ipv4 xt_tcpudp tun bridge stp llc kvm iptable_filter fuse binfmt_misc pseries_rng rng_core vmx_crypto ib_iser rdma_cm iw_cm ib_cm ib_core libiscsi scsi_transport_iscsi ip_tables x_tables autofs4 btrfs raid10 raid456 async_raid6_recov async_memcpy async_pq async_xor async_tx xor raid6_pq libcrc32c multipath virtio_net virtio_blk virtio_pci crc32c_vpmsum virtio_ring virtio
CPU: 8 PID: 8694 Comm: rmdir Not tainted 4.11.0-nnr+ #113
task: c0000000bab52800 task.stack: c0000000baba0000
NIP: c0000000021edde8 LR: c0000000021f0590 CTR: c000000002119620
REGS: c0000000baba3870 TRAP: 0300 Not tainted (4.11.0-nnr+)
MSR: 8000000000009033 <SF,EE,ME,IR,DR,RI,LE>
CR: 22002422 XER: 20000000
CFAR: 00007fffabb725a8 DAR: 0000000000000008 DSISR: 40000000 SOFTE: 0
GPR00: c00000000220f750 c0000000baba3af0 c000000003157e00 0000000000000000
GPR04: 0000000000000040 00000000000000eb 0000000000000040 0000000000000000
GPR08: 0000000000000000 0000000000000113 0000000000000000 c00000000305db98
GPR12: c000000002119620 c00000000fd42c00 0000000000000000 0000000000000000
GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR20: 0000000000000000 0000000000000000 c0000000bab52e90 0000000000000000
GPR24: 0000000000000000 00000000000000eb 0000000000000040 c0000000baba3bb0
GPR28: c00000009cb06eb0 c0000000bab52800 c00000009cb06eb0 c0000000baba3bb0
NIP [c0000000021edde8] ring_buffer_lock_reserve+0x8/0x4e0
LR [c0000000021f0590] trace_event_buffer_lock_reserve+0xe0/0x1a0
Call Trace:
[c0000000baba3af0] [c0000000021f96c8] trace_event_buffer_commit+0x1b8/0x280 (unreliable)
[c0000000baba3b60] [c00000000220f750] trace_event_buffer_reserve+0x80/0xd0
[c0000000baba3b90] [c0000000021196b8] trace_event_raw_event_sched_switch+0x98/0x180
[c0000000baba3c10] [c0000000029d9980] __schedule+0x6e0/0xab0
[c0000000baba3ce0] [c000000002122230] do_task_dead+0x70/0xc0
[c0000000baba3d10] [c0000000020ea9c8] do_exit+0x828/0xd00
[c0000000baba3dd0] [c0000000020eaf70] do_group_exit+0x60/0x100
[c0000000baba3e10] [c0000000020eb034] SyS_exit_group+0x24/0x30
[c0000000baba3e30] [c00000000200bcec] system_call+0x38/0x54
Instruction dump:
60000000 60420000 7d244b78 7f63db78 4bffaa09 393efff8 793e0020 39200000
4bfffecc 60420000 3c4c00f7 3842a020 <81230008> 2f890000 409e02f0 a14d0008
---[ end trace b917b8985d0e650b ]---
Unable to handle kernel paging request for data at address 0x00000008
Faulting instruction address: 0xc0000000021edde8
Unable to handle kernel paging request for data at address 0x00000008
Faulting instruction address: 0xc0000000021edde8
Faulting instruction address: 0xc0000000021edde8
To address this, let's clear all registered function probes before
deleting the ftrace instance.
Reported-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
kernel/trace/ftrace.c | 8 ++++++++
kernel/trace/trace.c | 1 +
kernel/trace/trace.h | 1 +
3 files changed, 10 insertions(+)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 28dc824ad072..1b96d927a082 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -4256,6 +4256,14 @@ unregister_ftrace_function_probe_func(char *glob, struct trace_array *tr,
return ret;
}
+void clear_ftrace_function_probes(struct trace_array *tr)
+{
+ struct ftrace_func_probe *probe, *n;
+
+ list_for_each_entry_safe(probe, n, &tr->func_probes, list)
+ unregister_ftrace_function_probe_func(NULL, tr, probe->probe_ops);
+}
+
static LIST_HEAD(ftrace_commands);
static DEFINE_MUTEX(ftrace_cmd_mutex);
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index c4536c449021..3f2aed4ad1ed 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -7550,6 +7550,7 @@ static int instance_rmdir(const char *name)
}
tracing_set_nop(tr);
+ clear_ftrace_function_probes(tr);
event_trace_del_tracer(tr);
ftrace_clear_pids(tr);
ftrace_destroy_function_files(tr);
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 291a1bca5748..98e0845f7235 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -980,6 +980,7 @@ register_ftrace_function_probe(char *glob, struct trace_array *tr,
extern int
unregister_ftrace_function_probe_func(char *glob, struct trace_array *tr,
struct ftrace_probe_ops *ops);
+extern void clear_ftrace_function_probes(struct trace_array *tr);
int register_ftrace_command(struct ftrace_func_command *cmd);
int unregister_ftrace_command(struct ftrace_func_command *cmd);
--
2.12.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 3/4] selftests/ftrace: Fix bashisms
2017-05-13 19:31 [PATCH 0/4] ftrace: Fix a few issues Naveen N. Rao
2017-05-13 19:31 ` [PATCH 1/4] ftrace: Simplify glob handling in unregister_ftrace_function_probe_func() Naveen N. Rao
2017-05-13 19:31 ` [PATCH 2/4] ftrace/instances: Clear function triggers when removing instances Naveen N. Rao
@ 2017-05-13 19:31 ` Naveen N. Rao
2017-05-15 15:16 ` Steven Rostedt
2017-05-13 19:31 ` [PATCH 4/4] selftests/ftrace: Add test to remove instance with active event triggers Naveen N. Rao
3 siblings, 1 reply; 18+ messages in thread
From: Naveen N. Rao @ 2017-05-13 19:31 UTC (permalink / raw)
To: Steven Rostedt, Shuah Khan
Cc: Michael Ellerman, linux-kernel, linux-kselftest
Fix a few bashisms in ftrace selftests.
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
tools/testing/selftests/ftrace/ftracetest | 2 +-
tools/testing/selftests/ftrace/test.d/ftrace/func_event_triggers.tc | 2 +-
tools/testing/selftests/ftrace/test.d/functions | 4 ++--
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest
index 32e6211e1c6e..717581145cfc 100755
--- a/tools/testing/selftests/ftrace/ftracetest
+++ b/tools/testing/selftests/ftrace/ftracetest
@@ -58,7 +58,7 @@ parse_opts() { # opts
;;
--verbose|-v|-vv)
VERBOSE=$((VERBOSE + 1))
- [ $1 == '-vv' ] && VERBOSE=$((VERBOSE + 1))
+ [ $1 = '-vv' ] && VERBOSE=$((VERBOSE + 1))
shift 1
;;
--debug|-d)
diff --git a/tools/testing/selftests/ftrace/test.d/ftrace/func_event_triggers.tc b/tools/testing/selftests/ftrace/test.d/ftrace/func_event_triggers.tc
index 07bb3e5930b4..aa31368851c9 100644
--- a/tools/testing/selftests/ftrace/test.d/ftrace/func_event_triggers.tc
+++ b/tools/testing/selftests/ftrace/test.d/ftrace/func_event_triggers.tc
@@ -48,7 +48,7 @@ test_event_enabled() {
e=`cat $EVENT_ENABLE`
if [ "$e" != $val ]; then
echo "Expected $val but found $e"
- exit -1
+ exit 1
fi
}
diff --git a/tools/testing/selftests/ftrace/test.d/functions b/tools/testing/selftests/ftrace/test.d/functions
index 9aec6fcb7729..f2019b37370d 100644
--- a/tools/testing/selftests/ftrace/test.d/functions
+++ b/tools/testing/selftests/ftrace/test.d/functions
@@ -34,10 +34,10 @@ reset_ftrace_filter() { # reset all triggers in set_ftrace_filter
echo > set_ftrace_filter
grep -v '^#' set_ftrace_filter | while read t; do
tr=`echo $t | cut -d: -f2`
- if [ "$tr" == "" ]; then
+ if [ "$tr" = "" ]; then
continue
fi
- if [ $tr == "enable_event" -o $tr == "disable_event" ]; then
+ if [ $tr = "enable_event" -o $tr = "disable_event" ]; then
tr=`echo $t | cut -d: -f1-4`
limit=`echo $t | cut -d: -f5`
else
--
2.12.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 4/4] selftests/ftrace: Add test to remove instance with active event triggers
2017-05-13 19:31 [PATCH 0/4] ftrace: Fix a few issues Naveen N. Rao
` (2 preceding siblings ...)
2017-05-13 19:31 ` [PATCH 3/4] selftests/ftrace: Fix bashisms Naveen N. Rao
@ 2017-05-13 19:31 ` Naveen N. Rao
3 siblings, 0 replies; 18+ messages in thread
From: Naveen N. Rao @ 2017-05-13 19:31 UTC (permalink / raw)
To: Steven Rostedt, Shuah Khan
Cc: Michael Ellerman, linux-kernel, linux-kselftest
Add a test to ensure we clean up properly when removing an instance
with active event triggers.
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
Steven,
I have also removed what looked like an incorrect 'exit' in the middle
of this test. Kindly take a look if that's ok.
- Naveen
tools/testing/selftests/ftrace/test.d/instances/instance-event.tc | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/ftrace/test.d/instances/instance-event.tc b/tools/testing/selftests/ftrace/test.d/instances/instance-event.tc
index 4c5a061a5b4e..c73db7863adb 100644
--- a/tools/testing/selftests/ftrace/test.d/instances/instance-event.tc
+++ b/tools/testing/selftests/ftrace/test.d/instances/instance-event.tc
@@ -75,9 +75,13 @@ rmdir foo
if [ -d foo ]; then
fail "foo still exists"
fi
-exit 0
-
+mkdir foo
+echo "schedule:enable_event:sched:sched_switch" > foo/set_ftrace_filter
+rmdir foo
+if [ -d foo ]; then
+ fail "foo still exists"
+fi
instance_slam() {
--
2.12.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 3/4] selftests/ftrace: Fix bashisms
2017-05-13 19:31 ` [PATCH 3/4] selftests/ftrace: Fix bashisms Naveen N. Rao
@ 2017-05-15 15:16 ` Steven Rostedt
2017-05-16 14:25 ` Masami Hiramatsu
0 siblings, 1 reply; 18+ messages in thread
From: Steven Rostedt @ 2017-05-15 15:16 UTC (permalink / raw)
To: Naveen N. Rao, Masami Hiramatsu
Cc: Shuah Khan, Michael Ellerman, linux-kernel, linux-kselftest
Masami's the original author of ftracetest.
Masami, are you OK with this change?
-- Steve
On Sun, 14 May 2017 01:01:03 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
> Fix a few bashisms in ftrace selftests.
>
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> ---
> tools/testing/selftests/ftrace/ftracetest | 2 +-
> tools/testing/selftests/ftrace/test.d/ftrace/func_event_triggers.tc | 2 +-
> tools/testing/selftests/ftrace/test.d/functions | 4 ++--
> 3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest
> index 32e6211e1c6e..717581145cfc 100755
> --- a/tools/testing/selftests/ftrace/ftracetest
> +++ b/tools/testing/selftests/ftrace/ftracetest
> @@ -58,7 +58,7 @@ parse_opts() { # opts
> ;;
> --verbose|-v|-vv)
> VERBOSE=$((VERBOSE + 1))
> - [ $1 == '-vv' ] && VERBOSE=$((VERBOSE + 1))
> + [ $1 = '-vv' ] && VERBOSE=$((VERBOSE + 1))
> shift 1
> ;;
> --debug|-d)
> diff --git a/tools/testing/selftests/ftrace/test.d/ftrace/func_event_triggers.tc b/tools/testing/selftests/ftrace/test.d/ftrace/func_event_triggers.tc
> index 07bb3e5930b4..aa31368851c9 100644
> --- a/tools/testing/selftests/ftrace/test.d/ftrace/func_event_triggers.tc
> +++ b/tools/testing/selftests/ftrace/test.d/ftrace/func_event_triggers.tc
> @@ -48,7 +48,7 @@ test_event_enabled() {
> e=`cat $EVENT_ENABLE`
> if [ "$e" != $val ]; then
> echo "Expected $val but found $e"
> - exit -1
> + exit 1
> fi
> }
>
> diff --git a/tools/testing/selftests/ftrace/test.d/functions b/tools/testing/selftests/ftrace/test.d/functions
> index 9aec6fcb7729..f2019b37370d 100644
> --- a/tools/testing/selftests/ftrace/test.d/functions
> +++ b/tools/testing/selftests/ftrace/test.d/functions
> @@ -34,10 +34,10 @@ reset_ftrace_filter() { # reset all triggers in set_ftrace_filter
> echo > set_ftrace_filter
> grep -v '^#' set_ftrace_filter | while read t; do
> tr=`echo $t | cut -d: -f2`
> - if [ "$tr" == "" ]; then
> + if [ "$tr" = "" ]; then
> continue
> fi
> - if [ $tr == "enable_event" -o $tr == "disable_event" ]; then
> + if [ $tr = "enable_event" -o $tr = "disable_event" ]; then
> tr=`echo $t | cut -d: -f1-4`
> limit=`echo $t | cut -d: -f5`
> else
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4] ftrace/instances: Clear function triggers when removing instances
2017-05-13 19:31 ` [PATCH 2/4] ftrace/instances: Clear function triggers when removing instances Naveen N. Rao
@ 2017-05-15 15:21 ` Steven Rostedt
2017-05-15 16:11 ` Steven Rostedt
2017-05-16 2:20 ` Steven Rostedt
1 sibling, 1 reply; 18+ messages in thread
From: Steven Rostedt @ 2017-05-15 15:21 UTC (permalink / raw)
To: Naveen N. Rao
Cc: Shuah Khan, Michael Ellerman, linux-kernel, linux-kselftest,
Masami Hiramatsu
On Sun, 14 May 2017 01:01:02 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
> If instance directories are deleted while there are registered function
> triggers:
>
> # cd /sys/kernel/debug/tracing/instances
> # mkdir test
> # echo "schedule:enable_event:sched:sched_switch" > test/set_ftrace_filter
> # rmdir test
> Unable to handle kernel paging request for data at address 0x00000008
> Unable to handle kernel paging request for data at address 0x00000008
> Faulting instruction address: 0xc0000000021edde8
> Oops: Kernel access of bad area, sig: 11 [#1]
> SMP NR_CPUS=2048
> NUMA
> pSeries
Looks like this patch needs to be marked for stable, and go in right
away. I may also do the same for the ftracetest bashism patch with
Masami's approval. I may also slip in the ftracetest for the bug that
this patch fixes.
The first patch can go into the queue for 4.13.
-- Steve
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4] ftrace/instances: Clear function triggers when removing instances
2017-05-15 15:21 ` Steven Rostedt
@ 2017-05-15 16:11 ` Steven Rostedt
2017-05-16 16:11 ` Masami Hiramatsu
0 siblings, 1 reply; 18+ messages in thread
From: Steven Rostedt @ 2017-05-15 16:11 UTC (permalink / raw)
To: Naveen N. Rao
Cc: Shuah Khan, Michael Ellerman, linux-kernel, linux-kselftest,
Masami Hiramatsu
On Mon, 15 May 2017 11:21:15 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> On Sun, 14 May 2017 01:01:02 +0530
> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
>
> > If instance directories are deleted while there are registered function
> > triggers:
> >
> > # cd /sys/kernel/debug/tracing/instances
> > # mkdir test
> > # echo "schedule:enable_event:sched:sched_switch" > test/set_ftrace_filter
> > # rmdir test
> > Unable to handle kernel paging request for data at address 0x00000008
> > Unable to handle kernel paging request for data at address 0x00000008
> > Faulting instruction address: 0xc0000000021edde8
> > Oops: Kernel access of bad area, sig: 11 [#1]
> > SMP NR_CPUS=2048
> > NUMA
> > pSeries
>
> Looks like this patch needs to be marked for stable, and go in right
> away. I may also do the same for the ftracetest bashism patch with
> Masami's approval. I may also slip in the ftracetest for the bug that
> this patch fixes.
As this patch fixes a bug that was introduced with the 4.12 merge
window. Thus, it's not stable worthy (stable isn't affected). Although,
I'll wait on Masami to see if that should go to stable.
-- Steve
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] ftrace: Simplify glob handling in unregister_ftrace_function_probe_func()
2017-05-13 19:31 ` [PATCH 1/4] ftrace: Simplify glob handling in unregister_ftrace_function_probe_func() Naveen N. Rao
@ 2017-05-15 17:22 ` Steven Rostedt
2017-05-16 8:07 ` Naveen N. Rao
0 siblings, 1 reply; 18+ messages in thread
From: Steven Rostedt @ 2017-05-15 17:22 UTC (permalink / raw)
To: Naveen N. Rao; +Cc: Shuah Khan, Michael Ellerman, linux-kernel, linux-kselftest
On Sun, 14 May 2017 01:01:01 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
> Handle a NULL glob properly.
>
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> ---
> kernel/trace/ftrace.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 39dca4e86a94..28dc824ad072 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -4144,9 +4144,9 @@ unregister_ftrace_function_probe_func(char *glob, struct trace_array *tr,
> int i, ret = -ENODEV;
> int size;
>
> - if (glob && (strcmp(glob, "*") == 0 || !strlen(glob)))
> + if (!glob || (glob && (strcmp(glob, "*") == 0 || !strlen(glob))))
Actually, this can also be simplified.
if (!glob || strcmp(glob, "*") == 0) || !strlen(glob))
No need to check if glob exists past the first expression.
-- Steve
> func_g.search = NULL;
> - else if (glob) {
> + else {
> int not;
>
> func_g.type = filter_parse_regex(glob, strlen(glob),
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4] ftrace/instances: Clear function triggers when removing instances
2017-05-13 19:31 ` [PATCH 2/4] ftrace/instances: Clear function triggers when removing instances Naveen N. Rao
2017-05-15 15:21 ` Steven Rostedt
@ 2017-05-16 2:20 ` Steven Rostedt
2017-05-16 14:01 ` Naveen N. Rao
1 sibling, 1 reply; 18+ messages in thread
From: Steven Rostedt @ 2017-05-16 2:20 UTC (permalink / raw)
To: Naveen N. Rao; +Cc: Shuah Khan, Michael Ellerman, linux-kernel, linux-kselftest
On Sun, 14 May 2017 01:01:02 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
> If instance directories are deleted while there are registered function
> triggers:
>
> # cd /sys/kernel/debug/tracing/instances
> # mkdir test
> # echo "schedule:enable_event:sched:sched_switch" > test/set_ftrace_filter
> # rmdir test
> Unable to handle kernel paging request for data at address 0x00000008
> Unable to handle kernel paging request for data at address 0x00000008
> Faulting instruction address: 0xc0000000021edde8
> Oops: Kernel access of bad area, sig: 11 [#1]
> SMP NR_CPUS=2048
> NUMA
> pSeries
> Modules linked in: iptable_mangle ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat nf_nat_ipv4 nf_nat nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack ipt_REJECT nf_reject_ipv4 xt_tcpudp tun bridge stp llc kvm iptable_filter fuse binfmt_misc pseries_rng rng_core vmx_crypto ib_iser rdma_cm iw_cm ib_cm ib_core libiscsi scsi_transport_iscsi ip_tables x_tables autofs4 btrfs raid10 raid456 async_raid6_recov async_memcpy async_pq async_xor async_tx xor raid6_pq libcrc32c multipath virtio_net virtio_blk virtio_pci crc32c_vpmsum virtio_ring virtio
> CPU: 8 PID: 8694 Comm: rmdir Not tainted 4.11.0-nnr+ #113
> task: c0000000bab52800 task.stack: c0000000baba0000
> NIP: c0000000021edde8 LR: c0000000021f0590 CTR: c000000002119620
> REGS: c0000000baba3870 TRAP: 0300 Not tainted (4.11.0-nnr+)
> MSR: 8000000000009033 <SF,EE,ME,IR,DR,RI,LE>
> CR: 22002422 XER: 20000000
> CFAR: 00007fffabb725a8 DAR: 0000000000000008 DSISR: 40000000 SOFTE: 0
> GPR00: c00000000220f750 c0000000baba3af0 c000000003157e00 0000000000000000
> GPR04: 0000000000000040 00000000000000eb 0000000000000040 0000000000000000
> GPR08: 0000000000000000 0000000000000113 0000000000000000 c00000000305db98
> GPR12: c000000002119620 c00000000fd42c00 0000000000000000 0000000000000000
> GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> GPR20: 0000000000000000 0000000000000000 c0000000bab52e90 0000000000000000
> GPR24: 0000000000000000 00000000000000eb 0000000000000040 c0000000baba3bb0
> GPR28: c00000009cb06eb0 c0000000bab52800 c00000009cb06eb0 c0000000baba3bb0
> NIP [c0000000021edde8] ring_buffer_lock_reserve+0x8/0x4e0
> LR [c0000000021f0590] trace_event_buffer_lock_reserve+0xe0/0x1a0
> Call Trace:
> [c0000000baba3af0] [c0000000021f96c8] trace_event_buffer_commit+0x1b8/0x280 (unreliable)
> [c0000000baba3b60] [c00000000220f750] trace_event_buffer_reserve+0x80/0xd0
> [c0000000baba3b90] [c0000000021196b8] trace_event_raw_event_sched_switch+0x98/0x180
> [c0000000baba3c10] [c0000000029d9980] __schedule+0x6e0/0xab0
> [c0000000baba3ce0] [c000000002122230] do_task_dead+0x70/0xc0
> [c0000000baba3d10] [c0000000020ea9c8] do_exit+0x828/0xd00
> [c0000000baba3dd0] [c0000000020eaf70] do_group_exit+0x60/0x100
> [c0000000baba3e10] [c0000000020eb034] SyS_exit_group+0x24/0x30
> [c0000000baba3e30] [c00000000200bcec] system_call+0x38/0x54
> Instruction dump:
> 60000000 60420000 7d244b78 7f63db78 4bffaa09 393efff8 793e0020 39200000
> 4bfffecc 60420000 3c4c00f7 3842a020 <81230008> 2f890000 409e02f0 a14d0008
> ---[ end trace b917b8985d0e650b ]---
> Unable to handle kernel paging request for data at address 0x00000008
> Faulting instruction address: 0xc0000000021edde8
> Unable to handle kernel paging request for data at address 0x00000008
> Faulting instruction address: 0xc0000000021edde8
> Faulting instruction address: 0xc0000000021edde8
>
> To address this, let's clear all registered function probes before
> deleting the ftrace instance.
>
> Reported-by: Michael Ellerman <mpe@ellerman.id.au>
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> ---
> kernel/trace/ftrace.c | 8 ++++++++
> kernel/trace/trace.c | 1 +
> kernel/trace/trace.h | 1 +
> 3 files changed, 10 insertions(+)
>
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 28dc824ad072..1b96d927a082 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -4256,6 +4256,14 @@ unregister_ftrace_function_probe_func(char *glob, struct trace_array *tr,
> return ret;
> }
>
> +void clear_ftrace_function_probes(struct trace_array *tr)
> +{
> + struct ftrace_func_probe *probe, *n;
> +
> + list_for_each_entry_safe(probe, n, &tr->func_probes, list)
> + unregister_ftrace_function_probe_func(NULL, tr, probe->probe_ops);
> +}
> +
> static LIST_HEAD(ftrace_commands);
> static DEFINE_MUTEX(ftrace_cmd_mutex);
>
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index c4536c449021..3f2aed4ad1ed 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -7550,6 +7550,7 @@ static int instance_rmdir(const char *name)
> }
>
> tracing_set_nop(tr);
> + clear_ftrace_function_probes(tr);
> event_trace_del_tracer(tr);
> ftrace_clear_pids(tr);
> ftrace_destroy_function_files(tr);
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index 291a1bca5748..98e0845f7235 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -980,6 +980,7 @@ register_ftrace_function_probe(char *glob, struct trace_array *tr,
> extern int
> unregister_ftrace_function_probe_func(char *glob, struct trace_array *tr,
> struct ftrace_probe_ops *ops);
> +extern void clear_ftrace_function_probes(struct trace_array *tr);
This needs to have a stub function when CONFIG_DYNAMIC_FTRACE is not
defined. Otherwise we have:
kernel/trace/trace.c:7553:2: error: implicit declaration of function 'clear_ftrace_function_probes' [-Werror=implicit-function-declaration]
clear_ftrace_function_probes(tr);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
-- Steve
>
> int register_ftrace_command(struct ftrace_func_command *cmd);
> int unregister_ftrace_command(struct ftrace_func_command *cmd);
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] ftrace: Simplify glob handling in unregister_ftrace_function_probe_func()
2017-05-15 17:22 ` Steven Rostedt
@ 2017-05-16 8:07 ` Naveen N. Rao
0 siblings, 0 replies; 18+ messages in thread
From: Naveen N. Rao @ 2017-05-16 8:07 UTC (permalink / raw)
To: Steven Rostedt
Cc: Shuah Khan, Michael Ellerman, linux-kernel, linux-kselftest
On 2017/05/15 01:22PM, Steven Rostedt wrote:
> On Sun, 14 May 2017 01:01:01 +0530
> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
>
> > Handle a NULL glob properly.
> >
> > Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> > ---
> > kernel/trace/ftrace.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> > index 39dca4e86a94..28dc824ad072 100644
> > --- a/kernel/trace/ftrace.c
> > +++ b/kernel/trace/ftrace.c
> > @@ -4144,9 +4144,9 @@ unregister_ftrace_function_probe_func(char *glob, struct trace_array *tr,
> > int i, ret = -ENODEV;
> > int size;
> >
> > - if (glob && (strcmp(glob, "*") == 0 || !strlen(glob)))
> > + if (!glob || (glob && (strcmp(glob, "*") == 0 || !strlen(glob))))
>
> Actually, this can also be simplified.
>
> if (!glob || strcmp(glob, "*") == 0) || !strlen(glob))
>
> No need to check if glob exists past the first expression.
:facepalm:
I'll respin. Thanks.
- Naveen
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4] ftrace/instances: Clear function triggers when removing instances
2017-05-16 2:20 ` Steven Rostedt
@ 2017-05-16 14:01 ` Naveen N. Rao
2017-05-16 17:44 ` Naveen N. Rao
0 siblings, 1 reply; 18+ messages in thread
From: Naveen N. Rao @ 2017-05-16 14:01 UTC (permalink / raw)
To: Steven Rostedt
Cc: Shuah Khan, Michael Ellerman, linux-kernel, linux-kselftest
On 2017/05/15 10:20PM, Steven Rostedt wrote:
> On Sun, 14 May 2017 01:01:02 +0530
> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
>
[snip]
> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > index c4536c449021..3f2aed4ad1ed 100644
> > --- a/kernel/trace/trace.c
> > +++ b/kernel/trace/trace.c
> > @@ -7550,6 +7550,7 @@ static int instance_rmdir(const char *name)
> > }
> >
> > tracing_set_nop(tr);
> > + clear_ftrace_function_probes(tr);
> > event_trace_del_tracer(tr);
> > ftrace_clear_pids(tr);
> > ftrace_destroy_function_files(tr);
> > diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> > index 291a1bca5748..98e0845f7235 100644
> > --- a/kernel/trace/trace.h
> > +++ b/kernel/trace/trace.h
> > @@ -980,6 +980,7 @@ register_ftrace_function_probe(char *glob, struct trace_array *tr,
> > extern int
> > unregister_ftrace_function_probe_func(char *glob, struct trace_array *tr,
> > struct ftrace_probe_ops *ops);
> > +extern void clear_ftrace_function_probes(struct trace_array *tr);
>
> This needs to have a stub function when CONFIG_DYNAMIC_FTRACE is not
> defined. Otherwise we have:
>
> kernel/trace/trace.c:7553:2: error: implicit declaration of function 'clear_ftrace_function_probes' [-Werror=implicit-function-declaration]
> clear_ftrace_function_probes(tr);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
Ah, ok. I'll respin once Masami comments on the other patches.
Thanks,
Naveen
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/4] selftests/ftrace: Fix bashisms
2017-05-15 15:16 ` Steven Rostedt
@ 2017-05-16 14:25 ` Masami Hiramatsu
0 siblings, 0 replies; 18+ messages in thread
From: Masami Hiramatsu @ 2017-05-16 14:25 UTC (permalink / raw)
To: Steven Rostedt
Cc: Naveen N. Rao, Shuah Khan, Michael Ellerman, linux-kernel,
linux-kselftest
On Mon, 15 May 2017 11:16:30 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
>
> Masami's the original author of ftracetest.
>
> Masami, are you OK with this change?
>
> -- Steve
>
>
> On Sun, 14 May 2017 01:01:03 +0530
> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
>
> > Fix a few bashisms in ftrace selftests.
Ah, what's a nice fix!
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Thanks!
> >
> > Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> > ---
> > tools/testing/selftests/ftrace/ftracetest | 2 +-
> > tools/testing/selftests/ftrace/test.d/ftrace/func_event_triggers.tc | 2 +-
> > tools/testing/selftests/ftrace/test.d/functions | 4 ++--
> > 3 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest
> > index 32e6211e1c6e..717581145cfc 100755
> > --- a/tools/testing/selftests/ftrace/ftracetest
> > +++ b/tools/testing/selftests/ftrace/ftracetest
> > @@ -58,7 +58,7 @@ parse_opts() { # opts
> > ;;
> > --verbose|-v|-vv)
> > VERBOSE=$((VERBOSE + 1))
> > - [ $1 == '-vv' ] && VERBOSE=$((VERBOSE + 1))
> > + [ $1 = '-vv' ] && VERBOSE=$((VERBOSE + 1))
> > shift 1
> > ;;
> > --debug|-d)
> > diff --git a/tools/testing/selftests/ftrace/test.d/ftrace/func_event_triggers.tc b/tools/testing/selftests/ftrace/test.d/ftrace/func_event_triggers.tc
> > index 07bb3e5930b4..aa31368851c9 100644
> > --- a/tools/testing/selftests/ftrace/test.d/ftrace/func_event_triggers.tc
> > +++ b/tools/testing/selftests/ftrace/test.d/ftrace/func_event_triggers.tc
> > @@ -48,7 +48,7 @@ test_event_enabled() {
> > e=`cat $EVENT_ENABLE`
> > if [ "$e" != $val ]; then
> > echo "Expected $val but found $e"
> > - exit -1
> > + exit 1
> > fi
> > }
> >
> > diff --git a/tools/testing/selftests/ftrace/test.d/functions b/tools/testing/selftests/ftrace/test.d/functions
> > index 9aec6fcb7729..f2019b37370d 100644
> > --- a/tools/testing/selftests/ftrace/test.d/functions
> > +++ b/tools/testing/selftests/ftrace/test.d/functions
> > @@ -34,10 +34,10 @@ reset_ftrace_filter() { # reset all triggers in set_ftrace_filter
> > echo > set_ftrace_filter
> > grep -v '^#' set_ftrace_filter | while read t; do
> > tr=`echo $t | cut -d: -f2`
> > - if [ "$tr" == "" ]; then
> > + if [ "$tr" = "" ]; then
> > continue
> > fi
> > - if [ $tr == "enable_event" -o $tr == "disable_event" ]; then
> > + if [ $tr = "enable_event" -o $tr = "disable_event" ]; then
> > tr=`echo $t | cut -d: -f1-4`
> > limit=`echo $t | cut -d: -f5`
> > else
>
--
Masami Hiramatsu
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4] ftrace/instances: Clear function triggers when removing instances
2017-05-15 16:11 ` Steven Rostedt
@ 2017-05-16 16:11 ` Masami Hiramatsu
0 siblings, 0 replies; 18+ messages in thread
From: Masami Hiramatsu @ 2017-05-16 16:11 UTC (permalink / raw)
To: Steven Rostedt
Cc: Naveen N. Rao, Shuah Khan, Michael Ellerman, linux-kernel,
linux-kselftest, Masami Hiramatsu
On Mon, 15 May 2017 12:11:03 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> On Mon, 15 May 2017 11:21:15 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
>
> > On Sun, 14 May 2017 01:01:02 +0530
> > "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
> >
> > > If instance directories are deleted while there are registered function
> > > triggers:
> > >
> > > # cd /sys/kernel/debug/tracing/instances
> > > # mkdir test
> > > # echo "schedule:enable_event:sched:sched_switch" > test/set_ftrace_filter
> > > # rmdir test
> > > Unable to handle kernel paging request for data at address 0x00000008
> > > Unable to handle kernel paging request for data at address 0x00000008
> > > Faulting instruction address: 0xc0000000021edde8
> > > Oops: Kernel access of bad area, sig: 11 [#1]
> > > SMP NR_CPUS=2048
> > > NUMA
> > > pSeries
> >
> > Looks like this patch needs to be marked for stable, and go in right
> > away. I may also do the same for the ftracetest bashism patch with
> > Masami's approval. I may also slip in the ftracetest for the bug that
> > this patch fixes.
>
> As this patch fixes a bug that was introduced with the 4.12 merge
> window. Thus, it's not stable worthy (stable isn't affected). Although,
> I'll wait on Masami to see if that should go to stable.
Hm, for the bashism fix, that is not a show-stopper... and also,
newer ftracetest should work on older kernel. So I don't think
it needs to go stable.
Thank you,
>
> -- Steve
>
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4] ftrace/instances: Clear function triggers when removing instances
2017-05-16 14:01 ` Naveen N. Rao
@ 2017-05-16 17:44 ` Naveen N. Rao
2017-05-16 18:33 ` Steven Rostedt
0 siblings, 1 reply; 18+ messages in thread
From: Naveen N. Rao @ 2017-05-16 17:44 UTC (permalink / raw)
To: Steven Rostedt
Cc: Shuah Khan, Michael Ellerman, linux-kernel, linux-kselftest
On 2017/05/16 07:31PM, Naveen N. Rao wrote:
> On 2017/05/15 10:20PM, Steven Rostedt wrote:
> > On Sun, 14 May 2017 01:01:02 +0530
> > "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
> >
>
> [snip]
>
> > > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > > index c4536c449021..3f2aed4ad1ed 100644
> > > --- a/kernel/trace/trace.c
> > > +++ b/kernel/trace/trace.c
> > > @@ -7550,6 +7550,7 @@ static int instance_rmdir(const char *name)
> > > }
> > >
> > > tracing_set_nop(tr);
> > > + clear_ftrace_function_probes(tr);
> > > event_trace_del_tracer(tr);
> > > ftrace_clear_pids(tr);
> > > ftrace_destroy_function_files(tr);
> > > diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> > > index 291a1bca5748..98e0845f7235 100644
> > > --- a/kernel/trace/trace.h
> > > +++ b/kernel/trace/trace.h
> > > @@ -980,6 +980,7 @@ register_ftrace_function_probe(char *glob, struct trace_array *tr,
> > > extern int
> > > unregister_ftrace_function_probe_func(char *glob, struct trace_array *tr,
> > > struct ftrace_probe_ops *ops);
> > > +extern void clear_ftrace_function_probes(struct trace_array *tr);
> >
> > This needs to have a stub function when CONFIG_DYNAMIC_FTRACE is not
> > defined. Otherwise we have:
> >
> > kernel/trace/trace.c:7553:2: error: implicit declaration of function 'clear_ftrace_function_probes' [-Werror=implicit-function-declaration]
> > clear_ftrace_function_probes(tr);
> > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
The prototype in trace.h is actually guarded by:
#if defined(CONFIG_FUNCTION_TRACER) && defined(CONFIG_DYNAMIC_FTRACE)
So, I will guard the call to clear_ftrace_function_probes() in trace.c
with the same.
- Naveen
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4] ftrace/instances: Clear function triggers when removing instances
2017-05-16 17:44 ` Naveen N. Rao
@ 2017-05-16 18:33 ` Steven Rostedt
2017-06-06 17:06 ` Shuah Khan
0 siblings, 1 reply; 18+ messages in thread
From: Steven Rostedt @ 2017-05-16 18:33 UTC (permalink / raw)
To: Naveen N. Rao; +Cc: Shuah Khan, Michael Ellerman, linux-kernel, linux-kselftest
On Tue, 16 May 2017 23:14:23 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
> On 2017/05/16 07:31PM, Naveen N. Rao wrote:
> > On 2017/05/15 10:20PM, Steven Rostedt wrote:
> > > On Sun, 14 May 2017 01:01:02 +0530
> > > "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
> > >
> >
> > [snip]
> >
> > > > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > > > index c4536c449021..3f2aed4ad1ed 100644
> > > > --- a/kernel/trace/trace.c
> > > > +++ b/kernel/trace/trace.c
> > > > @@ -7550,6 +7550,7 @@ static int instance_rmdir(const char *name)
> > > > }
> > > >
> > > > tracing_set_nop(tr);
> > > > + clear_ftrace_function_probes(tr);
> > > > event_trace_del_tracer(tr);
> > > > ftrace_clear_pids(tr);
> > > > ftrace_destroy_function_files(tr);
> > > > diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> > > > index 291a1bca5748..98e0845f7235 100644
> > > > --- a/kernel/trace/trace.h
> > > > +++ b/kernel/trace/trace.h
> > > > @@ -980,6 +980,7 @@ register_ftrace_function_probe(char *glob, struct trace_array *tr,
> > > > extern int
> > > > unregister_ftrace_function_probe_func(char *glob, struct trace_array *tr,
> > > > struct ftrace_probe_ops *ops);
> > > > +extern void clear_ftrace_function_probes(struct trace_array *tr);
> > >
> > > This needs to have a stub function when CONFIG_DYNAMIC_FTRACE is not
> > > defined. Otherwise we have:
> > >
> > > kernel/trace/trace.c:7553:2: error: implicit declaration of function 'clear_ftrace_function_probes' [-Werror=implicit-function-declaration]
> > > clear_ftrace_function_probes(tr);
> > > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> The prototype in trace.h is actually guarded by:
> #if defined(CONFIG_FUNCTION_TRACER) && defined(CONFIG_DYNAMIC_FTRACE)
>
> So, I will guard the call to clear_ftrace_function_probes() in trace.c
> with the same.
>
No, the proper thing to do is to make a stub function in the else part
of the #if that the prototype was declared in.
static inline void clear_ftrace_function_probes(struct trace_array *tr)
{
}
Please, lets limit the #ifdef ugliness from the code.
-- Steve
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4] ftrace/instances: Clear function triggers when removing instances
2017-05-16 18:33 ` Steven Rostedt
@ 2017-06-06 17:06 ` Shuah Khan
2017-06-06 17:37 ` Naveen N. Rao
0 siblings, 1 reply; 18+ messages in thread
From: Shuah Khan @ 2017-06-06 17:06 UTC (permalink / raw)
To: Steven Rostedt, Naveen N. Rao
Cc: Michael Ellerman, linux-kernel, linux-kselftest, Shuah Khan
On 05/16/2017 12:33 PM, Steven Rostedt wrote:
> On Tue, 16 May 2017 23:14:23 +0530
> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
>
>> On 2017/05/16 07:31PM, Naveen N. Rao wrote:
>>> On 2017/05/15 10:20PM, Steven Rostedt wrote:
>>>> On Sun, 14 May 2017 01:01:02 +0530
>>>> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
>>>>
>>>
>>> [snip]
>>>
>>>>> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
>>>>> index c4536c449021..3f2aed4ad1ed 100644
>>>>> --- a/kernel/trace/trace.c
>>>>> +++ b/kernel/trace/trace.c
>>>>> @@ -7550,6 +7550,7 @@ static int instance_rmdir(const char *name)
>>>>> }
>>>>>
>>>>> tracing_set_nop(tr);
>>>>> + clear_ftrace_function_probes(tr);
>>>>> event_trace_del_tracer(tr);
>>>>> ftrace_clear_pids(tr);
>>>>> ftrace_destroy_function_files(tr);
>>>>> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
>>>>> index 291a1bca5748..98e0845f7235 100644
>>>>> --- a/kernel/trace/trace.h
>>>>> +++ b/kernel/trace/trace.h
>>>>> @@ -980,6 +980,7 @@ register_ftrace_function_probe(char *glob, struct trace_array *tr,
>>>>> extern int
>>>>> unregister_ftrace_function_probe_func(char *glob, struct trace_array *tr,
>>>>> struct ftrace_probe_ops *ops);
>>>>> +extern void clear_ftrace_function_probes(struct trace_array *tr);
>>>>
>>>> This needs to have a stub function when CONFIG_DYNAMIC_FTRACE is not
>>>> defined. Otherwise we have:
>>>>
>>>> kernel/trace/trace.c:7553:2: error: implicit declaration of function 'clear_ftrace_function_probes' [-Werror=implicit-function-declaration]
>>>> clear_ftrace_function_probes(tr);
>>>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> The prototype in trace.h is actually guarded by:
>> #if defined(CONFIG_FUNCTION_TRACER) && defined(CONFIG_DYNAMIC_FTRACE)
>>
>> So, I will guard the call to clear_ftrace_function_probes() in trace.c
>> with the same.
>>
>
> No, the proper thing to do is to make a stub function in the else part
> of the #if that the prototype was declared in.
>
> static inline void clear_ftrace_function_probes(struct trace_array *tr)
> {
> }
>
> Please, lets limit the #ifdef ugliness from the code.
>
> -- Steve
>
>
Hi Steve,
Are you good with this patch series. I am looking to see which
of these should go into 4.13-rc1 - please ack individual patches
that should be pulled into 4.13-rc1
thanks,
-- Shuah
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4] ftrace/instances: Clear function triggers when removing instances
2017-06-06 17:06 ` Shuah Khan
@ 2017-06-06 17:37 ` Naveen N. Rao
0 siblings, 0 replies; 18+ messages in thread
From: Naveen N. Rao @ 2017-06-06 17:37 UTC (permalink / raw)
To: Shuah Khan
Cc: Steven Rostedt, Michael Ellerman, linux-kernel, linux-kselftest
On 2017/06/06 11:06AM, Shuah Khan wrote:
> On 05/16/2017 12:33 PM, Steven Rostedt wrote:
> > On Tue, 16 May 2017 23:14:23 +0530
> > "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
> >
> >> On 2017/05/16 07:31PM, Naveen N. Rao wrote:
> >>> On 2017/05/15 10:20PM, Steven Rostedt wrote:
> >>>> On Sun, 14 May 2017 01:01:02 +0530
> >>>> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
> >>>>
> >>>
> >>> [snip]
> >>>
> >>>>> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> >>>>> index c4536c449021..3f2aed4ad1ed 100644
> >>>>> --- a/kernel/trace/trace.c
> >>>>> +++ b/kernel/trace/trace.c
> >>>>> @@ -7550,6 +7550,7 @@ static int instance_rmdir(const char *name)
> >>>>> }
> >>>>>
> >>>>> tracing_set_nop(tr);
> >>>>> + clear_ftrace_function_probes(tr);
> >>>>> event_trace_del_tracer(tr);
> >>>>> ftrace_clear_pids(tr);
> >>>>> ftrace_destroy_function_files(tr);
> >>>>> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> >>>>> index 291a1bca5748..98e0845f7235 100644
> >>>>> --- a/kernel/trace/trace.h
> >>>>> +++ b/kernel/trace/trace.h
> >>>>> @@ -980,6 +980,7 @@ register_ftrace_function_probe(char *glob, struct trace_array *tr,
> >>>>> extern int
> >>>>> unregister_ftrace_function_probe_func(char *glob, struct trace_array *tr,
> >>>>> struct ftrace_probe_ops *ops);
> >>>>> +extern void clear_ftrace_function_probes(struct trace_array *tr);
> >>>>
> >>>> This needs to have a stub function when CONFIG_DYNAMIC_FTRACE is not
> >>>> defined. Otherwise we have:
> >>>>
> >>>> kernel/trace/trace.c:7553:2: error: implicit declaration of function 'clear_ftrace_function_probes' [-Werror=implicit-function-declaration]
> >>>> clear_ftrace_function_probes(tr);
> >>>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >>
> >> The prototype in trace.h is actually guarded by:
> >> #if defined(CONFIG_FUNCTION_TRACER) && defined(CONFIG_DYNAMIC_FTRACE)
> >>
> >> So, I will guard the call to clear_ftrace_function_probes() in trace.c
> >> with the same.
> >>
> >
> > No, the proper thing to do is to make a stub function in the else part
> > of the #if that the prototype was declared in.
> >
> > static inline void clear_ftrace_function_probes(struct trace_array *tr)
> > {
> > }
> >
> > Please, lets limit the #ifdef ugliness from the code.
> >
> > -- Steve
> >
> >
>
>
> Hi Steve,
>
> Are you good with this patch series. I am looking to see which
> of these should go into 4.13-rc1 - please ack individual patches
> that should be pulled into 4.13-rc1
Hi Shuah,
I see that Steven has already sent v2 of this series to Linus, including
the selftest patches:
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1399833.html
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1399832.html
And I also noticed (after I came back from vacation) that Steven had
fixed the #ifdef ugliness I had introduced..
Thanks,
Naveen
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2017-06-06 17:38 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-13 19:31 [PATCH 0/4] ftrace: Fix a few issues Naveen N. Rao
2017-05-13 19:31 ` [PATCH 1/4] ftrace: Simplify glob handling in unregister_ftrace_function_probe_func() Naveen N. Rao
2017-05-15 17:22 ` Steven Rostedt
2017-05-16 8:07 ` Naveen N. Rao
2017-05-13 19:31 ` [PATCH 2/4] ftrace/instances: Clear function triggers when removing instances Naveen N. Rao
2017-05-15 15:21 ` Steven Rostedt
2017-05-15 16:11 ` Steven Rostedt
2017-05-16 16:11 ` Masami Hiramatsu
2017-05-16 2:20 ` Steven Rostedt
2017-05-16 14:01 ` Naveen N. Rao
2017-05-16 17:44 ` Naveen N. Rao
2017-05-16 18:33 ` Steven Rostedt
2017-06-06 17:06 ` Shuah Khan
2017-06-06 17:37 ` Naveen N. Rao
2017-05-13 19:31 ` [PATCH 3/4] selftests/ftrace: Fix bashisms Naveen N. Rao
2017-05-15 15:16 ` Steven Rostedt
2017-05-16 14:25 ` Masami Hiramatsu
2017-05-13 19:31 ` [PATCH 4/4] selftests/ftrace: Add test to remove instance with active event triggers Naveen N. Rao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).