ftrace: Fix a few issues

PATCH Superseded Naveen N. Rao 5 participants 18 messages May 13, 2017 – Jun 6, 2017

Subsystems (3)

Kselftest Tracing ftrace

lore

Share & tools

lore

Naveen N. Rao

author

2017-05-13 19:32 UTC #0 lore raw

cover ftrace: Fix a few issues

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(-)

Signature · 1 line

2.12.2

Naveen N. Rao

author

2017-05-13 19:33 UTC #1 lore raw

PATCH 1/4 ftrace: Simplify glob handling in unregister_ftrace_function_probe_func()

Handle a NULL glob properly.

Signed-off-by Naveen N. Rao <[email protected]>

---
 kernel/trace/ftrace.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

kernel/trace/ftrace.c +2 -2

index 39dca4e86a94..28dc824ad072 100644
@@ -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),

Signature · 1 line

2.12.2

Steven Rostedt

maintainer

2017-05-15 17:22 UTC #2 lore raw

Re: [PATCH 1/4] ftrace: Simplify glob handling in unregister_ftrace_function_probe_func()

Quoted 12 lines · On Sun, 14 May 2017 01:01:01 +0530 "Naveen N. Rao"…

Handle a NULL glob properly.
Signed-off-by Naveen N. Rao <[email protected]>
---
 kernel/trace/ftrace.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
kernel/trace/ftrace.c +0 -0
index 39dca4e86a94..28dc824ad072 100644

