mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: linux-kernel@vger.kernel.org
Cc: Ingo Molnar <mingo@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: [for-next][PATCH 23/33] ftrace: Have each function probe use its own ftrace_ops
Date: Fri, 21 Apr 2017 17:30:43 -0400	[thread overview]
Message-ID: <20170421213331.540772419@goodmis.org> (raw)
In-Reply-To: <20170421213020.875637678@goodmis.org>

[-- Attachment #1: 0023-ftrace-Have-each-function-probe-use-its-own-ftrace_o.patch --]
[-- Type: text/plain, Size: 10092 bytes --]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

Have the function probes have their own ftrace_ops, and remove the
trace_probe_ops. This simplifies some of the ftrace infrastructure code.

Individual entries for each function is still allocated for the use of the
output for set_ftrace_filter, but they will be removed soon too.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/ftrace.c | 246 ++++++++++++++++++++------------------------------
 kernel/trace/trace.h  |   1 +
 2 files changed, 99 insertions(+), 148 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index cbae7fb1be15..cf6b7263199a 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -3789,63 +3789,6 @@ static void function_trace_probe_call(unsigned long ip, unsigned long parent_ip,
 	preempt_enable_notrace();
 }
 
-static struct ftrace_ops trace_probe_ops __read_mostly =
-{
-	.func		= function_trace_probe_call,
-	.flags		= FTRACE_OPS_FL_INITIALIZED,
-	INIT_OPS_HASH(trace_probe_ops)
-};
-
-static int ftrace_probe_registered;
-
-static void __enable_ftrace_function_probe(struct ftrace_ops_hash *old_hash)
-{
-	int ret;
-	int i;
-
-	if (ftrace_probe_registered) {
-		/* still need to update the function call sites */
-		if (ftrace_enabled)
-			ftrace_run_modify_code(&trace_probe_ops, FTRACE_UPDATE_CALLS,
-					       old_hash);
-		return;
-	}
-
-	for (i = 0; i < FTRACE_FUNC_HASHSIZE; i++) {
-		struct hlist_head *hhd = &ftrace_func_hash[i];
-		if (hhd->first)
-			break;
-	}
-	/* Nothing registered? */
-	if (i == FTRACE_FUNC_HASHSIZE)
-		return;
-
-	ret = ftrace_startup(&trace_probe_ops, 0);
-
-	ftrace_probe_registered = 1;
-}
-
-static bool __disable_ftrace_function_probe(void)
-{
-	int i;
-
-	if (!ftrace_probe_registered)
-		return false;
-
-	for (i = 0; i < FTRACE_FUNC_HASHSIZE; i++) {
-		struct hlist_head *hhd = &ftrace_func_hash[i];
-		if (hhd->first)
-			return false;
-	}
-
-	/* no more funcs left */
-	ftrace_shutdown(&trace_probe_ops, 0);
-
-	ftrace_probe_registered = 0;
-	return true;
-}
-
-
 static void ftrace_free_entry(struct ftrace_func_probe *entry)
 {
 	if (entry->ops->free)
@@ -3996,110 +3939,110 @@ void free_ftrace_func_mapper(struct ftrace_func_mapper *mapper,
 
 int
 register_ftrace_function_probe(char *glob, struct ftrace_probe_ops *ops,
-			      void *data)
+			       void *data)
 {
-	struct ftrace_ops_hash old_hash_ops;
-	struct ftrace_func_probe *entry;
-	struct ftrace_glob func_g;
-	struct ftrace_hash **orig_hash = &trace_probe_ops.func_hash->filter_hash;
-	struct ftrace_hash *old_hash = *orig_hash;
+	struct ftrace_func_entry *entry;
+	struct ftrace_func_probe *probe;
+	struct ftrace_hash **orig_hash;
+	struct ftrace_hash *old_hash;
 	struct ftrace_hash *hash;
-	struct ftrace_page *pg;
-	struct dyn_ftrace *rec;
-	int not;
+	struct hlist_head hl;
+	struct hlist_node *n;
 	unsigned long key;
 	int count = 0;
+	int size;
 	int ret;
+	int i;
 
-	func_g.type = filter_parse_regex(glob, strlen(glob),
-			&func_g.search, &not);
-	func_g.len = strlen(func_g.search);
-
-	/* we do not support '!' for function probes */
-	if (WARN_ON(not))
+	/* We do not support '!' for function probes */
+	if (WARN_ON(glob[0] == '!'))
 		return -EINVAL;
 
-	mutex_lock(&trace_probe_ops.func_hash->regex_lock);
-
-	old_hash_ops.filter_hash = old_hash;
-	/* Probes only have filters */
-	old_hash_ops.notrace_hash = NULL;
-
-	hash = alloc_and_copy_ftrace_hash(FTRACE_HASH_DEFAULT_BITS, old_hash);
-	if (!hash) {
-		count = -ENOMEM;
-		goto out;
+	if (!(ops->ops.flags & FTRACE_OPS_FL_INITIALIZED)) {
+		ops->ops.func = function_trace_probe_call;
+		ftrace_ops_init(&ops->ops);
 	}
 
-	if (unlikely(ftrace_disabled)) {
-		count = -ENODEV;
-		goto out;
-	}
-
-	mutex_lock(&ftrace_lock);
+	mutex_lock(&ops->ops.func_hash->regex_lock);
 
-	do_for_each_ftrace_rec(pg, rec) {
+	orig_hash = &ops->ops.func_hash->filter_hash;
+	old_hash = *orig_hash;
+	hash = alloc_and_copy_ftrace_hash(FTRACE_HASH_DEFAULT_BITS, old_hash);
 
-		if (rec->flags & FTRACE_FL_DISABLED)
-			continue;
+	ret = ftrace_match_records(hash, glob, strlen(glob));
 
-		if (!ftrace_match_record(rec, &func_g, NULL, 0))
-			continue;
+	/* Nothing found? */
+	if (!ret)
+		ret = -EINVAL;
 
-		entry = kmalloc(sizeof(*entry), GFP_KERNEL);
-		if (!entry) {
-			/* If we did not process any, then return error */
-			if (!count)
-				count = -ENOMEM;
-			goto out_unlock;
-		}
+	if (ret < 0)
+		goto out;
 
-		count++;
+	INIT_HLIST_HEAD(&hl);
 
-		/*
-		 * The caller might want to do something special
-		 * for each function we find. We call the callback
-		 * to give the caller an opportunity to do so.
-		 */
-		if (ops->init) {
-			if (ops->init(ops, rec->ip, data) < 0) {
-				/* caller does not like this func */
-				kfree(entry);
+	size = 1 << hash->size_bits;
+	for (i = 0; i < size; i++) {
+		hlist_for_each_entry(entry, &hash->buckets[i], hlist) {
+			if (ftrace_lookup_ip(old_hash, entry->ip))
 				continue;
+			probe = kmalloc(sizeof(*probe), GFP_KERNEL);
+			if (!probe) {
+				count = -ENOMEM;
+				goto err_free;
 			}
-		}
+			probe->ops = ops;
+			probe->ip = entry->ip;
+			/*
+			 * The caller might want to do something special
+			 * for each function we find. We call the callback
+			 * to give the caller an opportunity to do so.
+			 */
+			if (ops->init && ops->init(ops, entry->ip, data) < 0) {
+				kfree(probe);
+				goto err_free;
+			}
+			hlist_add_head(&probe->node, &hl);
 
-		ret = enter_record(hash, rec, 0);
-		if (ret < 0) {
-			kfree(entry);
-			count = ret;
-			goto out_unlock;
+			count++;
 		}
+	}
 
-		entry->ops = ops;
-		entry->ip = rec->ip;
+	mutex_lock(&ftrace_lock);
 
-		key = hash_long(entry->ip, FTRACE_HASH_BITS);
-		hlist_add_head_rcu(&entry->node, &ftrace_func_hash[key]);
+	ret = ftrace_hash_move_and_update_ops(&ops->ops, orig_hash,
+						      hash, 1);
+	if (ret < 0)
+		goto err_free_unlock;
 
-	} while_for_each_ftrace_rec();
+	hlist_for_each_entry_safe(probe, n, &hl, node) {
+		hlist_del(&probe->node);
+		key = hash_long(probe->ip, FTRACE_HASH_BITS);
+		hlist_add_head_rcu(&probe->node, &ftrace_func_hash[key]);
+	}
 
-	ret = ftrace_hash_move(&trace_probe_ops, 1, orig_hash, hash);
+	if (!(ops->ops.flags & FTRACE_OPS_FL_ENABLED))
+		ret = ftrace_startup(&ops->ops, 0);
 
-	__enable_ftrace_function_probe(&old_hash_ops);
+	mutex_unlock(&ftrace_lock);
 
 	if (!ret)
-		free_ftrace_hash_rcu(old_hash);
-	else
-		count = ret;
-
- out_unlock:
-	mutex_unlock(&ftrace_lock);
+		ret = count;
  out:
-	mutex_unlock(&trace_probe_ops.func_hash->regex_lock);
+	mutex_unlock(&ops->ops.func_hash->regex_lock);
 	free_ftrace_hash(hash);
 
-	return count;
+	return ret;
+
+ err_free_unlock:
+	mutex_unlock(&ftrace_lock);
+ err_free:
+	hlist_for_each_entry_safe(probe, n, &hl, node) {
+		hlist_del(&probe->node);
+		if (ops->free)
+			ops->free(ops, probe->ip, NULL);
+		kfree(probe);
+	}
+	goto out;
 }
 
 int
@@ -4110,14 +4053,16 @@ unregister_ftrace_function_probe_func(char *glob, struct ftrace_probe_ops *ops)
 	struct ftrace_func_probe *entry;
 	struct ftrace_func_probe *p;
 	struct ftrace_glob func_g;
-	struct ftrace_hash **orig_hash = &trace_probe_ops.func_hash->filter_hash;
-	struct ftrace_hash *old_hash = *orig_hash;
+	struct ftrace_hash **orig_hash;
+	struct ftrace_hash *old_hash;
 	struct list_head free_list;
-	struct ftrace_hash *hash;
+	struct ftrace_hash *hash = NULL;
 	struct hlist_node *tmp;
 	char str[KSYM_SYMBOL_LEN];
 	int i, ret;
-	bool disabled;
+
+	if (!(ops->ops.flags & FTRACE_OPS_FL_INITIALIZED))
+		return -EINVAL;
 
 	if (glob && (strcmp(glob, "*") == 0 || !strlen(glob)))
 		func_g.search = NULL;
@@ -4134,14 +4079,21 @@ unregister_ftrace_function_probe_func(char *glob, struct ftrace_probe_ops *ops)
 			return -EINVAL;
 	}
 
-	mutex_lock(&trace_probe_ops.func_hash->regex_lock);
+	mutex_lock(&ops->ops.func_hash->regex_lock);
+
+	orig_hash = &ops->ops.func_hash->filter_hash;
+	old_hash = *orig_hash;
+
+	ret = -EINVAL;
+	if (ftrace_hash_empty(old_hash))
+		goto out_unlock;
 
 	old_hash_ops.filter_hash = old_hash;
 	/* Probes only have filters */
 	old_hash_ops.notrace_hash = NULL;
 
 	ret = -ENOMEM;
-	hash = alloc_and_copy_ftrace_hash(FTRACE_HASH_DEFAULT_BITS, *orig_hash);
+	hash = alloc_and_copy_ftrace_hash(FTRACE_HASH_DEFAULT_BITS, old_hash);
 	if (!hash)
 		goto out_unlock;
 
@@ -4181,20 +4133,18 @@ unregister_ftrace_function_probe_func(char *glob, struct ftrace_probe_ops *ops)
 	}
 
 	mutex_lock(&ftrace_lock);
-	disabled = __disable_ftrace_function_probe();
-	/*
-	 * Remove after the disable is called. Otherwise, if the last
-	 * probe is removed, a null hash means *all enabled*.
-	 */
-	ret = ftrace_hash_move(&trace_probe_ops, 1, orig_hash, hash);
+
+	if (ftrace_hash_empty(hash))
+		ftrace_shutdown(&ops->ops, 0);
+
+	ret = ftrace_hash_move_and_update_ops(&ops->ops, orig_hash,
+					      hash, 1);
 
 	/* still need to update the function call sites */
-	if (ftrace_enabled && !disabled)
-		ftrace_run_modify_code(&trace_probe_ops, FTRACE_UPDATE_CALLS,
+	if (ftrace_enabled && !ftrace_hash_empty(hash))
+		ftrace_run_modify_code(&ops->ops, FTRACE_UPDATE_CALLS,
 				       &old_hash_ops);
 	synchronize_sched();
-	if (!ret)
-		free_ftrace_hash_rcu(old_hash);
 
 	list_for_each_entry_safe(entry, p, &free_list, free_list) {
 		list_del(&entry->free_list);
@@ -4203,7 +4153,7 @@ unregister_ftrace_function_probe_func(char *glob, struct ftrace_probe_ops *ops)
 	mutex_unlock(&ftrace_lock);
 
  out_unlock:
-	mutex_unlock(&trace_probe_ops.func_hash->regex_lock);
+	mutex_unlock(&ops->ops.func_hash->regex_lock);
 	free_ftrace_hash(hash);
 	return ret;
 }
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 31d80bff9ee6..e16c67c49de4 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -932,6 +932,7 @@ static inline void ftrace_pid_follow_fork(struct trace_array *tr, bool enable) {
 #if defined(CONFIG_FUNCTION_TRACER) && defined(CONFIG_DYNAMIC_FTRACE)
 
 struct ftrace_probe_ops {
+	struct ftrace_ops	ops;
 	void			(*func)(unsigned long ip,
 					unsigned long parent_ip,
 					struct ftrace_probe_ops *ops,
-- 
2.10.2

  parent reply	other threads:[~2017-04-21 21:38 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-21 21:30 [for-next][PATCH 00/33] tracing: More updates for 4.12 Steven Rostedt
2017-04-21 21:30 ` [for-next][PATCH 01/33] ftrace: Fix removing of second function probe Steven Rostedt
2017-04-21 21:30 ` [for-next][PATCH 02/33] ftrace: Fix indexing of t_hash_start() from t_next() Steven Rostedt
2017-04-21 21:30 ` [for-next][PATCH 03/33] tracing: Have the trace_event benchmark thread call cond_resched_rcu_qs() Steven Rostedt
2017-04-21 21:30 ` [for-next][PATCH 04/33] ftrace: Add function-fork trace option Steven Rostedt
2017-04-21 21:30 ` [for-next][PATCH 05/33] selftests: ftrace: Add -l/--logdir option Steven Rostedt
2017-04-21 21:30 ` [for-next][PATCH 06/33] selftests: ftrace: Add a way to reset triggers in the set_ftrace_filter file Steven Rostedt
2017-04-21 21:30 ` [for-next][PATCH 07/33] selftests: ftrace: Add a selftest to test event enable/disable func trigger Steven Rostedt
2017-04-21 21:30 ` [for-next][PATCH 08/33] selftests: ftrace: Add a test to test function triggers to start and stop tracing Steven Rostedt
2017-04-21 21:30 ` [for-next][PATCH 09/33] selftests: ftrace: Add test to test reading of set_ftrace_file Steven Rostedt
2017-05-22  3:23   ` Masami Hiramatsu
2017-05-26 15:02     ` Steven Rostedt
2017-04-21 21:30 ` [for-next][PATCH 10/33] ftrace: Move the probe function into the tracing directory Steven Rostedt
2017-04-21 21:30 ` [for-next][PATCH 11/33] ftrace: Move the function commands " Steven Rostedt
2017-04-21 21:30 ` [for-next][PATCH 12/33] ftrace: Remove unused "flags" field from struct ftrace_func_probe Steven Rostedt
2017-04-21 21:30 ` [for-next][PATCH 13/33] ftrace: Pass probe ops to probe function Steven Rostedt
2017-04-21 21:30 ` [for-next][PATCH 14/33] ftrace: Added ftrace_func_mapper for function probe triggers Steven Rostedt
2017-04-21 21:30 ` [for-next][PATCH 15/33] tracing: Have the snapshot trigger use the mapping helper functions Steven Rostedt
2017-04-21 21:30 ` [for-next][PATCH 16/33] ftrace: Convert the rest of the function trigger over to the mapping functions Steven Rostedt
2017-04-21 21:30 ` [for-next][PATCH 17/33] ftrace: Remove unused unregister_ftrace_function_probe() function Steven Rostedt
2017-04-21 21:30 ` [for-next][PATCH 18/33] ftrace: Remove unused unregister_ftrace_function_probe_all() function Steven Rostedt
2017-04-21 21:30 ` [for-next][PATCH 19/33] ftrace: Remove printing of data in showing of a function probe Steven Rostedt
2017-04-21 21:30 ` [for-next][PATCH 20/33] ftrace: Remove data field from ftrace_func_probe structure Steven Rostedt
2017-04-21 21:30 ` [for-next][PATCH 21/33] ftrace: Add helper function ftrace_hash_move_and_update_ops() Steven Rostedt
2017-04-21 21:30 ` [for-next][PATCH 22/33] ftrace: Have unregister_ftrace_function_probe_func() return a value Steven Rostedt
2017-04-21 21:30 ` Steven Rostedt [this message]
2017-04-21 21:30 ` [for-next][PATCH 24/33] ftrace: Have the function probes call their own function Steven Rostedt
2017-04-21 21:30 ` [for-next][PATCH 25/33] ftrace: If the hash for a probe fails to update then free what was initialized Steven Rostedt
2017-04-21 21:30 ` [for-next][PATCH 26/33] tracing: Have the trace_array hold the list of registered func probes Steven Rostedt
2017-04-21 21:30 ` [for-next][PATCH 27/33] tracing: Pass the trace_array into ftrace_probe_ops functions Steven Rostedt
2017-04-21 21:30 ` [for-next][PATCH 28/33] ftrace: Dynamically create the probe ftrace_ops for the trace_array Steven Rostedt
2017-04-21 21:30 ` [for-next][PATCH 29/33] tracing/ftrace: Add a better way to pass data via the probe functions Steven Rostedt
2017-04-21 21:30 ` [for-next][PATCH 30/33] tracing/ftrace: Allow instances to have their own function probes Steven Rostedt
2017-04-21 21:30 ` [for-next][PATCH 31/33] tracing/ftrace: Enable snapshot function trigger to work with instances Steven Rostedt
2017-04-21 21:30 ` [for-next][PATCH 32/33] tracing/ftrace: Allow for the traceonoff probe be unique to instances Steven Rostedt
2017-04-21 21:30 ` [for-next][PATCH 33/33] tracing/ftrace: Allow for instances to trigger their own stacktrace probes 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=20170421213331.540772419@goodmis.org \
    --to=rostedt@goodmis.org \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    /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

Powered by JetHome