ftrace,bpf: Use single direct ops for bpf trampolines

PATCH Superseded Jiri Olsa 4 participants 14 messages Dec 3, 2025

Subsystems (5)

x86 BPF Tracing BPF core +1 more subsystems

lore

Share & tools

lore

Jiri Olsa

author reviewer

2025-12-03 08:24 UTC #0 lore raw

v4 cover ftrace,bpf: Use single direct ops for bpf trampolines

hi,
while poking the multi-tracing interface I ended up with just one ftrace_ops object to attach all trampolines.

This change allows to use less direct API calls during the attachment changes in the future code, so in effect speeding up the attachment.

In current code we get a speed up from using just a single ftrace_ops object.

- with current code:

Performance counter stats for 'bpftrace -e fentry:vmlinux:ksys_* {} -c true':

     6,364,157,902      cycles:k
       828,728,902      cycles:u
     1,064,803,824      instructions:u                   #    1.28  insn per cycle
    23,797,500,067      instructions:k                   #    3.74  insn per cycle

       4.416004987 seconds time elapsed

       0.164121000 seconds user
       1.289550000 seconds sys

- with the fix:

Performance counter stats for 'bpftrace -e fentry:vmlinux:ksys_* {} -c true':

     6,535,857,905      cycles:k
       810,809,429      cycles:u
     1,064,594,027      instructions:u                   #    1.31  insn per cycle
    23,962,552,894      instructions:k                   #    3.67  insn per cycle

       1.666961239 seconds time elapsed

       0.157412000 seconds user
       1.283396000 seconds sys

The speedup seems to be related to the fact that with single ftrace_ops object we don't call ftrace_shutdown anymore (we use ftrace_update_ops instead) and we skip the synchronize rcu calls (each ~100ms) at the end of that function.

rfc: https://lore.kernel.org/bpf/[email protected]/
v1:  https://lore.kernel.org/bpf/[email protected]/
v2:  https://lore.kernel.org/bpf/[email protected]/
v3:  https://lore.kernel.org/bpf/[email protected]/

v4 changes:
- rebased on top of bpf-next/master (with jmp attach changes) added patch 1 to deal with that
- added extra checks for update_ftrace_direct_del/mod to address the ci bot review

v3 changes:
- rebased on top of bpf-next/master
- fixed update_ftrace_direct_del cleanup path
- added missing inline to update_ftrace_direct_* stubs

v2 changes:
- rebased on top fo bpf-next/master plus Song's livepatch fixes [1]
- renamed the API functions [2] [Steven]
- do not export the new api [Steven]
- kept the original direct interface:

I'm not sure if we want to melt both *_ftrace_direct and the new interface into single one. It's bit different in semantic (hence the name change as Steven suggested [2]) and I don't think the changes are not that big so we could easily keep both APIs.

v1 changes:
- make the change x86 specific, after discussing with Mark options for arm64 [Mark]

thanks,
jirka

[1] https://lore.kernel.org/bpf/[email protected]/
[2] https://lore.kernel.org/bpf/[email protected]/
---
Jiri Olsa (9):
ftrace,bpf: Remove FTRACE_OPS_FL_JMP ftrace_ops flag
ftrace: Make alloc_and_copy_ftrace_hash direct friendly
ftrace: Export some of hash related functions
ftrace: Add update_ftrace_direct_add function
ftrace: Add update_ftrace_direct_del function
ftrace: Add update_ftrace_direct_mod function
bpf: Add trampoline ip hash table
ftrace: Factor ftrace_ops ops_func interface
bpf,x86: Use single ftrace_ops for direct calls

 arch/x86/Kconfig        |   1 +
 include/linux/bpf.h     |   7 ++-
 include/linux/ftrace.h  |  38 +++++++++++++-
 kernel/bpf/trampoline.c | 234 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------------
 kernel/trace/Kconfig    |   3 ++
 kernel/trace/ftrace.c   | 358 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------
 kernel/trace/trace.h    |   8 ---
 7 files changed, 568 insertions(+), 81 deletions(-)

Jiri Olsa

author reviewer

2025-12-03 08:24 UTC #1 lore raw

PATCH v4 1/9 ftrace,bpf: Remove FTRACE_OPS_FL_JMP ftrace_ops flag

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 25e4e3565d45 ("ftrace: Introduce FTRACE_OPS_FL_JMP")

Signed-off-by Jiri Olsa <[email protected]>

---
 include/linux/ftrace.h  |  1 -
 kernel/bpf/trampoline.c | 32 ++++++++++++++------------------
 kernel/trace/ftrace.c   | 14 --------------
 3 files changed, 14 insertions(+), 33 deletions(-)

include/linux/ftrace.h +0 -1

index 015dd1049bea..505b7d3f5641 100644
@@ -359,7 +359,6 @@ enum {
 	FTRACE_OPS_FL_DIRECT			= BIT(17),
 	FTRACE_OPS_FL_SUBOP			= BIT(18),
 	FTRACE_OPS_FL_GRAPH			= BIT(19),
-	FTRACE_OPS_FL_JMP			= BIT(20),
 };
 
 #ifndef CONFIG_DYNAMIC_FTRACE_WITH_ARGS

kernel/bpf/trampoline.c +14 ~~-18~~

index 976d89011b15..b9a358d7a78f 100644
@@ -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);
 	}

