LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 0/7] [GIT PULL] tracing: Some more fixes for 4.17
@ 2018-05-02 20:00 Steven Rostedt
  2018-05-02 20:00 ` [PATCH 1/7] tracing: Fix bad use of igrab in trace_uprobe.c Steven Rostedt
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Steven Rostedt @ 2018-05-02 20:00 UTC (permalink / raw)
  To: linux-kernel; +Cc: Linus Torvalds, Ingo Molnar, Andrew Morton


Linus,

Various fixes in tracing:

 - Tracepoints should not give warning on OOM failures

 - Use special field for function pointer in trace event

 - Fix igrab issues in uprobes

 - Fixes to the new histogram triggers

Please pull the latest trace-v4.17-rc1-2 tree, which can be found at:


  git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
trace-v4.17-rc1-2

Tag SHA1: a21207bca13342cd75fdb77962f4bc0ab96d9371
Head SHA1: d66a270be3310d7aa132fec0cea77d3d32a0ff75


Mathieu Desnoyers (1):
      tracepoint: Do not warn on ENOMEM

Rishabh Bhatnagar (1):
      tracing: initcall: Ordered comparison of function pointers

Song Liu (2):
      tracing: Fix bad use of igrab in trace_uprobe.c
      tracing: Remove igrab() iput() call from uprobes.c

Tom Zanussi (3):
      tracing: Restore proper field flag printing when displaying triggers
      tracing: Add field parsing hist error for hist triggers
      tracing: Add field modifier parsing hist error for hist triggers

----
 include/trace/events/initcall.h  | 14 +++++++++++---
 kernel/events/uprobes.c          |  7 +++----
 kernel/trace/trace_events_hist.c | 12 ++++++++++++
 kernel/trace/trace_uprobe.c      | 35 ++++++++++++++---------------------
 kernel/tracepoint.c              |  4 ++--
 5 files changed, 42 insertions(+), 30 deletions(-)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/7] tracing: Fix bad use of igrab in trace_uprobe.c
  2018-05-02 20:00 [PATCH 0/7] [GIT PULL] tracing: Some more fixes for 4.17 Steven Rostedt
@ 2018-05-02 20:00 ` Steven Rostedt
  2018-05-02 20:00 ` [PATCH 2/7] tracing: Remove igrab() iput() call from uprobes.c Steven Rostedt
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Steven Rostedt @ 2018-05-02 20:00 UTC (permalink / raw)
  To: linux-kernel
  Cc: Linus Torvalds, Ingo Molnar, Andrew Morton, stable, Ingo Molnar,
	Howard McLauchlan, Josef Bacik, Srikar Dronamraju,
	Miklos Szeredi, Miklos Szeredi, Song Liu

[-- Attachment #1: 0001-tracing-Fix-bad-use-of-igrab-in-trace_uprobe.c.patch --]
[-- Type: text/plain, Size: 5415 bytes --]

From: Song Liu <songliubraving@fb.com>

As Miklos reported and suggested:

  This pattern repeats two times in trace_uprobe.c and in
  kernel/events/core.c as well:

      ret = kern_path(filename, LOOKUP_FOLLOW, &path);
      if (ret)
          goto fail_address_parse;

      inode = igrab(d_inode(path.dentry));
      path_put(&path);

  And it's wrong.  You can only hold a reference to the inode if you
  have an active ref to the superblock as well (which is normally
  through path.mnt) or holding s_umount.

  This way unmounting the containing filesystem while the tracepoint is
  active will give you the "VFS: Busy inodes after unmount..." message
  and a crash when the inode is finally put.

  Solution: store path instead of inode.

This patch fixes two instances in trace_uprobe.c. struct path is added to
struct trace_uprobe to keep the inode and containing mount point
referenced.

Link: http://lkml.kernel.org/r/20180423172135.4050588-1-songliubraving@fb.com

Fixes: f3f096cfedf8 ("tracing: Provide trace events interface for uprobes")
Fixes: 33ea4b24277b ("perf/core: Implement the 'perf_uprobe' PMU")
Cc: stable@vger.kernel.org
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Howard McLauchlan <hmclauchlan@fb.com>
Cc: Josef Bacik <jbacik@fb.com>
Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Acked-by: Miklos Szeredi <mszeredi@redhat.com>
Reported-by: Miklos Szeredi <miklos@szeredi.hu>
Signed-off-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace_uprobe.c | 35 ++++++++++++++---------------------
 1 file changed, 14 insertions(+), 21 deletions(-)

diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 34fd0e0ec51d..ac892878dbe6 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -55,6 +55,7 @@ struct trace_uprobe {
 	struct list_head		list;
 	struct trace_uprobe_filter	filter;
 	struct uprobe_consumer		consumer;
+	struct path			path;
 	struct inode			*inode;
 	char				*filename;
 	unsigned long			offset;
@@ -289,7 +290,7 @@ static void free_trace_uprobe(struct trace_uprobe *tu)
 	for (i = 0; i < tu->tp.nr_args; i++)
 		traceprobe_free_probe_arg(&tu->tp.args[i]);
 
-	iput(tu->inode);
+	path_put(&tu->path);
 	kfree(tu->tp.call.class->system);
 	kfree(tu->tp.call.name);
 	kfree(tu->filename);
@@ -363,7 +364,6 @@ static int register_trace_uprobe(struct trace_uprobe *tu)
 static int create_trace_uprobe(int argc, char **argv)
 {
 	struct trace_uprobe *tu;
-	struct inode *inode;
 	char *arg, *event, *group, *filename;
 	char buf[MAX_EVENT_NAME_LEN];
 	struct path path;
@@ -371,7 +371,6 @@ static int create_trace_uprobe(int argc, char **argv)
 	bool is_delete, is_return;
 	int i, ret;
 
-	inode = NULL;
 	ret = 0;
 	is_delete = false;
 	is_return = false;
@@ -437,21 +436,16 @@ static int create_trace_uprobe(int argc, char **argv)
 	}
 	/* Find the last occurrence, in case the path contains ':' too. */
 	arg = strrchr(argv[1], ':');
-	if (!arg) {
-		ret = -EINVAL;
-		goto fail_address_parse;
-	}
+	if (!arg)
+		return -EINVAL;
 
 	*arg++ = '\0';
 	filename = argv[1];
 	ret = kern_path(filename, LOOKUP_FOLLOW, &path);
 	if (ret)
-		goto fail_address_parse;
-
-	inode = igrab(d_real_inode(path.dentry));
-	path_put(&path);
+		return ret;
 
-	if (!inode || !S_ISREG(inode->i_mode)) {
+	if (!d_is_reg(path.dentry)) {
 		ret = -EINVAL;
 		goto fail_address_parse;
 	}
@@ -490,7 +484,7 @@ static int create_trace_uprobe(int argc, char **argv)
 		goto fail_address_parse;
 	}
 	tu->offset = offset;
-	tu->inode = inode;
+	tu->path = path;
 	tu->filename = kstrdup(filename, GFP_KERNEL);
 
 	if (!tu->filename) {
@@ -558,7 +552,7 @@ static int create_trace_uprobe(int argc, char **argv)
 	return ret;
 
 fail_address_parse:
-	iput(inode);
+	path_put(&path);
 
 	pr_info("Failed to parse address or file.\n");
 
@@ -922,6 +916,7 @@ probe_event_enable(struct trace_uprobe *tu, struct trace_event_file *file,
 		goto err_flags;
 
 	tu->consumer.filter = filter;
+	tu->inode = d_real_inode(tu->path.dentry);
 	ret = uprobe_register(tu->inode, tu->offset, &tu->consumer);
 	if (ret)
 		goto err_buffer;
@@ -967,6 +962,7 @@ probe_event_disable(struct trace_uprobe *tu, struct trace_event_file *file)
 	WARN_ON(!uprobe_filter_is_empty(&tu->filter));
 
 	uprobe_unregister(tu->inode, tu->offset, &tu->consumer);
+	tu->inode = NULL;
 	tu->tp.flags &= file ? ~TP_FLAG_TRACE : ~TP_FLAG_PROFILE;
 
 	uprobe_buffer_disable();
@@ -1337,7 +1333,6 @@ struct trace_event_call *
 create_local_trace_uprobe(char *name, unsigned long offs, bool is_return)
 {
 	struct trace_uprobe *tu;
-	struct inode *inode;
 	struct path path;
 	int ret;
 
@@ -1345,11 +1340,8 @@ create_local_trace_uprobe(char *name, unsigned long offs, bool is_return)
 	if (ret)
 		return ERR_PTR(ret);
 
-	inode = igrab(d_inode(path.dentry));
-	path_put(&path);
-
-	if (!inode || !S_ISREG(inode->i_mode)) {
-		iput(inode);
+	if (!d_is_reg(path.dentry)) {
+		path_put(&path);
 		return ERR_PTR(-EINVAL);
 	}
 
@@ -1364,11 +1356,12 @@ create_local_trace_uprobe(char *name, unsigned long offs, bool is_return)
 	if (IS_ERR(tu)) {
 		pr_info("Failed to allocate trace_uprobe.(%d)\n",
 			(int)PTR_ERR(tu));
+		path_put(&path);
 		return ERR_CAST(tu);
 	}
 
 	tu->offset = offs;
-	tu->inode = inode;
+	tu->path = path;
 	tu->filename = kstrdup(name, GFP_KERNEL);
 	init_trace_event_call(tu, &tu->tp.call);
 
-- 
2.17.0

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 2/7] tracing: Remove igrab() iput() call from uprobes.c
  2018-05-02 20:00 [PATCH 0/7] [GIT PULL] tracing: Some more fixes for 4.17 Steven Rostedt
  2018-05-02 20:00 ` [PATCH 1/7] tracing: Fix bad use of igrab in trace_uprobe.c Steven Rostedt
