ftrace: Allow multiple users to pick and choose functions to trace

RFC PATCH In review Steven Rostedt 3 participants 17 messages May 6, 2011 – May 10, 2011

Subsystems (3)

Tracing ftrace Tracing OS noise / latency tracers

lore

Share & tools

lore

Steven Rostedt

author maintainer

2011-05-06 15:47 UTC #0 lore raw

cover ftrace: Allow multiple users to pick and choose functions to trace

Ingo, et. al.

This is an RFC patch set of the work I did to allow multiple users to pick and choose which functions they would like to trace without being affected by other users (well, they are still affected, but they can choose their own functions).

Since this rewrote a lot of the guts of ftrace's function handling I broke it up into steps that I can sequentially think about how to make the conversion. It also helped in understanding the changes that had to be made.

The end result now has this:

o The function record list flags item now stores a ref count, and we no longer need to have the flags if the records are filtered or set for "notrace"

o Each ftrace_ops has its own set of functions to trace or not trace which is used to update the ref counts of the function records mentioned above. If the ftrace_ops is not currently active, the records are not touched when the filters are changed. If the filters are changed while the ftrace_ops is active, it is done in a way with RCU to make the updates.

o If only one ftrace_ops is registered for tracing (actively tracing), its handler is called directly from mcount. Unless the ftrace_ops was allocated and not a static/global core kernel variable (see below).

o If more than one ftrace_ops is registered, then a function is called that will iterate over the list of registered ftrace_ops, performing a check of each of its hashes to know if ftrace_ops handler should be called for that function.

o If a ftrace_ops is allocated and not a static/global core kernel variable, it will be forced to use the function that loops through the ftrace_ops even if its the only one. This is because that function disables preemption when it performs the loop. This is needed, because dynamically allocated ftrace_ops must call synchronize_sched() before being freed (which is done when the dynamic ftrace_ops has been unregistered).

o A file has been created in the debugfs/tracing directory called enabled_functions. This file contains a list of all functions that any ftrace_ops has registered, along with each functions ref count. This can be used to make sure nothing is tracing.

I'm posting this patch series as an RFC, but I do not expect it to change much. I still have to put it through more tests even though I ran several on each set already.

-- Steve

Steven Rostedt (13):
ftrace: Replace FTRACE_FL_NOTRACE flag with a hash of ignored functions
ftrace: Use hash instead for FTRACE_FL_FILTER
ftrace: Create a global_ops to hold the filter and notrace hashes
ftrace: Separate hash allocation and assignment
ftrace: Use counters to enable functions to trace
ftrace: Add enabled_functions file
ftrace: Add ops parameter to ftrace_startup/shutdown functions
ftrace: Have global_ops store the functions that are to be traced
ftrace: Free hash with call_rcu_sched()
ftrace: Implement separate user function filtering
ftrace: Allow dynamically allocated function tracers
ftrace: Modify ftrace_set_filter/notrace to take ops
ftrace: Add self-tests for multiple function trace users

----

 include/linux/ftrace.h                |   33 +-
 include/linux/kernel.h                |    1 +
 kernel/extable.c                      |    8 +
 kernel/trace/ftrace.c                 |  992 ++++++++++++++++++++++++++++-----
 kernel/trace/trace.h                  |    2 +
 kernel/trace/trace_functions.c        |    2 +
 kernel/trace/trace_irqsoff.c          |    1 +
 kernel/trace/trace_sched_wakeup.c     |    1 +
 kernel/trace/trace_selftest.c         |  214 +++++++-
 kernel/trace/trace_selftest_dynamic.c |    6 +
 kernel/trace/trace_stack.c            |    1 +
 11 files changed, 1116 insertions(+), 145 deletions(-)

Steven Rostedt

author maintainer

2011-05-06 15:47 UTC #1 lore raw

RFC PATCH 1/13 ftrace: Replace FTRACE_FL_NOTRACE flag with a hash of ignored functions

From: Steven Rostedt <[email protected]>

To prepare for the accounting system that will allow multiple users of the function tracer, having the FTRACE_FL_NOTRACE as a flag in the dyn_trace record does not make sense.

All ftrace_ops will soon have a hash of functions they should trace and not trace. By making a global hash of functions not to trace makes this easier for the transition.

Signed-off-by Steven Rostedt <[email protected]>

---
 include/linux/ftrace.h |    1 -
 kernel/trace/ftrace.c  |  176 +++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 150 insertions(+), 27 deletions(-)

include/linux/ftrace.h +0 -1