kernel/trace/ftrace.c unregister_ftrace_function_probe_func(char *glob, struct trace_array *tr, · line 4147

Quoted 5 lines

kernel/trace/ftrace.c +1 -1

@@ -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

Quoted 6 lines

kernel/trace/ftrace.c +1 -1

 		func_g.search = NULL;
-	else if (glob) {
+	else {
 		int not;
 
 		func_g.type = filter_parse_regex(glob, strlen(glob),

Naveen N. Rao

author

2017-05-16 08:08 UTC #3 lore raw

Re: [PATCH 1/4] ftrace: Simplify glob handling in unregister_ftrace_function_probe_func()

Quoted 18 lines · Steven Rostedt

Quoted 15 lines · On Sun, 14 May 2017 01:01:01 +0530 "Naveen N. Rao"…
Handle a NULL glob properly.
Signed-off-by Naveen N. Rao <[email protected]>
---
 kernel/trace/ftrace.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
kernel/trace/ftrace.c +0 -0
index 39dca4e86a94..28dc824ad072 100644
@@ -4144,9 +4144,9 @@ unregister_ftrace_function_probe_func(char *glob, struct trace_array *tr,
 	int i, ret = -ENODEV;
 	int size;
 

Quoted 8 lines

kernel/trace/ftrace.c unregister_ftrace_function_probe_func(char *glob, struct trace_array *tr, · line 4147
Quoted 2 lines
kernel/trace/ftrace.c +1 -1
-	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

Naveen N. Rao

author

2017-05-13 19:32 UTC #4 lore raw

PATCH 2/4 ftrace/instances: Clear function triggers when removing instances

In reply to Naveen N. Rao

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 <[email protected]>

Signed-off-by Naveen N. Rao <[email protected]>

---
 kernel/trace/ftrace.c | 8 ++++++++
 kernel/trace/trace.c  | 1 +
 kernel/trace/trace.h  | 1 +
 3 files changed, 10 insertions(+)

kernel/trace/ftrace.c +8 -0

index 28dc824ad072..1b96d927a082 100644
@@ -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);

kernel/trace/trace.c +1 -0

index c4536c449021..3f2aed4ad1ed 100644
@@ -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);

kernel/trace/trace.h +1 -0

index 291a1bca5748..98e0845f7235 100644
@@ -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);

Signature · 1 line

2.12.2

Steven Rostedt

maintainer

2017-05-15 15:21 UTC #5 lore raw

Re: [PATCH 2/4] ftrace/instances: Clear function triggers when removing instances

Quoted 14 lines · On Sun, 14 May 2017 01:01:02 +0530 "Naveen N. Rao"…

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

Steven Rostedt

maintainer

2017-05-15 16:11 UTC #6 lore raw

Re: [PATCH 2/4] ftrace/instances: Clear function triggers when removing instances

Quoted 18 lines · On Mon, 15 May 2017 11:21:15 -0400 Steven Rostedt <[email protected]> wrote:

Quoted 14 lines · On Sun, 14 May 2017 01:01:02 +0530 "Naveen N. Rao"…

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

Quoted 4 lines

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

Masami Hiramatsu

maintainer

2017-05-16 16:11 UTC #7 lore raw

Re: [PATCH 2/4] ftrace/instances: Clear function triggers when removing instances

Quoted 21 lines · On Mon, 15 May 2017 12:11:03 -0400 Steven Rostedt <[email protected]> wrote:

Quoted 18 lines · On Mon, 15 May 2017 11:21:15 -0400 Steven Rostedt <[email protected]> wrote:
Quoted 14 lines · On Sun, 14 May 2017 01:01:02 +0530 "Naveen N. Rao"…
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  

Quoted 8 lines

Quoted 4 lines
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,

Quoted 3 lines

-- Steve

Signature · 1 line

Masami Hiramatsu <[email protected]>

Steven Rostedt

maintainer

2017-05-16 02:20 UTC #8 lore raw

Re: [PATCH 2/4] ftrace/instances: Clear function triggers when removing instances

In reply to Naveen N. Rao

Quoted 99 lines · On Sun, 14 May 2017 01:01:02 +0530 "Naveen N. Rao"…

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 <[email protected]>

Signed-off-by Naveen N. Rao <[email protected]>

---
 kernel/trace/ftrace.c | 8 ++++++++
 kernel/trace/trace.c  | 1 +
 kernel/trace/trace.h  | 1 +
 3 files changed, 10 insertions(+)

kernel/trace/ftrace.c +8 -0

index 28dc824ad072..1b96d927a082 100644
@@ -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);

kernel/trace/trace.c +1 -0

index c4536c449021..3f2aed4ad1ed 100644
@@ -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);

kernel/trace/trace.h +0 -0

index 291a1bca5748..98e0845f7235 100644

kernel/trace/trace.h register_ftrace_function_probe(char *glob, struct trace_array *tr, · line 983

Quoted 4 lines

kernel/trace/trace.h +1 -0

@@ -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

Quoted 3 lines

kernel/trace/trace.h +0 -0

 int register_ftrace_command(struct ftrace_func_command *cmd);
 int unregister_ftrace_command(struct ftrace_func_command *cmd);

Naveen N. Rao

author

2017-05-16 14:02 UTC #9 lore raw

Re: [PATCH 2/4] ftrace/instances: Clear function triggers when removing instances

Quoted 3 lines · Steven Rostedt

On Sun, 14 May 2017 01:01:02 +0530
"Naveen N. Rao" <[email protected]> wrote:

[snip]

Quoted 25 lines

Quoted 17 lines

kernel/trace/trace.c +1 -0

index c4536c449021..3f2aed4ad1ed 100644
@@ -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);

kernel/trace/trace.h +0 -0

index 291a1bca5748..98e0845f7235 100644

kernel/trace/trace.h register_ftrace_function_probe(char *glob, struct trace_array *tr, · line 983

Quoted 4 lines

kernel/trace/trace.h +1 -0

@@ -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:

Quoted 3 lines

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

Naveen N. Rao

author

2017-05-16 17:45 UTC #10 lore raw

Re: [PATCH 2/4] ftrace/instances: Clear function triggers when removing instances

Quoted 32 lines · Naveen N. Rao

Quoted 3 lines · Steven Rostedt
On Sun, 14 May 2017 01:01:02 +0530
"Naveen N. Rao" <[email protected]> wrote:
[snip]
Quoted 25 lines
Quoted 17 lines
kernel/trace/trace.c +1 -0
index c4536c449021..3f2aed4ad1ed 100644
@@ -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);
kernel/trace/trace.h +0 -0
index 291a1bca5748..98e0845f7235 100644
kernel/trace/trace.h register_ftrace_function_probe(char *glob, struct trace_array *tr, · line 983
Quoted 4 lines
kernel/trace/trace.h +1 -0
@@ -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:

Quoted 3 lines

Quoted 3 lines
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

Steven Rostedt

maintainer

2017-05-16 18:34 UTC #11 lore raw

Re: [PATCH 2/4] ftrace/instances: Clear function triggers when removing instances

Quoted 37 lines · On Tue, 16 May 2017 23:14:23 +0530 "Naveen N. Rao"…

Quoted 32 lines · Naveen N. Rao
Quoted 3 lines · Steven Rostedt
On Sun, 14 May 2017 01:01:02 +0530
"Naveen N. Rao" <[email protected]> wrote:
[snip]
Quoted 25 lines
Quoted 17 lines
kernel/trace/trace.c +1 -0
index c4536c449021..3f2aed4ad1ed 100644
@@ -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);
kernel/trace/trace.h +0 -0
index 291a1bca5748..98e0845f7235 100644
kernel/trace/trace.h register_ftrace_function_probe(char *glob, struct trace_array *tr, · line 983
Quoted 4 lines
kernel/trace/trace.h +1 -0
@@ -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:
Quoted 3 lines
Quoted 3 lines
kernel/trace/trace.c:7553:2: error: implicit declaration of function 'clear_ftrace_function_probes' [-Werror=implicit-function-declaration]
clear_ftrace_function_probes(tr);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~

Quoted 6 lines

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

Shuah Khan

maintainer

2017-06-06 17:06 UTC #12 lore raw

Re: [PATCH 2/4] ftrace/instances: Clear function triggers when removing instances

Quoted 50 lines · Steven Rostedt

Quoted 37 lines · On Tue, 16 May 2017 23:14:23 +0530 "Naveen N. Rao"…
Quoted 32 lines · Naveen N. Rao
Quoted 3 lines · Steven Rostedt
On Sun, 14 May 2017 01:01:02 +0530
"Naveen N. Rao" <[email protected]> wrote:
[snip]
Quoted 25 lines
Quoted 17 lines
kernel/trace/trace.c +1 -0
index c4536c449021..3f2aed4ad1ed 100644
@@ -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);
kernel/trace/trace.h +0 -0
index 291a1bca5748..98e0845f7235 100644
kernel/trace/trace.h register_ftrace_function_probe(char *glob, struct trace_array *tr, · line 983
Quoted 4 lines
kernel/trace/trace.h +1 -0
@@ -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:
Quoted 3 lines
Quoted 3 lines
kernel/trace/trace.c:7553:2: error: implicit declaration of function 'clear_ftrace_function_probes' [-Werror=implicit-function-declaration]
clear_ftrace_function_probes(tr);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
Quoted 6 lines
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.

Quoted 9 lines

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

Naveen N. Rao

author

2017-06-06 17:38 UTC #13 lore raw

Re: [PATCH 2/4] ftrace/instances: Clear function triggers when removing instances

Quoted 62 lines · Shuah Khan

Quoted 50 lines · Steven Rostedt
Quoted 37 lines · On Tue, 16 May 2017 23:14:23 +0530 "Naveen N. Rao"…
Quoted 32 lines · Naveen N. Rao
Quoted 3 lines · Steven Rostedt
On Sun, 14 May 2017 01:01:02 +0530
"Naveen N. Rao" <[email protected]> wrote:
[snip]
Quoted 25 lines
Quoted 17 lines
kernel/trace/trace.c +1 -0
index c4536c449021..3f2aed4ad1ed 100644
@@ -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);
kernel/trace/trace.h +0 -0
index 291a1bca5748..98e0845f7235 100644
kernel/trace/trace.h register_ftrace_function_probe(char *glob, struct trace_array *tr, · line 983
Quoted 4 lines
kernel/trace/trace.h +1 -0
@@ -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:
Quoted 3 lines
Quoted 3 lines
kernel/trace/trace.c:7553:2: error: implicit declaration of function 'clear_ftrace_function_probes' [-Werror=implicit-function-declaration]
clear_ftrace_function_probes(tr);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
Quoted 6 lines
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.
Quoted 9 lines
static inline void clear_ftrace_function_probes(struct trace_array *tr) {
}
Please, lets limit the #ifdef ugliness from the code.
-- Steve

Quoted 5 lines

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/[email protected]/msg1399833.html
https://www.mail-archive.com/[email protected]/msg1399832.html

And I also noticed (after I came back from vacation) that Steven had fixed the #ifdef ugliness I had introduced..

Thanks,
Naveen

Naveen N. Rao

author

2017-05-13 19:32 UTC #14 lore raw

PATCH 3/4 selftests/ftrace: Fix bashisms

In reply to Naveen N. Rao

Fix a few bashisms in ftrace selftests.

Signed-off-by Naveen N. Rao <[email protected]>

---
 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(-)

tools/testing/selftests/ftrace/ftracetest +1 -1

index 32e6211e1c6e..717581145cfc 100755
@@ -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)

tools/testing/selftests/ftrace/test.d/ftrace/func_event_triggers.tc +1 -1

index 07bb3e5930b4..aa31368851c9 100644
@@ -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
 }

