Adding interface to modify registered direct function
for ftrace_ops. Adding following function:
modify_ftrace_direct_multi(struct ftrace_ops *ops, unsigned long addr)
The function changes the currently registered direct
function for all attached functions.
Signed-off-by: Jiri Olsa <[email protected]>
---
include/linux/ftrace.h | 6 ++++++
kernel/trace/ftrace.c | 43 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 49 insertions(+)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index e40b5201c16e..f3ba6366f7af 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -318,6 +318,8 @@ int ftrace_modify_direct_caller(struct ftrace_func_entry *entry,
unsigned long ftrace_find_rec_direct(unsigned long ip);
int register_ftrace_direct_multi(struct ftrace_ops *ops, unsigned long addr);
int unregister_ftrace_direct_multi(struct ftrace_ops *ops);
+int modify_ftrace_direct_multi(struct ftrace_ops *ops, unsigned long addr);
+
#else
struct ftrace_ops;
# define ftrace_direct_func_count 0
@@ -357,6 +359,10 @@ static inline int unregister_ftrace_direct_multi(struct ftrace_ops *ops)
{
return -ENODEV;
}
+static inline int modify_ftrace_direct_multi(struct ftrace_ops *ops, unsigned long addr)
+{
+ return -ENODEV;
+}
#endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
#ifndef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 7243769493c9..59940a6a907c 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -5518,6 +5518,49 @@ int unregister_ftrace_direct_multi(struct ftrace_ops *ops)
return err;
}
EXPORT_SYMBOL_GPL(unregister_ftrace_direct_multi);
+
+int modify_ftrace_direct_multi(struct ftrace_ops *ops, unsigned long addr)
+{
+ struct ftrace_hash *hash = ops->func_hash->filter_hash;
+ struct ftrace_func_entry *entry, *iter;
+ int i, size;
+ int err;
+
+ if (check_direct_multi(ops))
+ return -EINVAL;
+ if (!(ops->flags & FTRACE_OPS_FL_ENABLED))
+ return -EINVAL;
+
+ mutex_lock(&direct_mutex);
+ mutex_lock(&ftrace_lock);
+
+ /*
+ * Shutdown the ops, change 'direct' pointer for each
+ * ops entry in direct_functions hash and startup the
+ * ops back again.
+ */
+ err = ftrace_shutdown(ops, 0);
+ if (err)
+ goto out_unlock;
+
+ size = 1 << hash->size_bits;
+ for (i = 0; i < size; i++) {
+ hlist_for_each_entry(iter, &hash->buckets[i], hlist) {
+ entry = __ftrace_lookup_ip(direct_functions, iter->ip);
+ if (!entry)
+ continue;
+ entry->direct = addr;
+ }
+ }
+
+ err = ftrace_startup(ops, 0);
+
+ out_unlock:
+ mutex_unlock(&ftrace_lock);
+ mutex_unlock(&direct_mutex);
+ return err;
+}
+EXPORT_SYMBOL_GPL(modify_ftrace_direct_multi);
#endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
/**
--
2.31.1
On Tue, 31 Aug 2021 11:50:16 +0200
Jiri Olsa <[email protected]> wrote:
> Adding interface to modify registered direct function
> for ftrace_ops. Adding following function:
>
> modify_ftrace_direct_multi(struct ftrace_ops *ops, unsigned long addr)
>
> The function changes the currently registered direct
> function for all attached functions.
>
> Signed-off-by: Jiri Olsa <[email protected]>
> ---
> include/linux/ftrace.h | 6 ++++++
> kernel/trace/ftrace.c | 43 ++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 49 insertions(+)
>
> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index e40b5201c16e..f3ba6366f7af 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
> @@ -318,6 +318,8 @@ int ftrace_modify_direct_caller(struct ftrace_func_entry *entry,
> unsigned long ftrace_find_rec_direct(unsigned long ip);
> int register_ftrace_direct_multi(struct ftrace_ops *ops, unsigned long addr);
> int unregister_ftrace_direct_multi(struct ftrace_ops *ops);
> +int modify_ftrace_direct_multi(struct ftrace_ops *ops, unsigned long addr);
> +
> #else
> struct ftrace_ops;
> # define ftrace_direct_func_count 0
> @@ -357,6 +359,10 @@ static inline int unregister_ftrace_direct_multi(struct ftrace_ops *ops)
> {
> return -ENODEV;
> }
> +static inline int modify_ftrace_direct_multi(struct ftrace_ops *ops, unsigned long addr)
> +{
> + return -ENODEV;
> +}
> #endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
>
> #ifndef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 7243769493c9..59940a6a907c 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -5518,6 +5518,49 @@ int unregister_ftrace_direct_multi(struct ftrace_ops *ops)
> return err;
> }
> EXPORT_SYMBOL_GPL(unregister_ftrace_direct_multi);
> +
Needs kernel doc comments.
> +int modify_ftrace_direct_multi(struct ftrace_ops *ops, unsigned long addr)
> +{
> + struct ftrace_hash *hash = ops->func_hash->filter_hash;
> + struct ftrace_func_entry *entry, *iter;
> + int i, size;
> + int err;
> +
> + if (check_direct_multi(ops))
> + return -EINVAL;
> + if (!(ops->flags & FTRACE_OPS_FL_ENABLED))
> + return -EINVAL;
> +
> + mutex_lock(&direct_mutex);
> + mutex_lock(&ftrace_lock);
> +
> + /*
> + * Shutdown the ops, change 'direct' pointer for each
> + * ops entry in direct_functions hash and startup the
> + * ops back again.
> + */
> + err = ftrace_shutdown(ops, 0);
This needs to be commented that there's going to be a rather large time
frame that there will be no callbacks happening while this update occurs.
A better solution, that prevents having to do this, is to first change
the function fentry's to call the ftrace list loop function, that calls
the ftrace_ops list, and will call the direct call via the ops in the
loop. Have the ops->func call the new direct function (all will be
immediately affected). Update the entries, and then switch from the
loop back to the direct caller.
-- Steve
> + if (err)
> + goto out_unlock;
> +
> + size = 1 << hash->size_bits;
> + for (i = 0; i < size; i++) {
> + hlist_for_each_entry(iter, &hash->buckets[i], hlist) {
> + entry = __ftrace_lookup_ip(direct_functions, iter->ip);
> + if (!entry)
> + continue;
> + entry->direct = addr;
> + }
> + }
> +
> + err = ftrace_startup(ops, 0);
> +
> + out_unlock:
> + mutex_unlock(&ftrace_lock);
> + mutex_unlock(&direct_mutex);
> + return err;
> +}
> +EXPORT_SYMBOL_GPL(modify_ftrace_direct_multi);
> #endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
>
> /**
On Tue, 14 Sep 2021 17:41:34 -0400
Steven Rostedt <[email protected]> wrote:
> A better solution, that prevents having to do this, is to first change
> the function fentry's to call the ftrace list loop function, that calls
> the ftrace_ops list, and will call the direct call via the ops in the
> loop. Have the ops->func call the new direct function (all will be
> immediately affected). Update the entries, and then switch from the
> loop back to the direct caller.
An easy way to force the loop function to be called instead of the direct
trampoline, is to register a stub ftrace_ops to each of the functions that
the direct function attaches to. You can even share the hash in doing so.
Having the ftrace_ops attached in the same locations as the direct
trampoline, will force the loop function to be called (to call the stub
ftrace_ops as well as the direct trampoline ftrace_ops helper).
Then change the direct trampoline address, which will have the ftrace_ops
helper use that direct trampoline immediately*. Then when you remove the
ftrace_ops stub, it will update all the call sites to call the new direct
trampoline directly.
(*) not quite immediately, as there's no read memory barrier with the
direct helper, so it may still be calling the old trampoline. But this
shouldn't be an issue. If it is, then you would need to include some memory
barrier synchronization.
I'm curious to what the use case is for the multi direct modify interface
is?
-- Steve
On Wed, Sep 15, 2021 at 05:47:18PM -0400, Steven Rostedt wrote:
> On Tue, 14 Sep 2021 17:41:34 -0400
> Steven Rostedt <[email protected]> wrote:
>
> > A better solution, that prevents having to do this, is to first change
> > the function fentry's to call the ftrace list loop function, that calls
> > the ftrace_ops list, and will call the direct call via the ops in the
> > loop. Have the ops->func call the new direct function (all will be
> > immediately affected). Update the entries, and then switch from the
> > loop back to the direct caller.
>
> An easy way to force the loop function to be called instead of the direct
> trampoline, is to register a stub ftrace_ops to each of the functions that
> the direct function attaches to. You can even share the hash in doing so.
>
> Having the ftrace_ops attached in the same locations as the direct
> trampoline, will force the loop function to be called (to call the stub
> ftrace_ops as well as the direct trampoline ftrace_ops helper).
>
> Then change the direct trampoline address, which will have the ftrace_ops
> helper use that direct trampoline immediately*. Then when you remove the
> ftrace_ops stub, it will update all the call sites to call the new direct
> trampoline directly.
ok, that's the way the current direct modify interface is using, right?
I thought it'd be not so easy to adopt for multiple functions, I'll check
on that again and come for help ;-)
>
> (*) not quite immediately, as there's no read memory barrier with the
> direct helper, so it may still be calling the old trampoline. But this
> shouldn't be an issue. If it is, then you would need to include some memory
> barrier synchronization.
>
> I'm curious to what the use case is for the multi direct modify interface
> is?
when the trampoline is re-generated by adding or removing program,
we have same functions to trace and new trampoline to attach
thanks,
jirka
>
> -- Steve
>
On Thu, 16 Sep 2021 21:49:37 +0200
Jiri Olsa <[email protected]> wrote:
> > I'm curious to what the use case is for the multi direct modify interface
> > is?
>
> when the trampoline is re-generated by adding or removing program,
> we have same functions to trace and new trampoline to attach
>
Then it probably doesn't matter for the slight delay in synchronization.
-- Steve