From: Douglas Anderson <dianders@chromium.org>
To: Steven Rostedt <rostedt@goodmis.org>,
Ingo Molnar <mingo@redhat.com>,
Jason Wessel <jason.wessel@windriver.com>,
Daniel Thompson <daniel.thompson@linaro.org>
Cc: kgdb-bugreport@lists.sourceforge.net,
Brian Norris <briannorris@chromium.org>,
Douglas Anderson <dianders@chromium.org>,
Vasyl Gomonovych <gomonovych@gmail.com>,
Tom Zanussi <tom.zanussi@linux.intel.com>,
linux-kernel@vger.kernel.org, Baohong Liu <baohong.liu@intel.com>,
Masami Hiramatsu <mhiramat@kernel.org>
Subject: [PATCH 1/2] tracing: kdb: Fix ftdump to not sleep
Date: Tue, 5 Mar 2019 15:31:49 -0800 [thread overview]
Message-ID: <20190305233150.159633-1-dianders@chromium.org> (raw)
As reported back in 2016-11 [1], the "ftdump" kdb command triggers a
BUG for "sleeping function called from invalid context".
kdb's "ftdump" command wants to call ring_buffer_read_prepare() in
atomic context. A very simple solution for this is to just provide a
variant of ring_buffer_read_prepare that takes in allocation flags so
kdb can call it without triggering the allocation error. This patch
does that.
Note that in the original email thread about this, it was suggested
that perhaps the solution for kdb was to either preallocate the buffer
ahead of time or create our own iterator. I'm hoping that this
alternative of creating a variant of ring_buffer_read_prepare() can be
considered since it means I don't need to duplicate more of the core
trace code into "trace_kdb.c" (for either creating my own iterator or
re-preparing a ring allocator whose memory was already allocated).
NOTE: another option for kdb is to actually figure out how to make it
reuse the existing ftrace_dump() function and totally eliminate the
duplication. This sounds very appealing and actually works (the "sr
z" command can be seen to properly dump the ftrace buffer). The
downside here is that ftrace_dump() fully consumes the trace buffer.
Unless that is changed I'd rather not use it because it means "ftdump
| grep xyz" won't be very useful to search the ftrace buffer since it
will throw away the whole trace on the first grep. A future patch to
dump only the last few lines of the buffer will also be hard to
implement.
[1] https://lkml.kernel.org/r/20161117191605.GA21459@google.com
Reported-by: Brian Norris <briannorris@chromium.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
include/linux/ring_buffer.h | 2 ++
kernel/trace/ring_buffer.c | 42 +++++++++++++++++++++----------------
kernel/trace/trace_kdb.c | 6 ++++--
3 files changed, 30 insertions(+), 20 deletions(-)
diff --git a/include/linux/ring_buffer.h b/include/linux/ring_buffer.h
index 5b9ae62272bb..0c8ab8bf4cfb 100644
--- a/include/linux/ring_buffer.h
+++ b/include/linux/ring_buffer.h
@@ -127,6 +127,8 @@ struct ring_buffer_event *
ring_buffer_consume(struct ring_buffer *buffer, int cpu, u64 *ts,
unsigned long *lost_events);
+struct ring_buffer_iter *
+_ring_buffer_read_prepare(struct ring_buffer *buffer, int cpu, gfp_t flags);
struct ring_buffer_iter *
ring_buffer_read_prepare(struct ring_buffer *buffer, int cpu);
void ring_buffer_read_prepare_sync(void);
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 06e864a334bb..fd60007aa7a1 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -4201,6 +4201,29 @@ ring_buffer_consume(struct ring_buffer *buffer, int cpu, u64 *ts,
}
EXPORT_SYMBOL_GPL(ring_buffer_consume);
+struct ring_buffer_iter *
+_ring_buffer_read_prepare(struct ring_buffer *buffer, int cpu, gfp_t flags)
+{
+ struct ring_buffer_per_cpu *cpu_buffer;
+ struct ring_buffer_iter *iter;
+
+ if (!cpumask_test_cpu(cpu, buffer->cpumask))
+ return NULL;
+
+ iter = kmalloc(sizeof(*iter), flags);
+ if (!iter)
+ return NULL;
+
+ cpu_buffer = buffer->buffers[cpu];
+
+ iter->cpu_buffer = cpu_buffer;
+
+ atomic_inc(&buffer->resize_disabled);
+ atomic_inc(&cpu_buffer->record_disabled);
+
+ return iter;
+}
+
/**
* ring_buffer_read_prepare - Prepare for a non consuming read of the buffer
* @buffer: The ring buffer to read from
@@ -4224,24 +4247,7 @@ EXPORT_SYMBOL_GPL(ring_buffer_consume);
struct ring_buffer_iter *
ring_buffer_read_prepare(struct ring_buffer *buffer, int cpu)
{
- struct ring_buffer_per_cpu *cpu_buffer;
- struct ring_buffer_iter *iter;
-
- if (!cpumask_test_cpu(cpu, buffer->cpumask))
- return NULL;
-
- iter = kmalloc(sizeof(*iter), GFP_KERNEL);
- if (!iter)
- return NULL;
-
- cpu_buffer = buffer->buffers[cpu];
-
- iter->cpu_buffer = cpu_buffer;
-
- atomic_inc(&buffer->resize_disabled);
- atomic_inc(&cpu_buffer->record_disabled);
-
- return iter;
+ return _ring_buffer_read_prepare(buffer, cpu, GFP_KERNEL);
}
EXPORT_SYMBOL_GPL(ring_buffer_read_prepare);
diff --git a/kernel/trace/trace_kdb.c b/kernel/trace/trace_kdb.c
index d953c163a079..be84fa1ded35 100644
--- a/kernel/trace/trace_kdb.c
+++ b/kernel/trace/trace_kdb.c
@@ -51,14 +51,16 @@ static void ftrace_dump_buf(int skip_lines, long cpu_file)
if (cpu_file == RING_BUFFER_ALL_CPUS) {
for_each_tracing_cpu(cpu) {
iter.buffer_iter[cpu] =
- ring_buffer_read_prepare(iter.trace_buffer->buffer, cpu);
+ _ring_buffer_read_prepare(iter.trace_buffer->buffer,
+ cpu, GFP_ATOMIC);
ring_buffer_read_start(iter.buffer_iter[cpu]);
tracing_iter_reset(&iter, cpu);
}
} else {
iter.cpu_file = cpu_file;
iter.buffer_iter[cpu_file] =
- ring_buffer_read_prepare(iter.trace_buffer->buffer, cpu_file);
+ _ring_buffer_read_prepare(iter.trace_buffer->buffer,
+ cpu_file, GFP_ATOMIC);
ring_buffer_read_start(iter.buffer_iter[cpu_file]);
tracing_iter_reset(&iter, cpu_file);
}
--
2.21.0.352.gf09ad66450-goog
next reply other threads:[~2019-03-05 23:32 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-05 23:31 Douglas Anderson [this message]
2019-03-05 23:31 ` [PATCH 2/2] tracing: kdb: Allow ftdump to skip all but the last few lines Douglas Anderson
2019-03-05 23:47 ` [PATCH 1/2] tracing: kdb: Fix ftdump to not sleep 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=20190305233150.159633-1-dianders@chromium.org \
--to=dianders@chromium.org \
--cc=baohong.liu@intel.com \
--cc=briannorris@chromium.org \
--cc=daniel.thompson@linaro.org \
--cc=gomonovych@gmail.com \
--cc=jason.wessel@windriver.com \
--cc=kgdb-bugreport@lists.sourceforge.net \
--cc=linux-kernel@vger.kernel.org \
--cc=mhiramat@kernel.org \
--cc=mingo@redhat.com \
--cc=rostedt@goodmis.org \
--cc=tom.zanussi@linux.intel.com \
/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
all inboxes | Powered by JetHome®