@ 2018-05-02 20:00 ` Steven Rostedt
  2018-05-02 20:00 ` [PATCH 3/7] tracing: initcall: Ordered comparison of function pointers Steven Rostedt
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Steven Rostedt @ 2018-05-02 20:00 UTC (permalink / raw)
  To: linux-kernel
  Cc: Linus Torvalds, Ingo Molnar, Andrew Morton, Ingo Molnar,
	Howard McLauchlan, Josef Bacik, Srikar Dronamraju,
	Miklos Szeredi, Song Liu

[-- Attachment #1: 0002-tracing-Remove-igrab-iput-call-from-uprobes.c.patch --]
[-- Type: text/plain, Size: 2618 bytes --]

From: Song Liu <songliubraving@fb.com>

Caller of uprobe_register is required to keep the inode and containing
mount point referenced.

There was misuse of igrab() in uprobes.c and trace_uprobe.c. This is
because igrab() will not prevent umount of the containing mount point.
To fix this, we added path to struct trace_uprobe, which keeps the inode
and containing mount reference.

For uprobes.c, it is not necessary to call igrab() in uprobe_register(),
as the caller is required to keep the inode reference. The igrab() is
removed and comments on this requirement is added to uprobe_register().

Link: http://lkml.kernel.org/r/CAELBmZB2XX=qEOLAdvGG4cPx4GEntcSnWQquJLUK1ongRj35cA@mail.gmail.com
Link: http://lkml.kernel.org/r/20180423172135.4050588-2-songliubraving@fb.com

Cc: Ingo Molnar <mingo@redhat.com>
Cc: Howard McLauchlan <hmclauchlan@fb.com>
Cc: Josef Bacik <jbacik@fb.com>
Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Acked-by: Miklos Szeredi <mszeredi@redhat.com>
Signed-off-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/events/uprobes.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index ce6848e46e94..1725b902983f 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -491,7 +491,7 @@ static struct uprobe *alloc_uprobe(struct inode *inode, loff_t offset)
 	if (!uprobe)
 		return NULL;
 
-	uprobe->inode = igrab(inode);
+	uprobe->inode = inode;
 	uprobe->offset = offset;
 	init_rwsem(&uprobe->register_rwsem);
 	init_rwsem(&uprobe->consumer_rwsem);
@@ -502,7 +502,6 @@ static struct uprobe *alloc_uprobe(struct inode *inode, loff_t offset)
 	if (cur_uprobe) {
 		kfree(uprobe);
 		uprobe = cur_uprobe;
-		iput(inode);
 	}
 
 	return uprobe;
@@ -701,7 +700,6 @@ static void delete_uprobe(struct uprobe *uprobe)
 	rb_erase(&uprobe->rb_node, &uprobes_tree);
 	spin_unlock(&uprobes_treelock);
 	RB_CLEAR_NODE(&uprobe->rb_node); /* for uprobe_is_active() */
-	iput(uprobe->inode);
 	put_uprobe(uprobe);
 }
 