@@ -499,13 +509,6 @@ static int bpf_trampoline_update(struct bpf_trampoline *tr, bool lock_direct_mut
 	if (err)
 		goto out_free;
 
-#ifdef CONFIG_DYNAMIC_FTRACE_WITH_JMP
-	if (bpf_trampoline_use_jmp(tr->flags))
-		tr->fops->flags |= FTRACE_OPS_FL_JMP;
-	else
-		tr->fops->flags &= ~FTRACE_OPS_FL_JMP;
-#endif
-
 	WARN_ON(tr->cur_image && total == 0);
 	if (tr->cur_image)
 		/* progs already running at this address */

@@ -533,15 +536,8 @@ static int bpf_trampoline_update(struct bpf_trampoline *tr, bool lock_direct_mut
 	tr->cur_image = im;
 out:
 	/* If any error happens, restore previous flags */
-	if (err) {
+	if (err)
 		tr->flags = orig_flags;
-#ifdef CONFIG_DYNAMIC_FTRACE_WITH_JMP
-		if (bpf_trampoline_use_jmp(tr->flags))
-			tr->fops->flags |= FTRACE_OPS_FL_JMP;
-		else
-			tr->fops->flags &= ~FTRACE_OPS_FL_JMP;
-#endif
-	}
 	kfree(tlinks);
 	return err;

kernel/trace/ftrace.c +0 ~~-14~~

index bbb37c0f8c6c..b0dc911411f1 100644
@@ -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;

Signature · 1 line

2.52.0

Menglong Dong

2025-12-03 09:16 UTC #2 lore raw

Re: [PATCHv4 bpf-next 1/9] ftrace,bpf: Remove FTRACE_OPS_FL_JMP ftrace_ops flag

Quoted 45 lines · Jiri Olsa

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 25e4e3565d45 ("ftrace: Introduce FTRACE_OPS_FL_JMP")
Signed-off-by Jiri Olsa <[email protected]>
---
 include/linux/ftrace.h  |  1 -
 kernel/bpf/trampoline.c | 32 ++++++++++++++------------------
 kernel/trace/ftrace.c   | 14 --------------
 3 files changed, 14 insertions(+), 33 deletions(-)
include/linux/ftrace.h +0 -0
index 015dd1049bea..505b7d3f5641 100644

include/linux/ftrace.h enum { · line 362

Quoted 4 lines

include/linux/ftrace.h +0 -1

@@ -359,7 +359,6 @@ enum {
        FTRACE_OPS_FL_DIRECT                    = BIT(17),
        FTRACE_OPS_FL_SUBOP                     = BIT(18),
        FTRACE_OPS_FL_GRAPH                     = BIT(19),
-       FTRACE_OPS_FL_JMP                       = BIT(20),

Yeah, the FTRACE_OPS_FL_JMP is not necessary. I added it in case that we maybe want to implement such "jmp" for ftrace trampoline in the feature. But it's OK to remove it now.

Quoted 8 lines

include/linux/ftrace.h +0 -0
 };
 
 #ifndef CONFIG_DYNAMIC_FTRACE_WITH_ARGS
kernel/bpf/trampoline.c +0 -0
index 976d89011b15..b9a358d7a78f 100644

kernel/bpf/trampoline.c static int modify_fentry(struct bpf_trampoline *tr, u32 orig_flags, · line 220

Quoted 7 lines

kernel/bpf/trampoline.c +4 -0

@@ -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);

nit: It seems that we can remove the variable "addr" can use the "new_addr" directly?

Quoted 11 lines

kernel/bpf/trampoline.c +3 -2

+
                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);

kernel/bpf/trampoline.c static int register_fentry(struct bpf_trampoline *tr, void *new_addr) · line 251

Quoted 7 lines

kernel/bpf/trampoline.c +4 -0

@@ -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);

And here.

Thanks!
Menglong Dong

Quoted 1 line

kernel/bpf/trampoline.c +1 -0
+

[...]

Quoted 1 line

Jiri Olsa

2025-12-03 20:23 UTC #3 lore raw

Re: [PATCHv4 bpf-next 1/9] ftrace,bpf: Remove FTRACE_OPS_FL_JMP ftrace_ops flag

Quoted 65 lines · Menglong Dong

Quoted 45 lines · Jiri Olsa
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 25e4e3565d45 ("ftrace: Introduce FTRACE_OPS_FL_JMP")
Signed-off-by Jiri Olsa <[email protected]>
---
 include/linux/ftrace.h  |  1 -
 kernel/bpf/trampoline.c | 32 ++++++++++++++------------------
 kernel/trace/ftrace.c   | 14 --------------
 3 files changed, 14 insertions(+), 33 deletions(-)
include/linux/ftrace.h +0 -0
index 015dd1049bea..505b7d3f5641 100644
include/linux/ftrace.h enum { · line 362
Quoted 4 lines
include/linux/ftrace.h +0 -1
@@ -359,7 +359,6 @@ enum {
        FTRACE_OPS_FL_DIRECT                    = BIT(17),
        FTRACE_OPS_FL_SUBOP                     = BIT(18),
        FTRACE_OPS_FL_GRAPH                     = BIT(19),
-       FTRACE_OPS_FL_JMP                       = BIT(20),
Yeah, the FTRACE_OPS_FL_JMP is not necessary. I added it in case that we maybe want to implement such "jmp" for ftrace trampoline in the feature. But it's OK to remove it now.
Quoted 10 lines
include/linux/ftrace.h +0 -0
 };
 
 #ifndef CONFIG_DYNAMIC_FTRACE_WITH_ARGS
kernel/bpf/trampoline.c +0 -0
index 976d89011b15..b9a358d7a78f 100644
@@ -214,10 +214,15 @@ static int modify_fentry(struct bpf_trampoline *tr, u32 orig_flags,
        int ret;
 

Quoted 5 lines

kernel/bpf/trampoline.c static int modify_fentry(struct bpf_trampoline *tr, u32 orig_flags, · line 220
Quoted 5 lines
kernel/bpf/trampoline.c +4 -0
        if (tr->func.ftrace_managed) {
+               unsigned long addr = (unsigned long) new_addr;
+
+               if (bpf_trampoline_use_jmp(tr->flags))
+                       addr = ftrace_jmp_set(addr);

I wanted to get rid of the void * -> unsigned long casting in all the places.. this way it has to be just on one place above, but maybe we could have already direct_ops_add with unsigned long addr, will check

jirka

Quoted 31 lines

nit: It seems that we can remove the variable "addr" can use the "new_addr" directly?

Quoted 11 lines

kernel/bpf/trampoline.c +3 -2

+
                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);

kernel/bpf/trampoline.c static int register_fentry(struct bpf_trampoline *tr, void *new_addr) · line 251

Quoted 7 lines

kernel/bpf/trampoline.c +4 -0

@@ -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);

And here.

Thanks!
Menglong Dong

Quoted 1 line

kernel/bpf/trampoline.c +1 -0
+

[...]

Quoted 1 line

Jiri Olsa

author reviewer

2025-12-03 08:24 UTC #4 lore raw

PATCH v4 2/9 ftrace: Make alloc_and_copy_ftrace_hash direct friendly

In reply to Jiri Olsa

Make alloc_and_copy_ftrace_hash to copy also direct address for each hash entry.

Signed-off-by Jiri Olsa <[email protected]>

---
 kernel/trace/ftrace.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

kernel/trace/ftrace.c +9 -2

index b0dc911411f1..7e3a81bd6f1e 100644
@@ -1186,7 +1186,7 @@ static void __add_hash_entry(struct ftrace_hash *hash,
 }
 
 static struct ftrace_func_entry *
-add_hash_entry(struct ftrace_hash *hash, unsigned long ip)
+add_hash_entry_direct(struct ftrace_hash *hash, unsigned long ip, unsigned long direct)
 {
 	struct ftrace_func_entry *entry;

@@ -1195,11 +1195,18 @@ add_hash_entry(struct ftrace_hash *hash, unsigned long ip)
 		return NULL;
 
 	entry->ip = ip;
+	entry->direct = direct;
 	__add_hash_entry(hash, entry);
 
 	return entry;
 }
 
+static struct ftrace_func_entry *
+add_hash_entry(struct ftrace_hash *hash, unsigned long ip)
+{
+	return add_hash_entry_direct(hash, ip, 0);
+}
+
 static void
 free_hash_entry(struct ftrace_hash *hash,
 		  struct ftrace_func_entry *entry)

@@ -1372,7 +1379,7 @@ alloc_and_copy_ftrace_hash(int size_bits, struct ftrace_hash *hash)
 	size = 1 << hash->size_bits;
 	for (i = 0; i < size; i++) {
 		hlist_for_each_entry(entry, &hash->buckets[i], hlist) {
-			if (add_hash_entry(new_hash, entry->ip) == NULL)
+			if (add_hash_entry_direct(new_hash, entry->ip, entry->direct) == NULL)
 				goto free_hash;
 		}
 	}

Signature · 1 line

2.52.0

Jiri Olsa

author reviewer

2025-12-03 08:24 UTC #5 lore raw

PATCH v4 3/9 ftrace: Export some of hash related functions

In reply to Jiri Olsa

We are going to use these functions in following changes.

Signed-off-by Jiri Olsa <[email protected]>

---
 include/linux/ftrace.h | 16 ++++++++++++++++
 kernel/trace/ftrace.c  |  7 +++----
 kernel/trace/trace.h   |  8 --------
 3 files changed, 19 insertions(+), 12 deletions(-)

include/linux/ftrace.h +16 -0

index 505b7d3f5641..8c034b028af4 100644
@@ -405,6 +405,22 @@ enum ftrace_ops_cmd {
 typedef int (*ftrace_ops_func_t)(struct ftrace_ops *op, enum ftrace_ops_cmd cmd);
 
 #ifdef CONFIG_DYNAMIC_FTRACE
+
+#define FTRACE_HASH_DEFAULT_BITS 10
+
+struct ftrace_hash {
+	unsigned long		size_bits;
+	struct hlist_head	*buckets;
+	unsigned long		count;
+	unsigned long		flags;
+	struct rcu_head		rcu;
+};
+
+struct ftrace_hash *alloc_ftrace_hash(int size_bits);
+void free_ftrace_hash(struct ftrace_hash *hash);
+struct ftrace_func_entry *add_hash_entry_direct(struct ftrace_hash *hash,
+						unsigned long ip, unsigned long direct);
+
 /* The hash used to know what functions callbacks trace */
 struct ftrace_ops_hash {
 	struct ftrace_hash __rcu	*notrace_hash;

kernel/trace/ftrace.c +3 -4

index 7e3a81bd6f1e..84aee9096a9e 100644
@@ -68,7 +68,6 @@
 	})
 
 /* hash bits for specific function selection */
-#define FTRACE_HASH_DEFAULT_BITS 10
 #define FTRACE_HASH_MAX_BITS 12
 
 #ifdef CONFIG_DYNAMIC_FTRACE

@@ -1185,7 +1184,7 @@ static void __add_hash_entry(struct ftrace_hash *hash,
 	hash->count++;
 }
 
-static struct ftrace_func_entry *
+struct ftrace_func_entry *
 add_hash_entry_direct(struct ftrace_hash *hash, unsigned long ip, unsigned long direct)
 {
 	struct ftrace_func_entry *entry;

@@ -1265,7 +1264,7 @@ static void clear_ftrace_mod_list(struct list_head *head)
 	mutex_unlock(&ftrace_lock);
 }
 
-static void free_ftrace_hash(struct ftrace_hash *hash)
+void free_ftrace_hash(struct ftrace_hash *hash)
 {
 	if (!hash || hash == EMPTY_HASH)
 		return;

@@ -1305,7 +1304,7 @@ void ftrace_free_filter(struct ftrace_ops *ops)
 }
 EXPORT_SYMBOL_GPL(ftrace_free_filter);
 
-static struct ftrace_hash *alloc_ftrace_hash(int size_bits)
+struct ftrace_hash *alloc_ftrace_hash(int size_bits)
 {
 	struct ftrace_hash *hash;
 	int size;

kernel/trace/trace.h +0 -8

index 85eabb454bee..62e0ac625f65 100644
@@ -899,14 +899,6 @@ enum {
 	FTRACE_HASH_FL_MOD	= (1 << 0),
 };
 
-struct ftrace_hash {
-	unsigned long		size_bits;
-	struct hlist_head	*buckets;
-	unsigned long		count;
-	unsigned long		flags;
-	struct rcu_head		rcu;
-};
-
 struct ftrace_func_entry *
 ftrace_lookup_ip(struct ftrace_hash *hash, unsigned long ip);

Signature · 1 line

2.52.0

Jiri Olsa

author reviewer

2025-12-03 08:24 UTC #6 lore raw

PATCH v4 4/9 ftrace: Add update_ftrace_direct_add function

In reply to Jiri Olsa

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]>

---
 include/linux/ftrace.h |   7 +++
 kernel/trace/ftrace.c  | 128 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 135 insertions(+)

include/linux/ftrace.h +7 -0

index 8c034b028af4..eb60d971ec1d 100644
@@ -550,6 +550,8 @@ int unregister_ftrace_direct(struct ftrace_ops *ops, unsigned long addr,
 int modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr);
 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);
+
 void ftrace_stub_direct_tramp(void);
 
 #else

@@ -576,6 +578,11 @@ static inline int modify_ftrace_direct_nolock(struct ftrace_ops *ops, unsigned l
 	return -ENODEV;
 }
 
+static inline int update_ftrace_direct_add(struct ftrace_ops *ops, struct ftrace_hash *hash)
+{
+	return -ENODEV;
+}
+
 /*
  * This must be implemented by the architecture.
  * It is the way the ftrace direct_ops helper, when called

kernel/trace/ftrace.c +128 -0

134 lines not on this page · raw message

Signature · 1 line

2.52.0

[email protected]

AI review

2025-12-03 08:47 UTC #7 lore raw

Re: [PATCHv4 bpf-next 4/9] ftrace: Add update_ftrace_direct_add function

Can new_filter_hash leak here? If the second hash_add() fails but the first one succeeds, update_ftrace_direct_add() will goto out_unlock with new_filter_hash pointing to allocated memory. At…

Quoted 4 lines

include/linux/ftrace.h +0 -0
index 8c034b028..eb60d971e 100644

[ ... ]

Quoted 4 lines

kernel/trace/ftrace.c +0 -0
index 84aee9096..1660a8754 100644

[ ... ]

Quoted 32 lines

kernel/trace/ftrace.c +32 -0

+int update_ftrace_direct_add(struct ftrace_ops *ops, struct ftrace_hash *hash)
+{
+	struct ftrace_hash *old_direct_functions = NULL, *new_direct_functions = NULL;
+	struct ftrace_hash *old_filter_hash = NULL, *new_filter_hash = NULL;
+	struct ftrace_func_entry *entry;
+	int i, size, err = -EINVAL;
+	bool reg;
+
+	if (!hash_count(hash))
+		return -EINVAL;
+
+	mutex_lock(&direct_mutex);
+
+	/* Make sure requested entries are not already registered. */
+	size = 1 << hash->size_bits;
+	for (i = 0; i < size; i++) {
+		hlist_for_each_entry(entry, &hash->buckets[i], hlist) {
+			if (__ftrace_lookup_ip(direct_functions, entry->ip))
+				goto out_unlock;
+		}
+	}
+
+	old_filter_hash = ops->func_hash ? ops->func_hash->filter_hash : NULL;
+	old_direct_functions = direct_functions;
+
+	/* If there's nothing in filter_hash we need to register the ops. */
+	reg = hash_count(old_filter_hash) == 0;
+	if (reg) {
+		if (ops->func || ops->trampoline)
+			goto out_unlock;
+		if (ops->flags & FTRACE_OPS_FL_ENABLED)
+			goto out_unlock;

kernel/trace/ftrace.c int modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr) · line 6334

Quoted 10 lines

kernel/trace/ftrace.c +10 -0

+	}
+
+	err = -ENOMEM;
+	new_filter_hash = hash_add(old_filter_hash, hash);
+	if (!new_filter_hash)
+		goto out_unlock;
+
+	new_direct_functions = hash_add(old_direct_functions, hash);
+	if (!new_direct_functions)
+		goto out_unlock;

Can new_filter_hash leak here? If the second hash_add() fails but the first one succeeds, update_ftrace_direct_add() will goto out_unlock with new_filter_hash pointing to allocated memory. At out_unlock, only old_filter_hash gets freed, not new_filter_hash.

register_ftrace_direct() handles this by freeing new_hash on error:

    out_unlock:
        mutex_unlock(&direct_mutex);

        if (free_hash && free_hash != EMPTY_HASH)
            call_rcu_tasks(&free_hash->rcu, register_ftrace_direct_cb);

        if (new_hash)
            free_ftrace_hash(new_hash);

        return err;

Quoted 34 lines

kernel/trace/ftrace.c +34 -0

+
+	rcu_assign_pointer(direct_functions, new_direct_functions);
+
+	if (reg) {
+		ops->func = call_direct_funcs;
+		ops->flags |= MULTI_FLAGS;
+		ops->trampoline = FTRACE_REGS_ADDR;
+		ops->local_hash.filter_hash = new_filter_hash;
+
+		err = register_ftrace_function_nolock(ops);
+		if (err) {
+			/* restore old filter on error */
+			ops->local_hash.filter_hash = old_filter_hash;
+			old_filter_hash = new_filter_hash;
+
+			/* cleanup for possible another register call */
+			ops->func = NULL;
+			ops->trampoline = 0;
+		}
+	} else {
+		err = ftrace_update_ops(ops, new_filter_hash, EMPTY_HASH);
+		/*
+		 * new_filter_hash is dup-ed, so we need to release it anyway,
+		 * old_filter_hash either stays on error or is released already
+		 */
+		old_filter_hash = new_filter_hash;
+	}
+
+	if (err) {
+		/* reset direct_functions and free the new one */
+		rcu_assign_pointer(direct_functions, old_direct_functions);
+		old_direct_functions = new_direct_functions;
+	}
+

kernel/trace/ftrace.c int modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr) · line 6378

