Skip to contentSkip to conversation

tracing: updates for 4.1

PATCH In review Steven Rostedt 2 participants 11 messages Mar 25, 2015 – Mar 30, 2015

Subsystems (5)
Stable branch Btrfs file system Tracing ext4 file system +1 more subsystems
lore
Share & tools
lore

Steven Rostedt

author maintainer
#0 lore raw

cover tracing: updates for 4.1

git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
for-next

Head SHA1: d9a16d3ab8770357015c85a07387f1d2676a4773

He Kuang (1):
tracing: remove ftrace:function TRACE_EVENT_FL_USE_CALL_FILTER flag

Scott Wood (1):
tracing: %pF is only for function pointers

Stephen Rothwell (1):
trace: Don't use __weak in header files

Steven Rostedt (1):

      ring-buffer: Replace this_cpu_*() with __this_cpu_*()

----

 include/trace/events/btrfs.h       |  4 ++--
 include/trace/events/ext3.h        |  2 +-
 include/trace/events/ext4.h        |  6 +++---
 include/trace/events/module.h      |  4 ++--
 include/trace/events/random.h      | 10 +++++-----
 kernel/trace/ring_buffer.c         | 11 +++++------
 kernel/trace/trace_entries.h       |  6 +++---
 kernel/trace/trace_export.c        |  2 +-
 kernel/trace/trace_kprobe.c        |  5 +++--
 kernel/trace/trace_probe.c         | 19 +++++++------------
 kernel/trace/trace_probe.h         | 10 ++--------
 kernel/trace/trace_uprobe.c        |  5 +++--
 tools/lib/traceevent/event-parse.c |  2 +-
 13 files changed, 38 insertions(+), 48 deletions(-)
author maintainer
#1 lore raw

PATCH 1/4 ring-buffer: Replace this_cpu_*() with __this_cpu_*()

From: Steven Rostedt <[email protected]>

It has come to my attention that this_cpu_read/write are horrible on architectures other than x86. Worse yet, they actually disable preemption or interrupts! This caused some unexpected tracing results on ARM.

101.356868: preempt_count_add <-ring_buffer_lock_reserve 101.356870: preempt_count_sub <-ring_buffer_lock_reserve

The ring_buffer_lock_reserve has recursion protection that requires accessing a per cpu variable. But since preempt_disable() is traced, it too got traced while accessing the variable that is suppose to prevent recursion like this.

The generic version of this_cpu_read() and write() are:

#define this_cpu_generic_read(pcp) \

 ({	typeof(pcp) ret__;						\
	preempt_disable();						\
	ret__ = *this_cpu_ptr(&(pcp));					\
	preempt_enable();						\
	ret__;								\

})

#define this_cpu_generic_to_op(pcp, val, op) \

 do {									\
	unsigned long flags;						\
	raw_local_irq_save(flags);					\
	*__this_cpu_ptr(&(pcp)) op val;					\
	raw_local_irq_restore(flags);					\

} while (0)

Which is unacceptable for locations that know they are within preempt disabled or interrupt disabled locations.

Paul McKenney stated that __this_cpu_() versions produce much better code on other architectures than this_cpu_() does, if we know that the call is done in a preempt disabled location.

I also changed the recursive_unlock() to use two local variables instead of accessing the per_cpu variable twice.

Link http://lkml.kernel.org/r/[email protected] open here
Link http://lkml.kernel.org/r/[email protected] open here
Cc [email protected]
Acked-by Christoph Lameter <[email protected]>
Reported-by Uwe Kleine-Koenig <[email protected]>
Tested-by Uwe Kleine-Koenig <[email protected]>
Signed-off-by Steven Rostedt <[email protected]>
---
 kernel/trace/ring_buffer.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)