@@ -873,7 +871,8 @@ static void __uprobe_unregister(struct uprobe *uprobe, struct uprobe_consumer *u
  * tuple).  Creation refcount stops uprobe_unregister from freeing the
  * @uprobe even before the register operation is complete. Creation
  * refcount is released when the last @uc for the @uprobe
- * unregisters.
+ * unregisters. Caller of uprobe_register() is required to keep @inode
+ * (and the containing mount) referenced.
  *
  * Return errno if it cannot successully install probes
  * else return 0 (success)
-- 
2.17.0

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 3/7] tracing: initcall: Ordered comparison of function pointers
  2018-05-02 20:00 [PATCH 0/7] [GIT PULL] tracing: Some more fixes for 4.17 Steven Rostedt
  2018-05-02 20:00 ` [PATCH 1/7] tracing: Fix bad use of igrab in trace_uprobe.c Steven Rostedt
  2018-05-02 20:00 ` [PATCH 2/7] tracing: Remove igrab() iput() call from uprobes.c Steven Rostedt
@ 2018-05-02 20:00 ` Steven Rostedt
  2018-05-02 20:00 ` [PATCH 4/7] tracing: Restore proper field flag printing when displaying triggers Steven Rostedt
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Steven Rostedt @ 2018-05-02 20:00 UTC (permalink / raw)
  To: linux-kernel
  Cc: Linus Torvalds, Ingo Molnar, Andrew Morton, Rishabh Bhatnagar

