Re: [PATCH v2 4/4] ftrace: Add an option for tracing console latencies

From: Viktor Rosendahl
Date: Thu May 02 2019 - 14:38:03 EST


> Instead of this being turned into a nop, don't have a kconfig option
> but instead have this call into the trace_irqsoff.c code, and depending
> on what the options are, it should stop it. Of course, this would need
> to be smart enough to pair it. Perhaps return the result of
> console_stop_critical_timings() and have that passed to
> console_start_critical_timings(), and only have start do something if
> stop did something. This way the option only needs to disable the stop
> part.

Something like this?

I have only compile tested it so far.

best regards,

Viktor
---
include/linux/irqflags.h | 17 +++++++++++++++++
kernel/printk/printk.c | 6 ++++--
kernel/trace/trace.h | 1 +
kernel/trace/trace_irqsoff.c | 13 +++++++++++++
4 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/include/linux/irqflags.h b/include/linux/irqflags.h
index 21619c92c377..e4f7ebc67a53 100644
--- a/include/linux/irqflags.h
+++ b/include/linux/irqflags.h
@@ -68,9 +68,26 @@ do { \
defined(CONFIG_PREEMPT_TRACER)
extern void stop_critical_timings(void);
extern void start_critical_timings(void);
+ extern bool console_tracing_disabled(void);
+
+#define console_stop_critical_timings(FLAG) \
+ do { \
+ FLAG = console_tracing_disabled(); \
+ if (FLAG) \
+ stop_critical_timings(); \
+ } while(0)
+
+#define console_start_critical_timings(FLAG) \
+ do { \
+ if (FLAG) \
+ start_critical_timings(); \
+ } while(0)
+
#else
# define stop_critical_timings() do { } while (0)
# define start_critical_timings() do { } while (0)
+# define console_stop_critical_timings(FLAG) do { } while (0)
+# define console_start_critical_timings(FLAG) do { } while (0)
#endif

/*
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 02ca827b8fac..3a18b7208399 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2348,6 +2348,7 @@ void console_unlock(void)
static char ext_text[CONSOLE_EXT_LOG_MAX];
static char text[LOG_LINE_MAX + PREFIX_MAX];
unsigned long flags;
+ bool cflag;
bool do_cond_resched, retry;

if (console_suspended) {
@@ -2448,9 +2449,10 @@ void console_unlock(void)
*/
console_lock_spinning_enable();

- stop_critical_timings(); /* don't trace print latency */
+ /* don't trace print latency if it's disabled */
+ console_stop_critical_timings(cflag);
call_console_drivers(ext_text, ext_len, text, len);
- start_critical_timings();
+ console_start_critical_timings(cflag);

if (console_lock_spinning_disable_and_check()) {
printk_safe_exit_irqrestore(flags);
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 59dc01ac52fd..3eed4756dba3 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1253,6 +1253,7 @@ extern int trace_get_user(struct trace_parser *parser, const char __user *ubuf,
C(PRINTK_MSGONLY, "printk-msg-only"), \
C(CONTEXT_INFO, "context-info"), /* Print pid/cpu/time */ \
C(LATENCY_FMT, "latency-format"), \
+ C(CONSOLE_LATENCY, "console-latency"), \
C(RECORD_CMD, "record-cmd"), \
C(RECORD_TGID, "record-tgid"), \
C(OVERWRITE, "overwrite"), \
diff --git a/kernel/trace/trace_irqsoff.c b/kernel/trace/trace_irqsoff.c
index 07a391e845de..659a82209c1e 100644
--- a/kernel/trace/trace_irqsoff.c
+++ b/kernel/trace/trace_irqsoff.c
@@ -490,6 +490,19 @@ void stop_critical_timings(void)
EXPORT_SYMBOL_GPL(stop_critical_timings);
NOKPROBE_SYMBOL(stop_critical_timings);

+bool console_tracing_disabled(void)
+{
+ struct trace_array *tr = irqsoff_trace;
+ int pc = preempt_count();
+
+ if (!preempt_trace(pc) && !irq_trace())
+ return false;
+
+ return !(tr->trace_flags & CONSOLE_LATENCY);
+}
+
+EXPORT_SYMBOL_GPL(console_tracing_disabled);
+
#ifdef CONFIG_FUNCTION_TRACER
static bool function_enabled;

--
2.17.1