From: Jungseok Lee <jungseoklee85@gmail.com> To: Steven Rostedt <rostedt@goodmis.org> Cc: AKASHI Takahiro <takahiro.akashi@linaro.org>, catalin.marinas@arm.com, will.deacon@arm.com, olof@lixom.net, broonie@kernel.org, david.griego@linaro.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [RFC 2/3] arm64: refactor save_stack_trace() Date: Fri, 17 Jul 2015 00:01:25 +0900 [thread overview] Message-ID: <12F47692-3010-4886-B87D-3D7820609177@gmail.com> (raw) In-Reply-To: <20150716102405.2cc8c406@gandalf.local.home> On Jul 16, 2015, at 11:24 PM, Steven Rostedt wrote: Hi, Steve > On Thu, 16 Jul 2015 22:29:05 +0900 > Jungseok Lee <jungseoklee85@gmail.com> wrote: [ snip ] >> The data looks odd in two points. >> 1) the number of entry >> There is a mismatch between start token and real data > > Yep, good catch. As soon as I read that, I realized exactly what the > issue was ;-) > >> >> 2) 80-byte gap >> stack_max_size is not aligned with "Depth" field of the first entry of stack_trace. >> >> IMHO, two items are not considered in this series as digging them out. >> >> 1) skipped entries >> As x variable is introduced in Steve's patch, it is needed to track down >> how many entries are recorded in both stack_dump_trace and stack_dump_index. > > Yep. > >> >> 2) max_stack_trace.skip value >> max_stack_trace.skip is 0 as applying Steve's patch. The above gap could be >> observed unless the value is not considered in arch code. In the above example, >> 80-byte gap is save_stack_trace function in arch/arm64/kernel/stacktrace.c. >> >> As applying the following fix, stack_trace and stack_max_size are okay. >> However, I'm not sure which code, arch or generic ftrace, should handle trace->skip. >> The latter one is responsible for it under current implementation, not Steve's change. >> >> Please correct me if I am wrong. > > No, it's a bug in my patch. I'll make an update. > > Does this new patch fix it for you? I've gathered stack tracer data with your update. 1) stack_trace Depth Size Location (35 entries) ----- ---- -------- 0) 4424 16 put_cpu_partial+0x28/0x1d0 1) 4408 80 get_partial_node.isra.64+0x13c/0x344 2) 4328 256 __slab_alloc.isra.65.constprop.67+0xd8/0x37c 3) 4072 32 kmem_cache_alloc+0x258/0x294 4) 4040 304 __alloc_skb+0x48/0x180 5) 3736 96 alloc_skb_with_frags+0x74/0x234 6) 3640 112 sock_alloc_send_pskb+0x1d0/0x294 7) 3528 160 sock_alloc_send_skb+0x44/0x54 8) 3368 64 __ip_append_data.isra.40+0x78c/0xb48 9) 3304 224 ip_append_data.part.42+0x98/0xe8 10) 3080 112 ip_append_data+0x68/0x7c 11) 2968 96 icmp_push_reply+0x7c/0x144 12) 2872 96 icmp_send+0x3c0/0x3c8 13) 2776 192 __udp4_lib_rcv+0x5b8/0x684 14) 2584 96 udp_rcv+0x2c/0x3c 15) 2488 32 ip_local_deliver+0xa0/0x224 16) 2456 48 ip_rcv+0x360/0x57c 17) 2408 64 __netif_receive_skb_core+0x4d0/0x80c 18) 2344 128 __netif_receive_skb+0x24/0x84 19) 2216 32 process_backlog+0x9c/0x15c 20) 2184 80 net_rx_action+0x1ec/0x32c 21) 2104 160 __do_softirq+0x114/0x2f0 22) 1944 128 do_softirq+0x60/0x68 23) 1816 32 __local_bh_enable_ip+0xb0/0xd4 24) 1784 32 ip_finish_output+0x1f4/0xabc 25) 1752 96 ip_output+0xf0/0x120 26) 1656 64 ip_local_out_sk+0x44/0x54 27) 1592 32 ip_send_skb+0x24/0xbc 28) 1560 48 udp_send_skb+0x1b4/0x2f4 29) 1512 80 udp_sendmsg+0x2a8/0x7a0 30) 1432 272 inet_sendmsg+0xa0/0xd0 31) 1160 48 sock_sendmsg+0x30/0x78 32) 1112 32 ___sys_sendmsg+0x15c/0x26c 33) 1080 400 __sys_sendmmsg+0x94/0x180 34) 680 320 SyS_sendmmsg+0x38/0x54 35) 360 360 el0_svc_naked+0x20/0x28 2) stack_max_size 4504 In case of the number of entries, the following diff might be needed as I suggested in the previous reply. ;) ----8<---- @@ -330,7 +333,7 @@ static int t_show(struct seq_file *m, void *v) seq_printf(m, " Depth Size Location" " (%d entries)\n" " ----- ---- --------\n", - max_stack_trace.nr_entries - 1); + max_stack_trace.nr_entries); if (!stack_tracer_enabled && !max_stack_size) print_disabled(m); ----8<---- However, 80-byte gap still appears. Since max_stack_trace.skip is 3 in your update, save_stack_trace in arm64 should be refactored to align with this value. max_stack_trace.skip should be set to 4 if AKASHI's [RFC 2/3] patch is merged. However, arch code is supposed to follow generic framework's rule in this case. Isn't it? Best Regards Jungseok Lee
WARNING: multiple messages have this Message-ID (diff)
From: jungseoklee85@gmail.com (Jungseok Lee) To: linux-arm-kernel@lists.infradead.org Subject: [RFC 2/3] arm64: refactor save_stack_trace() Date: Fri, 17 Jul 2015 00:01:25 +0900 [thread overview] Message-ID: <12F47692-3010-4886-B87D-3D7820609177@gmail.com> (raw) In-Reply-To: <20150716102405.2cc8c406@gandalf.local.home> On Jul 16, 2015, at 11:24 PM, Steven Rostedt wrote: Hi, Steve > On Thu, 16 Jul 2015 22:29:05 +0900 > Jungseok Lee <jungseoklee85@gmail.com> wrote: [ snip ] >> The data looks odd in two points. >> 1) the number of entry >> There is a mismatch between start token and real data > > Yep, good catch. As soon as I read that, I realized exactly what the > issue was ;-) > >> >> 2) 80-byte gap >> stack_max_size is not aligned with "Depth" field of the first entry of stack_trace. >> >> IMHO, two items are not considered in this series as digging them out. >> >> 1) skipped entries >> As x variable is introduced in Steve's patch, it is needed to track down >> how many entries are recorded in both stack_dump_trace and stack_dump_index. > > Yep. > >> >> 2) max_stack_trace.skip value >> max_stack_trace.skip is 0 as applying Steve's patch. The above gap could be >> observed unless the value is not considered in arch code. In the above example, >> 80-byte gap is save_stack_trace function in arch/arm64/kernel/stacktrace.c. >> >> As applying the following fix, stack_trace and stack_max_size are okay. >> However, I'm not sure which code, arch or generic ftrace, should handle trace->skip. >> The latter one is responsible for it under current implementation, not Steve's change. >> >> Please correct me if I am wrong. > > No, it's a bug in my patch. I'll make an update. > > Does this new patch fix it for you? I've gathered stack tracer data with your update. 1) stack_trace Depth Size Location (35 entries) ----- ---- -------- 0) 4424 16 put_cpu_partial+0x28/0x1d0 1) 4408 80 get_partial_node.isra.64+0x13c/0x344 2) 4328 256 __slab_alloc.isra.65.constprop.67+0xd8/0x37c 3) 4072 32 kmem_cache_alloc+0x258/0x294 4) 4040 304 __alloc_skb+0x48/0x180 5) 3736 96 alloc_skb_with_frags+0x74/0x234 6) 3640 112 sock_alloc_send_pskb+0x1d0/0x294 7) 3528 160 sock_alloc_send_skb+0x44/0x54 8) 3368 64 __ip_append_data.isra.40+0x78c/0xb48 9) 3304 224 ip_append_data.part.42+0x98/0xe8 10) 3080 112 ip_append_data+0x68/0x7c 11) 2968 96 icmp_push_reply+0x7c/0x144 12) 2872 96 icmp_send+0x3c0/0x3c8 13) 2776 192 __udp4_lib_rcv+0x5b8/0x684 14) 2584 96 udp_rcv+0x2c/0x3c 15) 2488 32 ip_local_deliver+0xa0/0x224 16) 2456 48 ip_rcv+0x360/0x57c 17) 2408 64 __netif_receive_skb_core+0x4d0/0x80c 18) 2344 128 __netif_receive_skb+0x24/0x84 19) 2216 32 process_backlog+0x9c/0x15c 20) 2184 80 net_rx_action+0x1ec/0x32c 21) 2104 160 __do_softirq+0x114/0x2f0 22) 1944 128 do_softirq+0x60/0x68 23) 1816 32 __local_bh_enable_ip+0xb0/0xd4 24) 1784 32 ip_finish_output+0x1f4/0xabc 25) 1752 96 ip_output+0xf0/0x120 26) 1656 64 ip_local_out_sk+0x44/0x54 27) 1592 32 ip_send_skb+0x24/0xbc 28) 1560 48 udp_send_skb+0x1b4/0x2f4 29) 1512 80 udp_sendmsg+0x2a8/0x7a0 30) 1432 272 inet_sendmsg+0xa0/0xd0 31) 1160 48 sock_sendmsg+0x30/0x78 32) 1112 32 ___sys_sendmsg+0x15c/0x26c 33) 1080 400 __sys_sendmmsg+0x94/0x180 34) 680 320 SyS_sendmmsg+0x38/0x54 35) 360 360 el0_svc_naked+0x20/0x28 2) stack_max_size 4504 In case of the number of entries, the following diff might be needed as I suggested in the previous reply. ;) ----8<---- @@ -330,7 +333,7 @@ static int t_show(struct seq_file *m, void *v) seq_printf(m, " Depth Size Location" " (%d entries)\n" " ----- ---- --------\n", - max_stack_trace.nr_entries - 1); + max_stack_trace.nr_entries); if (!stack_tracer_enabled && !max_stack_size) print_disabled(m); ----8<---- However, 80-byte gap still appears. Since max_stack_trace.skip is 3 in your update, save_stack_trace in arm64 should be refactored to align with this value. max_stack_trace.skip should be set to 4 if AKASHI's [RFC 2/3] patch is merged. However, arch code is supposed to follow generic framework's rule in this case. Isn't it? Best Regards Jungseok Lee
next prev parent reply other threads:[~2015-07-16 15:01 UTC|newest] Thread overview: 106+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-07-13 5:29 [RFC 0/3] arm64: ftrace: fix incorrect output from stack tracer AKASHI Takahiro 2015-07-13 5:29 ` AKASHI Takahiro 2015-07-13 5:29 ` [RFC 1/3] ftrace: adjust a function's pc to search for in check_stack() for arm64 AKASHI Takahiro 2015-07-13 5:29 ` AKASHI Takahiro 2015-07-13 15:24 ` Steven Rostedt 2015-07-13 15:24 ` Steven Rostedt 2015-07-15 0:22 ` AKASHI Takahiro 2015-07-15 0:22 ` AKASHI Takahiro 2015-07-13 5:29 ` [RFC 2/3] arm64: refactor save_stack_trace() AKASHI Takahiro 2015-07-13 5:29 ` AKASHI Takahiro 2015-07-14 12:47 ` Jungseok Lee 2015-07-14 12:47 ` Jungseok Lee 2015-07-14 13:31 ` Steven Rostedt 2015-07-14 13:31 ` Steven Rostedt 2015-07-15 0:20 ` AKASHI Takahiro 2015-07-15 0:20 ` AKASHI Takahiro 2015-07-15 2:51 ` Steven Rostedt 2015-07-15 2:51 ` Steven Rostedt 2015-07-15 11:41 ` AKASHI Takahiro 2015-07-15 11:41 ` AKASHI Takahiro 2015-07-15 14:55 ` Steven Rostedt 2015-07-15 14:55 ` Steven Rostedt 2015-07-15 16:13 ` Steven Rostedt 2015-07-15 16:13 ` Steven Rostedt 2015-07-16 0:27 ` AKASHI Takahiro 2015-07-16 0:27 ` AKASHI Takahiro 2015-07-16 1:08 ` AKASHI Takahiro 2015-07-16 1:08 ` AKASHI Takahiro 2015-07-16 1:38 ` Steven Rostedt 2015-07-16 1:38 ` Steven Rostedt 2015-07-17 10:46 ` Will Deacon 2015-07-17 10:46 ` Will Deacon 2015-07-16 13:29 ` Jungseok Lee 2015-07-16 13:29 ` Jungseok Lee 2015-07-16 13:54 ` Jungseok Lee 2015-07-16 13:54 ` Jungseok Lee 2015-07-16 14:24 ` Steven Rostedt 2015-07-16 14:24 ` Steven Rostedt 2015-07-16 15:01 ` Jungseok Lee [this message] 2015-07-16 15:01 ` Jungseok Lee 2015-07-16 15:31 ` Steven Rostedt 2015-07-16 15:31 ` Steven Rostedt 2015-07-16 15:52 ` Jungseok Lee 2015-07-16 15:52 ` Jungseok Lee 2015-07-16 20:22 ` Steven Rostedt 2015-07-16 20:22 ` Steven Rostedt 2015-07-17 2:49 ` AKASHI Takahiro 2015-07-17 2:49 ` AKASHI Takahiro 2015-07-17 3:21 ` Steven Rostedt 2015-07-17 3:21 ` Steven Rostedt 2015-07-16 16:16 ` Steven Rostedt 2015-07-16 16:16 ` Steven Rostedt 2015-07-17 12:40 ` Mark Rutland 2015-07-17 12:40 ` Mark Rutland 2015-07-17 12:51 ` Steven Rostedt 2015-07-17 12:51 ` Steven Rostedt 2015-07-17 13:00 ` Steven Rostedt 2015-07-17 13:00 ` Steven Rostedt 2015-07-17 14:28 ` Jungseok Lee 2015-07-17 14:28 ` Jungseok Lee 2015-07-17 14:41 ` Steven Rostedt 2015-07-17 14:41 ` Steven Rostedt 2015-07-17 14:59 ` Jungseok Lee 2015-07-17 14:59 ` Jungseok Lee 2015-07-17 15:34 ` Jungseok Lee 2015-07-17 15:34 ` Jungseok Lee 2015-07-17 16:01 ` Steven Rostedt 2015-07-17 16:01 ` Steven Rostedt 2015-07-20 16:20 ` Will Deacon 2015-07-20 16:20 ` Will Deacon 2015-07-20 23:53 ` AKASHI Takahiro 2015-07-20 23:53 ` AKASHI Takahiro 2015-07-21 10:26 ` AKASHI Takahiro 2015-07-21 10:26 ` AKASHI Takahiro 2015-07-21 14:34 ` Jungseok Lee 2015-07-21 14:34 ` Jungseok Lee 2015-08-03 9:09 ` Will Deacon 2015-08-03 9:09 ` Will Deacon 2015-08-03 14:01 ` Steven Rostedt 2015-08-03 14:01 ` Steven Rostedt 2015-08-03 14:04 ` Will Deacon 2015-08-03 14:04 ` Will Deacon 2015-08-03 16:30 ` Jungseok Lee 2015-08-03 16:30 ` Jungseok Lee 2015-08-03 16:57 ` Steven Rostedt 2015-08-03 16:57 ` Steven Rostedt 2015-08-03 17:22 ` Jungseok Lee 2015-08-03 17:22 ` Jungseok Lee 2015-08-03 17:32 ` Steven Rostedt 2015-08-03 17:32 ` Steven Rostedt 2015-08-04 7:41 ` AKASHI Takahiro 2015-08-04 7:41 ` AKASHI Takahiro 2015-07-17 2:04 ` AKASHI Takahiro 2015-07-17 2:04 ` AKASHI Takahiro 2015-07-17 14:38 ` Jungseok Lee 2015-07-17 14:38 ` Jungseok Lee 2015-07-16 14:28 ` Mark Rutland 2015-07-16 14:28 ` Mark Rutland 2015-07-16 14:34 ` Steven Rostedt 2015-07-16 14:34 ` Steven Rostedt 2015-07-17 2:09 ` AKASHI Takahiro 2015-07-17 2:09 ` AKASHI Takahiro 2015-07-13 5:29 ` [RFC 3/3] arm64: ftrace: mcount() should not create a stack frame AKASHI Takahiro 2015-07-13 5:29 ` AKASHI Takahiro 2015-07-13 15:01 ` [RFC 0/3] arm64: ftrace: fix incorrect output from stack tracer Jungseok Lee 2015-07-13 15:01 ` Jungseok Lee
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=12F47692-3010-4886-B87D-3D7820609177@gmail.com \ --to=jungseoklee85@gmail.com \ --cc=broonie@kernel.org \ --cc=catalin.marinas@arm.com \ --cc=david.griego@linaro.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=olof@lixom.net \ --cc=rostedt@goodmis.org \ --cc=takahiro.akashi@linaro.org \ --cc=will.deacon@arm.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.