tools/testing/selftests/ftrace/test.d/functions +2 -2

index 9aec6fcb7729..f2019b37370d 100644
@@ -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

Signature · 1 line

2.12.2

Steven Rostedt

maintainer

2017-05-15 15:16 UTC #15 lore raw

Re: [PATCH 3/4] selftests/ftrace: Fix bashisms

Masami's the original author of ftracetest.

Masami, are you OK with this change?

-- Steve

Quoted 52 lines · On Sun, 14 May 2017 01:01:03 +0530 "Naveen N. Rao"…

Fix a few bashisms in ftrace selftests.

Signed-off-by Naveen N. Rao <[email protected]>

---
 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(-)

tools/testing/selftests/ftrace/ftracetest +1 -1

index 32e6211e1c6e..717581145cfc 100755
@@ -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)

tools/testing/selftests/ftrace/test.d/ftrace/func_event_triggers.tc +1 -1

index 07bb3e5930b4..aa31368851c9 100644
@@ -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
 }

tools/testing/selftests/ftrace/test.d/functions +2 -2

index 9aec6fcb7729..f2019b37370d 100644
@@ -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

Acked-by

2017-05-16 14:26 UTC #16 lore raw

Re: [PATCH 3/4] selftests/ftrace: Fix bashisms

Quoted 12 lines · On Mon, 15 May 2017 11:16:30 -0400 Steven Rostedt <[email protected]> wrote:

