Skip to content

ftrace,bpf: Use single direct ops for bpf trampolines #10480

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
kernel-patches-daemon-bpf wants to merge 9 commits into
base: bpf-next_base
Choose a base branch
from
Open

ftrace,bpf: Use single direct ops for bpf trampolines #10480

kernel-patches-daemon-bpf wants to merge 9 commits into from
+560 −73

Conversation

@kernel-patches-daemon-bpf
Copy link

@kernel-patches-daemon-bpf kernel-patches-daemon-bpf bot commented Dec 15, 2025

Pull request for series with
subject: ftrace,bpf: Use single direct ops for bpf trampolines
version: 1
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=1033448

@kernel-patches-daemon-bpf
Copy link
Author

kernel-patches-daemon-bpf bot commented Dec 15, 2025

Upstream branch: 6f0b824
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1033448
version: 1

@kernel-patches-daemon-bpf
Copy link
Author

kernel-patches-daemon-bpf bot commented Dec 15, 2025

Upstream branch: 6f0b824
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1033448
version: 1

@kernel-patches-review-bot
Copy link

kernel-patches-review-bot bot commented Dec 15, 2025 

commit f8d7ac4f3dd3452dcfe481a5800dcf9a02d631ca
Author: Jiri Olsa <[email protected]>

ftrace,bpf: Remove FTRACE_OPS_FL_JMP ftrace_ops flag

This patch moves the responsibility for setting the LSB bit (indicating
jmp attachment) from ftrace core to the BPF trampoline code, removing
the FTRACE_OPS_FL_JMP flag entirely.

Fixes: 25e4e3565d45 ("ftrace: Introduce FTRACE_OPS_FL_JMP")

> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index 015dd1049bea..505b7d3f5641 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h

[ ... ]

> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> index 976d89011b15..b9a358d7a78f 100644
> --- a/kernel/bpf/trampoline.c
> +++ b/kernel/bpf/trampoline.c
> @@ -214,10 +214,15 @@ static int modify_fentry(struct bpf_trampoline *tr, u32 orig_flags,
>  	int ret;
>
>  	if (tr->func.ftrace_managed) {
> +		unsigned long addr = (unsigned long) new_addr;
> +
> +		if (bpf_trampoline_use_jmp(tr->flags))
> +			addr = ftrace_jmp_set(addr);
> +
>  		if (lock_direct_mutex)
> -			ret = modify_ftrace_direct(tr->fops, (long)new_addr);
> +			ret = modify_ftrace_direct(tr->fops, addr);
>  		else
> -			ret = modify_ftrace_direct_nolock(tr->fops, (long)new_addr);
> +			ret = modify_ftrace_direct_nolock(tr->fops, addr);
>  	} else {
>  		ret = bpf_trampoline_update_fentry(tr, orig_flags, old_addr,
>  						   new_addr);
> @@ -240,10 +245,15 @@ static int register_fentry(struct bpf_trampoline *tr, void *new_addr)
>  	}
>
>  	if (tr->func.ftrace_managed) {
> +		unsigned long addr = (unsigned long) new_addr;
> +
> +		if (bpf_trampoline_use_jmp(tr->flags))
> +			addr = ftrace_jmp_set(addr);
> +
>  		ret = ftrace_set_filter_ip(tr->fops, (unsigned long)ip, 0, 1);
>  		if (ret)
>  			return ret;
> -		ret = register_ftrace_direct(tr->fops, (long)new_addr);
> +		ret = register_ftrace_direct(tr->fops, addr);
>  	} else {
>  		ret = bpf_trampoline_update_fentry(tr, 0, NULL, new_addr);
>  	}

After this change, register_ftrace_direct() receives an address with
the LSB bit already set (via ftrace_jmp_set) and stores it as-is in
entry->direct. This address then flows through to ftrace_find_rec_direct()
which returns entry->direct unchanged.

In available_filter_functions_show():

    direct = ftrace_find_rec_direct(rec->ip);
    if (direct)
        seq_printf(m, "\n\tdirect-->%pS", (void *)direct);

Can the debug output show misleading symbol offsets? When the LSB is
set, %pS will resolve addresses like "function+0x1" instead of just
"function". This makes ftrace debug output harder to read for users
trying to understand direct call attachments.

The fix would be to strip the LSB bit before printing:
    seq_printf(m, "\n\tdirect-->%pS", (void *)ftrace_jmp_get(direct));

[ ... ]

> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index bbb37c0f8c6c..b0dc911411f1 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -6017,15 +6017,8 @@ int register_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
>  	if (ftrace_hash_empty(hash))
>  		return -EINVAL;
>
> -	/* This is a "raw" address, and this should never happen. */
> -	if (WARN_ON_ONCE(ftrace_is_jmp(addr)))
> -		return -EINVAL;
> -
>  	mutex_lock(&direct_mutex);
>
> -	if (ops->flags & FTRACE_OPS_FL_JMP)
> -		addr = ftrace_jmp_set(addr);
> -
>  	/* Make sure requested entries are not already registered.. */
>  	size = 1 << hash->size_bits;
>  	for (i = 0; i < size; i++) {

[ ... ]

> @@ -6146,13 +6139,6 @@ __modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
>
>  	lockdep_assert_held_once(&direct_mutex);
>
> -	/* This is a "raw" address, and this should never happen. */
> -	if (WARN_ON_ONCE(ftrace_is_jmp(addr)))
> -		return -EINVAL;
> -
> -	if (ops->flags & FTRACE_OPS_FL_JMP)
> -		addr = ftrace_jmp_set(addr);
> -
>  	/* 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;

[ ... ]

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

In-Reply-To-Subject: ftrace,bpf: Remove FTRACE_OPS_FL_JMP ftrace_ops flag
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/20247816064

@kernel-patches-daemon-bpf
Copy link
Author

kernel-patches-daemon-bpf bot commented Dec 15, 2025

Forwarding comment 3657681600 via email
In-Reply-To: [email protected]
Patch: https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/

@olsajiri
ftrace,bpf: Remove FTRACE_OPS_FL_JMP ftrace_ops flag
950015d 
At the moment the we allow the jmp attach only for ftrace_ops that
has FTRACE_OPS_FL_JMP set. This conflicts with following changes
where we use single ftrace_ops object for all direct call sites,
so all could be be attached via just call or jmp.

We already limit the jmp attach support with config option and bit
(LSB) set on the trampoline address. It turns out that's actually
enough to limit the jmp attach for architecture and only for chosen
addresses (with LSB bit set).

Each user of register_ftrace_direct or modify_ftrace_direct can set
the trampoline bit (LSB) to indicate it has to be attached by jmp.

The bpf trampoline generation code uses trampoline flags to generate
jmp-attach specific code and ftrace inner code uses the trampoline
bit (LSB) to handle return from jmp attachment, so there's no harm
to remove the FTRACE_OPS_FL_JMP bit.

The fexit/fmodret performance stays the same (did not drop),
current code:

  fentry         :   77.904 ± 0.546M/s
  fexit          :   62.430 ± 0.554M/s
  fmodret        :   66.503 ± 0.902M/s

with this change:

  fentry         :   80.472 ± 0.061M/s
  fexit          :   63.995 ± 0.127M/s
  fmodret        :   67.362 ± 0.175M/s

Fixes: 25e4e35 ("ftrace: Introduce FTRACE_OPS_FL_JMP")
Signed-off-by: Jiri Olsa <[email protected]>
@olsajiri
ftrace: Make alloc_and_copy_ftrace_hash direct friendly
5ad6990 
Make alloc_and_copy_ftrace_hash to copy also direct address
for each hash entry.

Signed-off-by: Jiri Olsa <[email protected]>
@olsajiri
ftrace: Export some of hash related functions
28c8b68 
We are going to use these functions in following changes.

Signed-off-by: Jiri Olsa <[email protected]>
@olsajiri
ftrace: Add update_ftrace_direct_add function
191b497 
Adding update_ftrace_direct_add function that adds all entries
(ip -> addr) provided in hash argument to direct ftrace ops
and updates its attachments.

The difference to current register_ftrace_direct is
 - hash argument that allows to register multiple ip -> direct
   entries at once
 - we can call update_ftrace_direct_add multiple times on the
   same ftrace_ops object, becase after first registration with
   register_ftrace_function_nolock, it uses ftrace_update_ops to
   update the ftrace_ops object

This change will allow us to have simple ftrace_ops for all bpf
direct interface users in following changes.

Signed-off-by: Jiri Olsa <[email protected]>
@kernel-patches-daemon-bpf
Copy link
Author

kernel-patches-daemon-bpf bot commented Dec 16, 2025

Upstream branch: e7a0adb
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1033448
version: 1

@olsajiri
ftrace: Add update_ftrace_direct_del function
6a99c10 
Adding update_ftrace_direct_del function that removes all entries
(ip -> addr) provided in hash argument to direct ftrace ops and
updates its attachments.

The difference to current unregister_ftrace_direct is
 - hash argument that allows to unregister multiple ip -> direct
   entries at once
 - we can call update_ftrace_direct_del multiple times on the
   same ftrace_ops object, becase we do not need to unregister
   all entries at once, we can do it gradualy with the help of
   ftrace_update_ops function

This change will allow us to have simple ftrace_ops for all bpf
direct interface users in following changes.

Signed-off-by: Jiri Olsa <[email protected]>
@olsajiri
ftrace: Add update_ftrace_direct_mod function
e1c537a 
Adding update_ftrace_direct_mod function that modifies all entries
(ip -> direct) provided in hash argument to direct ftrace ops and
updates its attachments.

The difference to current modify_ftrace_direct is:
- hash argument that allows to modify multiple ip -> direct
  entries at once

This change will allow us to have simple ftrace_ops for all bpf
direct interface users in following changes.

Signed-off-by: Jiri Olsa <[email protected]>
@olsajiri
bpf: Add trampoline ip hash table
7989051 
Following changes need to lookup trampoline based on its ip address,
adding hash table for that.

Signed-off-by: Jiri Olsa <[email protected]>
@olsajiri
ftrace: Factor ftrace_ops ops_func interface
893c6da 
We are going to remove "ftrace_ops->private == bpf_trampoline" setup
in following changes.

Adding ip argument to ftrace_ops_func_t callback function, so we can
use it to look up the trampoline.

Signed-off-by: Jiri Olsa <[email protected]>
@olsajiri
bpf,x86: Use single ftrace_ops for direct calls
13fd9f4 
Using single ftrace_ops for direct calls update instead of allocating
ftrace_ops object for each trampoline.

With single ftrace_ops object we can use update_ftrace_direct_* api
that allows multiple ip sites updates on single ftrace_ops object.

Adding HAVE_SINGLE_FTRACE_DIRECT_OPS config option to be enabled on
each arch that supports this.

At the moment we can enable this only on x86 arch, because arm relies
on ftrace_ops object representing just single trampoline image (stored
in ftrace_ops::direct_call). Ach that do not support this will continue
to use *_ftrace_direct api.

Signed-off-by: Jiri Olsa <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

bpf-next new V1-ci-fail V1

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@olsajiri