> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index 433c36c3a..bacb6d9ab 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
> @@ -544,6 +544,7 @@ int modify_ftrace_direct_nolock(struct ftrace_ops *ops,
> unsigned long addr);
>
> int update_ftrace_direct_add(struct ftrace_ops *ops, struct ftrace_hash
> *hash);
> int update_ftrace_direct_del(struct ftrace_ops *ops, struct ftrace_hash
> *hash);
> +int update_ftrace_direct_mod(struct ftrace_ops *ops, struct ftrace_hash
> *hash, bool do_direct_lock);
>
> void ftrace_stub_direct_tramp(void);
>
> @@ -581,6 +582,11 @@ int update_ftrace_direct_del(struct ftrace_ops *ops,
> struct ftrace_hash *hash)
> return -ENODEV;
> }
>
> +int modify_ftrace_direct_hash(struct ftrace_ops *ops, struct ftrace_hash
> *hash, bool do_direct_lock)
^^^^^^^^^^^^^^^^^^^^^^^^^^
The stub function name doesn't match the declaration. Should this be
update_ftrace_direct_mod() instead of modify_ftrace_direct_hash()?
> +{
> + return -ENODEV;
> +}
> +
[ ... ]
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 345f3fdd2..627d05b57 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -6445,6 +6445,74 @@ int update_ftrace_direct_del(struct ftrace_ops *ops,
> struct ftrace_hash *hash)
> return err;
> }
>
> +int update_ftrace_direct_mod(struct ftrace_ops *ops, struct ftrace_hash
> *hash, bool do_direct_lock)
> +{
> + struct ftrace_hash *orig_hash = ops->func_hash->filter_hash;
> + struct ftrace_func_entry *entry, *tmp;
> + static struct ftrace_ops tmp_ops = {
> + .func = ftrace_stub,
> + .flags = FTRACE_OPS_FL_STUB,
> + };
> + unsigned long size, i;
> + int err;
> +
> + if (!hash_count(hash))
> + return 0;
> + 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);
> +
> + /* 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);
^^^^
Does __ftrace_hash_update_ipmodify() accept a fourth argument? The
function signature shows it only takes three parameters (ops, old_hash,
new_hash). This looks like a compilation error.
> + if (err)
> + goto out;
[ ... ]
> +
> +out:
> + /* Removing the tmp_ops will add the updated direct callers to the
> functions */
> + unregister_ftrace_function(&tmp_ops);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Can this deadlock when do_direct_lock is true? Looking at the code:
update_ftrace_direct_mod() with do_direct_lock=true
-> mutex_lock(&direct_mutex)
-> register_ftrace_function_nolock(&tmp_ops) succeeds
-> __ftrace_hash_update_ipmodify() fails
-> goto out (still holding direct_mutex)
-> unregister_ftrace_function(&tmp_ops)
-> cleanup_direct_functions_after_ipmodify()
-> mutex_lock(&direct_mutex) /* deadlock */
The unregister path tries to reacquire direct_mutex that we're already
holding.
> +
> +unlock:
> + if (do_direct_lock)
> + mutex_unlock(&direct_mutex);
> + return err;
> +}
> +
> #endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/19332026793