[-- Attachment #1: 0003-tracing-initcall-Ordered-comparison-of-function-poin.patch --]
[-- Type: text/plain, Size: 1785 bytes --]

From: Rishabh Bhatnagar <rishabhb@codeaurora.org>

Using initcall_t in the __field macro generates the following warning
with clang version 6.0:

include/trace/events/initcall.h:34:3: warning: ordered comparison of
function pointers ('initcall_t' (aka 'int (*)(void)') and 'initcall_t')

__field macro expands to __field_ext macro which does is_signed_type
check on the type argument. Since initcall_t is defined as a function
pointer, using it as the type in the __field macro, leads to an ordered
comparison of function pointer warning, inside the check. Using
__field_struct macro avoids the issue.

Link: http://lkml.kernel.org/r/1524699755-29388-1-git-send-email-rishabhb@codeaurora.org

Signed-off-by: Rishabh Bhatnagar <rishabhb@codeaurora.org>
[ Added comment to why we are using field_struct() ]
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 include/trace/events/initcall.h | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/include/trace/events/initcall.h b/include/trace/events/initcall.h
index 8d6cf10d27c9..eb903c3f195f 100644
--- a/include/trace/events/initcall.h
+++ b/include/trace/events/initcall.h
@@ -31,7 +31,11 @@ TRACE_EVENT(initcall_start,
 	TP_ARGS(func),
 
 	TP_STRUCT__entry(
-		__field(initcall_t, func)
+		/*
+		 * Use field_struct to avoid is_signed_type()
+		 * comparison of a function pointer
+		 */
+		__field_struct(initcall_t, func)
 	),
 
 	TP_fast_assign(
@@ -48,8 +52,12 @@ TRACE_EVENT(initcall_finish,
 	TP_ARGS(func, ret),
 
 	TP_STRUCT__entry(
-		__field(initcall_t,	func)
-		__field(int,		ret)
+		/*
+		 * Use field_struct to avoid is_signed_type()
+		 * comparison of a function pointer
+		 */
+		__field_struct(initcall_t,	func)
+		__field(int,			ret)
 	),
 
 	TP_fast_assign(
-- 
2.17.0

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 4/7] tracing: Restore proper field flag printing when displaying triggers
  2018-05-02 20:00 [PATCH 0/7] [GIT PULL] tracing: Some more fixes for 4.17 Steven Rostedt
                   ` (2 preceding siblings ...)
  2018-05-02 20:00 ` [PATCH 3/7] tracing: initcall: Ordered comparison of function pointers Steven Rostedt
@ 2018-05-02 20:00 ` Steven Rostedt
  2018-05-02 20:00 ` [PATCH 5/7] tracing: Add field parsing hist error for hist triggers Steven Rostedt
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Steven Rostedt @ 2018-05-02 20:00 UTC (permalink / raw)
  To: linux-kernel; +Cc: Linus Torvalds, Ingo Molnar, Andrew Morton, Tom Zanussi

[-- Attachment #1: 0004-tracing-Restore-proper-field-flag-printing-when-disp.patch --]
[-- Type: text/plain, Size: 2691 bytes --]

From: Tom Zanussi <tom.zanussi@linux.intel.com>

The flag-printing code used when displaying hist triggers somehow got
dropped during refactoring of the inter-event patchset.  This restores
it.

Below are a couple examples - in the first case, .usecs wasn't being
displayed properly for common_timestamps and the second illustrates
the same for other flags such as .execname.

Before:

  # echo 'hist:key=common_pid.execname:val=count:sort=count' > /sys/kernel/debug/tracing/events/syscalls/sys_enter_read/trigger
  # cat /sys/kernel/debug/tracing/events/syscalls/sys_enter_read/trigger
  hist:keys=common_pid:vals=hitcount,count:sort=count:size=2048 [active]

  # echo 'hist:keys=pid:ts0=common_timestamp.usecs if comm=="cyclictest"' >> /sys/kernel/debug/tracing/events/sched/sched_wakeup/trigger
  # cat /sys/kernel/debug/tracing/events/sched/sched_wakeup/trigger
  hist:keys=pid:vals=hitcount:ts0=common_timestamp:sort=hitcount:size=2048:clock=global if comm=="cyclictest" [active]

After:

  # echo 'hist:key=common_pid.execname:val=count:sort=count' > /sys/kernel/debug/tracing/events/syscalls/sys_enter_read/trigger
  # cat /sys/kernel/debug/tracing/events/syscalls/sys_enter_read/trigger
  hist:keys=common_pid.execname:vals=hitcount,count:sort=count:size=2048 [active]

  # echo 'hist:keys=pid:ts0=common_timestamp.usecs if comm=="cyclictest"' >> /sys/kernel/debug/tracing/events/sched/sched_wakeup/trigger
  # cat /sys/kernel/debug/tracing/events/sched/sched_wakeup/trigger
  hist:keys=pid:vals=hitcount:ts0=common_timestamp.usecs:sort=hitcount:size=2048:clock=global if comm=="cyclictest" [active]

Link: http://lkml.kernel.org/r/492bab42ff21806600af98a8ea901af10efbee0c.1524790601.git.tom.zanussi@linux.intel.com

Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace_events_hist.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 0d7b3ffbecc2..66c87be4ebb2 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -4913,6 +4913,16 @@ static void hist_field_print(struct seq_file *m, struct hist_field *hist_field)
 		seq_printf(m, "%s", field_name);
 	} else if (hist_field->flags & HIST_FIELD_FL_TIMESTAMP)
 		seq_puts(m, "common_timestamp");
+
+	if (hist_field->flags) {
+		if (!(hist_field->flags & HIST_FIELD_FL_VAR_REF) &&
+		    !(hist_field->flags & HIST_FIELD_FL_EXPR)) {
+			const char *flags = get_hist_field_flags(hist_field);
+
+			if (flags)
+				seq_printf(m, ".%s", flags);
+		}
+	}
 }
 
 static int event_hist_trigger_print(struct seq_file *m,
-- 
2.17.0

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 5/7] tracing: Add field parsing hist error for hist triggers
  2018-05-02 20:00 [PATCH 0/7] [GIT PULL] tracing: Some more fixes for 4.17 Steven Rostedt
                   ` (3 preceding siblings ...)
  2018-05-02 20:00 ` [PATCH 4/7] tracing: Restore proper field flag printing when displaying triggers Steven Rostedt
@ 2018-05-02 20:00 ` Steven Rostedt
  2018-05-02 20:00 ` [PATCH 6/7] tracing: Add field modifier " Steven Rostedt
  2018-05-02 20:00 ` [PATCH 7/7] tracepoint: Do not warn on ENOMEM Steven Rostedt
  6 siblings, 0 replies; 8+ messages in thread
From: Steven Rostedt @ 2018-05-02 20:00 UTC (permalink / raw)
  To: linux-kernel
  Cc: Linus Torvalds, Ingo Molnar, Andrew Morton, Tom Zanussi,
	Masami Hiramatsu

[-- Attachment #1: 0005-tracing-Add-field-parsing-hist-error-for-hist-trigge.patch --]
[-- Type: text/plain, Size: 1739 bytes --]

From: Tom Zanussi <tom.zanussi@linux.intel.com>

If the user specifies a nonexistent field for a hist trigger, the
current code correctly flags that as an error, but doesn't tell the
user what happened.

Fix this by invoking hist_err() with an appropriate message when
nonexistent fields are specified.

Before:

  # echo 'hist:keys=pid:ts0=common_timestamp.usecs' >> /sys/kernel/debug/tracing/events/sched/sched_switch/trigger
  -su: echo: write error: Invalid argument
  # cat /sys/kernel/debug/tracing/events/sched/sched_switch/hist

After:

  # echo 'hist:keys=pid:ts0=common_timestamp.usecs' >> /sys/kernel/debug/tracing/events/sched/sched_switch/trigger
  -su: echo: write error: Invalid argument
  # cat /sys/kernel/debug/tracing/events/sched/sched_switch/hist
  ERROR: Couldn't find field: pid
    Last command: keys=pid:ts0=common_timestamp.usecs

Link: http://lkml.kernel.org/r/fdc8746969d16906120f162b99dd71c741e0b62c.1524790601.git.tom.zanussi@linux.intel.com

Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
Reported-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace_events_hist.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 66c87be4ebb2..f231fa2a3dcd 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -2481,6 +2481,7 @@ parse_field(struct hist_trigger_data *hist_data, struct trace_event_file *file,
 	else {
 		field = trace_find_event_field(file->event_call, field_name);
 		if (!field || !field->size) {
+			hist_err("Couldn't find field: ", field_name);
 			field = ERR_PTR(-EINVAL);
 			goto out;
 		}
-- 
2.17.0

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 6/7] tracing: Add field modifier parsing hist error for hist triggers
  2018-05-02 20:00 [PATCH 0/7] [GIT PULL] tracing: Some more fixes for 4.17 Steven Rostedt
                   ` (4 preceding siblings ...)
  2018-05-02 20:00 ` [PATCH 5/7] tracing: Add field parsing hist error for hist triggers Steven Rostedt
@ 2018-05-02 20:00 ` Steven Rostedt
  2018-05-02 20:00 ` [PATCH 7/7] tracepoint: Do not warn on ENOMEM Steven Rostedt
  6 siblings, 0 replies; 8+ messages in thread
From: Steven Rostedt @ 2018-05-02 20:00 UTC (permalink / raw)
  To: linux-kernel; +Cc: Linus Torvalds, Ingo Molnar, Andrew Morton, Tom Zanussi

[-- Attachment #1: 0006-tracing-Add-field-modifier-parsing-hist-error-for-hi.patch --]
[-- Type: text/plain, Size: 1706 bytes --]

From: Tom Zanussi <tom.zanussi@linux.intel.com>

If the user specifies an invalid field modifier for a hist trigger,
the current code correctly flags that as an error, but doesn't tell
the user what happened.

Fix this by invoking hist_err() with an appropriate message when
invalid modifiers are specified.

Before:

  # echo 'hist:keys=pid:ts0=common_timestamp.junkusecs' >> /sys/kernel/debug/tracing/events/sched/sched_wakeup/trigger
  -su: echo: write error: Invalid argument
  # cat /sys/kernel/debug/tracing/events/sched/sched_wakeup/hist

After:

  # echo 'hist:keys=pid:ts0=common_timestamp.junkusecs' >> /sys/kernel/debug/tracing/events/sched/sched_wakeup/trigger
  -su: echo: write error: Invalid argument
  # cat /sys/kernel/debug/tracing/events/sched/sched_wakeup/hist
  ERROR: Invalid field modifier: junkusecs
    Last command: keys=pid:ts0=common_timestamp.junkusecs

Link: http://lkml.kernel.org/r/b043c59fa79acd06a5f14a1d44dee9e5a3cd1248.1524790601.git.tom.zanussi@linux.intel.com

Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace_events_hist.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index f231fa2a3dcd..b9061ed59bbd 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -2466,6 +2466,7 @@ parse_field(struct hist_trigger_data *hist_data, struct trace_event_file *file,
 		else if (strcmp(modifier, "usecs") == 0)
 			*flags |= HIST_FIELD_FL_TIMESTAMP_USECS;
 		else {
+			hist_err("Invalid field modifier: ", modifier);
 			field = ERR_PTR(-EINVAL);
 			goto out;
 		}
-- 
2.17.0

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 7/7] tracepoint: Do not warn on ENOMEM
  2018-05-02 20:00 [PATCH 0/7] [GIT PULL] tracing: Some more fixes for 4.17 Steven Rostedt
                   ` (5 preceding siblings ...)
  2018-05-02 20:00 ` [PATCH 6/7] tracing: Add field modifier " Steven Rostedt
@ 2018-05-02 20:00 ` Steven Rostedt
  6 siblings, 0 replies; 8+ messages in thread
From: Steven Rostedt @ 2018-05-02 20:00 UTC (permalink / raw)
  To: linux-kernel
  Cc: Linus Torvalds, Ingo Molnar, Andrew Morton, Peter Zijlstra,
	Jiri Olsa, Arnaldo Carvalho de Melo, Alexander Shishkin,
	Namhyung Kim, stable, syzbot+9c0d616860575a73166a,
	syzbot+4e9ae7fa46233396f64d, Mathieu Desnoyers

[-- Attachment #1: 0007-tracepoint-Do-not-warn-on-ENOMEM.patch --]
[-- Type: text/plain, Size: 2091 bytes --]

From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

Tracepoint should only warn when a kernel API user does not respect the
required preconditions (e.g. same tracepoint enabled twice, or called
to remove a tracepoint that does not exist).

Silence warning in out-of-memory conditions, given that the error is
returned to the caller.

This ensures that out-of-memory error-injection testing does not trigger
warnings in tracepoint.c, which were seen by syzbot.

Link: https://lkml.kernel.org/r/001a114465e241a8720567419a72@google.com
Link: https://lkml.kernel.org/r/001a1140e0de15fc910567464190@google.com
Link: http://lkml.kernel.org/r/20180315124424.32319-1-mathieu.desnoyers@efficios.com

CC: Peter Zijlstra <peterz@infradead.org>
CC: Jiri Olsa <jolsa@redhat.com>
CC: Arnaldo Carvalho de Melo <acme@kernel.org>
CC: Alexander Shishkin <alexander.shishkin@linux.intel.com>
CC: Namhyung Kim <namhyung@kernel.org>
CC: stable@vger.kernel.org
Fixes: de7b2973903c6 ("tracepoint: Use struct pointer instead of name hash for reg/unreg tracepoints")
Reported-by: syzbot+9c0d616860575a73166a@syzkaller.appspotmail.com
Reported-by: syzbot+4e9ae7fa46233396f64d@syzkaller.appspotmail.com
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/tracepoint.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
index 671b13457387..1e37da2e0c25 100644
--- a/kernel/tracepoint.c
+++ b/kernel/tracepoint.c
@@ -207,7 +207,7 @@ static int tracepoint_add_func(struct tracepoint *tp,
 			lockdep_is_held(&tracepoints_mutex));
 	old = func_add(&tp_funcs, func, prio);
 	if (IS_ERR(old)) {
-		WARN_ON_ONCE(1);
+		WARN_ON_ONCE(PTR_ERR(old) != -ENOMEM);
 		return PTR_ERR(old);
 	}
 
@@ -239,7 +239,7 @@ static int tracepoint_remove_func(struct tracepoint *tp,
 			lockdep_is_held(&tracepoints_mutex));
 	old = func_remove(&tp_funcs, func);
 	if (IS_ERR(old)) {
-		WARN_ON_ONCE(1);
+		WARN_ON_ONCE(PTR_ERR(old) != -ENOMEM);
 		return PTR_ERR(old);
 	}
 
-- 
2.17.0

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-05-02 20:02 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-02 20:00 [PATCH 0/7] [GIT PULL] tracing: Some more fixes for 4.17 Steven Rostedt
2018-05-02 20:00 ` [PATCH 1/7] tracing: Fix bad use of igrab in trace_uprobe.c Steven Rostedt
2018-05-02 20:00 ` [PATCH 2/7] tracing: Remove igrab() iput() call from uprobes.c Steven Rostedt
2018-05-02 20:00 ` [PATCH 3/7] tracing: initcall: Ordered comparison of function pointers Steven Rostedt
2018-05-02 20:00 ` [PATCH 4/7] tracing: Restore proper field flag printing when displaying triggers Steven Rostedt
2018-05-02 20:00 ` [PATCH 5/7] tracing: Add field parsing hist error for hist triggers Steven Rostedt
2018-05-02 20:00 ` [PATCH 6/7] tracing: Add field modifier " Steven Rostedt
2018-05-02 20:00 ` [PATCH 7/7] tracepoint: Do not warn on ENOMEM Steven Rostedt

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).