On Thu, Oct 14, 2021 at 04:28:19PM -0400, Steven Rostedt wrote:
> On Fri, 8 Oct 2021 11:13:35 +0200
> Jiri Olsa <jolsa@xxxxxxxxxx> wrote:
>
> > + /*
> > + * Shutdown the ops, change 'direct' pointer for each
> > + * ops entry in direct_functions hash and startup the
> > + * ops back again.
> > + *
> > + * Note there is no callback called for @ops object after
> > + * this ftrace_shutdown call until ftrace_startup is called
> > + * later on.
> > + */
> > + err = ftrace_shutdown(ops, 0);
> > + if (err)
> > + goto out_unlock;
>
> I believe I said before that we can do this by adding a stub ops that match
> all the functions with the direct ops being modified. This will cause the
> loop function to be called, which will call the direct function helper,
> which will then call the direct function that is found. That is, there is
> no "pause" in calling the direct callers. Either the old direct is called,
> or the new one. When the function returns, all are calling the new one.
>
> That is, instead of:
>
> [ Changing direct call from my_direct_1 to my_direct_2 ]
>
> <traced_func>:
> call my_direct_1
>
> ||||||||||||||||||||
> vvvvvvvvvvvvvvvvvvvv
>
> <traced_func>:
> nop
>
> ||||||||||||||||||||
> vvvvvvvvvvvvvvvvvvvv
>
> <traced_func>:
> call my_direct_2
>
>
> We have it do:
>
> <traced_func>:
> call my_direct_1
>
> ||||||||||||||||||||
> vvvvvvvvvvvvvvvvvvvv
>
> <traced_func>:
> call ftrace_caller
>
>
> <ftrace_caller>:
> [..]
> call ftrace_ops_list_func
>
>
> ftrace_ops_list_func()
> {
> ops->func() -> direct_helper -> set rax to my_direct_1 or my_direct_2
> }
>
> call rax (to either my_direct_1 or my_direct_2
nice! :) I did not see that as a problem and something that can be
done later, thanks for doing this
>
> ||||||||||||||||||||
> vvvvvvvvvvvvvvvvvvvv
>
> <traced_func>:
> call my_direct_2
>
>
> I did this on top of this patch:
ATM I'm bit stuck on the bpf side of this whole change, I'll test
it with my other changes when I unstuck myself ;-)
thanks,
jirka
>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@xxxxxxxxxxx>
> ---
> kernel/trace/ftrace.c | 33 ++++++++++++++++++++-------------
> 1 file changed, 20 insertions(+), 13 deletions(-)
>
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 30120342176e..7ad1e8ae5855 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -5561,8 +5561,12 @@ 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_hash *hash;
> struct ftrace_func_entry *entry, *iter;
> + static struct ftrace_ops tmp_ops = {
> + .func = ftrace_stub,
> + .flags = FTRACE_OPS_FL_STUB,
> + };
> int i, size;
> int err;
>
> @@ -5572,21 +5576,22 @@ int modify_ftrace_direct_multi(struct ftrace_ops *ops, unsigned long addr)
> return -EINVAL;
>
> mutex_lock(&direct_mutex);
> - mutex_lock(&ftrace_lock);
> +
> + /* Enable the tmp_ops to have the same functions as the direct ops */
> + ftrace_ops_init(&tmp_ops);
> + tmp_ops.func_hash = ops->func_hash;
> +
> + err = register_ftrace_function(&tmp_ops);
> + if (err)
> + goto out_direct;
>
> /*
> - * Shutdown the ops, change 'direct' pointer for each
> - * ops entry in direct_functions hash and startup the
> - * ops back again.
> - *
> - * Note there is no callback called for @ops object after
> - * this ftrace_shutdown call until ftrace_startup is called
> - * later on.
> + * Now the ftrace_ops_list_func() is called to do the direct callers.
> + * We can safely change the direct functions attached to each entry.
> */
> - err = ftrace_shutdown(ops, 0);
> - if (err)
> - goto out_unlock;
> + mutex_lock(&ftrace_lock);
>
> + hash = ops->func_hash->filter_hash;
> size = 1 << hash->size_bits;
> for (i = 0; i < size; i++) {
> hlist_for_each_entry(iter, &hash->buckets[i], hlist) {
> @@ -5597,10 +5602,12 @@ int modify_ftrace_direct_multi(struct ftrace_ops *ops, unsigned long addr)
> }
> }
>
> - err = ftrace_startup(ops, 0);
> + /* Removing the tmp_ops will add the updated direct callers to the functions */
> + unregister_ftrace_function(&tmp_ops);
>
> out_unlock:
> mutex_unlock(&ftrace_lock);
> + out_direct:
> mutex_unlock(&direct_mutex);
> return err;
> }
> --
> 2.31.1
>