Quoted 10 lines

kernel/trace/ftrace.c +10 -0

+ out_unlock:
+	mutex_unlock(&direct_mutex);
+
+	if (old_direct_functions && old_direct_functions != EMPTY_HASH)
+		call_rcu_tasks(&old_direct_functions->rcu, register_ftrace_direct_cb);
+	if (old_filter_hash)
+		free_ftrace_hash(old_filter_hash);
+
+	return err;
+}

---
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/19887401362

Jiri Olsa

2025-12-03 20:25 UTC #8 lore raw

Re: [PATCHv4 bpf-next 4/9] ftrace: Add update_ftrace_direct_add function

Quoted 53 lines · On Wed, Dec 03, 2025 at 08:47:45AM +0000, [email protected] wrote:

Quoted 4 lines

include/linux/ftrace.h +0 -0
index 8c034b028..eb60d971e 100644

[ ... ]

Quoted 4 lines

kernel/trace/ftrace.c +0 -0
index 84aee9096..1660a8754 100644

[ ... ]

Quoted 39 lines

kernel/trace/ftrace.c +39 -0

+int update_ftrace_direct_add(struct ftrace_ops *ops, struct ftrace_hash *hash)
+{
+	struct ftrace_hash *old_direct_functions = NULL, *new_direct_functions = NULL;
+	struct ftrace_hash *old_filter_hash = NULL, *new_filter_hash = NULL;
+	struct ftrace_func_entry *entry;
+	int i, size, err = -EINVAL;
+	bool reg;
+
+	if (!hash_count(hash))
+		return -EINVAL;
+
+	mutex_lock(&direct_mutex);
+
+	/* Make sure requested entries are not already registered. */
+	size = 1 << hash->size_bits;
+	for (i = 0; i < size; i++) {
+		hlist_for_each_entry(entry, &hash->buckets[i], hlist) {
+			if (__ftrace_lookup_ip(direct_functions, entry->ip))
+				goto out_unlock;
+		}
+	}
+
+	old_filter_hash = ops->func_hash ? ops->func_hash->filter_hash : NULL;
+	old_direct_functions = direct_functions;
+
+	/* If there's nothing in filter_hash we need to register the ops. */
+	reg = hash_count(old_filter_hash) == 0;
+	if (reg) {
+		if (ops->func || ops->trampoline)
+			goto out_unlock;
+		if (ops->flags & FTRACE_OPS_FL_ENABLED)
+			goto out_unlock;
+	}
+
+	err = -ENOMEM;
+	new_filter_hash = hash_add(old_filter_hash, hash);
+	if (!new_filter_hash)
+		goto out_unlock;
+

Quoted 8 lines

kernel/trace/ftrace.c int modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr) · line 6334
Quoted 3 lines
kernel/trace/ftrace.c +3 -0
+	new_direct_functions = hash_add(old_direct_functions, hash);
+	if (!new_direct_functions)
+		goto out_unlock;
Can new_filter_hash leak here? If the second hash_add() fails but the first one succeeds, update_ftrace_direct_add() will goto out_unlock with new_filter_hash pointing to allocated memory. At out_unlock, only old_filter_hash gets freed, not new_filter_hash.