kernel/trace/ring_buffer.c +5 -6
index 5040d44fe5a3..922048a0f7ea 100644
@@ -2679,7 +2679,7 @@ static DEFINE_PER_CPU(unsigned int, current_context);
 
 static __always_inline int trace_recursive_lock(void)
 {
-	unsigned int val = this_cpu_read(current_context);
+	unsigned int val = __this_cpu_read(current_context);
 	int bit;
 
 	if (in_interrupt()) {

@@ -2696,18 +2696,17 @@ static __always_inline int trace_recursive_lock(void)
 		return 1;
 
 	val |= (1 << bit);
-	this_cpu_write(current_context, val);
+	__this_cpu_write(current_context, val);
 
 	return 0;
 }
 
 static __always_inline void trace_recursive_unlock(void)
 {
-	unsigned int val = this_cpu_read(current_context);
+	unsigned int val = __this_cpu_read(current_context);
 
-	val--;
-	val &= this_cpu_read(current_context);
-	this_cpu_write(current_context, val);
+	val &= val & (val - 1);
+	__this_cpu_write(current_context, val);
 }
 
 #else
Signature · 1 line
2.1.4
#2 lore raw

Re: [for-next][PATCH 1/4] ring-buffer: Replace this_cpu_*() with __this_cpu_*()

Quoted 4 lines · Steven Rostedt

It has come to my attention that this_cpu_read/write are horrible on architectures other than x86. Worse yet, they actually disable preemption or interrupts! This caused some unexpected tracing results on ARM.

This isnt something new and I thought the comment was dropped from the patch? This is a plain error in using this_cpu_* where __this_cpu_* would have been sufficient. Code was uselessly disabling preemption twice.

Quoted 2 lines

Which is unacceptable for locations that know they are within preempt disabled or interrupt disabled locations.

Well yes. Thats why the __this_cpu ops are there to avoid this overhead.

Quoted 2 lines

I also changed the recursive_unlock() to use two local variables instead of accessing the per_cpu variable twice.

Ok gotta look at that.

kernel/trace/ring_buffer.c static __always_inline int trace_recursive_lock(void) · line 2710

Quoted 11 lines

static __always_inline void trace_recursive_unlock(void) {

kernel/trace/ring_buffer.c +3 -4
-	unsigned int val = this_cpu_read(current_context);
+	unsigned int val = __this_cpu_read(current_context);
 
-	val--;
-	val &= this_cpu_read(current_context);
-	this_cpu_write(current_context, val);
+	val &= val & (val - 1);
+	__this_cpu_write(current_context, val);
 }

Ummm... This is does not look like an equivalent thing. Should this not be:

	unsigned int val = __this_cpu_read(current_context);
	unsigned int newval = val - 1;

	newval &= val;
	__this_cpu_write(current_context, newval);

or more compact

	unsigned int val = __this_cpu_read(current_context);

	__this_cpu_write(current_context, val & (val - 1));
author maintainer
#3 lore raw

Re: [for-next][PATCH 1/4] ring-buffer: Replace this_cpu_*() with __this_cpu_*()

Quoted 11 lines · On Fri, 27 Mar 2015 14:41:44 -0500 (CDT) Christoph Lameter <[email protected]> wrote:
Quoted 4 lines · Steven Rostedt

It has come to my attention that this_cpu_read/write are horrible on architectures other than x86. Worse yet, they actually disable preemption or interrupts! This caused some unexpected tracing results on ARM.

This isnt something new and I thought the comment was dropped from the patch? This is a plain error in using this_cpu_* where __this_cpu_* would have been sufficient. Code was uselessly disabling preemption twice.

Where in the patch do you see the comment? Or were you talking about the change log? The original patch did have a comment, an it was dropped, that's what I thought you were talking about.

Quoted 26 lines
Quoted 2 lines

Which is unacceptable for locations that know they are within preempt disabled or interrupt disabled locations.

Well yes. Thats why the __this_cpu ops are there to avoid this overhead.

Quoted 2 lines

I also changed the recursive_unlock() to use two local variables instead of accessing the per_cpu variable twice.

Ok gotta look at that.

kernel/trace/ring_buffer.c static __always_inline int trace_recursive_lock(void) · line 2710

Quoted 11 lines

static __always_inline void trace_recursive_unlock(void) {

kernel/trace/ring_buffer.c +3 -4
-	unsigned int val = this_cpu_read(current_context);
+	unsigned int val = __this_cpu_read(current_context);
 
-	val--;
-	val &= this_cpu_read(current_context);
-	this_cpu_write(current_context, val);
+	val &= val & (val - 1);
+	__this_cpu_write(current_context, val);
 }

Ummm... This is does not look like an equivalent thing. Should this not be:

Quoted 5 lines
	unsigned int val = __this_cpu_read(current_context);
	unsigned int newval = val - 1;

	newval &= val;
	__this_cpu_write(current_context, newval);

Actually, it is equivalent, but I do see a issue with my patch.

	val &= val & (val - 1);

is the same as the more reasonable:

	val &= val - 1;

I think I meant to replace &= with = :-/

Quoted 6 lines

or more compact

	unsigned int val = __this_cpu_read(current_context);

	__this_cpu_write(current_context, val & (val - 1));

Maybe I'll just use your compact version.

Thanks,

-- Steve

#4 lore raw

Re: [for-next][PATCH 1/4] ring-buffer: Replace this_cpu_*() with __this_cpu_*()

Quoted 3 lines · Steven Rostedt

Where in the patch do you see the comment? Or were you talking about the change log? The original patch did have a comment, an it was dropped, that's what I thought you were talking about.

Sorry yes the changelog.

Quoted 11 lines

Actually, it is equivalent, but I do see a issue with my patch.

	val &= val & (val - 1);

is the same as the more reasonable:

	val &= val - 1;

I think I meant to replace &= with = :-/

Quoted 1 line
Quoted 7 lines
Quoted 5 lines

or more compact

	unsigned int val = __this_cpu_read(current_context);

	__this_cpu_write(current_context, val & (val - 1));

Maybe I'll just use your compact version.

Hmmm... It could even be more compact

__this_cpu_and(current_context, __this_cpu_read(current_context) - 1);

author maintainer
#5 lore raw

Re: [for-next][PATCH 1/4] ring-buffer: Replace this_cpu_*() with __this_cpu_*()

Quoted 5 lines · On Mon, 30 Mar 2015 07:44:30 -0500 (CDT) Christoph Lameter <[email protected]> wrote:
Quoted 5 lines
Quoted 5 lines

or more compact

	unsigned int val = __this_cpu_read(current_context);
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
	__this_cpu_write(current_context, val & (val - 1));

Maybe I'll just use your compact version.

Hmmm... It could even be more compact

__this_cpu_and(current_context, __this_cpu_read(current_context) - 1);

Hmm, I didn't realize there was an "and" version. I'm guessing this would bring down the instruction count even more?

/me tries it.

I just finished testing my previous version. If this does prove to be more compact, I'll have to replace that one with this one.

-- Steve

#6 lore raw

Re: [for-next][PATCH 1/4] ring-buffer: Replace this_cpu_*() with __this_cpu_*()

Quoted 2 lines · Steven Rostedt

Hmm, I didn't realize there was an "and" version. I'm guessing this would bring down the instruction count even more?

Yes two segment prefixed instructions and a decrement.

author maintainer
#7 lore raw

PATCH 1/1 ring-buffer: Remove duplicate use of '&' in recursive code

In reply to Christoph Lameter

Steven Rostedt (Red Hat) (1):
ring-buffer: Remove duplicate use of '&' in recursive code

----

 kernel/trace/ring_buffer.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
---------------------------
commit 7eb867195b9f3990da60738b1f26d0a71f37f77f
Author: Steven Rostedt (Red Hat) <[email protected]>
Date:   Fri Mar 27 17:39:49 2015 -0400

ring-buffer: Remove duplicate use of '&' in recursive code

A clean up of the recursive protection code changed

val = this_cpu_read(current_context);
val--;
val &= this_cpu_read(current_context);

to

val = this_cpu_read(current_context);
val &= val & (val - 1);

Which has a duplicate use of '&' as the above is the same as

val = val & (val - 1);

Actually, it would be best to remove that line altogether and just add it to where it is used.

Link http://lkml.kernel.org/[email protected] open here
Suggested-by Christoph Lameter <[email protected]>
Signed-off-by Steven Rostedt <[email protected]>
kernel/trace/ring_buffer.c +1 -2
index 922048a0f7ea..93caf56567cb 100644
@@ -2705,8 +2705,7 @@ static __always_inline void trace_recursive_unlock(void)
 {
 	unsigned int val = __this_cpu_read(current_context);
 
-	val &= val & (val - 1);
-	__this_cpu_write(current_context, val);
+	__this_cpu_write(current_context, val & (val - 1));
 }
 
 #else
author maintainer
#8 lore raw

PATCH 2/4 tracing: %pF is only for function pointers

In reply to Steven Rostedt

From: Scott Wood <[email protected]>

Use %pS for actual addresses, otherwise you'll get bad output on arches like ppc64 where %pF expects a function descriptor.

Link http://lkml.kernel.org/r/[email protected] open here
Signed-off-by Scott Wood <[email protected]>
Signed-off-by Steven Rostedt <[email protected]>
---
 include/trace/events/btrfs.h       |  4 ++--
 include/trace/events/ext3.h        |  2 +-
 include/trace/events/ext4.h        |  6 +++---
 include/trace/events/module.h      |  4 ++--
 include/trace/events/random.h      | 10 +++++-----
 kernel/trace/trace_entries.h       |  6 +++---
 tools/lib/traceevent/event-parse.c |  2 +-
 7 files changed, 17 insertions(+), 17 deletions(-)
include/trace/events/btrfs.h +2 -2
index 1faecea101f3..572e6503394a 100644
@@ -962,7 +962,7 @@ TRACE_EVENT(alloc_extent_state,
 		__entry->ip	= IP
 	),
 
-	TP_printk("state=%p; mask = %s; caller = %pF", __entry->state,
+	TP_printk("state=%p; mask = %s; caller = %pS", __entry->state,
 		  show_gfp_flags(__entry->mask), (void *)__entry->ip)
 );
 

@@ -982,7 +982,7 @@ TRACE_EVENT(free_extent_state,
 		__entry->ip = IP
 	),
 
-	TP_printk(" state=%p; caller = %pF", __entry->state,
+	TP_printk(" state=%p; caller = %pS", __entry->state,
 		  (void *)__entry->ip)
 );
include/trace/events/ext3.h +1 -1
index 6797b9de90ed..7f20707849bb 100644
@@ -144,7 +144,7 @@ TRACE_EVENT(ext3_mark_inode_dirty,
 		__entry->ip	= IP;
 	),
 
-	TP_printk("dev %d,%d ino %lu caller %pF",
+	TP_printk("dev %d,%d ino %lu caller %pS",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
 		  (unsigned long) __entry->ino, (void *)__entry->ip)
 );
include/trace/events/ext4.h +3 -3
index 6e5abd6d38a2..47fca36ee426 100644
@@ -240,7 +240,7 @@ TRACE_EVENT(ext4_mark_inode_dirty,
 		__entry->ip	= IP;
 	),
 
-	TP_printk("dev %d,%d ino %lu caller %pF",
+	TP_printk("dev %d,%d ino %lu caller %pS",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
 		  (unsigned long) __entry->ino, (void *)__entry->ip)
 );

@@ -1762,7 +1762,7 @@ TRACE_EVENT(ext4_journal_start,
 		__entry->rsv_blocks	 = rsv_blocks;
 	),
 
-	TP_printk("dev %d,%d blocks, %d rsv_blocks, %d caller %pF",
+	TP_printk("dev %d,%d blocks, %d rsv_blocks, %d caller %pS",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
 		  __entry->blocks, __entry->rsv_blocks, (void *)__entry->ip)
 );

@@ -1784,7 +1784,7 @@ TRACE_EVENT(ext4_journal_start_reserved,
 		__entry->blocks		 = blocks;
 	),
 
-	TP_printk("dev %d,%d blocks, %d caller %pF",
+	TP_printk("dev %d,%d blocks, %d caller %pS",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
 		  __entry->blocks, (void *)__entry->ip)
 );
include/trace/events/module.h +2 -2
index 81c4c183d348..28c45997e451 100644
@@ -84,7 +84,7 @@ DECLARE_EVENT_CLASS(module_refcnt,
 		__assign_str(name, mod->name);
 	),
 
-	TP_printk("%s call_site=%pf refcnt=%d",
+	TP_printk("%s call_site=%ps refcnt=%d",
 		  __get_str(name), (void *)__entry->ip, __entry->refcnt)
 );
 

@@ -121,7 +121,7 @@ TRACE_EVENT(module_request,
 		__assign_str(name, name);
 	),
 
-	TP_printk("%s wait=%d call_site=%pf",
+	TP_printk("%s wait=%d call_site=%ps",
 		  __get_str(name), (int)__entry->wait, (void *)__entry->ip)
 );
include/trace/events/random.h +5 -5
index 805af6db41cc..4684de344c5d 100644
@@ -22,7 +22,7 @@ TRACE_EVENT(add_device_randomness,
 		__entry->IP		= IP;
 	),
 
-	TP_printk("bytes %d caller %pF",
+	TP_printk("bytes %d caller %pS",
 		__entry->bytes, (void *)__entry->IP)
 );
 

@@ -43,7 +43,7 @@ DECLARE_EVENT_CLASS(random__mix_pool_bytes,
 		__entry->IP		= IP;
 	),
 
-	TP_printk("%s pool: bytes %d caller %pF",
+	TP_printk("%s pool: bytes %d caller %pS",
 		  __entry->pool_name, __entry->bytes, (void *)__entry->IP)
 );
 

@@ -82,7 +82,7 @@ TRACE_EVENT(credit_entropy_bits,
 	),
 
 	TP_printk("%s pool: bits %d entropy_count %d entropy_total %d "
-		  "caller %pF", __entry->pool_name, __entry->bits,
+		  "caller %pS", __entry->pool_name, __entry->bits,
 		  __entry->entropy_count, __entry->entropy_total,
 		  (void *)__entry->IP)
 );

