linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>,
	Namhyung Kim <namhyung@kernel.org>,
	Srikar Dronamraju <srikar@linux.vnet.ibm.com>,
	Tom Zanussi <tom.zanussi@linux.intel.com>,
	"zhangwei(Jovi)" <jovi.zhangwei@huawei.com>,
	linux-kernel@vger.kernel.org
Subject: [PATCH 1/4] tracing/uprobes: Revert "Support mix of ftrace and perf"
Date: Fri, 27 Jun 2014 19:01:36 +0200	[thread overview]
Message-ID: <20140627170136.GA18319@redhat.com> (raw)
In-Reply-To: <20140627170116.GA18298@redhat.com>

This reverts commit 43fe98913c9f67e3b523615ee3316f9520a623e0.

This patch is very wrong. Firstly, this change leads to unbalanced
uprobe_unregister(). Just for example,

	# perf probe -x /lib/libc.so.6 syscall
	# echo 1 >> /sys/kernel/debug/tracing/events/probe_libc/enable
	# perf record -e probe_libc:syscall whatever

after that uprobe is dead (unregistered) but the user of ftrace/perf
can't know this, and it looks as if nobody hits this probe.

This would be easy to fix, but there are other reasons why it is not
simple to mix ftrace and perf. If nothing else, they can't share the
same ->consumer.filter. This is fixable too, but probably we need to
fix the poorly designed uprobe_register() interface first. At least
"register" and "apply" should be clearly separated.

Cc: stable@vger.kernel.org # v3.14
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 kernel/trace/trace_uprobe.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 04fdb5d..08e7970 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -893,6 +893,9 @@ probe_event_enable(struct trace_uprobe *tu, struct ftrace_event_file *file,
 	int ret;
 
 	if (file) {
+		if (tu->tp.flags & TP_FLAG_PROFILE)
+			return -EINTR;
+
 		link = kmalloc(sizeof(*link), GFP_KERNEL);
 		if (!link)
 			return -ENOMEM;
@@ -901,8 +904,12 @@ probe_event_enable(struct trace_uprobe *tu, struct ftrace_event_file *file,
 		list_add_tail_rcu(&link->list, &tu->tp.files);
 
 		tu->tp.flags |= TP_FLAG_TRACE;
-	} else
+	} else {
+		if (tu->tp.flags & TP_FLAG_TRACE)
+			return -EINTR;
+
 		tu->tp.flags |= TP_FLAG_PROFILE;
+	}
 
 	ret = uprobe_buffer_enable();
 	if (ret < 0)
-- 
1.5.5.1


  reply	other threads:[~2014-06-27 18:33 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-27 17:01 [PATCH 0/4] tracing/uprobes fixes Oleg Nesterov
2014-06-27 17:01 ` Oleg Nesterov [this message]
2014-06-30  5:49   ` [PATCH 1/4] tracing/uprobes: Revert "Support mix of ftrace and perf" Namhyung Kim
2014-06-30 18:48     ` Oleg Nesterov
2014-07-01 19:31       ` probe_event_disable()->synchronize_sched() (Was: tracing/uprobes: Revert "Support mix of ftrace and perf") Oleg Nesterov
2014-07-03  0:54         ` probe_event_disable()->synchronize_sched() Namhyung Kim
2014-07-03 15:41           ` probe_event_disable()->synchronize_sched() Oleg Nesterov
2014-07-03  5:35         ` probe_event_disable()->synchronize_sched() (Was: tracing/uprobes: Revert "Support mix of ftrace and perf") Masami Hiramatsu
2014-07-03  5:46         ` Masami Hiramatsu
2014-07-03  7:44           ` probe_event_disable()->synchronize_sched() Namhyung Kim
2014-07-04  1:00             ` probe_event_disable()->synchronize_sched() Masami Hiramatsu
2014-07-04  8:01               ` probe_event_disable()->synchronize_sched() Namhyung Kim
2014-07-03 16:22           ` probe_event_disable()->synchronize_sched() (Was: tracing/uprobes: Revert "Support mix of ftrace and perf") Oleg Nesterov
2014-07-03 17:01             ` __trace_remove_event_dirs() leaks file->filter ? (Was: probe_event_disable()->synchronize_sched()) Oleg Nesterov
2014-07-04  5:21               ` Masami Hiramatsu
2014-07-04 19:38                 ` Oleg Nesterov
2014-07-04  4:46             ` probe_event_disable()->synchronize_sched() (Was: tracing/uprobes: Revert "Support mix of ftrace and perf") Masami Hiramatsu
2014-06-30 11:52   ` [PATCH 1/4] tracing/uprobes: Revert "Support mix of ftrace and perf" Masami Hiramatsu
2014-06-30 16:56   ` Srikar Dronamraju
2014-06-27 17:01 ` [PATCH 2/4] uprobes: Change unregister/apply to WARN() if uprobe/consumer is gone Oleg Nesterov
2014-06-30  5:50   ` Namhyung Kim
2014-06-30 16:57   ` Srikar Dronamraju
2014-06-27 17:01 ` [PATCH 3/4] tracing/uprobes: Kill the bogus UPROBE_HANDLER_REMOVE code in uprobe_dispatcher() Oleg Nesterov
2014-06-30  6:03   ` Namhyung Kim
2014-06-30 16:57   ` Srikar Dronamraju
2014-06-27 17:01 ` [PATCH 4/4] tracing/uprobes: Fix the usage of uprobe_buffer_enable() in probe_event_enable() Oleg Nesterov
2014-06-30  6:18   ` Namhyung Kim
2014-06-30 11:49   ` Masami Hiramatsu
2014-06-30 17:04   ` Srikar Dronamraju
2014-06-30 17:21     ` Steven Rostedt
2014-06-30 17:58       ` Oleg Nesterov
2014-06-30 18:22         ` Steven Rostedt
2014-06-30 17:50     ` Oleg Nesterov
2014-06-30 18:01       ` Steven Rostedt
2014-06-30 13:28 ` [PATCH 0/4] tracing/uprobes fixes Steven Rostedt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140627170136.GA18319@redhat.com \
    --to=oleg@redhat.com \
    --cc=jovi.zhangwei@huawei.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=masami.hiramatsu.pt@hitachi.com \
    --cc=namhyung@kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=srikar@linux.vnet.ibm.com \
    --cc=tom.zanussi@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).