* [PATCH 07/13] tracing/kdb: remove redundant checking @ 2013-03-11 7:13 zhangwei(Jovi) 2013-03-11 14:09 ` Steven Rostedt 0 siblings, 1 reply; 4+ messages in thread From: zhangwei(Jovi) @ 2013-03-11 7:13 UTC (permalink / raw) To: linux-kernel@vger.kernel.org, Steven Rostedt, Frederic Weisbecker, Ingo Molnar trace_empty is checking in while-loop, so the previous checking is totally redundant, and more worse, the first trace entry is losted. so remove it. Signed-off-by: zhangwei(Jovi) <jovi.zhangwei@huawei.com> --- kernel/trace/trace_kdb.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/kernel/trace/trace_kdb.c b/kernel/trace/trace_kdb.c index 3c5c5df..6489b2e 100644 --- a/kernel/trace/trace_kdb.c +++ b/kernel/trace/trace_kdb.c @@ -57,8 +57,7 @@ static void ftrace_dump_buf(int skip_lines, long cpu_file) ring_buffer_read_start(iter.buffer_iter[cpu_file]); tracing_iter_reset(&iter, cpu_file); } - if (!trace_empty(&iter)) - trace_find_next_entry_inc(&iter); + while (!trace_empty(&iter)) { if (!cnt) kdb_printf("---------------------------------\n"); -- 1.7.9.7 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 07/13] tracing/kdb: remove redundant checking 2013-03-11 7:13 [PATCH 07/13] tracing/kdb: remove redundant checking zhangwei(Jovi) @ 2013-03-11 14:09 ` Steven Rostedt 2013-03-11 14:46 ` Jason Wessel 0 siblings, 1 reply; 4+ messages in thread From: Steven Rostedt @ 2013-03-11 14:09 UTC (permalink / raw) To: zhangwei(Jovi), Jason Wessel Cc: linux-kernel@vger.kernel.org, Frederic Weisbecker, Ingo Molnar This is Jason's code. Jason, please give an Ack or Nack. Thanks, -- Steve On Mon, 2013-03-11 at 15:13 +0800, zhangwei(Jovi) wrote: > trace_empty is checking in while-loop, so the previous checking > is totally redundant, and more worse, the first trace entry is losted. > > so remove it. > > Signed-off-by: zhangwei(Jovi) <jovi.zhangwei@huawei.com> > --- > kernel/trace/trace_kdb.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/kernel/trace/trace_kdb.c b/kernel/trace/trace_kdb.c > index 3c5c5df..6489b2e 100644 > --- a/kernel/trace/trace_kdb.c > +++ b/kernel/trace/trace_kdb.c > @@ -57,8 +57,7 @@ static void ftrace_dump_buf(int skip_lines, long cpu_file) > ring_buffer_read_start(iter.buffer_iter[cpu_file]); > tracing_iter_reset(&iter, cpu_file); > } > - if (!trace_empty(&iter)) > - trace_find_next_entry_inc(&iter); > + > while (!trace_empty(&iter)) { > if (!cnt) > kdb_printf("---------------------------------\n"); ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 07/13] tracing/kdb: remove redundant checking 2013-03-11 14:09 ` Steven Rostedt @ 2013-03-11 14:46 ` Jason Wessel 2013-03-11 15:54 ` Jovi Zhang 0 siblings, 1 reply; 4+ messages in thread From: Jason Wessel @ 2013-03-11 14:46 UTC (permalink / raw) To: Steven Rostedt Cc: zhangwei(Jovi), linux-kernel@vger.kernel.org, Frederic Weisbecker, Ingo Molnar On 03/11/2013 09:09 AM, Steven Rostedt wrote: > This is Jason's code. > > Jason, please give an Ack or Nack. > > Thanks, > > -- Steve > > > On Mon, 2013-03-11 at 15:13 +0800, zhangwei(Jovi) wrote: >> trace_empty is checking in while-loop, so the previous checking >> is totally redundant, and more worse, the first trace entry is losted. >> >> so remove it. >> >> Signed-off-by: zhangwei(Jovi) <jovi.zhangwei@huawei.com> >> --- >> kernel/trace/trace_kdb.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/kernel/trace/trace_kdb.c b/kernel/trace/trace_kdb.c >> index 3c5c5df..6489b2e 100644 >> --- a/kernel/trace/trace_kdb.c >> +++ b/kernel/trace/trace_kdb.c >> @@ -57,8 +57,7 @@ static void ftrace_dump_buf(int skip_lines, long cpu_file) >> ring_buffer_read_start(iter.buffer_iter[cpu_file]); >> tracing_iter_reset(&iter, cpu_file); >> } >> - if (!trace_empty(&iter)) >> - trace_find_next_entry_inc(&iter); >> + >> while (!trace_empty(&iter)) { >> if (!cnt) >> kdb_printf("---------------------------------\n"); > This was just a copy of part of the logic used for printing the information in a similar manner to what you get when you cat the trace buffer to obtain the human readable version. May I ask how you tested it though? As far as I know the patch I sent Stephen quite a while ago got lost and the mainline version of the "ftdump" doesn't actually work. Example: [0]kdb> ftdump Dumping ftrace buffer: 3BUG: sleeping function called from invalid context at /space/jw/git/kgdb/linux-2.6-kgdb/mm/slub.c:925 3in_atomic(): 1, irqs_disabled(): 1, pid: 81, name: sh Pid: 81, comm: sh Not tainted 3.9.0-rc1-WR4.3.0.0_standard+ #568 Call Trace: <#DB> [<ffffffff81069ffe>] __might_sleep+0xde/0x100 [<ffffffff8112611b>] kmem_cache_alloc_trace+0xdb/0x170 [<ffffffff810c01bd>] ring_buffer_read_prepare+0x4d/0x70 [<ffffffff810d4d28>] kdb_ftdump+0x1e8/0x410 [<ffffffff810a9a89>] kdb_parse+0x209/0x690 [<ffffffff810aad0c>] kdb_main_loop+0x67c/0x8c0 [<ffffffff810ad4b3>] kdb_stub+0x1d3/0x420 [<ffffffff8104ccfd>] ? __send_signal+0x1ad/0x3e0 [<ffffffff810a33be>] kgdb_cpu_enter+0x27e/0x590 [<ffffffff810a3981>] kgdb_handle_exception+0x161/0x1c0 [<ffffffff81027cf1>] __kgdb_notify+0x31/0xe0 [<ffffffff81027e10>] kgdb_ll_trap+0x40/0x50 [<ffffffff81002e12>] do_int3+0x52/0x130 [<ffffffff81674345>] int3+0x25/0x40 [<ffffffff810a2be2>] ? sysrq_handle_dbg+0x32/0x60 <<EOE>> [<ffffffff813e1e69>] __handle_sysrq+0x129/0x190 I think we need to actually empirically prove the code change is right or wrong before merging it, as well as cleaning up the change log slightly. I'll go hunt down the patch the fixes the oops first. Cheers, Jason. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 07/13] tracing/kdb: remove redundant checking 2013-03-11 14:46 ` Jason Wessel @ 2013-03-11 15:54 ` Jovi Zhang 0 siblings, 0 replies; 4+ messages in thread From: Jovi Zhang @ 2013-03-11 15:54 UTC (permalink / raw) To: Jason Wessel Cc: Steven Rostedt, zhangwei(Jovi), linux-kernel@vger.kernel.org, Frederic Weisbecker, Ingo Molnar On Mon, Mar 11, 2013 at 10:46 PM, Jason Wessel <jason.wessel@windriver.com> wrote: > On 03/11/2013 09:09 AM, Steven Rostedt wrote: >> This is Jason's code. >> >> Jason, please give an Ack or Nack. >> >> Thanks, >> >> -- Steve >> >> >> On Mon, 2013-03-11 at 15:13 +0800, zhangwei(Jovi) wrote: >>> trace_empty is checking in while-loop, so the previous checking >>> is totally redundant, and more worse, the first trace entry is losted. >>> >>> so remove it. >>> >>> Signed-off-by: zhangwei(Jovi) <jovi.zhangwei@huawei.com> >>> --- >>> kernel/trace/trace_kdb.c | 3 +-- >>> 1 file changed, 1 insertion(+), 2 deletions(-) >>> >>> diff --git a/kernel/trace/trace_kdb.c b/kernel/trace/trace_kdb.c >>> index 3c5c5df..6489b2e 100644 >>> --- a/kernel/trace/trace_kdb.c >>> +++ b/kernel/trace/trace_kdb.c >>> @@ -57,8 +57,7 @@ static void ftrace_dump_buf(int skip_lines, long cpu_file) >>> ring_buffer_read_start(iter.buffer_iter[cpu_file]); >>> tracing_iter_reset(&iter, cpu_file); >>> } >>> - if (!trace_empty(&iter)) >>> - trace_find_next_entry_inc(&iter); >>> + >>> while (!trace_empty(&iter)) { >>> if (!cnt) >>> kdb_printf("---------------------------------\n"); >> > > > This was just a copy of part of the logic used for printing the information in a similar manner to what you get when you cat the trace buffer to obtain the human readable version. > > May I ask how you tested it though? No, just watch the code, and that part looks weird. kdb_ftdump seems copied from ftrace_dump function, it have similar functionality, can we have some way to unify these two function into one common function? this at least can save trace_iterator static memory ~8k+. (I'm not sure this can work or not) > > As far as I know the patch I sent Stephen quite a while ago got lost and the mainline version of the "ftdump" doesn't actually work. > > Example: > [0]kdb> ftdump > Dumping ftrace buffer: > 3BUG: sleeping function called from invalid context at /space/jw/git/kgdb/linux-2.6-kgdb/mm/slub.c:925 > 3in_atomic(): 1, irqs_disabled(): 1, pid: 81, name: sh > Pid: 81, comm: sh Not tainted 3.9.0-rc1-WR4.3.0.0_standard+ #568 > Call Trace: > <#DB> [<ffffffff81069ffe>] __might_sleep+0xde/0x100 > [<ffffffff8112611b>] kmem_cache_alloc_trace+0xdb/0x170 > [<ffffffff810c01bd>] ring_buffer_read_prepare+0x4d/0x70 > [<ffffffff810d4d28>] kdb_ftdump+0x1e8/0x410 > [<ffffffff810a9a89>] kdb_parse+0x209/0x690 > [<ffffffff810aad0c>] kdb_main_loop+0x67c/0x8c0 > [<ffffffff810ad4b3>] kdb_stub+0x1d3/0x420 > [<ffffffff8104ccfd>] ? __send_signal+0x1ad/0x3e0 > [<ffffffff810a33be>] kgdb_cpu_enter+0x27e/0x590 > [<ffffffff810a3981>] kgdb_handle_exception+0x161/0x1c0 > [<ffffffff81027cf1>] __kgdb_notify+0x31/0xe0 > [<ffffffff81027e10>] kgdb_ll_trap+0x40/0x50 > [<ffffffff81002e12>] do_int3+0x52/0x130 > [<ffffffff81674345>] int3+0x25/0x40 > [<ffffffff810a2be2>] ? sysrq_handle_dbg+0x32/0x60 > <<EOE>> [<ffffffff813e1e69>] __handle_sysrq+0x129/0x190 > > > I think we need to actually empirically prove the code change is right or wrong before merging it, as well as cleaning up the change log slightly. > > I'll go hunt down the patch the fixes the oops first. > > Cheers, > Jason. > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-03-11 15:54 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-03-11 7:13 [PATCH 07/13] tracing/kdb: remove redundant checking zhangwei(Jovi) 2013-03-11 14:09 ` Steven Rostedt 2013-03-11 14:46 ` Jason Wessel 2013-03-11 15:54 ` Jovi Zhang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox