Messages in this thread | | | Date | Mon, 11 Mar 2013 23:54:00 +0800 | Subject | Re: [PATCH 07/13] tracing/kdb: remove redundant checking | From | Jovi Zhang <> |
| |
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/
| |