From: Steven Rostedt <rostedt@goodmis.org>
To: Petr Mladek <pmladek@suse.cz>
Cc: linux-kernel@vger.kernel.org, Ingo Molnar <mingo@kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
Jiri Kosina <jkosina@suse.cz>
Subject: Re: [PATCH 14/26 v5] tracing: Create seq_buf layer in trace_seq
Date: Wed, 19 Nov 2014 10:08:41 -0500 [thread overview]
Message-ID: <20141119100841.316e865f@gandalf.local.home> (raw)
In-Reply-To: <20141119145133.GC2332@dhcp128.suse.cz>
On Wed, 19 Nov 2014 15:51:38 +0100
Petr Mladek <pmladek@suse.cz> wrote:
> On Fri 2014-11-14 23:59:01, Steven Rostedt wrote:
> > From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
> >
> > Create a seq_buf layer that trace_seq sits on. The seq_buf will not
> > be limited to page size. This will allow other usages of seq_buf
> > instead of a hard set PAGE_SIZE one that trace_seq has.
> >
> > Link: http://lkml.kernel.org/r/20141104160221.864997179@goodmis.org
> > Link: http://lkml.kernel.org/r/20141114011412.170377300@goodmis.org
> >
> > Tested-by: Jiri Kosina <jkosina@suse.cz>
> > Acked-by: Jiri Kosina <jkosina@suse.cz>
> > Reviewed-by: Petr Mladek <pmladek@suse.cz>
> > Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> > ---
> > include/linux/seq_buf.h | 81 +++++++++
> > include/linux/trace_seq.h | 12 +-
> > kernel/trace/Makefile | 1 +
> > kernel/trace/seq_buf.c | 341 +++++++++++++++++++++++++++++++++++
> > kernel/trace/trace.c | 39 ++--
> > kernel/trace/trace_events.c | 6 +-
> > kernel/trace/trace_functions_graph.c | 6 +-
> > kernel/trace/trace_seq.c | 177 +++++++++---------
> > 8 files changed, 538 insertions(+), 125 deletions(-)
> > create mode 100644 include/linux/seq_buf.h
> > create mode 100644 kernel/trace/seq_buf.c
>
> [...]
>
> > diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
> > index 100288d10e1f..6d1342ae7a44 100644
> > --- a/kernel/trace/trace_functions_graph.c
> > +++ b/kernel/trace/trace_functions_graph.c
>
> [...]
>
> > @@ -163,21 +176,22 @@ EXPORT_SYMBOL_GPL(trace_seq_vprintf);
> > */
> > void trace_seq_bprintf(struct trace_seq *s, const char *fmt, const u32 *binary)
> > {
> > - unsigned int len = TRACE_SEQ_BUF_LEFT(s);
> > + unsigned int save_len = s->seq.len;
> > int ret;
>
> Last but one small thing. The "ret" is assigned but not longer used.
>
> JFYI, I have skipped this patch yesterday because it was long and already
> marked as reviewed. I could not help myself and double checked it today.
>
OK, I'll fix that up.
> > - if (s->full || !len)
> > + if (s->full)
> > return;
> >
> > - ret = bstr_printf(s->buffer + s->len, len, fmt, binary);
> > + __trace_seq_init(s);
> > +
> > + ret = seq_buf_bprintf(&s->seq, fmt, binary);
> >
> > /* If we can't write it all, don't bother writing anything */
> > - if (ret >= len) {
> > + if (unlikely(seq_buf_has_overflowed(&s->seq))) {
> > + s->seq.len = save_len;
> > s->full = 1;
> > return;
> > }
> > -
> > - s->len += ret;
> > }
> > EXPORT_SYMBOL_GPL(trace_seq_bprintf);
>
> The last small thing is the same unused "ret" in trace_seq_path(). But
> this one should get fixed in "[PATCH 15/26 v5] tracing: Convert
> seq_buf_path() to be like seq_path()".
OK, I'll try to remember that :-)
>
> Both these changes are rather cosmetic. Feel free to leave them for
> some later clean up.
>
>
> Eureka, I have finished review of the patchset and am happy with it \o/
> I am looking forward to see it in the main tree.
Well now it comes down to Linus or Andrew giving me the OK. I've pulled
all the trace_seq updates into my ftrace/core branch (which will be
going into my for-next branch shortly). I stopped just before this
patch. I didn't want to create the seq_buf until I knew it was going to
be used by something other than trace_seq.
>
> Thanks a lot for the hard work.
>
Yeah, let's hope that it was worth it.
Thanks for the in depth reviews.
-- Steve
next prev parent reply other threads:[~2014-11-19 15:08 UTC|newest]
Thread overview: 70+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-15 4:58 [PATCH 00/26 v5] trace-seq/seq-buf/x86/printk: Print all stacks from NMI safely Steven Rostedt
2014-11-15 4:58 ` [PATCH 01/26 v5] tracing: Fix trace_seq_bitmask() to start at current position Steven Rostedt
2014-11-15 4:58 ` [PATCH 02/26 v5] tracing: Add trace_seq_has_overflowed() and trace_handle_return() Steven Rostedt
2014-11-15 4:58 ` [PATCH 03/26 v5] blktrace/tracing: Use trace_seq_has_overflowed() helper function Steven Rostedt
2014-11-15 4:58 ` [PATCH 04/26 v5] ring-buffer: Remove check of trace_seq_{puts,printf}() return values Steven Rostedt
2014-11-15 4:58 ` [PATCH 05/26 v5] tracing: Have branch tracer use trace_handle_return() helper function Steven Rostedt
2014-11-15 4:58 ` [PATCH 06/26 v5] tracing: Have function_graph use trace_seq_has_overflowed() Steven Rostedt
2014-11-15 4:58 ` [PATCH 07/26 v5] kprobes/tracing: Use trace_seq_has_overflowed() for overflow checks Steven Rostedt
2014-11-18 14:02 ` Petr Mladek
2014-11-15 4:58 ` [PATCH 08/26 v5] tracing: Do not check return values of trace_seq_p*() for mmio tracer Steven Rostedt
2014-11-18 14:06 ` Petr Mladek
2014-11-15 4:58 ` [PATCH 09/26 v5] tracing/probes: Do not use return value of trace_seq_printf() Steven Rostedt
2014-11-18 14:09 ` Petr Mladek
2014-11-15 4:58 ` [PATCH 10/26 v5] tracing/uprobes: Do not use return values " Steven Rostedt
2014-11-17 5:28 ` Masami Hiramatsu
2014-11-17 5:58 ` Srikar Dronamraju
2014-11-18 14:13 ` Petr Mladek
2014-11-15 4:58 ` [PATCH 11/26 v5] tracing: Do not use return values of trace_seq_printf() in syscall tracing Steven Rostedt
2014-11-15 4:58 ` [PATCH 12/26 v5] tracing: Remove return values of most trace_seq_*() functions Steven Rostedt
2014-11-18 14:18 ` Petr Mladek
2014-11-15 4:59 ` [PATCH 13/26 v5] tracing: Fix return value of ftrace_raw_output_prep() Steven Rostedt
2014-11-18 14:24 ` Petr Mladek
2014-11-15 4:59 ` [PATCH 14/26 v5] tracing: Create seq_buf layer in trace_seq Steven Rostedt
2014-11-19 14:51 ` Petr Mladek
2014-11-19 15:08 ` Steven Rostedt [this message]
2014-11-15 4:59 ` [PATCH 15/26 v5] tracing: Convert seq_buf_path() to be like seq_path() Steven Rostedt
2014-11-15 4:59 ` [PATCH 16/26 v5] tracing: Convert seq_buf fields to be like seq_file fields Steven Rostedt
2014-11-15 4:59 ` [PATCH 17/26 v5] tracing: Add a seq_buf_clear() helper and clear len and readpos in init Steven Rostedt
2014-11-15 4:59 ` [PATCH 18/26 v5] seq_buf: Create seq_buf_used() to find out how much was written Steven Rostedt
2014-11-18 15:02 ` Petr Mladek
2014-11-19 15:49 ` Steven Rostedt
2014-11-19 16:30 ` Petr Mladek
2014-11-15 4:59 ` [PATCH 19/26 v5] tracing: Use trace_seq_used() and seq_buf_used() instead of len Steven Rostedt
2014-11-17 17:32 ` Steven Rostedt
2014-11-17 19:11 ` [PATCH 1/2] tracing: Clean up tracing_fill_pipe_page() Steven Rostedt
2014-11-18 16:15 ` Petr Mladek
2014-11-19 16:17 ` Steven Rostedt
2014-11-19 16:51 ` Petr Mladek
2014-11-19 17:12 ` Steven Rostedt
2014-11-17 19:12 ` [PATCH 2/2] tracing: Use trace_seq_used() and seq_buf_used() instead of len Steven Rostedt
2014-11-18 16:33 ` Petr Mladek
2014-11-18 17:37 ` Steven Rostedt
2014-11-19 11:40 ` Petr Mladek
2014-11-19 13:48 ` Steven Rostedt
2014-11-19 14:40 ` Petr Mladek
2014-11-19 15:01 ` Steven Rostedt
2014-11-19 16:00 ` Steven Rostedt
2014-11-19 16:44 ` Petr Mladek
2014-11-15 4:59 ` [PATCH 20/26 v5] seq_buf: Add seq_buf_can_fit() helper function Steven Rostedt
2014-11-17 17:36 ` Steven Rostedt
2014-11-18 0:07 ` Joe Perches
2014-11-18 0:27 ` Steven Rostedt
2014-11-18 0:35 ` Joe Perches
2014-11-18 0:56 ` Steven Rostedt
2014-11-18 1:07 ` Joe Perches
2014-11-18 1:24 ` Steven Rostedt
2014-11-18 1:48 ` Joe Perches
2014-11-18 2:37 ` Steven Rostedt
2014-11-18 2:50 ` Joe Perches
2014-11-18 3:00 ` Steven Rostedt
2014-11-18 16:40 ` Petr Mladek
2014-11-15 4:59 ` [PATCH 21/26 v5] tracing: Have seq_buf use full buffer Steven Rostedt
2014-11-15 4:59 ` [PATCH 22/26 v5] tracing: Add seq_buf_get_buf() and seq_buf_commit() helper functions Steven Rostedt
2014-11-15 4:59 ` [PATCH 23/26 v5] seq-buf: Make seq_buf_bprintf() conditional on CONFIG_BINARY_PRINTF Steven Rostedt
2014-11-15 4:59 ` [PATCH 24/26 v5] seq_buf: Move the seq_buf code to lib/ Steven Rostedt
2014-11-15 4:59 ` [PATCH 25/26 v5] printk: Add per_cpu printk func to allow printk to be diverted Steven Rostedt
2014-11-15 4:59 ` [PATCH 26/26 v5] x86/nmi: Perform a safe NMI stack trace on all CPUs Steven Rostedt
2014-11-18 17:02 ` Petr Mladek
2014-11-15 5:08 ` [PATCH 00/26 v5] trace-seq/seq-buf/x86/printk: Print all stacks from NMI safely Steven Rostedt
2014-11-18 3:21 ` Steven Rostedt
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=20141119100841.316e865f@gandalf.local.home \
--to=rostedt@goodmis.org \
--cc=akpm@linux-foundation.org \
--cc=jkosina@suse.cz \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=pmladek@suse.cz \
/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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).