Messages in this thread | | | From | Doug Anderson <> | Date | Fri, 15 Mar 2019 12:55:31 -0700 | Subject | Re: [PATCH v3] tracing: kdb: Allow ftdump to skip all but the last few lines |
| |
Hi,
On Fri, Mar 15, 2019 at 11:41 AM Steven Rostedt <rostedt@goodmis.org> wrote: > > On Fri, 15 Mar 2019 10:45:28 -0700 > Douglas Anderson <dianders@chromium.org> wrote: > > > > Changes in v3: > > - Optimize counting as per Steven Rostedt. > > - Down to 1 patch since patch #1 from v2 landed. > > > > kernel/trace/trace_kdb.c | 38 +++++++++++++++++++++++++++++--------- > > 1 file changed, 29 insertions(+), 9 deletions(-) > > > > diff --git a/kernel/trace/trace_kdb.c b/kernel/trace/trace_kdb.c > > index 810d78a8d14c..cc6ca6c0d6de 100644 > > --- a/kernel/trace/trace_kdb.c > > +++ b/kernel/trace/trace_kdb.c > > @@ -17,7 +17,7 @@ > > #include "trace.h" > > #include "trace_output.h" > > > > -static void ftrace_dump_buf(int skip_lines, long cpu_file) > > +static int ftrace_dump_buf(int skip_lines, long cpu_file, bool quiet) > > Not quite what I wanted you to do. > > > > { > > /* use static because iter can be a bit big for the stack */ > > static struct trace_iterator iter; > > @@ -39,8 +39,6 @@ static void ftrace_dump_buf(int skip_lines, long cpu_file) > > /* don't look at user memory in panic mode */ > > tr->trace_flags &= ~TRACE_ITER_SYM_USEROBJ; > > > > - kdb_printf("Dumping ftrace buffer:\n"); > > - > > /* reset all but tr, trace, and overruns */ > > memset(&iter.seq, 0, > > sizeof(struct trace_iterator) - > > @@ -55,6 +53,9 @@ static void ftrace_dump_buf(int skip_lines, long cpu_file) > > cpu, GFP_ATOMIC); > > ring_buffer_read_start(iter.buffer_iter[cpu]); > > tracing_iter_reset(&iter, cpu); > > + > > + cnt += > > + ring_buffer_entries_cpu(iter.trace_buffer->buffer, cpu); > > BTW, 80 char limit is a guideline not a rule. Don't break a line to > make it look worse. But that doesn't matter because this isn't the > change I meant.
Yeah, I didn't like the look of it either but decided to match the style of the function, which uses nearly the same solution for word wrapping a few lines above. :-) I'm happy to just break 80 characters though...
> > } > > } else { > > iter.cpu_file = cpu_file; > > @@ -63,13 +64,21 @@ static void ftrace_dump_buf(int skip_lines, long cpu_file) > > cpu_file, GFP_ATOMIC); > > ring_buffer_read_start(iter.buffer_iter[cpu_file]); > > tracing_iter_reset(&iter, cpu_file); > > + > > + cnt += ring_buffer_entries_cpu(iter.trace_buffer->buffer, > > + cpu_file); > > } > > > > - while (trace_find_next_entry_inc(&iter)) { > > - if (!cnt) > > - kdb_printf("---------------------------------\n"); > > - cnt++; > > + if (quiet) > > + goto out; > > + > > + kdb_printf("Dumping ftrace buffer (skipping %d lines):\n", > > + skip_lines); > > + > > + if (cnt) > > + kdb_printf("---------------------------------\n"); > > > > + while (trace_find_next_entry_inc(&iter)) { > > if (!skip_lines) { > > print_trace_line(&iter); > > trace_printk_seq(&iter.seq); > > @@ -99,6 +108,8 @@ static void ftrace_dump_buf(int skip_lines, long cpu_file) > > iter.buffer_iter[cpu] = NULL; > > } > > } > > + > > + return cnt; > > } > > > > /* > > @@ -109,6 +120,7 @@ static int kdb_ftdump(int argc, const char **argv) > > int skip_lines = 0; > > long cpu_file; > > char *cp; > > + int cnt; > > > > if (argc > 2) > > return KDB_ARGCOUNT; > > @@ -129,7 +141,14 @@ static int kdb_ftdump(int argc, const char **argv) > > } > > > > kdb_trap_printk++; > > - ftrace_dump_buf(skip_lines, cpu_file); > > + > > + /* A negative skip_lines means skip all but the last lines */ > > + if (skip_lines < 0) { > > + cnt = ftrace_dump_buf(0, cpu_file, true); > > Actually, you can add a function in trace.c that exposes > get_total_entries: > > unsigned long trace_total_entries(struct trace_array *tr) > { > unsigned long total, entries; > > if (!tr) > tr = &global_trace; > > get_total_entries(tr->trace_buffer, &total, &entries); > > return entries; > } > > and then do: > cnt = trace_total_entries(NULL);
OK. I guess I'll need to figure out how to add an argument to limit it to just one CPU too since the kdb command allows you to specify a single CPU or all CPUs. I think the best way would mean adding an argument to get_total_entries(). ...or I can just change the kdb command to not allow specifying a CPU? Which do you prefer? I personally haven't ever used the feature to just print the ftrace buffer for a certain CPU so I'd be OK removing it.
> Don't modify ftrace_dump_buf()
I still kinda prefer modifying ftrace_dump_buf() just because it's pretty important that the "count" we come up with match pretty exactly the count that ftrace_dump_buf() will come up with. My worry probably comes from my lack of experience with the internals of ftrace but, for instance, I see things like "tr->trace_flags &= ~TRACE_ITER_SYM_USEROBJ" in ftrace_dump_buf() and I worry that it will affect the count. ...but if you tell me that I need not worry about things like that then I won't. :-)
Thanks for your help!
-Doug
| |