Masami's the original author of ftracetest.
Masami, are you OK with this change?
-- Steve
Quoted 1 line · On Sun, 14 May 2017 01:01:03 +0530 "Naveen N. Rao"…
Fix a few bashisms in ftrace selftests.

Ah, what's a nice fix!

Acked-by Masami Hiramatsu <[email protected]>

Thanks!

Quoted 52 lines

Quoted 51 lines

Signed-off-by Naveen N. Rao <[email protected]>

---
 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(-)

tools/testing/selftests/ftrace/ftracetest +1 -1

index 32e6211e1c6e..717581145cfc 100755
@@ -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)

tools/testing/selftests/ftrace/test.d/ftrace/func_event_triggers.tc +1 -1

index 07bb3e5930b4..aa31368851c9 100644
@@ -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
 }

tools/testing/selftests/ftrace/test.d/functions +2 -2

index 9aec6fcb7729..f2019b37370d 100644
@@ -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

Signature · 1 line

Masami Hiramatsu

Naveen N. Rao

author

2017-05-13 19:32 UTC #17 lore raw

PATCH 4/4 selftests/ftrace: Add test to remove instance with active event triggers

In reply to Naveen N. Rao

Add a test to ensure we clean up properly when removing an instance with active event triggers.

Signed-off-by Naveen N. Rao <[email protected]>

---
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(-)

tools/testing/selftests/ftrace/test.d/instances/instance-event.tc +6 -2

index 4c5a061a5b4e..c73db7863adb 100644
@@ -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() {

Signature · 1 line

2.12.2