@@ -207,7 +207,7 @@ DECLARE_EVENT_CLASS(random__get_random_bytes,
 		__entry->IP		= IP;
 	),
 
-	TP_printk("nbytes %d caller %pF", __entry->nbytes, (void *)__entry->IP)
+	TP_printk("nbytes %d caller %pS", __entry->nbytes, (void *)__entry->IP)
 );
 
 DEFINE_EVENT(random__get_random_bytes, get_random_bytes,

@@ -242,7 +242,7 @@ DECLARE_EVENT_CLASS(random__extract_entropy,
 		__entry->IP		= IP;
 	),
 
-	TP_printk("%s pool: nbytes %d entropy_count %d caller %pF",
+	TP_printk("%s pool: nbytes %d entropy_count %d caller %pS",
 		  __entry->pool_name, __entry->nbytes, __entry->entropy_count,
 		  (void *)__entry->IP)
 );
kernel/trace/trace_entries.h +3 -3
index e2d027ac66a2..ee7b94a4810a 100644
@@ -223,7 +223,7 @@ FTRACE_ENTRY(bprint, bprint_entry,
 		__dynamic_array(	u32,	buf	)
 	),
 
-	F_printk("%pf: %s",
+	F_printk("%ps: %s",
 		 (void *)__entry->ip, __entry->fmt),
 
 	FILTER_OTHER

@@ -238,7 +238,7 @@ FTRACE_ENTRY(print, print_entry,
 		__dynamic_array(	char,	buf	)
 	),
 
-	F_printk("%pf: %s",
+	F_printk("%ps: %s",
 		 (void *)__entry->ip, __entry->buf),
 
 	FILTER_OTHER

@@ -253,7 +253,7 @@ FTRACE_ENTRY(bputs, bputs_entry,
 		__field(	const char *,	str	)
 	),
 
-	F_printk("%pf: %s",
+	F_printk("%ps: %s",
 		 (void *)__entry->ip, __entry->str),
 
 	FILTER_OTHER
tools/lib/traceevent/event-parse.c +1 -1
index afe20ed9fac8..2c0bd8f2aad0 100644
@@ -3976,7 +3976,7 @@ static struct print_arg *make_bprint_args(char *fmt, void *data, int size, struc
 	if (asprintf(&arg->atom.atom, "%lld", ip) < 0)
 		goto out_free;
 
-	/* skip the first "%pf: " */
+	/* skip the first "%ps: " */
 	for (ptr = fmt + 5, bptr = data + field->offset;
 	     bptr < data + size && *ptr; ptr++) {
 		int ls = 0;
Signature · 1 line
2.1.4
author maintainer
#9 lore raw

PATCH 3/4 tracing: remove ftrace:function TRACE_EVENT_FL_USE_CALL_FILTER flag

In reply to Steven Rostedt

From: He Kuang <[email protected]>

TRACE_EVENT_FL_USE_CALL_FILTER flag in ftrace:functon event can be removed. This flag was first introduced in commit f306cc82a93d ("tracing: Update event filters for multibuffer").

Now, the only place uses this flag is ftrace:function, but the filter of ftrace:function has a different code path with events/syscalls and events/tracepoints. It uses ftrace_filter_write() and perf's ftrace_profile_set_filter() to set the filter, the functionality of file 'tracing/events/ftrace/function/filter' is bypassed in function init_pred(), in which case, neither call->filter nor file->filter is used.

So we can safely remove TRACE_EVENT_FL_USE_CALL_FILTER flag from ftrace:function events.

Link http://lkml.kernel.org/r/[email protected] open here
Signed-off-by He Kuang <[email protected]>
Signed-off-by Steven Rostedt <[email protected]>
---
 kernel/trace/trace_export.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/trace/trace_export.c +1 -1
index 12e2b99be862..174a6a71146c 100644
@@ -177,7 +177,7 @@ struct ftrace_event_call __used event_##call = {			\
 	},								\
 	.event.type		= etype,				\
 	.print_fmt		= print,				\
-	.flags			= TRACE_EVENT_FL_IGNORE_ENABLE | TRACE_EVENT_FL_USE_CALL_FILTER, \
+	.flags			= TRACE_EVENT_FL_IGNORE_ENABLE,		\
 };									\
 struct ftrace_event_call __used						\
 __attribute__((section("_ftrace_events"))) *__event_##call = &event_##call;
Signature · 1 line
2.1.4
author maintainer
#10 lore raw

PATCH 4/4 trace: Dont use __weak in header files

In reply to Steven Rostedt

From: Stephen Rothwell <[email protected]>

The commit that added a check for this to checkpatch says:

"Using weak declarations can have unintended link defects. The __weak on the declaration causes non-weak definitions to become weak."

In this case, when a PowerPC kernel is built with CONFIG_KPROBE_EVENT but not CONFIG_UPROBE_EVENT, it generates the following warning:

WARNING: 1 bad relocations
c0000000014f2190 R_PPC64_ADDR64 uprobes_fetch_type_table

This is fixed by passing the fetch_table arrays to traceprobe_parse_probe_arg() which also means that they can never be NULL.

Link http://lkml.kernel.org/r/[email protected] open here
Acked-by Masami Hiramatsu <[email protected]>
Signed-off-by Stephen Rothwell <[email protected]>
Signed-off-by Steven Rostedt <[email protected]>
---
 kernel/trace/trace_kprobe.c |  5 +++--
 kernel/trace/trace_probe.c  | 19 +++++++------------
 kernel/trace/trace_probe.h  | 10 ++--------
 kernel/trace/trace_uprobe.c |  5 +++--
 4 files changed, 15 insertions(+), 24 deletions(-)
kernel/trace/trace_kprobe.c +3 -2
index d73f565b4e06..f34c3ad1b5f4 100644
@@ -250,7 +250,7 @@ DEFINE_FETCH_symbol(string_size)
 #define fetch_file_offset_string_size	NULL
 
 /* Fetch type information table */
-const struct fetch_type kprobes_fetch_type_table[] = {
+static const struct fetch_type kprobes_fetch_type_table[] = {
 	/* Special types */
 	[FETCH_TYPE_STRING] = __ASSIGN_FETCH_TYPE("string", string, string,
 					sizeof(u32), 1, "__data_loc char[]"),

@@ -760,7 +760,8 @@ static int create_trace_kprobe(int argc, char **argv)
 
 		/* Parse fetch argument */
 		ret = traceprobe_parse_probe_arg(arg, &tk->tp.size, parg,
-						is_return, true);
+						is_return, true,
+						kprobes_fetch_type_table);
 		if (ret) {
 			pr_info("Parse error at argument[%d]. (%d)\n", i, ret);
 			goto error;
kernel/trace/trace_probe.c +7 -12
index b983b2fd2ca1..1769a81da8a7 100644
@@ -356,17 +356,14 @@ static int parse_probe_vars(char *arg, const struct fetch_type *t,
 
 /* Recursive argument parser */
 static int parse_probe_arg(char *arg, const struct fetch_type *t,
-		     struct fetch_param *f, bool is_return, bool is_kprobe)
+		     struct fetch_param *f, bool is_return, bool is_kprobe,
+		     const struct fetch_type *ftbl)
 {
-	const struct fetch_type *ftbl;
 	unsigned long param;
 	long offset;
 	char *tmp;
 	int ret = 0;
 
-	ftbl = is_kprobe ? kprobes_fetch_type_table : uprobes_fetch_type_table;
-	BUG_ON(ftbl == NULL);
-
 	switch (arg[0]) {
 	case '$':
 		ret = parse_probe_vars(arg + 1, t, f, is_return, is_kprobe);

@@ -447,7 +444,7 @@ static int parse_probe_arg(char *arg, const struct fetch_type *t,
 			dprm->fetch_size = get_fetch_size_function(t,
 							dprm->fetch, ftbl);
 			ret = parse_probe_arg(arg, t2, &dprm->orig, is_return,
-							is_kprobe);
+							is_kprobe, ftbl);
 			if (ret)
 				kfree(dprm);
 			else {

@@ -505,15 +502,12 @@ static int __parse_bitfield_probe_arg(const char *bf,
 
 /* String length checking wrapper */
 int traceprobe_parse_probe_arg(char *arg, ssize_t *size,
-		struct probe_arg *parg, bool is_return, bool is_kprobe)
+		struct probe_arg *parg, bool is_return, bool is_kprobe,
+		const struct fetch_type *ftbl)
 {
-	const struct fetch_type *ftbl;
 	const char *t;
 	int ret;
 
-	ftbl = is_kprobe ? kprobes_fetch_type_table : uprobes_fetch_type_table;
-	BUG_ON(ftbl == NULL);
-
 	if (strlen(arg) > MAX_ARGSTR_LEN) {
 		pr_info("Argument is too long.: %s\n",  arg);
 		return -ENOSPC;

@@ -535,7 +529,8 @@ int traceprobe_parse_probe_arg(char *arg, ssize_t *size,
 	}
 	parg->offset = *size;
 	*size += parg->type->size;
-	ret = parse_probe_arg(arg, parg->type, &parg->fetch, is_return, is_kprobe);
+	ret = parse_probe_arg(arg, parg->type, &parg->fetch, is_return,
+			      is_kprobe, ftbl);
 
 	if (ret >= 0 && t != NULL)
 		ret = __parse_bitfield_probe_arg(t, parg->type, &parg->fetch);
kernel/trace/trace_probe.h +2 -8
index 4f815fbce16d..e30f6cce4af6 100644
@@ -229,13 +229,6 @@ ASSIGN_FETCH_FUNC(file_offset, ftype),			\
 #define FETCH_TYPE_STRING	0
 #define FETCH_TYPE_STRSIZE	1
 
-/*
- * Fetch type information table.
- * It's declared as a weak symbol due to conditional compilation.
- */
-extern __weak const struct fetch_type kprobes_fetch_type_table[];
-extern __weak const struct fetch_type uprobes_fetch_type_table[];
-
 #ifdef CONFIG_KPROBE_EVENT
 struct symbol_cache;
 unsigned long update_symbol_cache(struct symbol_cache *sc);

@@ -333,7 +326,8 @@ find_event_file_link(struct trace_probe *tp, struct ftrace_event_file *file)
 }
 
 extern int traceprobe_parse_probe_arg(char *arg, ssize_t *size,
-		   struct probe_arg *parg, bool is_return, bool is_kprobe);
+		   struct probe_arg *parg, bool is_return, bool is_kprobe,
+		   const struct fetch_type *ftbl);
 
 extern int traceprobe_conflict_field_name(const char *name,
 			       struct probe_arg *args, int narg);
kernel/trace/trace_uprobe.c +3 -2
index 7dc1c8abecd6..74865465e0b7 100644
@@ -196,7 +196,7 @@ DEFINE_FETCH_file_offset(string)
 DEFINE_FETCH_file_offset(string_size)
 
 /* Fetch type information table */
-const struct fetch_type uprobes_fetch_type_table[] = {
+static const struct fetch_type uprobes_fetch_type_table[] = {
 	/* Special types */
 	[FETCH_TYPE_STRING] = __ASSIGN_FETCH_TYPE("string", string, string,
 					sizeof(u32), 1, "__data_loc char[]"),

@@ -535,7 +535,8 @@ static int create_trace_uprobe(int argc, char **argv)
 
 		/* Parse fetch argument */
 		ret = traceprobe_parse_probe_arg(arg, &tu->tp.size, parg,
-						 is_return, false);
+						 is_return, false,
+						 uprobes_fetch_type_table);
 		if (ret) {
 			pr_info("Parse error at argument[%d]. (%d)\n", i, ret);
 			goto error;
Signature · 1 line
2.1.4