index 3204744..fe0a90a 100644
@@ -149,7 +149,6 @@ enum {
 	FTRACE_FL_FREE		= (1 << 0),
 	FTRACE_FL_FILTER	= (1 << 1),
 	FTRACE_FL_ENABLED	= (1 << 2),
-	FTRACE_FL_NOTRACE	= (1 << 3),
 };
 
 struct dyn_ftrace {

kernel/trace/ftrace.c +150 ~~-26~~

267 lines not on this page · raw message

Signature · 1 line

1.7.2.3

Steven Rostedt

author maintainer

2011-05-06 15:48 UTC #2 lore raw

RFC PATCH 2/13 ftrace: Use hash instead for FTRACE_FL_FILTER

In reply to Steven Rostedt

From: Steven Rostedt <[email protected]>

When multiple users are allowed to have their own set of functions to trace, having the FTRACE_FL_FILTER flag will not be enough to handle the accounting of those users. Each user will need their own set of functions.

Replace the FTRACE_FL_FILTER with a filter_hash instead. This is temporary until the rest of the function filtering accounting gets in.

Signed-off-by Steven Rostedt <[email protected]>

---
 include/linux/ftrace.h |    3 +-
 kernel/trace/ftrace.c  |  151 ++++++++++++++++++++++--------------------------
 2 files changed, 70 insertions(+), 84 deletions(-)

include/linux/ftrace.h +1 -2

index fe0a90a..52fc5d4 100644
@@ -147,8 +147,7 @@ extern int ftrace_text_reserved(void *start, void *end);
 
 enum {
 	FTRACE_FL_FREE		= (1 << 0),
-	FTRACE_FL_FILTER	= (1 << 1),
-	FTRACE_FL_ENABLED	= (1 << 2),
+	FTRACE_FL_ENABLED	= (1 << 1),
 };
 
 struct dyn_ftrace {

kernel/trace/ftrace.c +69 ~~-82~~

357 lines not on this page · raw message

Signature · 1 line

1.7.2.3

Steven Rostedt

author maintainer

2011-05-06 15:50 UTC #3 lore raw

RFC PATCH 3/13 ftrace: Create a global_ops to hold the filter and notrace hashes

In reply to Steven Rostedt

From: Steven Rostedt <[email protected]>

Combine the filter and notrace hashes to be accessed by a single entity, the global_ops. The global_ops is a ftrace_ops structure that is passed to different functions that can read or modify the filtering of the function tracer.

The ftrace_ops structure was modified to hold a filter and notrace hashes so that later patches may allow each ftrace_ops to have its own set of rules to what functions may be filtered.

Signed-off-by Steven Rostedt <[email protected]>

---
 include/linux/ftrace.h |   10 ++++++-
 kernel/trace/ftrace.c  |   65 ++++++++++++++++++++++++++++++++++--------------
 2 files changed, 54 insertions(+), 21 deletions(-)

include/linux/ftrace.h +8 -2

index 52fc5d4..6658a51 100644
@@ -29,9 +29,15 @@ ftrace_enable_sysctl(struct ctl_table *table, int write,
 
 typedef void (*ftrace_func_t)(unsigned long ip, unsigned long parent_ip);
 
+struct ftrace_hash;
+
 struct ftrace_ops {
-	ftrace_func_t	  func;
-	struct ftrace_ops *next;
+	ftrace_func_t			func;
+	struct ftrace_ops		*next;
+#ifdef CONFIG_DYNAMIC_FTRACE
+	struct ftrace_hash		*notrace_hash;
+	struct ftrace_hash		*filter_hash;
+#endif
 };
 
 extern int function_trace_stop;

kernel/trace/ftrace.c +46 ~~-19~~

201 lines not on this page · raw message

Signature · 1 line

1.7.2.3

Steven Rostedt

author maintainer

2011-05-06 15:50 UTC #4 lore raw

RFC PATCH 4/13 ftrace: Separate hash allocation and assignment

In reply to Steven Rostedt

From: Steven Rostedt <[email protected]>

When filtering, allocate a hash to insert the function records. After the filtering is complete, assign it to the ftrace_ops structure.

This allows the ftrace_ops structure to have a much smaller array of hash buckets instead of wasting a lot of memory.

A read only empty_hash is created to be the minimum size that any ftrace_ops can point to.

When a new hash is created, it has the following steps:

o Allocate a default hash.
o Walk the function records assigning the filtered records to the hash
o Allocate a new hash with the appropriate size buckets
o Move the entries from the default hash to the new hash.

Signed-off-by Steven Rostedt <[email protected]>

---
 kernel/trace/ftrace.c |  275 +++++++++++++++++++++++++++++++++++++++++--------
 1 files changed, 233 insertions(+), 42 deletions(-)

kernel/trace/ftrace.c +233 ~~-42~~

index a517a6c..46f0826 100644
@@ -57,7 +57,8 @@
 /* hash bits for specific function selection */
 #define FTRACE_HASH_BITS 7
 #define FTRACE_FUNC_HASHSIZE (1 << FTRACE_HASH_BITS)
-#define FTRACE_HASH_MAX_BITS 10
+#define FTRACE_HASH_DEFAULT_BITS 10
+#define FTRACE_HASH_MAX_BITS 12
 
 /* ftrace_enabled is a method to turn ftrace on or off */
 int ftrace_enabled __read_mostly;

@@ -877,22 +878,22 @@ struct ftrace_hash {
 	unsigned long		count;
 };
 
-static struct hlist_head notrace_buckets[1 << FTRACE_HASH_MAX_BITS];
-static struct ftrace_hash notrace_hash = {
-	.size_bits = FTRACE_HASH_MAX_BITS,
-	.buckets = notrace_buckets,
-};
-
-static struct hlist_head filter_buckets[1 << FTRACE_HASH_MAX_BITS];
-static struct ftrace_hash filter_hash = {
-	.size_bits = FTRACE_HASH_MAX_BITS,
-	.buckets = filter_buckets,
+/*
+ * We make these constant because no one should touch them,
+ * but they are used as the default "empty hash", to avoid allocating
+ * it all the time. These are in a read only section such that if
+ * anyone does try to modify it, it will cause an exception.
+ */
+static const struct hlist_head empty_buckets[1];
+static const struct ftrace_hash empty_hash = {
+	.buckets = (struct hlist_head *)empty_buckets,
 };
+#define EMPTY_HASH	((struct ftrace_hash *)&empty_hash)
 
 struct ftrace_ops global_ops = {
 	.func			= ftrace_stub,
-	.notrace_hash		= &notrace_hash,
-	.filter_hash		= &filter_hash,
+	.notrace_hash		= EMPTY_HASH,
+	.filter_hash		= EMPTY_HASH,
 };
 
 static struct dyn_ftrace *ftrace_new_addrs;

@@ -941,31 +942,38 @@ ftrace_lookup_ip(struct ftrace_hash *hash, unsigned long ip)
 	return NULL;
 }
 
-static int add_hash_entry(struct ftrace_hash *hash, unsigned long ip)
+static void __add_hash_entry(struct ftrace_hash *hash,
+			     struct ftrace_func_entry *entry)
 {
-	struct ftrace_func_entry *entry;
 	struct hlist_head *hhd;
 	unsigned long key;
 
-	entry = kmalloc(sizeof(*entry), GFP_KERNEL);
-	if (!entry)
-		return -ENOMEM;
-
 	if (hash->size_bits)
-		key = hash_long(ip, hash->size_bits);
+		key = hash_long(entry->ip, hash->size_bits);
 	else
 		key = 0;
 
-	entry->ip = ip;
 	hhd = &hash->buckets[key];
 	hlist_add_head(&entry->hlist, hhd);
 	hash->count++;
+}
+
+static int add_hash_entry(struct ftrace_hash *hash, unsigned long ip)
+{
+	struct ftrace_func_entry *entry;
+
+	entry = kmalloc(sizeof(*entry), GFP_KERNEL);
+	if (!entry)
+		return -ENOMEM;
+
+	entry->ip = ip;
+	__add_hash_entry(hash, entry);
 
 	return 0;
 }
 
 static void
-remove_hash_entry(struct ftrace_hash *hash,
+free_hash_entry(struct ftrace_hash *hash,
 		  struct ftrace_func_entry *entry)
 {
 	hlist_del(&entry->hlist);

@@ -973,6 +981,14 @@ remove_hash_entry(struct ftrace_hash *hash,
 	hash->count--;
 }
 
+static void
+remove_hash_entry(struct ftrace_hash *hash,
+		  struct ftrace_func_entry *entry)
+{
+	hlist_del(&entry->hlist);
+	hash->count--;
+}
+
 static void ftrace_hash_clear(struct ftrace_hash *hash)
 {
 	struct hlist_head *hhd;

@@ -981,14 +997,156 @@ static void ftrace_hash_clear(struct ftrace_hash *hash)
 	int size = 1 << hash->size_bits;
 	int i;
 
+	if (!hash->count)
+		return;
+
 	for (i = 0; i < size; i++) {
 		hhd = &hash->buckets[i];
 		hlist_for_each_entry_safe(entry, tp, tn, hhd, hlist)
-			remove_hash_entry(hash, entry);
+			free_hash_entry(hash, entry);
 	}
 	FTRACE_WARN_ON(hash->count);
 }
 
+static void free_ftrace_hash(struct ftrace_hash *hash)
+{
+	if (!hash || hash == EMPTY_HASH)
+		return;
+	ftrace_hash_clear(hash);
+	kfree(hash->buckets);
+	kfree(hash);
+}
+
+static struct ftrace_hash *alloc_ftrace_hash(int size_bits)
+{
+	struct ftrace_hash *hash;
+	int size;
+
+	hash = kzalloc(sizeof(*hash), GFP_KERNEL);
+	if (!hash)
+		return NULL;
+
+	size = 1 << size_bits;
+	hash->buckets = kzalloc(sizeof(*hash->buckets) * size, GFP_KERNEL);
+
+	if (!hash->buckets) {
+		kfree(hash);
+		return NULL;
+	}
+
+	hash->size_bits = size_bits;
+
+	return hash;
+}
+
+static struct ftrace_hash *
+alloc_and_copy_ftrace_hash(int size_bits, struct ftrace_hash *hash)
+{
+	struct ftrace_func_entry *entry;
+	struct ftrace_hash *new_hash;
+	struct hlist_node *tp;
+	int size;
+	int ret;
+	int i;
+
+	new_hash = alloc_ftrace_hash(size_bits);
+	if (!new_hash)
+		return NULL;
+
+	/* Empty hash? */
+	if (!hash || !hash->count)
+		return new_hash;
+
+	size = 1 << hash->size_bits;
+	for (i = 0; i < size; i++) {
+		hlist_for_each_entry(entry, tp, &hash->buckets[i], hlist) {
+			ret = add_hash_entry(new_hash, entry->ip);
+			if (ret < 0)
+				goto free_hash;
+		}
+	}
+
+	FTRACE_WARN_ON(new_hash->count != hash->count);
+
+	return new_hash;
+
+ free_hash:
+	free_ftrace_hash(new_hash);
+	return NULL;
+}
+
+static int
+ftrace_hash_move(struct ftrace_hash **dst, struct ftrace_hash *src)
+{
+	struct ftrace_func_entry *entry;
+	struct hlist_node *tp, *tn;
+	struct hlist_head *hhd;
+	struct ftrace_hash *hash = *dst;
+	unsigned long key;
+	int size = src->count;
+	int bits = 0;
+	int i;
+
+	/*
+	 * If the new source is empty, just free dst and assign it
+	 * the empty_hash.
+	 */
+	if (!src->count) {
+		free_ftrace_hash(*dst);
+		*dst = EMPTY_HASH;
+		return 0;
+	}
+
+	ftrace_hash_clear(hash);
+
+	/*
+	 * Make the hash size about 1/2 the # found
+	 */
+	for (size /= 2; size; size >>= 1)
+		bits++;
+
+	/* Don't allocate too much */
+	if (bits > FTRACE_HASH_MAX_BITS)
+		bits = FTRACE_HASH_MAX_BITS;
+
+	/* We can't modify the empty_hash */
+	if (hash == EMPTY_HASH) {
+		/* Create a new hash */
+		*dst = alloc_ftrace_hash(bits);
+		if (!*dst) {
+			*dst = EMPTY_HASH;
+			return -ENOMEM;
+		}
+		hash = *dst;
+	} else {
+		size = 1 << bits;
+
+		/* Use the old hash, but create new buckets */
+		hhd = kzalloc(sizeof(*hhd) * size, GFP_KERNEL);
+		if (!hhd)
+			return -ENOMEM;
+
+		kfree(hash->buckets);
+		hash->buckets = hhd;
+		hash->size_bits = bits;
+	}
+
+	size = 1 << src->size_bits;
+	for (i = 0; i < size; i++) {
+		hhd = &src->buckets[i];
+		hlist_for_each_entry_safe(entry, tp, tn, hhd, hlist) {
+			if (bits > 0)
+				key = hash_long(entry->ip, bits);
+			else
+				key = 0;
+			remove_hash_entry(src, entry);
+			__add_hash_entry(hash, entry);
+		}
+	}
+
+	return 0;
+}
+
 /*
  * This is a double for. Do not use 'break' to break out of the loop,
  * you must use a goto.

@@ -1443,6 +1601,7 @@ struct ftrace_iterator {
 	struct ftrace_func_probe	*probe;
 	struct trace_parser		parser;
 	struct ftrace_hash		*hash;
+	struct ftrace_ops		*ops;
 	int				hidx;
 	int				idx;
 	unsigned			flags;

@@ -1742,22 +1901,37 @@ ftrace_regex_open(struct ftrace_ops *ops, int flag,
 	else
 		hash = ops->filter_hash;
 
-	iter->hash = hash;
+	iter->ops = ops;
+	iter->flags = flag;
+
+	if (file->f_mode & FMODE_WRITE) {
+		mutex_lock(&ftrace_lock);
+		iter->hash = alloc_and_copy_ftrace_hash(FTRACE_HASH_DEFAULT_BITS, hash);
+		mutex_unlock(&ftrace_lock);
+
+		if (!iter->hash) {
+			trace_parser_put(&iter->parser);
+			kfree(iter);
+			return -ENOMEM;
+		}
+	}
 
 	mutex_lock(&ftrace_regex_lock);
+
 	if ((file->f_mode & FMODE_WRITE) &&
 	    (file->f_flags & O_TRUNC))
-		ftrace_filter_reset(hash);
+		ftrace_filter_reset(iter->hash);
 
 	if (file->f_mode & FMODE_READ) {
 		iter->pg = ftrace_pages_start;
-		iter->flags = flag;
 
 		ret = seq_open(file, &show_ftrace_seq_ops);
 		if (!ret) {
 			struct seq_file *m = file->private_data;
 			m->private = iter;
 		} else {
+			/* Failed */
+			free_ftrace_hash(iter->hash);
 			trace_parser_put(&iter->parser);
 			kfree(iter);
 		}

@@ -1835,7 +2009,7 @@ enter_record(struct ftrace_hash *hash, struct dyn_ftrace *rec, int not)
 		if (!entry)
 			return 0;
 
-		remove_hash_entry(hash, entry);
+		free_hash_entry(hash, entry);
 	} else {
 		/* Do nothing if it exists */
 		if (entry)

@@ -2259,19 +2433,13 @@ int unregister_ftrace_command(struct ftrace_func_command *cmd)
 	return ret;
 }
 
-static int ftrace_process_regex(char *buff, int len, int enable)
+static int ftrace_process_regex(struct ftrace_hash *hash,
+				char *buff, int len, int enable)
 {
 	char *func, *command, *next = buff;
-	struct ftrace_ops *ops = &global_ops;
 	struct ftrace_func_command *p;
-	struct ftrace_hash *hash;
 	int ret;
 
-	if (enable)
-		hash = ops->filter_hash;
-	else
-		hash = ops->notrace_hash;
-
 	func = strsep(&next, ":");
 
 	if (!next) {

@@ -2328,7 +2496,7 @@ ftrace_regex_write(struct file *file, const char __user *ubuf,
 
 	if (read >= 0 && trace_parser_loaded(parser) &&
 	    !trace_parser_cont(parser)) {
-		ret = ftrace_process_regex(parser->buffer,
+		ret = ftrace_process_regex(iter->hash, parser->buffer,
 					   parser->idx, enable);
 		trace_parser_clear(parser);
 		if (ret)

@@ -2356,26 +2524,40 @@ ftrace_notrace_write(struct file *file, const char __user *ubuf,
 	return ftrace_regex_write(file, ubuf, cnt, ppos, 0);
 }
 
-static void
+static int
 ftrace_set_regex(struct ftrace_ops *ops, unsigned char *buf, int len,
 		 int reset, int enable)
 {
+	struct ftrace_hash **orig_hash;
 	struct ftrace_hash *hash;
+	int ret;
 
 	if (unlikely(ftrace_disabled))
-		return;
+		return -ENODEV;
 
 	if (enable)
-		hash = ops->filter_hash;
+		orig_hash = &ops->filter_hash;
 	else
-		hash = ops->notrace_hash;
+		orig_hash = &ops->notrace_hash;
+
+	hash = alloc_and_copy_ftrace_hash(FTRACE_HASH_DEFAULT_BITS, *orig_hash);
+	if (!hash)
+		return -ENOMEM;
 
 	mutex_lock(&ftrace_regex_lock);
 	if (reset)
 		ftrace_filter_reset(hash);
 	if (buf)
 		ftrace_match_records(hash, buf, len);
+
+	mutex_lock(&ftrace_lock);
+	ret = ftrace_hash_move(orig_hash, hash);
+	mutex_unlock(&ftrace_lock);
+
 	mutex_unlock(&ftrace_regex_lock);
+
+	free_ftrace_hash(hash);
+	return ret;
 }
 
 /**

@@ -2484,7 +2666,9 @@ ftrace_regex_release(struct inode *inode, struct file *file)
 {
 	struct seq_file *m = (struct seq_file *)file->private_data;
 	struct ftrace_iterator *iter;
+	struct ftrace_hash **orig_hash;
 	struct trace_parser *parser;
+	int ret;
 
 	mutex_lock(&ftrace_regex_lock);
 	if (file->f_mode & FMODE_READ) {

@@ -2501,14 +2685,21 @@ ftrace_regex_release(struct inode *inode, struct file *file)
 	}
 
 	trace_parser_put(parser);
-	kfree(iter);
 
 	if (file->f_mode & FMODE_WRITE) {
+		if (iter->flags & FTRACE_ITER_NOTRACE)
+			orig_hash = &iter->ops->notrace_hash;
+		else
+			orig_hash = &iter->ops->filter_hash;
+
 		mutex_lock(&ftrace_lock);
-		if (ftrace_start_up && ftrace_enabled)
+		ret = ftrace_hash_move(orig_hash, iter->hash);
+		if (!ret && ftrace_start_up && ftrace_enabled)
 			ftrace_run_update_code(FTRACE_ENABLE_CALLS);
 		mutex_unlock(&ftrace_lock);
 	}
+	free_ftrace_hash(iter->hash);
+	kfree(iter);
 
 	mutex_unlock(&ftrace_regex_lock);
 	return 0;

Signature · 1 line

1.7.2.3

Steven Rostedt

author maintainer

2011-05-06 15:49 UTC #5 lore raw

RFC PATCH 5/13 ftrace: Use counters to enable functions to trace

In reply to Steven Rostedt

From: Steven Rostedt <[email protected]>

Every function has its own record that stores the instruction pointer and flags for the function to be traced. There are only two flags: enabled and free. The enabled flag states that tracing for the function has been enabled (actively traced), and the free flag states that the record no longer points to a function and can be used by new functions (loaded modules).

These flags are now moved to the MSB of the flags (actually just the top 32bits). The rest of the bits (30 bits) are now used as a ref counter. Everytime a tracer register functions to trace, those functions will have its counter incremented.

When tracing is enabled, to determine if a function should be traced, the counter is examined, and if it is non-zero it is set to trace.

When a ftrace_ops is registered to trace functions, its hashes are examined. If the ftrace_ops filter_hash count is zero, then all functions are set to be traced, otherwise only the functions in the hash are to be traced. The exception to this is if a function is also in the ftrace_ops notrace_hash. Then that function's counter is not incremented for this ftrace_ops.

Signed-off-by Steven Rostedt <[email protected]>

---
 include/linux/ftrace.h |    8 ++-
 kernel/trace/ftrace.c  |  158 +++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 148 insertions(+), 18 deletions(-)

include/linux/ftrace.h +6 -2

index 6658a51..ab1c46e 100644
@@ -37,6 +37,7 @@ struct ftrace_ops {
 #ifdef CONFIG_DYNAMIC_FTRACE
 	struct ftrace_hash		*notrace_hash;
 	struct ftrace_hash		*filter_hash;
+	unsigned long			flags;
 #endif
 };

@@ -152,10 +153,13 @@ extern void unregister_ftrace_function_probe_all(char *glob);
 extern int ftrace_text_reserved(void *start, void *end);
 
 enum {
-	FTRACE_FL_FREE		= (1 << 0),
-	FTRACE_FL_ENABLED	= (1 << 1),
+	FTRACE_FL_ENABLED	= (1 << 30),
+	FTRACE_FL_FREE		= (1 << 31),
 };
 
+#define FTRACE_FL_MASK		(0x3UL << 30)
+#define FTRACE_REF_MAX		((1 << 30) - 1)
+
 struct dyn_ftrace {
 	union {
 		unsigned long		ip; /* address of mcount call-site */

kernel/trace/ftrace.c +142 ~~-16~~

225 lines not on this page · raw message

Signature · 1 line

1.7.2.3

Steven Rostedt

author maintainer

2011-05-06 15:47 UTC #6 lore raw

RFC PATCH 6/13 ftrace: Add enabled_functions file

In reply to Steven Rostedt

From: Steven Rostedt <[email protected]>

Add the enabled_functions file that is used to show all the functions that have been enabled for tracing as well as their ref counts. This helps seeing if any function has been registered and what functions are being traced.

Signed-off-by Steven Rostedt <[email protected]>

---
 kernel/trace/ftrace.c |   51 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 49 insertions(+), 2 deletions(-)

kernel/trace/ftrace.c +49 -2

index 5dd332c..065f1e6 100644
@@ -1703,6 +1703,7 @@ enum {
 	FTRACE_ITER_NOTRACE	= (1 << 1),
 	FTRACE_ITER_PRINTALL	= (1 << 2),
 	FTRACE_ITER_HASH	= (1 << 3),
+	FTRACE_ITER_ENABLED	= (1 << 4),
 };
 
 #define FTRACE_BUFF_MAX (KSYM_SYMBOL_LEN+4) /* room for wildcards */

@@ -1842,7 +1843,11 @@ t_next(struct seq_file *m, void *v, loff_t *pos)
 		     !(ftrace_lookup_ip(ops->filter_hash, rec->ip))) ||
 
 		    ((iter->flags & FTRACE_ITER_NOTRACE) &&
-		     !ftrace_lookup_ip(ops->notrace_hash, rec->ip))) {
+		     !ftrace_lookup_ip(ops->notrace_hash, rec->ip)) ||
+
+		    ((iter->flags & FTRACE_ITER_ENABLED) &&
+		     !(rec->flags & ~FTRACE_FL_MASK))) {
+
 			rec = NULL;
 			goto retry;
 		}

@@ -1944,7 +1949,11 @@ static int t_show(struct seq_file *m, void *v)
 	if (!rec)
 		return 0;
 
-	seq_printf(m, "%ps\n", (void *)rec->ip);
+	seq_printf(m, "%ps", (void *)rec->ip);
+	if (iter->flags & FTRACE_ITER_ENABLED)
+		seq_printf(m, " (%ld)",
+			   rec->flags & ~FTRACE_FL_MASK);
+	seq_printf(m, "\n");
 
 	return 0;
 }

@@ -1983,6 +1992,34 @@ ftrace_avail_open(struct inode *inode, struct file *file)
 	return ret;
 }
 
+static int
+ftrace_enabled_open(struct inode *inode, struct file *file)
+{
+	struct ftrace_iterator *iter;
+	int ret;
+
+	if (unlikely(ftrace_disabled))
+		return -ENODEV;
+
+	iter = kzalloc(sizeof(*iter), GFP_KERNEL);
+	if (!iter)
+		return -ENOMEM;
+
+	iter->pg = ftrace_pages_start;
+	iter->flags = FTRACE_ITER_ENABLED;
+
+	ret = seq_open(file, &show_ftrace_seq_ops);
+	if (!ret) {
+		struct seq_file *m = file->private_data;
+
+		m->private = iter;
+	} else {
+		kfree(iter);
+	}
+
+	return ret;
+}
+
 static void ftrace_filter_reset(struct ftrace_hash *hash)
 {
 	mutex_lock(&ftrace_lock);

@@ -2838,6 +2875,13 @@ static const struct file_operations ftrace_avail_fops = {
 	.release = seq_release_private,
 };
 
+static const struct file_operations ftrace_enabled_fops = {
+	.open = ftrace_enabled_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = seq_release_private,
+};
+
 static const struct file_operations ftrace_filter_fops = {
 	.open = ftrace_filter_open,
 	.read = seq_read,

@@ -3069,6 +3113,9 @@ static __init int ftrace_init_dyn_debugfs(struct dentry *d_tracer)
 	trace_create_file("available_filter_functions", 0444,
 			d_tracer, NULL, &ftrace_avail_fops);
 
+	trace_create_file("enabled_functions", 0444,
+			d_tracer, NULL, &ftrace_enabled_fops);
+
 	trace_create_file("set_ftrace_filter", 0644, d_tracer,
 			NULL, &ftrace_filter_fops);

Signature · 1 line

1.7.2.3

Steven Rostedt

author maintainer

2011-05-06 15:47 UTC #7 lore raw

RFC PATCH 7/13 ftrace: Add ops parameter to ftrace_startup/shutdown functions

In reply to Steven Rostedt

From: Steven Rostedt <[email protected]>

In order to allow different ops to enable different functions, the ftrace_startup() and ftrace_shutdown() functions need the ops parameter passed to them.

Signed-off-by Steven Rostedt <[email protected]>

---
 kernel/trace/ftrace.c |   28 ++++++++++++++--------------
 1 files changed, 14 insertions(+), 14 deletions(-)

kernel/trace/ftrace.c +14 ~~-14~~

index 065f1e6..8fef1d9 100644
@@ -1522,10 +1522,8 @@ static void ftrace_startup_enable(int command)
 	ftrace_run_update_code(command);
 }
 
-static void ftrace_startup(int command)
+static void ftrace_startup(struct ftrace_ops *ops, int command)
 {
-	struct ftrace_ops *ops = &global_ops;
-
 	if (unlikely(ftrace_disabled))
 		return;

@@ -1539,10 +1537,8 @@ static void ftrace_startup(int command)
 	ftrace_startup_enable(command);
 }
 
-static void ftrace_shutdown(int command)
+static void ftrace_shutdown(struct ftrace_ops *ops, int command)
 {
-	struct ftrace_ops *ops = &global_ops;
-
 	if (unlikely(ftrace_disabled))
 		return;

@@ -2362,7 +2358,7 @@ static void __enable_ftrace_function_probe(void)
 		return;
 
 	__register_ftrace_function(&trace_probe_ops);
-	ftrace_startup(0);
+	ftrace_startup(&global_ops, 0);
 	ftrace_probe_registered = 1;
 }

@@ -2381,7 +2377,7 @@ static void __disable_ftrace_function_probe(void)
 
 	/* no more funcs left */
 	__unregister_ftrace_function(&trace_probe_ops);
-	ftrace_shutdown(0);
+	ftrace_shutdown(&global_ops, 0);
 	ftrace_probe_registered = 0;
 }

@@ -3267,6 +3263,10 @@ void __init ftrace_init(void)
 
 #else
 
+struct ftrace_ops global_ops = {
+	.func			= ftrace_stub,
+};
+
 static int __init ftrace_nodyn_init(void)
 {
 	ftrace_enabled = 1;

@@ -3277,8 +3277,8 @@ device_initcall(ftrace_nodyn_init);
 static inline int ftrace_init_dyn_debugfs(struct dentry *d_tracer) { return 0; }
 static inline void ftrace_startup_enable(int command) { }
 /* Keep as macros so we do not need to define the commands */
-# define ftrace_startup(command)	do { } while (0)
-# define ftrace_shutdown(command)	do { } while (0)
+# define ftrace_startup(ops, command)	do { } while (0)
+# define ftrace_shutdown(ops, command)	do { } while (0)
 # define ftrace_startup_sysctl()	do { } while (0)
 # define ftrace_shutdown_sysctl()	do { } while (0)
 #endif /* CONFIG_DYNAMIC_FTRACE */

@@ -3583,7 +3583,7 @@ int register_ftrace_function(struct ftrace_ops *ops)
 		goto out_unlock;
 
 	ret = __register_ftrace_function(ops);
-	ftrace_startup(0);
+	ftrace_startup(&global_ops, 0);
 
  out_unlock:
 	mutex_unlock(&ftrace_lock);

@@ -3602,7 +3602,7 @@ int unregister_ftrace_function(struct ftrace_ops *ops)
 
 	mutex_lock(&ftrace_lock);
 	ret = __unregister_ftrace_function(ops);
-	ftrace_shutdown(0);
+	ftrace_shutdown(&global_ops, 0);
 	mutex_unlock(&ftrace_lock);
 
 	return ret;

@@ -3825,7 +3825,7 @@ int register_ftrace_graph(trace_func_graph_ret_t retfunc,
 	ftrace_graph_return = retfunc;
 	ftrace_graph_entry = entryfunc;
 
-	ftrace_startup(FTRACE_START_FUNC_RET);
+	ftrace_startup(&global_ops, FTRACE_START_FUNC_RET);
 
 out:
 	mutex_unlock(&ftrace_lock);

@@ -3842,7 +3842,7 @@ void unregister_ftrace_graph(void)
 	ftrace_graph_active--;
 	ftrace_graph_return = (trace_func_graph_ret_t)ftrace_stub;
 	ftrace_graph_entry = ftrace_graph_entry_stub;
-	ftrace_shutdown(FTRACE_STOP_FUNC_RET);
+	ftrace_shutdown(&global_ops, FTRACE_STOP_FUNC_RET);
 	unregister_pm_notifier(&ftrace_suspend_notifier);
 	unregister_trace_sched_switch(ftrace_graph_probe_sched_switch, NULL);

Signature · 1 line

1.7.2.3

Steven Rostedt

author maintainer

2011-05-06 15:48 UTC #8 lore raw

RFC PATCH 8/13 ftrace: Have global_ops store the functions that are to be traced

In reply to Steven Rostedt

From: Steven Rostedt <[email protected]>

This is a step towards each ops structure defining its own set of functions to trace. As the current code with pid's and such are specific to the global_ops, it is restructured to be used with the global ops.

Signed-off-by Steven Rostedt <[email protected]>

---
 kernel/trace/ftrace.c |   69 +++++++++++++++++++++++++++++++++++++-----------
 1 files changed, 53 insertions(+), 16 deletions(-)

kernel/trace/ftrace.c +53 ~~-16~~

index 8fef1d9..dcce0bf 100644
@@ -91,6 +91,7 @@ static struct ftrace_ops *ftrace_list __read_mostly = &ftrace_list_end;
 ftrace_func_t ftrace_trace_function __read_mostly = ftrace_stub;
 ftrace_func_t __ftrace_trace_function __read_mostly = ftrace_stub;
 ftrace_func_t ftrace_pid_function __read_mostly = ftrace_stub;
+static struct ftrace_ops global_ops;
 
 /*
  * Traverse the ftrace_list, invoking all entries.  The reason that we

@@ -153,7 +154,7 @@ static void ftrace_test_stop_func(unsigned long ip, unsigned long parent_ip)
 }
 #endif
 
-static void update_ftrace_function(void)
+static void update_global_ops(void)
 {
 	ftrace_func_t func;

@@ -173,6 +174,18 @@ static void update_ftrace_function(void)
 		set_ftrace_pid_function(func);
 		func = ftrace_pid_func;
 	}
+
+	global_ops.func = func;
+}
+
+static void update_ftrace_function(void)
+{
+	ftrace_func_t func;
+
+	update_global_ops();
+
+	func = global_ops.func;
+
 #ifdef CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST
 	ftrace_trace_function = func;
 #else

@@ -181,24 +194,19 @@ static void update_ftrace_function(void)
 #endif
 }
 
-static int __register_ftrace_function(struct ftrace_ops *ops)
+static void add_ftrace_ops(struct ftrace_ops **list, struct ftrace_ops *ops)
 {
-	ops->next = ftrace_list;
+	ops->next = *list;
 	/*
 	 * We are entering ops into the ftrace_list but another
 	 * CPU might be walking that list. We need to make sure
 	 * the ops->next pointer is valid before another CPU sees
 	 * the ops pointer included into the ftrace_list.
 	 */
-	rcu_assign_pointer(ftrace_list, ops);
-
-	if (ftrace_enabled)
-		update_ftrace_function();
-
-	return 0;
+	rcu_assign_pointer(*list, ops);
 }
 
-static int __unregister_ftrace_function(struct ftrace_ops *ops)
+static int remove_ftrace_ops(struct ftrace_ops **list, struct ftrace_ops *ops)
 {
 	struct ftrace_ops **p;

@@ -206,13 +214,12 @@ static int __unregister_ftrace_function(struct ftrace_ops *ops)
 	 * If we are removing the last function, then simply point
 	 * to the ftrace_stub.
 	 */
-	if (ftrace_list == ops && ops->next == &ftrace_list_end) {
-		ftrace_trace_function = ftrace_stub;
-		ftrace_list = &ftrace_list_end;
+	if (*list == ops && ops->next == &ftrace_list_end) {
+		*list = &ftrace_list_end;
 		return 0;
 	}
 
-	for (p = &ftrace_list; *p != &ftrace_list_end; p = &(*p)->next)
+	for (p = list; *p != &ftrace_list_end; p = &(*p)->next)
 		if (*p == ops)
 			break;

@@ -220,7 +227,37 @@ static int __unregister_ftrace_function(struct ftrace_ops *ops)
 		return -1;
 
 	*p = (*p)->next;
+	return 0;
+}
+
+static int __register_ftrace_function(struct ftrace_ops *ops)
+{
+	if (ftrace_disabled)
+		return -ENODEV;
+
+	if (FTRACE_WARN_ON(ops == &global_ops))
+		return -EINVAL;
+
+	add_ftrace_ops(&ftrace_list, ops);
+	if (ftrace_enabled)
+		update_ftrace_function();
+
+	return 0;
+}
 
+static int __unregister_ftrace_function(struct ftrace_ops *ops)
+{
+	int ret;
+
+	if (ftrace_disabled)
+		return -ENODEV;
+
+	if (FTRACE_WARN_ON(ops == &global_ops))
+		return -EINVAL;
+
+	ret = remove_ftrace_ops(&ftrace_list, ops);
+	if (ret < 0)
+		return ret;
 	if (ftrace_enabled)
 		update_ftrace_function();

@@ -894,7 +931,7 @@ enum {
 	FTRACE_OPS_FL_ENABLED		= 1,
 };
 
-struct ftrace_ops global_ops = {
+static struct ftrace_ops global_ops = {
 	.func			= ftrace_stub,
 	.notrace_hash		= EMPTY_HASH,
 	.filter_hash		= EMPTY_HASH,

@@ -3263,7 +3300,7 @@ void __init ftrace_init(void)
 
 #else
 
-struct ftrace_ops global_ops = {
+static struct ftrace_ops global_ops = {
 	.func			= ftrace_stub,
 };

Signature · 1 line

1.7.2.3

Steven Rostedt

author maintainer

2011-05-06 15:49 UTC #9 lore raw

RFC PATCH 9/13 ftrace: Free hash with call_rcu_sched()

In reply to Steven Rostedt

From: Steven Rostedt <[email protected]>

When a hash is modified and might be in use, we need to perform a schedule RCU operation on it, as the hashes will soon be used directly in the function tracer callback.

Cc Paul E. McKenney <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

---
 kernel/trace/ftrace.c |   55 +++++++++++++++++++++++++------------------------
 1 files changed, 28 insertions(+), 27 deletions(-)

kernel/trace/ftrace.c +28 ~~-27~~

index dcce0bf..92b6fdf 100644
@@ -913,6 +913,7 @@ struct ftrace_hash {
 	unsigned long		size_bits;
 	struct hlist_head	*buckets;
 	unsigned long		count;
+	struct rcu_head		rcu;
 };
 
 /*

@@ -1058,6 +1059,21 @@ static void free_ftrace_hash(struct ftrace_hash *hash)
 	kfree(hash);
 }
 
+static void __free_ftrace_hash_rcu(struct rcu_head *rcu)
+{
+	struct ftrace_hash *hash;
+
+	hash = container_of(rcu, struct ftrace_hash, rcu);
+	free_ftrace_hash(hash);
+}
+
+static void free_ftrace_hash_rcu(struct ftrace_hash *hash)
+{
+	if (!hash || hash == EMPTY_HASH)
+		return;
+	call_rcu_sched(&hash->rcu, __free_ftrace_hash_rcu);
+}
+
 static struct ftrace_hash *alloc_ftrace_hash(int size_bits)
 {
 	struct ftrace_hash *hash;

@@ -1122,7 +1138,8 @@ ftrace_hash_move(struct ftrace_hash **dst, struct ftrace_hash *src)
 	struct ftrace_func_entry *entry;
 	struct hlist_node *tp, *tn;
 	struct hlist_head *hhd;
-	struct ftrace_hash *hash = *dst;
+	struct ftrace_hash *old_hash;
+	struct ftrace_hash *new_hash;
 	unsigned long key;
 	int size = src->count;
 	int bits = 0;

@@ -1133,13 +1150,11 @@ ftrace_hash_move(struct ftrace_hash **dst, struct ftrace_hash *src)
 	 * the empty_hash.
 	 */
 	if (!src->count) {
-		free_ftrace_hash(*dst);
-		*dst = EMPTY_HASH;
+		free_ftrace_hash_rcu(*dst);
+		rcu_assign_pointer(*dst, EMPTY_HASH);
 		return 0;
 	}
 
-	ftrace_hash_clear(hash);
-
 	/*
 	 * Make the hash size about 1/2 the # found
 	 */

@@ -1150,27 +1165,9 @@ ftrace_hash_move(struct ftrace_hash **dst, struct ftrace_hash *src)
 	if (bits > FTRACE_HASH_MAX_BITS)
 		bits = FTRACE_HASH_MAX_BITS;
 
-	/* We can't modify the empty_hash */
-	if (hash == EMPTY_HASH) {
-		/* Create a new hash */
-		*dst = alloc_ftrace_hash(bits);
-		if (!*dst) {
-			*dst = EMPTY_HASH;
-			return -ENOMEM;
-		}
-		hash = *dst;
-	} else {
-		size = 1 << bits;
-
-		/* Use the old hash, but create new buckets */
-		hhd = kzalloc(sizeof(*hhd) * size, GFP_KERNEL);
-		if (!hhd)
-			return -ENOMEM;
-
-		kfree(hash->buckets);
-		hash->buckets = hhd;
-		hash->size_bits = bits;
-	}
+	new_hash = alloc_ftrace_hash(bits);
+	if (!new_hash)
+		return -ENOMEM;
 
 	size = 1 << src->size_bits;
 	for (i = 0; i < size; i++) {

@@ -1181,10 +1178,14 @@ ftrace_hash_move(struct ftrace_hash **dst, struct ftrace_hash *src)
 			else
 				key = 0;
 			remove_hash_entry(src, entry);
-			__add_hash_entry(hash, entry);
+			__add_hash_entry(new_hash, entry);
 		}
 	}
 
+	old_hash = *dst;
+	rcu_assign_pointer(*dst, new_hash);
+	free_ftrace_hash_rcu(old_hash);
+
 	return 0;
 }

Signature · 1 line

1.7.2.3

Steven Rostedt

author maintainer

2011-05-06 15:48 UTC #10 lore raw

RFC PATCH 10/13 ftrace: Implement separate user function filtering

In reply to Steven Rostedt

From: Steven Rostedt <[email protected]>

ftrace_ops that are registered to trace functions can now be agnostic to each other in respect to what functions they trace. Each ops has their own hash of the functions they want to trace and a hash to what they do not want to trace. A empty hash for the functions they want to trace denotes all functions should be traced that are not in the notrace hash.

Cc Paul E. McKenney <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

---
 include/linux/ftrace.h            |    7 +-
 kernel/trace/ftrace.c             |  193 +++++++++++++++++++++++++++++-------
 kernel/trace/trace_functions.c    |    2 +
 kernel/trace/trace_irqsoff.c      |    1 +
 kernel/trace/trace_sched_wakeup.c |    1 +
 kernel/trace/trace_stack.c        |    1 +
 6 files changed, 166 insertions(+), 39 deletions(-)

include/linux/ftrace.h +6 -1

index ab1c46e..4609c0e 100644
@@ -31,13 +31,18 @@ typedef void (*ftrace_func_t)(unsigned long ip, unsigned long parent_ip);
 
 struct ftrace_hash;
 
+enum {
+	FTRACE_OPS_FL_ENABLED		= 1 << 0,
+	FTRACE_OPS_FL_GLOBAL		= 1 << 1,
+};
+
 struct ftrace_ops {
 	ftrace_func_t			func;
 	struct ftrace_ops		*next;
+	unsigned long			flags;
 #ifdef CONFIG_DYNAMIC_FTRACE
 	struct ftrace_hash		*notrace_hash;
 	struct ftrace_hash		*filter_hash;
-	unsigned long			flags;
 #endif
 };

kernel/trace/ftrace.c +155 ~~-38~~

366 lines not on this page · raw message

kernel/trace/trace_functions.c +2 -0

index 16aee4d..8d0e1cc 100644
@@ -149,11 +149,13 @@ function_stack_trace_call(unsigned long ip, unsigned long parent_ip)
 static struct ftrace_ops trace_ops __read_mostly =
 {
 	.func = function_trace_call,
+	.flags = FTRACE_OPS_FL_GLOBAL,
 };
 
 static struct ftrace_ops trace_stack_ops __read_mostly =
 {
 	.func = function_stack_trace_call,
+	.flags = FTRACE_OPS_FL_GLOBAL,
 };
 
 /* Our two options */

kernel/trace/trace_irqsoff.c +1 -0

index a4969b4..c77424b 100644
@@ -153,6 +153,7 @@ irqsoff_tracer_call(unsigned long ip, unsigned long parent_ip)
 static struct ftrace_ops trace_ops __read_mostly =
 {
 	.func = irqsoff_tracer_call,
+	.flags = FTRACE_OPS_FL_GLOBAL,
 };
 #endif /* CONFIG_FUNCTION_TRACER */

kernel/trace/trace_sched_wakeup.c +1 -0

index 7319559..f029dd4 100644
@@ -129,6 +129,7 @@ wakeup_tracer_call(unsigned long ip, unsigned long parent_ip)
 static struct ftrace_ops trace_ops __read_mostly =
 {
 	.func = wakeup_tracer_call,
+	.flags = FTRACE_OPS_FL_GLOBAL,
 };
 #endif /* CONFIG_FUNCTION_TRACER */

kernel/trace/trace_stack.c +1 -0

index 4c5dead..b0b53b8 100644
@@ -133,6 +133,7 @@ stack_trace_call(unsigned long ip, unsigned long parent_ip)
 static struct ftrace_ops trace_ops __read_mostly =
 {
 	.func = stack_trace_call,
+	.flags = FTRACE_OPS_FL_GLOBAL,
 };
 
 static ssize_t

Signature · 1 line

1.7.2.3

Steven Rostedt

author maintainer

2011-05-06 15:48 UTC #11 lore raw

RFC PATCH 11/13 ftrace: Allow dynamically allocated function tracers

In reply to Steven Rostedt

From: Steven Rostedt <[email protected]>

Now that functions may be selected individually, it only makes sense that we should allow dynamically allocated trace structures to be traced. This will allow perf to allocate a ftrace_ops structure at runtime and use it to pick and choose which functions that structure will trace.

Note, a dynamically allocated ftrace_ops will always be called indirectly instead of being called directly from the mcount in entry.S. This is because there's no safe way to prevent mcount from being preempted before calling the function, unless we modify every entry.S to do so (not likely). Thus, dynamically allocated functions will now be called by the ftrace_ops_list_func() that loops through the ops that are allocated if there are more than one op allocated at a time. This loop is protected with a preempt_disable.

To determine if an ftrace_ops structure is allocated or not, a new util function was added to the kernel/extable.c called core_kernel_data(), which returns 1 if the address is between _sdata and _edata.

Cc Paul E. McKenney <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

---
 include/linux/ftrace.h |    1 +
 include/linux/kernel.h |    1 +
 kernel/extable.c       |    8 ++++++++
 kernel/trace/ftrace.c  |   37 ++++++++++++++++++++++++++++++-------
 4 files changed, 40 insertions(+), 7 deletions(-)

include/linux/ftrace.h +1 -0

index 4609c0e..caba694 100644
@@ -34,6 +34,7 @@ struct ftrace_hash;
 enum {
 	FTRACE_OPS_FL_ENABLED		= 1 << 0,
 	FTRACE_OPS_FL_GLOBAL		= 1 << 1,
+	FTRACE_OPS_FL_DYNAMIC		= 1 << 2,
 };
 
 struct ftrace_ops {

include/linux/kernel.h +1 -0

index 00cec4d..f37ba71 100644
@@ -283,6 +283,7 @@ extern char *get_options(const char *str, int nints, int *ints);
 extern unsigned long long memparse(const char *ptr, char **retptr);
 
 extern int core_kernel_text(unsigned long addr);
+extern int core_kernel_data(unsigned long addr);
 extern int __kernel_text_address(unsigned long addr);
 extern int kernel_text_address(unsigned long addr);
 extern int func_ptr_is_kernel_text(void *ptr);

kernel/extable.c +8 -0

index 7f8f263..c2d625f 100644
@@ -72,6 +72,14 @@ int core_kernel_text(unsigned long addr)
 	return 0;
 }
 
+int core_kernel_data(unsigned long addr)
+{
+	if (addr >= (unsigned long)_sdata &&
+	    addr < (unsigned long)_edata)
+		return 1;
+	return 0;
+}
+
 int __kernel_text_address(unsigned long addr)
 {
 	if (core_kernel_text(addr))

kernel/trace/ftrace.c +30 -7

index 6c7e1df..5b3ee04 100644
@@ -189,8 +189,14 @@ static void update_ftrace_function(void)
 
 	update_global_ops();
 
+	/*
+	 * If we are at the end of the list and this ops is
+	 * not dynamic, then have the mcount trampoline call
+	 * the function directly
+	 */
 	if (ftrace_ops_list == &ftrace_list_end ||
-	    ftrace_ops_list->next == &ftrace_list_end)
+	    (ftrace_ops_list->next == &ftrace_list_end &&
+	     !(ftrace_ops_list->flags & FTRACE_OPS_FL_DYNAMIC)))
 		func = ftrace_ops_list->func;
 	else
 		func = ftrace_ops_list_func;

@@ -250,6 +256,9 @@ static int __register_ftrace_function(struct ftrace_ops *ops)
 	if (WARN_ON(ops->flags & FTRACE_OPS_FL_ENABLED))
 		return -EBUSY;
 
+	if (!core_kernel_data((unsigned long)ops))
+		ops->flags |= FTRACE_OPS_FL_DYNAMIC;
+
 	if (ops->flags & FTRACE_OPS_FL_GLOBAL) {
 		int first = ftrace_global_list == &ftrace_list_end;
 		add_ftrace_ops(&ftrace_global_list, ops);

@@ -293,6 +302,13 @@ static int __unregister_ftrace_function(struct ftrace_ops *ops)
 	if (ftrace_enabled)
 		update_ftrace_function();
 
+	/*
+	 * Dynamic ops may be freed, we must make sure that all
+	 * callers are done before leaving this function.
+	 */
+	if (ops->flags & FTRACE_OPS_FL_DYNAMIC)
+		synchronize_sched();
+
 	return 0;
 }

@@ -1225,6 +1241,9 @@ ftrace_hash_move(struct ftrace_hash **dst, struct ftrace_hash *src)
  * the filter_hash does not exist or is empty,
  *  AND
  * the ip is not in the ops->notrace_hash.
+ *
+ * This needs to be called with preemption disabled as
+ * the hashes are freed with call_rcu_sched().
  */
 static int
 ftrace_ops_test(struct ftrace_ops *ops, unsigned long ip)

@@ -1233,9 +1252,6 @@ ftrace_ops_test(struct ftrace_ops *ops, unsigned long ip)
 	struct ftrace_hash *notrace_hash;
 	int ret;
 
-	/* The hashes are freed with call_rcu_sched() */
-	preempt_disable_notrace();
-
 	filter_hash = rcu_dereference_raw(ops->filter_hash);
 	notrace_hash = rcu_dereference_raw(ops->notrace_hash);

@@ -1246,7 +1262,6 @@ ftrace_ops_test(struct ftrace_ops *ops, unsigned long ip)
 		ret = 1;
 	else
 		ret = 0;
-	preempt_enable_notrace();
 
 	return ret;
 }

@@ -3425,14 +3440,20 @@ ftrace_ops_test(struct ftrace_ops *ops, unsigned long ip)
 static void
 ftrace_ops_list_func(unsigned long ip, unsigned long parent_ip)
 {
-	/* see comment above ftrace_global_list_func */
-	struct ftrace_ops *op = rcu_dereference_raw(ftrace_ops_list);
+	struct ftrace_ops *op;
 
+	/*
+	 * Some of the ops may be dynamically allocated,
+	 * they must be freed after a synchronize_sched().
+	 */
+	preempt_disable_notrace();
+	op = rcu_dereference_raw(ftrace_ops_list);
 	while (op != &ftrace_list_end) {
 		if (ftrace_ops_test(op, ip))
 			op->func(ip, parent_ip);
 		op = rcu_dereference_raw(op->next);
 	};
+	preempt_enable_notrace();
 }
 
 static void clear_ftrace_swapper(void)

@@ -3743,6 +3764,7 @@ int register_ftrace_function(struct ftrace_ops *ops)
 	mutex_unlock(&ftrace_lock);
 	return ret;
 }
+EXPORT_SYMBOL_GPL(register_ftrace_function);
 
 /**
  * unregister_ftrace_function - unregister a function for profiling.

@@ -3762,6 +3784,7 @@ int unregister_ftrace_function(struct ftrace_ops *ops)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(unregister_ftrace_function);
 
 int
 ftrace_enable_sysctl(struct ctl_table *table, int write,

Signature · 1 line

1.7.2.3

Steven Rostedt

author maintainer

2011-05-06 15:48 UTC #12 lore raw

RFC PATCH 12/13 ftrace: Modify ftrace_set_filter/notrace to take ops

In reply to Steven Rostedt

From: Steven Rostedt <[email protected]>

Since users of the function tracer can now pick and choose which functions they want to trace agnostically from other users of the function tracer, we need to pass the ops struct to the ftrace_set_filter() functions.

The functions ftrace_set_global_filter() and ftrace_set_global_notrace() is added to keep the old filter functions which are used to modify the generic function tracers.

Signed-off-by Steven Rostedt <[email protected]>

---
 include/linux/ftrace.h        |    7 +++++-
 kernel/trace/ftrace.c         |   46 +++++++++++++++++++++++++++++++++++++++-
 kernel/trace/trace_selftest.c |    7 ++++-
 3 files changed, 55 insertions(+), 5 deletions(-)

include/linux/ftrace.h +6 -1

index caba694..9d88e1c 100644
@@ -179,7 +179,12 @@ struct dyn_ftrace {
 };
 
 int ftrace_force_update(void);
-void ftrace_set_filter(unsigned char *buf, int len, int reset);
+void ftrace_set_filter(struct ftrace_ops *ops, unsigned char *buf,
+		       int len, int reset);
+void ftrace_set_notrace(struct ftrace_ops *ops, unsigned char *buf,
+			int len, int reset);
+void ftrace_set_global_filter(unsigned char *buf, int len, int reset);
+void ftrace_set_global_notrace(unsigned char *buf, int len, int reset);
 
 int register_ftrace_command(struct ftrace_func_command *cmd);
 int unregister_ftrace_command(struct ftrace_func_command *cmd);

kernel/trace/ftrace.c +44 -2

79 lines not on this page · raw message

kernel/trace/trace_selftest.c +5 -2

index 659732e..0d563a9 100644
@@ -131,7 +131,7 @@ int trace_selftest_startup_dynamic_tracing(struct tracer *trace,
 	func_name = "*" __stringify(DYN_FTRACE_TEST_NAME);
 
 	/* filter only on our function */
-	ftrace_set_filter(func_name, strlen(func_name), 1);
+	ftrace_set_global_filter(func_name, strlen(func_name), 1);
 
 	/* enable tracing */
 	ret = tracer_init(trace, tr);

@@ -181,7 +181,10 @@ int trace_selftest_startup_dynamic_tracing(struct tracer *trace,
 	tracer_enabled = save_tracer_enabled;
 
 	/* Enable tracing on all functions again */
-	ftrace_set_filter(NULL, 0, 1);
+	ftrace_set_global_filter(NULL, 0, 1);
+
+	if (!ret)
+		ret = trace_selftest_probes();
 
 	return ret;
 }

Signature · 1 line

1.7.2.3

Steven Rostedt

author maintainer

2011-05-06 15:49 UTC #13 lore raw

RFC PATCH 13/13 ftrace: Add self-tests for multiple function trace users

In reply to Steven Rostedt

From: Steven Rostedt <[email protected]>

Add some basic sanity tests for multiple users of the function tracer at startup.

Signed-off-by Steven Rostedt <[email protected]>

---
 kernel/trace/trace.h                  |    2 +
 kernel/trace/trace_selftest.c         |  209 ++++++++++++++++++++++++++++++++-
 kernel/trace/trace_selftest_dynamic.c |    6 +
 3 files changed, 215 insertions(+), 2 deletions(-)

kernel/trace/trace.h +2 -0

index 5e9dfc6..6b69c4b 100644
@@ -419,6 +419,8 @@ extern void trace_find_cmdline(int pid, char comm[]);
 extern unsigned long ftrace_update_tot_cnt;
 #define DYN_FTRACE_TEST_NAME trace_selftest_dynamic_test_func
 extern int DYN_FTRACE_TEST_NAME(void);
+#define DYN_FTRACE_TEST_NAME2 trace_selftest_dynamic_test_func2
+extern int DYN_FTRACE_TEST_NAME2(void);
 #endif
 
 extern int ring_buffer_expanded;

kernel/trace/trace_selftest.c +207 -2

237 lines not on this page · raw message

kernel/trace/trace_selftest_dynamic.c +6 -0

index 54dd77c..b4c475a 100644
@@ -5,3 +5,9 @@ int DYN_FTRACE_TEST_NAME(void)
 	/* used to call mcount */
 	return 0;
 }
+
+int DYN_FTRACE_TEST_NAME2(void)
+{
+	/* used to call mcount */
+	return 0;
+}

Signature · 1 line

1.7.2.3

Ingo Molnar

2011-05-10 08:00 UTC #14 lore raw

In reply to Steven Rostedt

Quoted 43 lines · Steven Rostedt

Ingo, et. al.
This is an RFC patch set of the work I did to allow multiple users to pick and choose which functions they would like to trace without being affected by other users (well, they are still affected, but they can choose their own functions).
Since this rewrote a lot of the guts of ftrace's function handling I broke it up into steps that I can sequentially think about how to make the conversion. It also helped in understanding the changes that had to be made.
The end result now has this:
o The function record list flags item now stores a ref count, and we no longer need to have the flags if the records are filtered or set for "notrace"
o Each ftrace_ops has its own set of functions to trace or not trace which is used to update the ref counts of the function records mentioned above. If the ftrace_ops is not currently active, the records are not touched when the filters are changed. If the filters are changed while the ftrace_ops is active, it is done in a way with RCU to make the updates.
o If only one ftrace_ops is registered for tracing (actively tracing), its handler is called directly from mcount. Unless the ftrace_ops was allocated and not a static/global core kernel variable (see below).
o If more than one ftrace_ops is registered, then a function is called that will iterate over the list of registered ftrace_ops, performing a check of each of its hashes to know if ftrace_ops handler should be called for that function.
o If a ftrace_ops is allocated and not a static/global core kernel variable, it will be forced to use the function that loops through the ftrace_ops even if its the only one. This is because that function disables preemption when it performs the loop. This is needed, because dynamically allocated ftrace_ops must call synchronize_sched() before being freed (which is done when the dynamic ftrace_ops has been unregistered).

Quoted 4 lines

o A file has been created in the debugfs/tracing directory called enabled_functions. This file contains a list of all functions that any ftrace_ops has registered, along with each functions ref count. This can be used to make sure nothing is tracing.

Well, i think we'd like to see the end result as well: i.e. function tracing exposed to a tracer that can do per task tracing (perf) and then demonstrating that independent tracing sessions can trace functions without impacting each other.

Thanks,

Ingo

Steven Rostedt

author maintainer

2011-05-10 10:48 UTC #15 lore raw

Quoted 7 lines · Ingo Molnar

* Steven Rostedt <[email protected]> wrote:
Well, i think we'd like to see the end result as well: i.e. function tracing exposed to a tracer that can do per task tracing (perf) and then demonstrating that independent tracing sessions can trace functions without impacting each other.

This could be done. I just wanted to post this RFC for now, just in case someone found an issue with this design. When I get back home, I'll complete this, and work with Frederic to get a RFC patch set with perf working.

I don't want to rush this out as it can get complex and I want us to "learn" the correct way of enabling this for perf. Although, it would be nice to get this into 2.6.40. Perhaps we should be conservative and aim for 2.6.41 and let it settle for a release.

-- Steve

Frederic Weisbecker

2011-05-10 14:43 UTC #16 lore raw

Quoted 14 lines · Steven Rostedt

Quoted 7 lines · Ingo Molnar
* Steven Rostedt <[email protected]> wrote:
Well, i think we'd like to see the end result as well: i.e. function tracing exposed to a tracer that can do per task tracing (perf) and then demonstrating that independent tracing sessions can trace functions without impacting each other.
This could be done. I just wanted to post this RFC for now, just in case someone found an issue with this design. When I get back home, I'll complete this, and work with Frederic to get a RFC patch set with perf working.

Quoted 4 lines

I don't want to rush this out as it can get complex and I want us to "learn" the correct way of enabling this for perf. Although, it would be nice to get this into 2.6.40. Perhaps we should be conservative and aim for 2.6.41 and let it settle for a release.

In fact getting the function tracer able to run with multi users and making it usable by perf are two different (although for the same final purpose) and complicated tasks on their own.

Thus it seems to me more reasonable to advance step by step like we are doing.