mamma mia, can't get this right apparently.. yea, will fix in next version, thanks

jirka

Quoted 65 lines

register_ftrace_direct() handles this by freeing new_hash on error:

    out_unlock:
        mutex_unlock(&direct_mutex);

        if (free_hash && free_hash != EMPTY_HASH)
            call_rcu_tasks(&free_hash->rcu, register_ftrace_direct_cb);

        if (new_hash)
            free_ftrace_hash(new_hash);

        return err;

Quoted 34 lines

kernel/trace/ftrace.c +34 -0

+
+	rcu_assign_pointer(direct_functions, new_direct_functions);
+
+	if (reg) {
+		ops->func = call_direct_funcs;
+		ops->flags |= MULTI_FLAGS;
+		ops->trampoline = FTRACE_REGS_ADDR;
+		ops->local_hash.filter_hash = new_filter_hash;
+
+		err = register_ftrace_function_nolock(ops);
+		if (err) {
+			/* restore old filter on error */
+			ops->local_hash.filter_hash = old_filter_hash;
+			old_filter_hash = new_filter_hash;
+
+			/* cleanup for possible another register call */
+			ops->func = NULL;
+			ops->trampoline = 0;
+		}
+	} else {
+		err = ftrace_update_ops(ops, new_filter_hash, EMPTY_HASH);
+		/*
+		 * new_filter_hash is dup-ed, so we need to release it anyway,
+		 * old_filter_hash either stays on error or is released already
+		 */
+		old_filter_hash = new_filter_hash;
+	}
+
+	if (err) {
+		/* reset direct_functions and free the new one */
+		rcu_assign_pointer(direct_functions, old_direct_functions);
+		old_direct_functions = new_direct_functions;
+	}
+

