On Mon, 15 Dec 2025 22:13:59 +0100
Jiri Olsa <[email protected]> wrote:
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 48dc0de5f2ce..95a38fb18ed7 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -6489,6 +6489,78 @@ int update_ftrace_direct_del(struct ftrace_ops *ops,
> struct ftrace_hash *hash)
> return err;
> }
>
Kerneldoc needed.
> +int update_ftrace_direct_mod(struct ftrace_ops *ops, struct ftrace_hash
> *hash, bool do_direct_lock)
> +{
> + struct ftrace_func_entry *entry, *tmp;
> + static struct ftrace_ops tmp_ops = {
> + .func = ftrace_stub,
> + .flags = FTRACE_OPS_FL_STUB,
> + };
> + struct ftrace_hash *orig_hash;
> + unsigned long size, i;
> + int err = -EINVAL;
> +
> + if (!hash_count(hash))
> + return -EINVAL;
> + if (check_direct_multi(ops))
> + return -EINVAL;
> + if (!(ops->flags & FTRACE_OPS_FL_ENABLED))
> + return -EINVAL;
> + if (direct_functions == EMPTY_HASH)
> + return -EINVAL;
> +
> + if (do_direct_lock)
> + mutex_lock(&direct_mutex);
This optional taking of the direct_mutex lock needs some serious rationale
and documentation.
> +
> + orig_hash = ops->func_hash ? ops->func_hash->filter_hash : NULL;
> + if (!orig_hash)
> + goto unlock;
> +
> + /* 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_nolock(&tmp_ops);
> + if (err)
> + goto unlock;
> +
> + /*
> + * Call __ftrace_hash_update_ipmodify() here, so that we can call
> + * ops->ops_func for the ops. This is needed because the above
> + * register_ftrace_function_nolock() worked on tmp_ops.
> + */
> + err = __ftrace_hash_update_ipmodify(ops, orig_hash, orig_hash, true);
> + if (err)
> + goto out;
> +
> + /*
> + * Now the ftrace_ops_list_func() is called to do the direct callers.
> + * We can safely change the direct functions attached to each entry.
> + */
> + mutex_lock(&ftrace_lock);
I'm going to need some time staring at this code. It looks like it may be
relying on some internals here.
-- Steve
> +
> + size = 1 << hash->size_bits;
> + for (i = 0; i < size; i++) {
> + hlist_for_each_entry(entry, &hash->buckets[i], hlist) {
> + tmp = __ftrace_lookup_ip(direct_functions, entry->ip);
> + if (!tmp)
> + continue;
> + tmp->direct = entry->direct;
> + }
> + }
> +
> + mutex_unlock(&ftrace_lock);
> +
> +out:
> + /* Removing the tmp_ops will add the updated direct callers to the
> functions */
> + unregister_ftrace_function(&tmp_ops);
> +
> +unlock:
> + if (do_direct_lock)
> + mutex_unlock(&direct_mutex);
> + return err;
> +}
> +
> #endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
>
> /**
