All of 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 related	[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 related	[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 related	[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 related	[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 related	[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 related	[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 related	[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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.