kernel/trace/ftrace.c int modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr) · line 6378

Quoted 10 lines

kernel/trace/ftrace.c +10 -0

+ out_unlock:
+	mutex_unlock(&direct_mutex);
+
+	if (old_direct_functions && old_direct_functions != EMPTY_HASH)
+		call_rcu_tasks(&old_direct_functions->rcu, register_ftrace_direct_cb);
+	if (old_filter_hash)
+		free_ftrace_hash(old_filter_hash);
+
+	return err;
+}

---
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/19887401362

Jiri Olsa

author reviewer

2025-12-03 08:25 UTC #9 lore raw

PATCH v4 5/9 ftrace: Add update_ftrace_direct_del function

In reply to Jiri Olsa

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]>

---
 include/linux/ftrace.h |   6 +++
 kernel/trace/ftrace.c  | 113 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 119 insertions(+)

include/linux/ftrace.h +6 -0

index eb60d971ec1d..bac9dd784826 100644
@@ -551,6 +551,7 @@ int modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr);
 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);
 
 void ftrace_stub_direct_tramp(void);

@@ -583,6 +584,11 @@ static inline int update_ftrace_direct_add(struct ftrace_ops *ops, struct ftrace
 	return -ENODEV;
 }
 
+static inline int update_ftrace_direct_del(struct ftrace_ops *ops, struct ftrace_hash *hash)
+{
+	return -ENODEV;
+}
+
 /*
  * This must be implemented by the architecture.
  * It is the way the ftrace direct_ops helper, when called

kernel/trace/ftrace.c +113 -0

119 lines not on this page · raw message

Signature · 1 line

2.52.0

Jiri Olsa

author reviewer

2025-12-03 08:25 UTC #10 lore raw

PATCH v4 6/9 ftrace: Add update_ftrace_direct_mod function

In reply to Jiri Olsa

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]>

---
 include/linux/ftrace.h |  6 ++++
 kernel/trace/ftrace.c  | 72 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+)

include/linux/ftrace.h +6 -0

index bac9dd784826..c27b7381c5f1 100644
@@ -552,6 +552,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);

@@ -589,6 +590,11 @@ static inline int update_ftrace_direct_del(struct ftrace_ops *ops, struct ftrace
 	return -ENODEV;
 }
 
+static inline int update_ftrace_direct_mod(struct ftrace_ops *ops, struct ftrace_hash *hash, bool do_direct_lock)
+{
+	return -ENODEV;
+}
+
 /*
  * This must be implemented by the architecture.
  * It is the way the ftrace direct_ops helper, when called

kernel/trace/ftrace.c +72 -0

78 lines not on this page · raw message

Signature · 1 line

2.52.0

Jiri Olsa

author reviewer

2025-12-03 08:25 UTC #11 lore raw

PATCH v4 7/9 bpf: Add trampoline ip hash table

In reply to Jiri Olsa

Following changes need to lookup trampoline based on its ip address, adding hash table for that.

Signed-off-by Jiri Olsa <[email protected]>

---
 include/linux/bpf.h     |  7 +++++--
 kernel/bpf/trampoline.c | 30 +++++++++++++++++++-----------
 2 files changed, 24 insertions(+), 13 deletions(-)

include/linux/bpf.h +5 -2

index 6498be4c44f8..ef44653eae44 100644
@@ -1307,14 +1307,17 @@ struct bpf_tramp_image {
 };
 
 struct bpf_trampoline {
-	/* hlist for trampoline_table */
-	struct hlist_node hlist;
+	/* hlist for trampoline_key_table */
+	struct hlist_node hlist_key;
+	/* hlist for trampoline_ip_table */
+	struct hlist_node hlist_ip;
 	struct ftrace_ops *fops;
 	/* serializes access to fields of this trampoline */
 	struct mutex mutex;
 	refcount_t refcnt;
 	u32 flags;
 	u64 key;
