Re: [PATCH v3 bpf 1/3] ftrace: Fix BPF fexit with livepatch

[Song Liu]

> On Oct 27, 2025, at 10:01 AM, Steven Rostedt <rost...@goodmis.org> wrote:
> 
> On Sun, 26 Oct 2025 13:54:43 -0700
> Song Liu <s...@kernel.org> wrote:
> 
>> --- a/kernel/trace/ftrace.c
>> +++ b/kernel/trace/ftrace.c
>> @@ -6048,6 +6048,12 @@ int register_ftrace_direct(struct ftrace_ops *ops, 
>> unsigned long addr)
>> ops->direct_call = addr;
>> 
>> err = register_ftrace_function_nolock(ops);
>> + if (err) {
>> + /* cleanup for possible another register call */
>> + ops->func = NULL;
>> + ops->trampoline = 0;
>> + remove_direct_functions_hash(hash, addr);
>> + }
>> 
> 
> As you AI bot noticed that it was missing what unregister_ftrace_direct()
> does, instead, can we make a helper function that both use? This way it
> will not get out of sync again.
> 
> static void reset_direct(struct ftrace_ops *ops, unsigned long addr)
> {
> struct ftrace_hash *hash = ops->func_hash->filter_hash;
> 
> ops->func = NULL;
> ops->trampoline = 0;
> remove_direct_functions_hash(hash, addr);
> }
> 
> Then we could have:
> 
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 0c91247a95ab..51c3f5d46fde 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -6062,7 +6062,7 @@ int register_ftrace_direct(struct ftrace_ops *ops, 
> unsigned long addr)
> 
> err = register_ftrace_function_nolock(ops);
> if (err)
> - remove_direct_functions_hash(hash, addr);
> + reset_direct(ops, addr);
> 
>  out_unlock:
> mutex_unlock(&direct_mutex);
> @@ -6095,7 +6095,6 @@ EXPORT_SYMBOL_GPL(register_ftrace_direct);
> int unregister_ftrace_direct(struct ftrace_ops *ops, unsigned long addr,
>     bool free_filters)
> {
> - struct ftrace_hash *hash = ops->func_hash->filter_hash;
> int err;
> 
> if (check_direct_multi(ops))
> @@ -6105,13 +6104,9 @@ int unregister_ftrace_direct(struct ftrace_ops *ops, 
> unsigned long addr,
> 
> mutex_lock(&direct_mutex);
> err = unregister_ftrace_function(ops);
> - remove_direct_functions_hash(hash, addr);
> + reset_direct(ops, addr);
> mutex_unlock(&direct_mutex);
> 
> - /* cleanup for possible another register call */
> - ops->func = NULL;
> - ops->trampoline = 0;
> -
> if (free_filters)
> ftrace_free_filter(ops);
> return err;

Make sense. I will use this in v4. 

Thanks,
Song