Date: Fri, 16 Dec 2016 11:37:39 -0500
From: Steven Rostedt <rostedt@goodmis.org>
To: David Howells <dhowells@redhat.com>
Cc: Christian =?UTF-8?B?S8O2bmln?= <deathsimple@vodafone.de>,
        LKML <linux-kernel@vger.kernel.org>,
        Alex Deucher <alexander.deucher@amd.com>
Subject: Re: Allow tracepoints to use direct indexing for number->string
 translation
Message-ID: <20161216113739.5b5a4a5e@gandalf.local.home>
In-Reply-To: <31530.1481905345@warthog.procyon.org.uk>
References: <20161216100241.160a81b2@gandalf.local.home>
        <23920.1481823475@warthog.procyon.org.uk>
        <ad6fb376-4fa8-3001-8cc9-7e0a41393616@vodafone.de>
        <25916.1481889335@warthog.procyon.org.uk>
        <31530.1481905345@warthog.procyon.org.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 5194
Lines: 148

On Fri, 16 Dec 2016 16:22:25 +0000
David Howells <dhowells@redhat.com> wrote:

> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > > Another feature that would be very nice to have is the ability to turn a
> > > number into a string by direct array index rather than by table search or
> > > serial ?: ternary operators.  
> > 
> > Which function are you talking about?  
> 
> See the attached patch that converts rxrpc's tracing from using string arrays
> to using TRACE_DEFINE_ENUM so that utilities like crash's trace buffer
> disassembler can find the string values.
> 
> When __print_symbolic() is used, each set of strings is rendered as an array
> of trace_print_flag structs which trace_print_symbols_seq() then iterates
> over to find a match.
> 

You mean to do something like this? (untested, not even compiled)

-- Steve

---
 include/linux/trace_events.h |    6 ++++--
 include/trace/trace_events.h |   10 ++++++----
 kernel/trace/trace_output.c  |   28 ++++++++++++++++++++++++----
 3 files changed, 34 insertions(+), 10 deletions(-)

Index: linux-trace.git/include/linux/trace_events.h
===================================================================
--- linux-trace.git.orig/include/linux/trace_events.h	2016-11-15 17:27:19.397749523 -0500
+++ linux-trace.git/include/linux/trace_events.h	2016-12-16 11:34:38.783424430 -0500
@@ -20,13 +20,15 @@ const char *trace_print_flags_seq(struct
 				  const struct trace_print_flags *flag_array);
 
 const char *trace_print_symbols_seq(struct trace_seq *p, unsigned long val,
-				    const struct trace_print_flags *symbol_array);
+				    const struct trace_print_flags *symbol_array,
+				    unsigned long nr);
 
 #if BITS_PER_LONG == 32
 const char *trace_print_symbols_seq_u64(struct trace_seq *p,
 					unsigned long long val,
 					const struct trace_print_flags_u64
-								 *symbol_array);
+								 *symbol_array,
+					unsigned long long nr);
 #endif
 
 const char *trace_print_bitmask_seq(struct trace_seq *p, void *bitmask_ptr,
Index: linux-trace.git/include/trace/trace_events.h
===================================================================
--- linux-trace.git.orig/include/trace/trace_events.h	2016-12-16 11:29:39.599950900 -0500
+++ linux-trace.git/include/trace/trace_events.h	2016-12-16 11:30:39.927844742 -0500
@@ -279,8 +279,9 @@ TRACE_MAKE_SYSTEM_STR();
 #define __print_symbolic(value, symbol_array...)			\
 	({								\
 		static const struct trace_print_flags symbols[] =	\
-			{ symbol_array, { -1, NULL }};			\
-		trace_print_symbols_seq(p, value, symbols);		\
+			{ symbol_array}};				\
+		trace_print_symbols_seq(p, value, symbols,		\
+					ARRAY_SIZE(symbols));		\
 	})
 
 #undef __print_symbolic_u64
@@ -288,8 +289,9 @@ TRACE_MAKE_SYSTEM_STR();
 #define __print_symbolic_u64(value, symbol_array...)			\
 	({								\
 		static const struct trace_print_flags_u64 symbols[] =	\
-			{ symbol_array, { -1, NULL } };			\
-		trace_print_symbols_seq_u64(p, value, symbols);	\
+			{ symbol_array };				\
+		trace_print_symbols_seq_u64(p, value, symbols,		\
+					    ARRAY_SIZE(symbols));	\
 	})
 #else
 #define __print_symbolic_u64(value, symbol_array...)			\
Index: linux-trace.git/kernel/trace/trace_output.c
===================================================================
--- linux-trace.git.orig/kernel/trace/trace_output.c	2016-12-14 10:33:28.581929129 -0500
+++ linux-trace.git/kernel/trace/trace_output.c	2016-12-16 11:36:41.695208145 -0500
@@ -99,12 +99,21 @@ EXPORT_SYMBOL(trace_print_flags_seq);
 
 const char *
 trace_print_symbols_seq(struct trace_seq *p, unsigned long val,
-			const struct trace_print_flags *symbol_array)
+			const struct trace_print_flags *symbol_array,
+			unsigned long nr)
 {
 	int i;
 	const char *ret = trace_seq_buffer_ptr(p);
 
-	for (i = 0;  symbol_array[i].name; i++) {
+	/* A lot of arrays are simply enums that map to the array index */
+	if (val < nr) {
+		if (val == symbol_array[val].mask) {
+			trace_seq_puts(p, symbol_array[val].name);
+			goto out;
+		}
+	}
+
+	for (i = 0;  i < nr; i++) {
 
 		if (val != symbol_array[i].mask)
 			continue;
@@ -116,6 +125,7 @@ trace_print_symbols_seq(struct trace_seq
 	if (ret == (const char *)(trace_seq_buffer_ptr(p)))
 		trace_seq_printf(p, "0x%lx", val);
 
+ out:
 	trace_seq_putc(p, 0);
 
 	return ret;
@@ -125,12 +135,21 @@ EXPORT_SYMBOL(trace_print_symbols_seq);
 #if BITS_PER_LONG == 32
 const char *
 trace_print_symbols_seq_u64(struct trace_seq *p, unsigned long long val,
-			 const struct trace_print_flags_u64 *symbol_array)
+			 const struct trace_print_flags_u64 *symbol_array,
+			    unsigned long long nr)
 {
 	int i;
 	const char *ret = trace_seq_buffer_ptr(p);
 
-	for (i = 0;  symbol_array[i].name; i++) {
+	/* A lot of arrays are simply enums that map to the array index */
+	if (val < nr) {
+		if (val == symbol_array[val].mask) {
+			trace_seq_puts(p, symbol_array[val].name);
+			goto out;
+		}
+	}
+
+	for (i = 0;  i < nr; i++) {
 
 		if (val != symbol_array[i].mask)
 			continue;
@@ -142,6 +161,7 @@ trace_print_symbols_seq_u64(struct trace
 	if (ret == (const char *)(trace_seq_buffer_ptr(p)))
 		trace_seq_printf(p, "0x%llx", val);
 
+ out:
 	trace_seq_putc(p, 0);
 
 	return ret;