+	unsigned long ip;
 	struct {
 		struct btf_func_model model;
 		void *addr;

kernel/bpf/trampoline.c +19 ~~-11~~

index b9a358d7a78f..f298bafab76e 100644
@@ -24,9 +24,10 @@ const struct bpf_prog_ops bpf_extension_prog_ops = {
 #define TRAMPOLINE_HASH_BITS 10
 #define TRAMPOLINE_TABLE_SIZE (1 << TRAMPOLINE_HASH_BITS)
 
-static struct hlist_head trampoline_table[TRAMPOLINE_TABLE_SIZE];
+static struct hlist_head trampoline_key_table[TRAMPOLINE_TABLE_SIZE];
+static struct hlist_head trampoline_ip_table[TRAMPOLINE_TABLE_SIZE];
 
-/* serializes access to trampoline_table */
+/* serializes access to trampoline tables */
 static DEFINE_MUTEX(trampoline_mutex);
 
 #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS

@@ -135,15 +136,15 @@ void bpf_image_ksym_del(struct bpf_ksym *ksym)
 			   PAGE_SIZE, true, ksym->name);
 }
 
-static struct bpf_trampoline *bpf_trampoline_lookup(u64 key)
+static struct bpf_trampoline *bpf_trampoline_lookup(u64 key, unsigned long ip)
 {
 	struct bpf_trampoline *tr;
 	struct hlist_head *head;
 	int i;
 
 	mutex_lock(&trampoline_mutex);
-	head = &trampoline_table[hash_64(key, TRAMPOLINE_HASH_BITS)];
-	hlist_for_each_entry(tr, head, hlist) {
+	head = &trampoline_key_table[hash_64(key, TRAMPOLINE_HASH_BITS)];
+	hlist_for_each_entry(tr, head, hlist_key) {
 		if (tr->key == key) {
 			refcount_inc(&tr->refcnt);
 			goto out;

@@ -164,8 +165,12 @@ static struct bpf_trampoline *bpf_trampoline_lookup(u64 key)
 #endif
 
 	tr->key = key;
-	INIT_HLIST_NODE(&tr->hlist);
-	hlist_add_head(&tr->hlist, head);
+	tr->ip = ftrace_location(ip);
+	INIT_HLIST_NODE(&tr->hlist_key);
+	INIT_HLIST_NODE(&tr->hlist_ip);
+	hlist_add_head(&tr->hlist_key, head);
+	head = &trampoline_ip_table[hash_64(tr->ip, TRAMPOLINE_HASH_BITS)];
+	hlist_add_head(&tr->hlist_ip, head);
 	refcount_set(&tr->refcnt, 1);
 	mutex_init(&tr->mutex);
 	for (i = 0; i < BPF_TRAMP_MAX; i++)

@@ -846,7 +851,7 @@ void bpf_trampoline_unlink_cgroup_shim(struct bpf_prog *prog)
 					 prog->aux->attach_btf_id);
 
 	bpf_lsm_find_cgroup_shim(prog, &bpf_func);
-	tr = bpf_trampoline_lookup(key);
+	tr = bpf_trampoline_lookup(key, 0);
 	if (WARN_ON_ONCE(!tr))
 		return;

@@ -866,7 +871,7 @@ struct bpf_trampoline *bpf_trampoline_get(u64 key,
 {
 	struct bpf_trampoline *tr;
 
-	tr = bpf_trampoline_lookup(key);
+	tr = bpf_trampoline_lookup(key, tgt_info->tgt_addr);
 	if (!tr)
 		return NULL;

@@ -902,7 +907,8 @@ void bpf_trampoline_put(struct bpf_trampoline *tr)
 	 * fexit progs. The fentry-only trampoline will be freed via
 	 * multiple rcu callbacks.
 	 */
-	hlist_del(&tr->hlist);
+	hlist_del(&tr->hlist_key);
+	hlist_del(&tr->hlist_ip);
 	if (tr->fops) {
 		ftrace_free_filter(tr->fops);
 		kfree(tr->fops);

@@ -1175,7 +1181,9 @@ static int __init init_trampolines(void)
 	int i;
 
 	for (i = 0; i < TRAMPOLINE_TABLE_SIZE; i++)
-		INIT_HLIST_HEAD(&trampoline_table[i]);
+		INIT_HLIST_HEAD(&trampoline_key_table[i]);
+	for (i = 0; i < TRAMPOLINE_TABLE_SIZE; i++)
+		INIT_HLIST_HEAD(&trampoline_ip_table[i]);
 	return 0;
 }
 late_initcall(init_trampolines);

Signature · 1 line

2.52.0

Jiri Olsa

author reviewer

2025-12-03 08:25 UTC #12 lore raw

PATCH v4 8/9 ftrace: Factor ftrace_ops ops_func interface

In reply to Jiri Olsa

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]>

---
 include/linux/ftrace.h  | 2 +-
 kernel/bpf/trampoline.c | 3 ++-
 kernel/trace/ftrace.c   | 6 +++---
 3 files changed, 6 insertions(+), 5 deletions(-)

include/linux/ftrace.h +1 -1

index c27b7381c5f1..515cf5b723dd 100644
@@ -402,7 +402,7 @@ enum ftrace_ops_cmd {
  *        Negative on failure. The return value is dependent on the
  *        callback.
  */
-typedef int (*ftrace_ops_func_t)(struct ftrace_ops *op, enum ftrace_ops_cmd cmd);
+typedef int (*ftrace_ops_func_t)(struct ftrace_ops *op, unsigned long ip, enum ftrace_ops_cmd cmd);
 
 #ifdef CONFIG_DYNAMIC_FTRACE

kernel/bpf/trampoline.c +2 -1

index f298bafab76e..17af2aad8382 100644
@@ -33,7 +33,8 @@ static DEFINE_MUTEX(trampoline_mutex);
 #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
 static int bpf_trampoline_update(struct bpf_trampoline *tr, bool lock_direct_mutex);
 
-static int bpf_tramp_ftrace_ops_func(struct ftrace_ops *ops, enum ftrace_ops_cmd cmd)
+static int bpf_tramp_ftrace_ops_func(struct ftrace_ops *ops, unsigned long ip,
+				     enum ftrace_ops_cmd cmd)
 {
 	struct bpf_trampoline *tr = ops->private;
 	int ret = 0;

kernel/trace/ftrace.c +3 -3

index c77f620b3eb3..9582cb374d4f 100644
@@ -2049,7 +2049,7 @@ static int __ftrace_hash_update_ipmodify(struct ftrace_ops *ops,
 				 */
 				if (!ops->ops_func)
 					return -EBUSY;
-				ret = ops->ops_func(ops, FTRACE_OPS_CMD_ENABLE_SHARE_IPMODIFY_SELF);
+				ret = ops->ops_func(ops, rec->ip, FTRACE_OPS_CMD_ENABLE_SHARE_IPMODIFY_SELF);
 				if (ret)
 					return ret;
 			} else if (is_ipmodify) {

@@ -8984,7 +8984,7 @@ static int prepare_direct_functions_for_ipmodify(struct ftrace_ops *ops)
 				if (!op->ops_func)
 					return -EBUSY;
 
-				ret = op->ops_func(op, FTRACE_OPS_CMD_ENABLE_SHARE_IPMODIFY_PEER);
+				ret = op->ops_func(op, ip, FTRACE_OPS_CMD_ENABLE_SHARE_IPMODIFY_PEER);
 				if (ret)
 					return ret;
 			}

@@ -9031,7 +9031,7 @@ static void cleanup_direct_functions_after_ipmodify(struct ftrace_ops *ops)
 
 			/* The cleanup is optional, ignore any errors */
 			if (found_op && op->ops_func)
-				op->ops_func(op, FTRACE_OPS_CMD_DISABLE_SHARE_IPMODIFY_PEER);
+				op->ops_func(op, ip, FTRACE_OPS_CMD_DISABLE_SHARE_IPMODIFY_PEER);
 		}
 	}
 	mutex_unlock(&direct_mutex);

Signature · 1 line

2.52.0

Jiri Olsa

author reviewer

2025-12-03 08:25 UTC #13 lore raw

PATCH v4 9/9 bpf,x86: Use single ftrace_ops for direct calls

In reply to Jiri Olsa

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]>

---
 arch/x86/Kconfig        |   1 +
 kernel/bpf/trampoline.c | 195 ++++++++++++++++++++++++++++++++++------
 kernel/trace/Kconfig    |   3 +
 kernel/trace/ftrace.c   |   7 +-
 4 files changed, 177 insertions(+), 29 deletions(-)

arch/x86/Kconfig +1 -0

index 462250a20311..737dff48342b 100644
@@ -333,6 +333,7 @@ config X86
 	select SCHED_SMT			if SMP
 	select ARCH_SUPPORTS_SCHED_CLUSTER	if SMP
 	select ARCH_SUPPORTS_SCHED_MC		if SMP
+	select HAVE_SINGLE_FTRACE_DIRECT_OPS	if X86_64 && DYNAMIC_FTRACE_WITH_DIRECT_CALLS
 
 config INSTRUCTION_DECODER
 	def_bool y

kernel/bpf/trampoline.c +167 ~~-28~~

246 lines not on this page · raw message

kernel/trace/Kconfig +3 -0

index 4661b9e606e0..1ad2e307c834 100644
@@ -50,6 +50,9 @@ config HAVE_DYNAMIC_FTRACE_WITH_REGS
 config HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
 	bool
 
+config HAVE_SINGLE_FTRACE_DIRECT_OPS
+	bool
+
 config HAVE_DYNAMIC_FTRACE_WITH_CALL_OPS
 	bool

kernel/trace/ftrace.c +6 -1

index 9582cb374d4f..0bdee1758fa1 100644
@@ -2605,8 +2605,13 @@ unsigned long ftrace_find_rec_direct(unsigned long ip)
 static void call_direct_funcs(unsigned long ip, unsigned long pip,
 			      struct ftrace_ops *ops, struct ftrace_regs *fregs)
 {
-	unsigned long addr = READ_ONCE(ops->direct_call);
+	unsigned long addr;
 
+#ifdef CONFIG_HAVE_SINGLE_FTRACE_DIRECT_OPS
+	addr = ftrace_find_rec_direct(ip);
+#else
+	addr = READ_ONCE(ops->direct_call);
+#endif
 	if (!addr)
 		return;

Signature · 1 line

2.52.0