* [for-next][PATCH 1/9] ftrace: Add separate function for non recursive callbacks
2014-09-11 14:47 [for-next][PATCH 0/9] ftrace: 3.18 queue Steven Rostedt
@ 2014-09-11 14:47 ` Steven Rostedt
2014-09-11 14:47 ` [for-next][PATCH 2/9] ftrace: Add helper function ftrace_ops_get_func() Steven Rostedt
` (7 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Steven Rostedt @ 2014-09-11 14:47 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton
[-- Attachment #1: 0001-ftrace-Add-separate-function-for-non-recursive-callb.patch --]
[-- Type: text/plain, Size: 2750 bytes --]
From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
Instead of using the generic list function for callbacks that
are not recursive, call a new helper function from the mcount
trampoline called ftrace_ops_recur_func() that will do the recursion
checking for the callback.
This eliminates an indirection as well as will help in future code
that will use dynamically allocated trampolines.
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
kernel/trace/ftrace.c | 33 +++++++++++++++++++++++++++++++--
1 file changed, 31 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 5916a8e59e87..17b606362ab4 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -113,6 +113,9 @@ ftrace_func_t ftrace_pid_function __read_mostly = ftrace_stub;
static struct ftrace_ops global_ops;
static struct ftrace_ops control_ops;
+static void ftrace_ops_recurs_func(unsigned long ip, unsigned long parent_ip,
+ struct ftrace_ops *op, struct pt_regs *regs);
+
#if ARCH_SUPPORTS_FTRACE_OPS
static void ftrace_ops_list_func(unsigned long ip, unsigned long parent_ip,
struct ftrace_ops *op, struct pt_regs *regs);
@@ -258,11 +261,18 @@ static void update_ftrace_function(void)
if (ftrace_ops_list == &ftrace_list_end ||
(ftrace_ops_list->next == &ftrace_list_end &&
!(ftrace_ops_list->flags & FTRACE_OPS_FL_DYNAMIC) &&
- (ftrace_ops_list->flags & FTRACE_OPS_FL_RECURSION_SAFE) &&
!FTRACE_FORCE_LIST_FUNC)) {
/* Set the ftrace_ops that the arch callback uses */
set_function_trace_op = ftrace_ops_list;
- func = ftrace_ops_list->func;
+ /*
+ * If the func handles its own recursion, call it directly.
+ * Otherwise call the recursion protected function that
+ * will call the ftrace ops function.
+ */
+ if (ftrace_ops_list->flags & FTRACE_OPS_FL_RECURSION_SAFE)
+ func = ftrace_ops_list->func;
+ else
+ func = ftrace_ops_recurs_func;
} else {
/* Just use the default ftrace_ops */
set_function_trace_op = &ftrace_list_end;
@@ -4827,6 +4837,25 @@ static void ftrace_ops_no_ops(unsigned long ip, unsigned long parent_ip)
}
#endif
+/*
+ * If there's only one function registered but it does not support
+ * recursion, this function will be called by the mcount trampoline.
+ * This function will handle recursion protection.
+ */
+static void ftrace_ops_recurs_func(unsigned long ip, unsigned long parent_ip,
+ struct ftrace_ops *op, struct pt_regs *regs)
+{
+ int bit;
+
+ bit = trace_test_and_set_recursion(TRACE_LIST_START, TRACE_LIST_MAX);
+ if (bit < 0)
+ return;
+
+ op->func(ip, parent_ip, op, regs);
+
+ trace_clear_recursion(bit);
+}
+
static void clear_ftrace_swapper(void)
{
struct task_struct *p;
--
2.0.1
^ permalink raw reply [flat|nested] 10+ messages in thread* [for-next][PATCH 2/9] ftrace: Add helper function ftrace_ops_get_func()
2014-09-11 14:47 [for-next][PATCH 0/9] ftrace: 3.18 queue Steven Rostedt
2014-09-11 14:47 ` [for-next][PATCH 1/9] ftrace: Add separate function for non recursive callbacks Steven Rostedt
@ 2014-09-11 14:47 ` Steven Rostedt
2014-09-11 14:47 ` [for-next][PATCH 3/9] ftrace: Set callback to ftrace_stub when no ops are registered Steven Rostedt
` (6 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Steven Rostedt @ 2014-09-11 14:47 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton
[-- Attachment #1: 0002-ftrace-Add-helper-function-ftrace_ops_get_func.patch --]
[-- Type: text/plain, Size: 3419 bytes --]
From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
Add the helper function to what the mcount trampoline is to call
for a ftrace_ops function. This helper will be used by arch code
in the future to set up dynamic trampolines. But as this does the
same tests that are performed in choosing what function to call for
the default mcount trampoline, might as well use it to clean up
the existing code.
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
include/linux/ftrace.h | 2 ++
kernel/trace/ftrace.c | 47 +++++++++++++++++++++++++++++++++++------------
2 files changed, 37 insertions(+), 12 deletions(-)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index f0b0edbf55a9..ef37286547fc 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -56,6 +56,8 @@ struct ftrace_ops;
typedef void (*ftrace_func_t)(unsigned long ip, unsigned long parent_ip,
struct ftrace_ops *op, struct pt_regs *regs);
+ftrace_func_t ftrace_ops_get_func(struct ftrace_ops *ops);
+
/*
* FTRACE_OPS_FL_* bits denote the state of ftrace_ops struct and are
* set in the flags member.
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 17b606362ab4..dabf734f909c 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -259,20 +259,12 @@ static void update_ftrace_function(void)
* 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->flags & FTRACE_OPS_FL_DYNAMIC) &&
- !FTRACE_FORCE_LIST_FUNC)) {
+ (ftrace_ops_list->next == &ftrace_list_end)) {
+
/* Set the ftrace_ops that the arch callback uses */
set_function_trace_op = ftrace_ops_list;
- /*
- * If the func handles its own recursion, call it directly.
- * Otherwise call the recursion protected function that
- * will call the ftrace ops function.
- */
- if (ftrace_ops_list->flags & FTRACE_OPS_FL_RECURSION_SAFE)
- func = ftrace_ops_list->func;
- else
- func = ftrace_ops_recurs_func;
+
+ func = ftrace_ops_get_func(ftrace_ops_list);
} else {
/* Just use the default ftrace_ops */
set_function_trace_op = &ftrace_list_end;
@@ -4856,6 +4848,37 @@ static void ftrace_ops_recurs_func(unsigned long ip, unsigned long parent_ip,
trace_clear_recursion(bit);
}
+/**
+ * ftrace_ops_get_func - get the function a trampoline should call
+ * @ops: the ops to get the function for
+ *
+ * Normally the mcount trampoline will call the ops->func, but there
+ * are times that it should not. For example, if the ops does not
+ * have its own recursion protection, then it should call the
+ * ftrace_ops_recurs_func() instead.
+ *
+ * Returns the function that the trampoline should call for @ops.
+ */
+ftrace_func_t ftrace_ops_get_func(struct ftrace_ops *ops)
+{
+ /*
+ * If this is a dynamic ops or we force list func,
+ * then it needs to call the list anyway.
+ */
+ if (ops->flags & FTRACE_OPS_FL_DYNAMIC || FTRACE_FORCE_LIST_FUNC)
+ return ftrace_ops_list_func;
+
+ /*
+ * If the func handles its own recursion, call it directly.
+ * Otherwise call the recursion protected function that
+ * will call the ftrace ops function.
+ */
+ if (!(ops->flags & FTRACE_OPS_FL_RECURSION_SAFE))
+ return ftrace_ops_recurs_func;
+
+ return ops->func;
+}
+
static void clear_ftrace_swapper(void)
{
struct task_struct *p;
--
2.0.1
^ permalink raw reply [flat|nested] 10+ messages in thread* [for-next][PATCH 3/9] ftrace: Set callback to ftrace_stub when no ops are registered
2014-09-11 14:47 [for-next][PATCH 0/9] ftrace: 3.18 queue Steven Rostedt
2014-09-11 14:47 ` [for-next][PATCH 1/9] ftrace: Add separate function for non recursive callbacks Steven Rostedt
2014-09-11 14:47 ` [for-next][PATCH 2/9] ftrace: Add helper function ftrace_ops_get_func() Steven Rostedt
@ 2014-09-11 14:47 ` Steven Rostedt
2014-09-11 14:47 ` [for-next][PATCH 4/9] ftrace: Remove freeing of old_hash from ftrace_hash_move() Steven Rostedt
` (5 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Steven Rostedt @ 2014-09-11 14:47 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton
[-- Attachment #1: 0003-ftrace-Set-callback-to-ftrace_stub-when-no-ops-are-r.patch --]
[-- Type: text/plain, Size: 1969 bytes --]
From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
The clean up that adds the helper function ftrace_ops_get_func()
caused the default function to not change when DYNAMIC_FTRACE was not
set and no ftrace_ops were registered. Although static tracing is
not very useful (not having DYNAMIC_FTRACE set), it is still supported
and we don't want to break it.
Clean up the if statement even more to specifically have the default
function call ftrace_stub when no ftrace_ops are registered. This
fixes the small bug for static tracing as well as makes the code a
bit more understandable.
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
kernel/trace/ftrace.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index dabf734f909c..708aea493d96 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -254,17 +254,24 @@ static void update_ftrace_function(void)
ftrace_func_t func;
/*
+ * Prepare the ftrace_ops that the arch callback will use.
+ * If there's only one ftrace_ops registered, the ftrace_ops_list
+ * will point to the ops we want.
+ */
+ set_function_trace_op = ftrace_ops_list;
+
+ /* If there's no ftrace_ops registered, just call the stub function */
+ if (ftrace_ops_list == &ftrace_list_end) {
+ func = ftrace_stub;
+
+ /*
* If we are at the end of the list and this ops is
* recursion safe and not dynamic and the arch supports passing ops,
* then have the mcount trampoline call the function directly.
*/
- if (ftrace_ops_list == &ftrace_list_end ||
- (ftrace_ops_list->next == &ftrace_list_end)) {
-
- /* Set the ftrace_ops that the arch callback uses */
- set_function_trace_op = ftrace_ops_list;
-
+ } else if (ftrace_ops_list->next == &ftrace_list_end) {
func = ftrace_ops_get_func(ftrace_ops_list);
+
} else {
/* Just use the default ftrace_ops */
set_function_trace_op = &ftrace_list_end;
--
2.0.1
^ permalink raw reply [flat|nested] 10+ messages in thread* [for-next][PATCH 4/9] ftrace: Remove freeing of old_hash from ftrace_hash_move()
2014-09-11 14:47 [for-next][PATCH 0/9] ftrace: 3.18 queue Steven Rostedt
` (2 preceding siblings ...)
2014-09-11 14:47 ` [for-next][PATCH 3/9] ftrace: Set callback to ftrace_stub when no ops are registered Steven Rostedt
@ 2014-09-11 14:47 ` Steven Rostedt
2014-09-11 14:47 ` [for-next][PATCH 5/9] ftrace: Grab any ops for a rec for enabled_functions output Steven Rostedt
` (4 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Steven Rostedt @ 2014-09-11 14:47 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton
[-- Attachment #1: 0004-ftrace-Remove-freeing-of-old_hash-from-ftrace_hash_m.patch --]
[-- Type: text/plain, Size: 4721 bytes --]
From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
ftrace_hash_move() currently frees the old hash that is passed to it
after replacing the pointer with the new hash. Instead of having the
function do that chore, have the caller perform the free.
This lets the ftrace_hash_move() be used a bit more freely, which
is needed for changing the way the trampoline logic is done.
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
kernel/trace/ftrace.c | 31 +++++++++++++++++++++----------
1 file changed, 21 insertions(+), 10 deletions(-)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 708aea493d96..2c4eef49b1af 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -1316,7 +1316,6 @@ ftrace_hash_move(struct ftrace_ops *ops, int enable,
struct ftrace_func_entry *entry;
struct hlist_node *tn;
struct hlist_head *hhd;
- struct ftrace_hash *old_hash;
struct ftrace_hash *new_hash;
int size = src->count;
int bits = 0;
@@ -1361,9 +1360,7 @@ update:
*/
ftrace_hash_rec_disable_modify(ops, enable);
- old_hash = *dst;
rcu_assign_pointer(*dst, new_hash);
- free_ftrace_hash_rcu(old_hash);
ftrace_hash_rec_enable_modify(ops, enable);
@@ -3408,6 +3405,7 @@ register_ftrace_function_probe(char *glob, struct ftrace_probe_ops *ops,
{
struct ftrace_func_probe *entry;
struct ftrace_hash **orig_hash = &trace_probe_ops.func_hash->filter_hash;
+ struct ftrace_hash *old_hash = *orig_hash;
struct ftrace_hash *hash;
struct ftrace_page *pg;
struct dyn_ftrace *rec;
@@ -3426,7 +3424,7 @@ register_ftrace_function_probe(char *glob, struct ftrace_probe_ops *ops,
mutex_lock(&trace_probe_ops.func_hash->regex_lock);
- 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) {
count = -ENOMEM;
goto out;
@@ -3485,7 +3483,9 @@ register_ftrace_function_probe(char *glob, struct ftrace_probe_ops *ops,
} while_for_each_ftrace_rec();
ret = ftrace_hash_move(&trace_probe_ops, 1, orig_hash, hash);
- if (ret < 0)
+ if (!ret)
+ free_ftrace_hash_rcu(old_hash);
+ else
count = ret;
__enable_ftrace_function_probe();
@@ -3512,6 +3512,7 @@ __unregister_ftrace_function_probe(char *glob, struct ftrace_probe_ops *ops,
struct ftrace_func_probe *entry;
struct ftrace_func_probe *p;
struct ftrace_hash **orig_hash = &trace_probe_ops.func_hash->filter_hash;
+ struct ftrace_hash *old_hash = *orig_hash;
struct list_head free_list;
struct ftrace_hash *hash;
struct hlist_node *tmp;
@@ -3519,6 +3520,7 @@ __unregister_ftrace_function_probe(char *glob, struct ftrace_probe_ops *ops,
int type = MATCH_FULL;
int i, len = 0;
char *search;
+ int ret;
if (glob && (strcmp(glob, "*") == 0 || !strlen(glob)))
glob = NULL;
@@ -3577,8 +3579,11 @@ __unregister_ftrace_function_probe(char *glob, struct ftrace_probe_ops *ops,
* Remove after the disable is called. Otherwise, if the last
* probe is removed, a null hash means *all enabled*.
*/
- ftrace_hash_move(&trace_probe_ops, 1, orig_hash, hash);
+ ret = ftrace_hash_move(&trace_probe_ops, 1, orig_hash, hash);
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);
ftrace_free_entry(entry);
@@ -3776,6 +3781,7 @@ ftrace_set_hash(struct ftrace_ops *ops, unsigned char *buf, int len,
unsigned long ip, int remove, int reset, int enable)
{
struct ftrace_hash **orig_hash;
+ struct ftrace_hash *old_hash;
struct ftrace_hash *hash;
int ret;
@@ -3810,10 +3816,12 @@ ftrace_set_hash(struct ftrace_ops *ops, unsigned char *buf, int len,
}
mutex_lock(&ftrace_lock);
+ old_hash = *orig_hash;
ret = ftrace_hash_move(ops, enable, orig_hash, hash);
- if (!ret)
+ if (!ret) {
ftrace_ops_update_code(ops);
-
+ free_ftrace_hash_rcu(old_hash);
+ }
mutex_unlock(&ftrace_lock);
out_regex_unlock:
@@ -4022,6 +4030,7 @@ int 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 ftrace_hash *old_hash;
struct trace_parser *parser;
int filter_hash;
int ret;
@@ -4051,11 +4060,13 @@ int ftrace_regex_release(struct inode *inode, struct file *file)
orig_hash = &iter->ops->func_hash->notrace_hash;
mutex_lock(&ftrace_lock);
+ old_hash = *orig_hash;
ret = ftrace_hash_move(iter->ops, filter_hash,
orig_hash, iter->hash);
- if (!ret)
+ if (!ret) {
ftrace_ops_update_code(iter->ops);
-
+ free_ftrace_hash_rcu(old_hash);
+ }
mutex_unlock(&ftrace_lock);
}
--
2.0.1
^ permalink raw reply [flat|nested] 10+ messages in thread* [for-next][PATCH 5/9] ftrace: Grab any ops for a rec for enabled_functions output
2014-09-11 14:47 [for-next][PATCH 0/9] ftrace: 3.18 queue Steven Rostedt
` (3 preceding siblings ...)
2014-09-11 14:47 ` [for-next][PATCH 4/9] ftrace: Remove freeing of old_hash from ftrace_hash_move() Steven Rostedt
@ 2014-09-11 14:47 ` Steven Rostedt
2014-09-11 14:47 ` [for-next][PATCH 6/9] ftrace: Annotate the ops operation on update Steven Rostedt
` (3 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Steven Rostedt @ 2014-09-11 14:47 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton
[-- Attachment #1: 0005-ftrace-Grab-any-ops-for-a-rec-for-enabled_functions-.patch --]
[-- Type: text/plain, Size: 1560 bytes --]
From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
When dumping the enabled_functions, use the first op that is
found with a trampoline to the record, as there should only be
one, as only one ops can be registered to a function that has
a trampoline.
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
kernel/trace/ftrace.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 2c4eef49b1af..858ac16f8492 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -1901,6 +1901,25 @@ int ftrace_test_record(struct dyn_ftrace *rec, int enable)
}
static struct ftrace_ops *
+ftrace_find_tramp_ops_any(struct dyn_ftrace *rec)
+{
+ struct ftrace_ops *op;
+
+ do_for_each_ftrace_op(op, ftrace_ops_list) {
+
+ if (!op->trampoline)
+ continue;
+
+ if (ftrace_lookup_ip(op->func_hash->filter_hash, rec->ip) &&
+ (ftrace_hash_empty(op->func_hash->notrace_hash) ||
+ !ftrace_lookup_ip(op->func_hash->notrace_hash, rec->ip)))
+ return op;
+ } while_for_each_ftrace_op(op);
+
+ return NULL;
+}
+
+static struct ftrace_ops *
ftrace_find_tramp_ops_curr(struct dyn_ftrace *rec)
{
struct ftrace_ops *op;
@@ -2966,7 +2985,7 @@ static int t_show(struct seq_file *m, void *v)
if (rec->flags & FTRACE_FL_TRAMP_EN) {
struct ftrace_ops *ops;
- ops = ftrace_find_tramp_ops_curr(rec);
+ ops = ftrace_find_tramp_ops_any(rec);
if (ops && ops->trampoline)
seq_printf(m, "\ttramp: %pS",
(void *)ops->trampoline);
--
2.0.1
^ permalink raw reply [flat|nested] 10+ messages in thread* [for-next][PATCH 6/9] ftrace: Annotate the ops operation on update
2014-09-11 14:47 [for-next][PATCH 0/9] ftrace: 3.18 queue Steven Rostedt
` (4 preceding siblings ...)
2014-09-11 14:47 ` [for-next][PATCH 5/9] ftrace: Grab any ops for a rec for enabled_functions output Steven Rostedt
@ 2014-09-11 14:47 ` Steven Rostedt
2014-09-11 14:47 ` [for-next][PATCH 7/9] ftrace: Replace tramp_hash with old_*_hash to save space Steven Rostedt
` (2 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Steven Rostedt @ 2014-09-11 14:47 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton
[-- Attachment #1: 0006-ftrace-Annotate-the-ops-operation-on-update.patch --]
[-- Type: text/plain, Size: 5954 bytes --]
From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
Add three new flags for ftrace_ops:
FTRACE_OPS_FL_ADDING
FTRACE_OPS_FL_REMOVING
FTRACE_OPS_FL_MODIFYING
These will be set for the ftrace_ops when they are first added
to the function tracing, being removed from function tracing
or just having their functions changed from function tracing,
respectively.
This will be needed to remove the tramp_hash, which can grow quite
big. The tramp_hash is used to note what functions a ftrace_ops
is using a trampoline for. Denoting which ftrace_ops is being
modified, will allow us to use the ftrace_ops hashes themselves,
which are much smaller as they have a global flag to denote if
a ftrace_ops is tracing all functions, as well as a notrace hash
if the ftrace_ops is tracing all but a few. The tramp_hash just
creates a hash item for every function, which can go into the 10s
of thousands if all functions are using the ftrace_ops trampoline.
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
include/linux/ftrace.h | 6 ++++++
kernel/trace/ftrace.c | 45 +++++++++++++++++++++++++++++++++++++++------
2 files changed, 45 insertions(+), 6 deletions(-)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index ef37286547fc..d9216f6385d9 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -91,6 +91,9 @@ ftrace_func_t ftrace_ops_get_func(struct ftrace_ops *ops);
* INITIALIZED - The ftrace_ops has already been initialized (first use time
* register_ftrace_function() is called, it will initialized the ops)
* DELETED - The ops are being deleted, do not let them be registered again.
+ * ADDING - The ops is in the process of being added.
+ * REMOVING - The ops is in the process of being removed.
+ * MODIFYING - The ops is in the process of changing its filter functions.
*/
enum {
FTRACE_OPS_FL_ENABLED = 1 << 0,
@@ -102,6 +105,9 @@ enum {
FTRACE_OPS_FL_STUB = 1 << 6,
FTRACE_OPS_FL_INITIALIZED = 1 << 7,
FTRACE_OPS_FL_DELETED = 1 << 8,
+ FTRACE_OPS_FL_ADDING = 1 << 9,
+ FTRACE_OPS_FL_REMOVING = 1 << 10,
+ FTRACE_OPS_FL_MODIFYING = 1 << 11,
};
#ifdef CONFIG_DYNAMIC_FTRACE
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 858ac16f8492..e43c793093e5 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -1057,6 +1057,12 @@ static struct pid * const ftrace_swapper_pid = &init_struct_pid;
static struct ftrace_ops *removed_ops;
+/*
+ * Set when doing a global update, like enabling all recs or disabling them.
+ * It is not set when just updating a single ftrace_ops.
+ */
+static bool update_all_ops;
+
#ifndef CONFIG_FTRACE_MCOUNT_RECORD
# error Dynamic ftrace depends on MCOUNT_RECORD
#endif
@@ -2366,6 +2372,13 @@ static void ftrace_run_update_code(int command)
FTRACE_WARN_ON(ret);
}
+static void ftrace_run_modify_code(struct ftrace_ops *ops, int command)
+{
+ ops->flags |= FTRACE_OPS_FL_MODIFYING;
+ ftrace_run_update_code(command);
+ ops->flags &= ~FTRACE_OPS_FL_MODIFYING;
+}
+
static ftrace_func_t saved_ftrace_func;
static int ftrace_start_up;
@@ -2387,6 +2400,13 @@ static void ftrace_startup_enable(int command)
ftrace_run_update_code(command);
}
+static void ftrace_startup_all(int command)
+{
+ update_all_ops = true;
+ ftrace_startup_enable(command);
+ update_all_ops = false;
+}
+
static int ftrace_startup(struct ftrace_ops *ops, int command)
{
int ret;
@@ -2401,12 +2421,22 @@ static int ftrace_startup(struct ftrace_ops *ops, int command)
ftrace_start_up++;
command |= FTRACE_UPDATE_CALLS;
- ops->flags |= FTRACE_OPS_FL_ENABLED;
+ /*
+ * Note that ftrace probes uses this to start up
+ * and modify functions it will probe. But we still
+ * set the ADDING flag for modification, as probes
+ * do not have trampolines. If they add them in the
+ * future, then the probes will need to distinguish
+ * between adding and updating probes.
+ */
+ ops->flags |= FTRACE_OPS_FL_ENABLED | FTRACE_OPS_FL_ADDING;
ftrace_hash_rec_enable(ops, 1);
ftrace_startup_enable(command);
+ ops->flags &= ~FTRACE_OPS_FL_ADDING;
+
return 0;
}
@@ -2456,11 +2486,12 @@ static int ftrace_shutdown(struct ftrace_ops *ops, int command)
* If the ops uses a trampoline, then it needs to be
* tested first on update.
*/
+ ops->flags |= FTRACE_OPS_FL_REMOVING;
removed_ops = ops;
ftrace_run_update_code(command);
- removed_ops = NULL;
+ ops->flags &= ~FTRACE_OPS_FL_REMOVING;
/*
* Dynamic ops may be freed, we must make sure that all
@@ -3373,7 +3404,7 @@ static void __enable_ftrace_function_probe(void)
if (ftrace_probe_registered) {
/* still need to update the function call sites */
if (ftrace_enabled)
- ftrace_run_update_code(FTRACE_UPDATE_CALLS);
+ ftrace_run_modify_code(&trace_probe_ops, FTRACE_UPDATE_CALLS);
return;
}
@@ -3792,7 +3823,7 @@ ftrace_match_addr(struct ftrace_hash *hash, unsigned long ip, int remove)
static void ftrace_ops_update_code(struct ftrace_ops *ops)
{
if (ops->flags & FTRACE_OPS_FL_ENABLED && ftrace_enabled)
- ftrace_run_update_code(FTRACE_UPDATE_CALLS);
+ ftrace_run_modify_code(ops, FTRACE_UPDATE_CALLS);
}
static int
@@ -4717,6 +4748,7 @@ core_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) { }
+static inline void ftrace_startup_all(int command) { }
/* Keep as macros so we do not need to define the commands */
# define ftrace_startup(ops, command) \
({ \
@@ -5016,7 +5048,8 @@ static int ftrace_pid_add(int p)
set_ftrace_pid_task(pid);
ftrace_update_pid_func();
- ftrace_startup_enable(0);
+
+ ftrace_startup_all(0);
mutex_unlock(&ftrace_lock);
return 0;
@@ -5045,7 +5078,7 @@ static void ftrace_pid_reset(void)
}
ftrace_update_pid_func();
- ftrace_startup_enable(0);
+ ftrace_startup_all(0);
mutex_unlock(&ftrace_lock);
}
--
2.0.1
^ permalink raw reply [flat|nested] 10+ messages in thread* [for-next][PATCH 7/9] ftrace: Replace tramp_hash with old_*_hash to save space
2014-09-11 14:47 [for-next][PATCH 0/9] ftrace: 3.18 queue Steven Rostedt
` (5 preceding siblings ...)
2014-09-11 14:47 ` [for-next][PATCH 6/9] ftrace: Annotate the ops operation on update Steven Rostedt
@ 2014-09-11 14:47 ` Steven Rostedt
2014-09-11 14:47 ` [for-next][PATCH 8/9] tracing: generate RCU warnings even when tracepoints are disabled Steven Rostedt
2014-09-11 14:47 ` [for-next][PATCH 9/9] kernel: trace_syscalls: Replace rcu_assign_pointer() with RCU_INIT_POINTER() Steven Rostedt
8 siblings, 0 replies; 10+ messages in thread
From: Steven Rostedt @ 2014-09-11 14:47 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton
[-- Attachment #1: 0007-ftrace-Replace-tramp_hash-with-old_-_hash-to-save-sp.patch --]
[-- Type: text/plain, Size: 13352 bytes --]
From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
Allowing function callbacks to declare their own trampolines requires
that each ftrace_ops that has a trampoline must have some sort of
accounting that keeps track of which ops has a trampoline attached
to a record.
The easy way to solve this was to add a "tramp_hash" that created a
hash entry for every function that a ops uses with a trampoline.
But since we can have literally tens of thousands of functions being
traced, that means we need tens of thousands of descriptors to map
the ops to the function in the hash. This is quite expensive and
can cause enabling and disabling the function graph tracer to take
some time to start and stop. It can take up to several seconds to
disable or enable all functions in the function graph tracer for this
reason.
The better approach albeit more complex, is to keep track of how ops
are being enabled and disabled, and use that along with the counting
of the number of ops attached to records, to determive what ops has
a trampoline attached to a record at enabling and disabling of
tracing.
To do this, the tramp_hash has been replaced with an old_filter_hash
and old_notrace_hash, which get the copy of the ops filter_hash and
notrace_hash respectively. The old hashes is kept until the ops has
been modified or removed and the old hashes are used with the logic
of the accounting to determine the ops that have the trampoline of
a record. The reason this has less of a footprint is due to the trick
that an "empty" hash in the filter_hash means "all functions" and
an empty hash in the notrace hash means "no functions" in the hash.
This is much more efficienct, doesn't have the delay, and takes up
much less memory, as we do not need to map all the functions but
just figure out which functions are mapped at the time it is
enabled or disabled.
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
include/linux/ftrace.h | 2 +-
kernel/trace/ftrace.c | 239 +++++++++++++++++--------------------------------
2 files changed, 85 insertions(+), 156 deletions(-)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index d9216f6385d9..662697babd48 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -140,7 +140,7 @@ struct ftrace_ops {
int nr_trampolines;
struct ftrace_ops_hash local_hash;
struct ftrace_ops_hash *func_hash;
- struct ftrace_hash *tramp_hash;
+ struct ftrace_ops_hash old_hash;
unsigned long trampoline;
#endif
};
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index e43c793093e5..d325a1e76554 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -1373,6 +1373,21 @@ update:
return 0;
}
+static bool hash_contains_ip(unsigned long ip,
+ struct ftrace_ops_hash *hash)
+{
+ /*
+ * The function record is a match if it exists in the filter
+ * hash and not in the notrace hash. Note, an emty hash is
+ * considered a match for the filter hash, but an empty
+ * notrace hash is considered not in the notrace hash.
+ */
+ return (ftrace_hash_empty(hash->filter_hash) ||
+ ftrace_lookup_ip(hash->filter_hash, ip)) &&
+ (ftrace_hash_empty(hash->notrace_hash) ||
+ !ftrace_lookup_ip(hash->notrace_hash, ip));
+}
+
/*
* Test the hashes for this ops to see if we want to call
* the ops->func or not.
@@ -1388,8 +1403,7 @@ update:
static int
ftrace_ops_test(struct ftrace_ops *ops, unsigned long ip, void *regs)
{
- struct ftrace_hash *filter_hash;
- struct ftrace_hash *notrace_hash;
+ struct ftrace_ops_hash hash;
int ret;
#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
@@ -1402,13 +1416,10 @@ ftrace_ops_test(struct ftrace_ops *ops, unsigned long ip, void *regs)
return 0;
#endif
- filter_hash = rcu_dereference_raw_notrace(ops->func_hash->filter_hash);
- notrace_hash = rcu_dereference_raw_notrace(ops->func_hash->notrace_hash);
+ hash.filter_hash = rcu_dereference_raw_notrace(ops->func_hash->filter_hash);
+ hash.notrace_hash = rcu_dereference_raw_notrace(ops->func_hash->notrace_hash);
- if ((ftrace_hash_empty(filter_hash) ||
- ftrace_lookup_ip(filter_hash, ip)) &&
- (ftrace_hash_empty(notrace_hash) ||
- !ftrace_lookup_ip(notrace_hash, ip)))
+ if (hash_contains_ip(ip, &hash))
ret = 1;
else
ret = 0;
@@ -1520,46 +1531,6 @@ static bool test_rec_ops_needs_regs(struct dyn_ftrace *rec)
return keep_regs;
}
-static void ftrace_remove_tramp(struct ftrace_ops *ops,
- struct dyn_ftrace *rec)
-{
- /* If TRAMP is not set, no ops should have a trampoline for this */
- if (!(rec->flags & FTRACE_FL_TRAMP))
- return;
-
- rec->flags &= ~FTRACE_FL_TRAMP;
-
- if ((!ftrace_hash_empty(ops->func_hash->filter_hash) &&
- !ftrace_lookup_ip(ops->func_hash->filter_hash, rec->ip)) ||
- ftrace_lookup_ip(ops->func_hash->notrace_hash, rec->ip))
- return;
- /*
- * The tramp_hash entry will be removed at time
- * of update.
- */
- ops->nr_trampolines--;
-}
-
-static void ftrace_clear_tramps(struct dyn_ftrace *rec, struct ftrace_ops *ops)
-{
- struct ftrace_ops *op;
-
- /* If TRAMP is not set, no ops should have a trampoline for this */
- if (!(rec->flags & FTRACE_FL_TRAMP))
- return;
-
- do_for_each_ftrace_op(op, ftrace_ops_list) {
- /*
- * This function is called to clear other tramps
- * not the one that is being updated.
- */
- if (op == ops)
- continue;
- if (op->nr_trampolines)
- ftrace_remove_tramp(op, rec);
- } while_for_each_ftrace_op(op);
-}
-
static void __ftrace_hash_rec_update(struct ftrace_ops *ops,
int filter_hash,
bool inc)
@@ -1648,18 +1619,16 @@ static void __ftrace_hash_rec_update(struct ftrace_ops *ops,
* function, and the ops has a trampoline registered
* for it, then we can call it directly.
*/
- if (ftrace_rec_count(rec) == 1 && ops->trampoline) {
+ if (ftrace_rec_count(rec) == 1 && ops->trampoline)
rec->flags |= FTRACE_FL_TRAMP;
- ops->nr_trampolines++;
- } else {
+ else
/*
* If we are adding another function callback
* to this function, and the previous had a
* custom trampoline in use, then we need to go
* back to the default trampoline.
*/
- ftrace_clear_tramps(rec, ops);
- }
+ rec->flags &= ~FTRACE_FL_TRAMP;
/*
* If any ops wants regs saved for this function
@@ -1672,9 +1641,6 @@ static void __ftrace_hash_rec_update(struct ftrace_ops *ops,
return;
rec->flags--;
- if (ops->trampoline && !ftrace_rec_count(rec))
- ftrace_remove_tramp(ops, rec);
-
/*
* If the rec had REGS enabled and the ops that is
* being removed had REGS set, then see if there is
@@ -1689,6 +1655,17 @@ static void __ftrace_hash_rec_update(struct ftrace_ops *ops,
}
/*
+ * If the rec had TRAMP enabled, then it needs to
+ * be cleared. As TRAMP can only be enabled iff
+ * there is only a single ops attached to it.
+ * In otherwords, always disable it on decrementing.
+ * In the future, we may set it if rec count is
+ * decremented to one, and the ops that is left
+ * has a trampoline.
+ */
+ rec->flags &= ~FTRACE_FL_TRAMP;
+
+ /*
* flags will be cleared in ftrace_check_record()
* if rec count is zero.
*/
@@ -1910,15 +1887,14 @@ static struct ftrace_ops *
ftrace_find_tramp_ops_any(struct dyn_ftrace *rec)
{
struct ftrace_ops *op;
+ unsigned long ip = rec->ip;
do_for_each_ftrace_op(op, ftrace_ops_list) {
if (!op->trampoline)
continue;
- if (ftrace_lookup_ip(op->func_hash->filter_hash, rec->ip) &&
- (ftrace_hash_empty(op->func_hash->notrace_hash) ||
- !ftrace_lookup_ip(op->func_hash->notrace_hash, rec->ip)))
+ if (hash_contains_ip(ip, op->func_hash))
return op;
} while_for_each_ftrace_op(op);
@@ -1929,18 +1905,51 @@ static struct ftrace_ops *
ftrace_find_tramp_ops_curr(struct dyn_ftrace *rec)
{
struct ftrace_ops *op;
+ unsigned long ip = rec->ip;
- /* Removed ops need to be tested first */
- if (removed_ops && removed_ops->tramp_hash) {
- if (ftrace_lookup_ip(removed_ops->tramp_hash, rec->ip))
+ /*
+ * Need to check removed ops first.
+ * If they are being removed, and this rec has a tramp,
+ * and this rec is in the ops list, then it would be the
+ * one with the tramp.
+ */
+ if (removed_ops) {
+ if (hash_contains_ip(ip, &removed_ops->old_hash))
return removed_ops;
}
+ /*
+ * Need to find the current trampoline for a rec.
+ * Now, a trampoline is only attached to a rec if there
+ * was a single 'ops' attached to it. But this can be called
+ * when we are adding another op to the rec or removing the
+ * current one. Thus, if the op is being added, we can
+ * ignore it because it hasn't attached itself to the rec
+ * yet. That means we just need to find the op that has a
+ * trampoline and is not beeing added.
+ */
do_for_each_ftrace_op(op, ftrace_ops_list) {
- if (!op->tramp_hash)
+
+ if (!op->trampoline)
+ continue;
+
+ /*
+ * If the ops is being added, it hasn't gotten to
+ * the point to be removed from this tree yet.
+ */
+ if (op->flags & FTRACE_OPS_FL_ADDING)
continue;
- if (ftrace_lookup_ip(op->tramp_hash, rec->ip))
+ /*
+ * If the ops is not being added and has a trampoline,
+ * then it must be the one that we want!
+ */
+ if (hash_contains_ip(ip, op->func_hash))
+ return op;
+
+ /* If the ops is being modified, it may be in the old hash. */
+ if ((op->flags & FTRACE_OPS_FL_MODIFYING) &&
+ hash_contains_ip(ip, &op->old_hash))
return op;
} while_for_each_ftrace_op(op);
@@ -1952,10 +1961,11 @@ static struct ftrace_ops *
ftrace_find_tramp_ops_new(struct dyn_ftrace *rec)
{
struct ftrace_ops *op;
+ unsigned long ip = rec->ip;
do_for_each_ftrace_op(op, ftrace_ops_list) {
/* pass rec in as regs to have non-NULL val */
- if (ftrace_ops_test(op, rec->ip, rec))
+ if (hash_contains_ip(ip, op->func_hash))
return op;
} while_for_each_ftrace_op(op);
@@ -2262,92 +2272,6 @@ void __weak arch_ftrace_update_code(int command)
ftrace_run_stop_machine(command);
}
-static int ftrace_save_ops_tramp_hash(struct ftrace_ops *ops)
-{
- struct ftrace_page *pg;
- struct dyn_ftrace *rec;
- int size, bits;
- int ret;
-
- size = ops->nr_trampolines;
- bits = 0;
- /*
- * Make the hash size about 1/2 the # found
- */
- for (size /= 2; size; size >>= 1)
- bits++;
-
- ops->tramp_hash = alloc_ftrace_hash(bits);
- /*
- * TODO: a failed allocation is going to screw up
- * the accounting of what needs to be modified
- * and not. For now, we kill ftrace if we fail
- * to allocate here. But there are ways around this,
- * but that will take a little more work.
- */
- if (!ops->tramp_hash)
- return -ENOMEM;
-
- do_for_each_ftrace_rec(pg, rec) {
- if (ftrace_rec_count(rec) == 1 &&
- ftrace_ops_test(ops, rec->ip, rec)) {
-
- /*
- * If another ops adds to a rec, the rec will
- * lose its trampoline and never get it back
- * until all ops are off of it.
- */
- if (!(rec->flags & FTRACE_FL_TRAMP))
- continue;
-
- /* This record had better have a trampoline */
- if (FTRACE_WARN_ON(!(rec->flags & FTRACE_FL_TRAMP_EN)))
- return -1;
-
- ret = add_hash_entry(ops->tramp_hash, rec->ip);
- if (ret < 0)
- return ret;
- }
- } while_for_each_ftrace_rec();
-
- /* The number of recs in the hash must match nr_trampolines */
- if (FTRACE_WARN_ON(ops->tramp_hash->count != ops->nr_trampolines))
- pr_warn("count=%ld trampolines=%d\n",
- ops->tramp_hash->count,
- ops->nr_trampolines);
-
- return 0;
-}
-
-static int ftrace_save_tramp_hashes(void)
-{
- struct ftrace_ops *op;
- int ret;
-
- /*
- * Now that any trampoline is being used, we need to save the
- * hashes for the ops that have them. This allows the mapping
- * back from the record to the ops that has the trampoline to
- * know what code is being replaced. Modifying code must always
- * verify what it is changing.
- */
- do_for_each_ftrace_op(op, ftrace_ops_list) {
-
- /* The tramp_hash is recreated each time. */
- free_ftrace_hash(op->tramp_hash);
- op->tramp_hash = NULL;
-
- if (op->nr_trampolines) {
- ret = ftrace_save_ops_tramp_hash(op);
- if (ret)
- return ret;
- }
-
- } while_for_each_ftrace_op(op);
-
- return 0;
-}
-
static void ftrace_run_update_code(int command)
{
int ret;
@@ -2367,9 +2291,6 @@ static void ftrace_run_update_code(int command)
ret = ftrace_arch_code_modify_post_process();
FTRACE_WARN_ON(ret);
-
- ret = ftrace_save_tramp_hashes();
- FTRACE_WARN_ON(ret);
}
static void ftrace_run_modify_code(struct ftrace_ops *ops, int command)
@@ -2489,8 +2410,16 @@ static int ftrace_shutdown(struct ftrace_ops *ops, int command)
ops->flags |= FTRACE_OPS_FL_REMOVING;
removed_ops = ops;
+ /* The trampoline logic checks the old hashes */
+ ops->old_hash.filter_hash = ops->func_hash->filter_hash;
+ ops->old_hash.notrace_hash = ops->func_hash->notrace_hash;
+
ftrace_run_update_code(command);
+ ops->old_hash.filter_hash = NULL;
+ ops->old_hash.notrace_hash = NULL;
+
+ removed_ops = NULL;
ops->flags &= ~FTRACE_OPS_FL_REMOVING;
/*
@@ -3017,7 +2946,7 @@ static int t_show(struct seq_file *m, void *v)
struct ftrace_ops *ops;
ops = ftrace_find_tramp_ops_any(rec);
- if (ops && ops->trampoline)
+ if (ops)
seq_printf(m, "\ttramp: %pS",
(void *)ops->trampoline);
else
--
2.0.1
^ permalink raw reply [flat|nested] 10+ messages in thread* [for-next][PATCH 8/9] tracing: generate RCU warnings even when tracepoints are disabled
2014-09-11 14:47 [for-next][PATCH 0/9] ftrace: 3.18 queue Steven Rostedt
` (6 preceding siblings ...)
2014-09-11 14:47 ` [for-next][PATCH 7/9] ftrace: Replace tramp_hash with old_*_hash to save space Steven Rostedt
@ 2014-09-11 14:47 ` Steven Rostedt
2014-09-11 14:47 ` [for-next][PATCH 9/9] kernel: trace_syscalls: Replace rcu_assign_pointer() with RCU_INIT_POINTER() Steven Rostedt
8 siblings, 0 replies; 10+ messages in thread
From: Steven Rostedt @ 2014-09-11 14:47 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Dave Hansen, Dave Hansen, paulmck,
Ingo Molnar
[-- Attachment #1: 0008-tracing-generate-RCU-warnings-even-when-tracepoints-.patch --]
[-- Type: text/plain, Size: 2692 bytes --]
From: Dave Hansen <dave.hansen@linux.intel.com>
Dave Jones reported seeing a bug from one of my TLB tracepoints:
http://lkml.kernel.org/r/20140806181801.GA4605@redhat.com
I've been running these patches for months and never saw this.
But, a big chunk of my testing, especially with all the debugging
enabled, was in a vm where intel_idle doesn't work. On the
systems where I was using intel_idle, I never had lockdep enabled
and this tracepoint on at the same time.
This patch ensures that whenever we have lockdep available, we do
_some_ RCU activity at the site of the tracepoint, despite
whether the tracepoint's condition matches or even if the
tracepoint itself is completely disabled. This is a bit of a
hack, but it is pretty self-contained.
I confirmed that with this patch plus lockdep I get the same
splat as Dave Jones did, but without enabling the tracepoint
explicitly.
Link: http://lkml.kernel.org/p/20140807175204.C257CAC5@viggo.jf.intel.com
Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Dave Hansen <dave@sr71.net>
Cc: Dave Jones <davej@redhat.com>,
Cc: paulmck@linux.vnet.ibm.com
Cc: Ingo Molnar <mingo@redhat.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
include/linux/tracepoint.h | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index b1293f15f592..e08e21e5f601 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -157,6 +157,12 @@ extern void syscall_unregfunc(void);
* Make sure the alignment of the structure in the __tracepoints section will
* not add unwanted padding between the beginning of the section and the
* structure. Force alignment to the same alignment as the section start.
+ *
+ * When lockdep is enabled, we make sure to always do the RCU portions of
+ * the tracepoint code, regardless of whether tracing is on or we match the
+ * condition. This lets us find RCU issues triggered with tracepoints even
+ * when this tracepoint is off. This code has no purpose other than poking
+ * RCU a bit.
*/
#define __DECLARE_TRACE(name, proto, args, cond, data_proto, data_args) \
extern struct tracepoint __tracepoint_##name; \
@@ -167,6 +173,11 @@ extern void syscall_unregfunc(void);
TP_PROTO(data_proto), \
TP_ARGS(data_args), \
TP_CONDITION(cond),,); \
+ if (IS_ENABLED(CONFIG_LOCKDEP)) { \
+ rcu_read_lock_sched_notrace(); \
+ rcu_dereference_sched(__tracepoint_##name.funcs);\
+ rcu_read_unlock_sched_notrace(); \
+ } \
} \
__DECLARE_TRACE_RCU(name, PARAMS(proto), PARAMS(args), \
PARAMS(cond), PARAMS(data_proto), PARAMS(data_args)) \
--
2.0.1
^ permalink raw reply [flat|nested] 10+ messages in thread* [for-next][PATCH 9/9] kernel: trace_syscalls: Replace rcu_assign_pointer() with RCU_INIT_POINTER()
2014-09-11 14:47 [for-next][PATCH 0/9] ftrace: 3.18 queue Steven Rostedt
` (7 preceding siblings ...)
2014-09-11 14:47 ` [for-next][PATCH 8/9] tracing: generate RCU warnings even when tracepoints are disabled Steven Rostedt
@ 2014-09-11 14:47 ` Steven Rostedt
8 siblings, 0 replies; 10+ messages in thread
From: Steven Rostedt @ 2014-09-11 14:47 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Andreea-Cristina Bernat
[-- Attachment #1: 0009-kernel-trace_syscalls-Replace-rcu_assign_pointer-wit.patch --]
[-- Type: text/plain, Size: 1701 bytes --]
From: Andreea-Cristina Bernat <bernat.ada@gmail.com>
The uses of "rcu_assign_pointer()" are NULLing out the pointers.
According to RCU_INIT_POINTER()'s block comment:
"1. This use of RCU_INIT_POINTER() is NULLing out the pointer"
it is better to use it instead of rcu_assign_pointer() because it has a
smaller overhead.
The following Coccinelle semantic patch was used:
@@
@@
- rcu_assign_pointer
+ RCU_INIT_POINTER
(..., NULL)
Link: http://lkml.kernel.org/p/20140822142822.GA32391@ada
Signed-off-by: Andreea-Cristina Bernat <bernat.ada@gmail.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
kernel/trace/trace_syscalls.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index 759d5e004517..4dc8b79c5f75 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -425,7 +425,7 @@ static void unreg_event_syscall_enter(struct ftrace_event_file *file,
return;
mutex_lock(&syscall_trace_lock);
tr->sys_refcount_enter--;
- rcu_assign_pointer(tr->enter_syscall_files[num], NULL);
+ RCU_INIT_POINTER(tr->enter_syscall_files[num], NULL);
if (!tr->sys_refcount_enter)
unregister_trace_sys_enter(ftrace_syscall_enter, tr);
mutex_unlock(&syscall_trace_lock);
@@ -463,7 +463,7 @@ static void unreg_event_syscall_exit(struct ftrace_event_file *file,
return;
mutex_lock(&syscall_trace_lock);
tr->sys_refcount_exit--;
- rcu_assign_pointer(tr->exit_syscall_files[num], NULL);
+ RCU_INIT_POINTER(tr->exit_syscall_files[num], NULL);
if (!tr->sys_refcount_exit)
unregister_trace_sys_exit(ftrace_syscall_exit, tr);
mutex_unlock(&syscall_trace_lock);
--
2.0.1
^ permalink raw reply [flat|nested] 10+ messages in thread