Messages in this thread | | | From | Doug Anderson <> | Date | Tue, 19 Mar 2019 07:45:38 -0700 | Subject | Re: [PATCH v5 2/3] tracing: Add trace_total_entries() / trace_total_entries_cpu() |
| |
Hi,
On Tue, Mar 19, 2019 at 4:25 AM Daniel Thompson <daniel.thompson@linaro.org> wrote: > > On Mon, Mar 18, 2019 at 01:47:40PM -0700, Douglas Anderson wrote: > > These two new exported functions will be used in a future patch by > > kdb_ftdump() to quickly skip all but the last few trace entries. > > > > Suggested-by: Steven Rostedt <rostedt@goodmis.org> > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > > --- > > > > Changes in v5: None > > Changes in v4: > > - trace_total_entries() / trace_total_entries_cpu() new for v4 > > > > Changes in v3: None > > > > kernel/trace/trace.c | 65 ++++++++++++++++++++++++++++++++++---------- > > kernel/trace/trace.h | 3 ++ > > 2 files changed, 53 insertions(+), 15 deletions(-) > > > > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c > > index ccd759eaad79..7afc90f82e53 100644 > > --- a/kernel/trace/trace.c > > +++ b/kernel/trace/trace.c > > @@ -3490,34 +3490,69 @@ static void s_stop(struct seq_file *m, void *p) > > trace_event_read_unlock(); > > } > > > > +static void > > +get_total_entries_cpu(struct trace_buffer *buf, unsigned long *total, > > + unsigned long *entries, int cpu) > > +{ > > + unsigned long count; > > + > > + count = ring_buffer_entries_cpu(buf->buffer, cpu); > > + /* > > + * If this buffer has skipped entries, then we hold all > > + * entries for the trace and we need to ignore the > > + * ones before the time stamp. > > + */ > > + if (per_cpu_ptr(buf->data, cpu)->skipped_entries) { > > + count -= per_cpu_ptr(buf->data, cpu)->skipped_entries; > > + /* total is the same as the entries */ > > + *total = count; > > + } else > > + *total = count + > > + ring_buffer_overrun_cpu(buf->buffer, cpu); > > + *entries = count; > > +} > > + > > static void > > get_total_entries(struct trace_buffer *buf, > > unsigned long *total, unsigned long *entries) > > { > > - unsigned long count; > > + unsigned long t, e; > > int cpu; > > > > *total = 0; > > *entries = 0; > > > > for_each_tracing_cpu(cpu) { > > - count = ring_buffer_entries_cpu(buf->buffer, cpu); > > - /* > > - * If this buffer has skipped entries, then we hold all > > - * entries for the trace and we need to ignore the > > - * ones before the time stamp. > > - */ > > - if (per_cpu_ptr(buf->data, cpu)->skipped_entries) { > > - count -= per_cpu_ptr(buf->data, cpu)->skipped_entries; > > - /* total is the same as the entries */ > > - *total += count; > > - } else > > - *total += count + > > - ring_buffer_overrun_cpu(buf->buffer, cpu); > > - *entries += count; > > + get_total_entries_cpu(buf, &t, &e, cpu); > > + *total += t; > > + *entries += e; > > } > > } > > > > +unsigned long trace_total_entries_cpu(struct trace_array *tr, int cpu) > > +{ > > + unsigned long total, entries; > > + > > + if (!tr) > > + tr = &global_trace; > > This function is only ever called with tr set to NULL which means tr is > an argument looking for a user. > > I wouldn't mind except if this was following copying prior art to keep > the API feel the same but I can't find any other trace function where > the trace_array can be substituted for NULL. AFAICT all the existing > sites where global_trace is used will use it unconditionally.
Happy to change this if you guys want. At the moment the trace_total_entries() comes straight from Steven's suggestion at:
https://lkml.kernel.org/r/20190315144130.1aa36931@gandalf.local.home
Ugh, but while looking at this it looks like I totally forgot one piece of feedback from Steven: to move the disabling of the tracing out so it's not re-enabled after counting but before dumping. Not sure how I missed that. :(
I'll wait to spin until I hear from Steven if he wants me to remove the "struct trace_array *tr" argument, then I'll post out a v6 with the fix I forgot...
-Doug
| |