* [PATCHv2 bpf-next 1/8] ftrace: Make alloc_and_copy_ftrace_hash direct friendly
2025-11-13 12:37 [PATCHv2 bpf-next 0/9] ftrace,bpf: Use single direct ops for bpf trampolines Jiri Olsa
@ 2025-11-13 12:37 ` Jiri Olsa
2025-11-13 12:37 ` [PATCHv2 bpf-next 2/8] ftrace: Export some of hash related functions Jiri Olsa
` (6 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Jiri Olsa @ 2025-11-13 12:37 UTC (permalink / raw)
To: Steven Rostedt, Florent Revest, Mark Rutland
Cc: bpf, linux-kernel, linux-trace-kernel, linux-arm-kernel,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Menglong Dong, Song Liu
Make alloc_and_copy_ftrace_hash to copy also direct address
for each hash entry.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
kernel/trace/ftrace.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 59cfacb8a5bb..0e3714b796d9 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -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;
}
}
--
2.51.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCHv2 bpf-next 2/8] ftrace: Export some of hash related functions
2025-11-13 12:37 [PATCHv2 bpf-next 0/9] ftrace,bpf: Use single direct ops for bpf trampolines Jiri Olsa
2025-11-13 12:37 ` [PATCHv2 bpf-next 1/8] ftrace: Make alloc_and_copy_ftrace_hash direct friendly Jiri Olsa
@ 2025-11-13 12:37 ` Jiri Olsa
2025-11-13 12:37 ` [PATCHv2 bpf-next 3/8] ftrace: Add update_ftrace_direct_add function Jiri Olsa
` (5 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Jiri Olsa @ 2025-11-13 12:37 UTC (permalink / raw)
To: Steven Rostedt, Florent Revest, Mark Rutland
Cc: bpf, linux-kernel, linux-trace-kernel, linux-arm-kernel,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Menglong Dong, Song Liu
We are going to use these functions in following changes.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
include/linux/ftrace.h | 16 ++++++++++++++++
kernel/trace/ftrace.c | 7 +++----
kernel/trace/trace.h | 8 --------
3 files changed, 19 insertions(+), 12 deletions(-)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 7ded7df6e9b5..e23e6a859e08 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -397,6 +397,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;
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 0e3714b796d9..e6ccf572c5f6 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -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;
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 85eabb454bee..62e0ac625f65 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -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);
--
2.51.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCHv2 bpf-next 3/8] ftrace: Add update_ftrace_direct_add function
2025-11-13 12:37 [PATCHv2 bpf-next 0/9] ftrace,bpf: Use single direct ops for bpf trampolines Jiri Olsa
2025-11-13 12:37 ` [PATCHv2 bpf-next 1/8] ftrace: Make alloc_and_copy_ftrace_hash direct friendly Jiri Olsa
2025-11-13 12:37 ` [PATCHv2 bpf-next 2/8] ftrace: Export some of hash related functions Jiri Olsa
@ 2025-11-13 12:37 ` Jiri Olsa
2025-11-13 13:02 ` bot+bpf-ci
2025-11-13 12:37 ` [PATCHv2 bpf-next 4/8] ftrace: Add update_ftrace_direct_del function Jiri Olsa
` (4 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Jiri Olsa @ 2025-11-13 12:37 UTC (permalink / raw)
To: Steven Rostedt, Florent Revest, Mark Rutland
Cc: bpf, linux-kernel, linux-trace-kernel, linux-arm-kernel,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Menglong Dong, Song Liu
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 <jolsa@kernel.org>
---
include/linux/ftrace.h | 7 +++
kernel/trace/ftrace.c | 128 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 135 insertions(+)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index e23e6a859e08..ded3a306a8b2 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -542,6 +542,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
@@ -568,6 +570,11 @@ static inline int modify_ftrace_direct_nolock(struct ftrace_ops *ops, unsigned l
return -ENODEV;
}
+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
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index e6ccf572c5f6..0b6e1af8f922 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -6248,6 +6248,134 @@ int modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
return err;
}
EXPORT_SYMBOL_GPL(modify_ftrace_direct);
+
+static unsigned long hash_count(struct ftrace_hash *hash)
+{
+ return hash ? hash->count : 0;
+}
+
+/**
+ * hash_add - adds two struct ftrace_hash and returns the result
+ * @a: struct ftrace_hash object
+ * @b: struct ftrace_hash object
+ *
+ * Returns struct ftrace_hash object on success, NULL on error.
+ */
+static struct ftrace_hash *hash_add(struct ftrace_hash *a, struct ftrace_hash *b)
+{
+ struct ftrace_func_entry *entry;
+ struct ftrace_hash *add;
+ int size, i;
+
+ size = hash_count(a) + hash_count(b);
+ if (size > 32)
+ size = 32;
+
+ add = alloc_and_copy_ftrace_hash(fls(size), a);
+ if (!add)
+ goto error;
+
+ size = 1 << b->size_bits;
+ for (i = 0; i < size; i++) {
+ hlist_for_each_entry(entry, &b->buckets[i], hlist) {
+ if (add_hash_entry_direct(add, entry->ip, entry->direct) == NULL)
+ goto error;
+ }
+ }
+ return add;
+
+ error:
+ free_ftrace_hash(add);
+ return NULL;
+}
+
+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 0;
+
+ 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;
+
+ new_direct_functions = hash_add(old_direct_functions, hash);
+ if (!new_direct_functions)
+ goto out_unlock;
+
+ 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;
+ }
+
+ 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;
+}
+
#endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
/**
--
2.51.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCHv2 bpf-next 3/8] ftrace: Add update_ftrace_direct_add function
2025-11-13 12:37 ` [PATCHv2 bpf-next 3/8] ftrace: Add update_ftrace_direct_add function Jiri Olsa
@ 2025-11-13 13:02 ` bot+bpf-ci
2025-11-13 15:59 ` Jiri Olsa
0 siblings, 1 reply; 16+ messages in thread
From: bot+bpf-ci @ 2025-11-13 13:02 UTC (permalink / raw)
To: jolsa, rostedt, revest, mark.rutland
Cc: bpf, linux-kernel, linux-trace-kernel, linux-arm-kernel, ast,
daniel, andrii, menglong8.dong, song, ast, andrii, daniel,
martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 1154 bytes --]
> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index e23e6a859..ded3a306a 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
[ ... ]
> @@ -568,6 +570,11 @@ static inline int modify_ftrace_direct_nolock(struct ftrace_ops *ops, unsigned l
> return -ENODEV;
> }
>
> +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
Should this stub function have static inline qualifiers? All the other
stub functions in this section (ftrace_find_rec_direct,
register_ftrace_direct, unregister_ftrace_direct, modify_ftrace_direct,
and modify_ftrace_direct_nolock) use static inline. Without it, this
could cause multiple definition linker errors when the header is
included in multiple compilation units.
---
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
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCHv2 bpf-next 3/8] ftrace: Add update_ftrace_direct_add function
2025-11-13 13:02 ` bot+bpf-ci
@ 2025-11-13 15:59 ` Jiri Olsa
0 siblings, 0 replies; 16+ messages in thread
From: Jiri Olsa @ 2025-11-13 15:59 UTC (permalink / raw)
To: bot+bpf-ci
Cc: rostedt, revest, mark.rutland, bpf, linux-kernel,
linux-trace-kernel, linux-arm-kernel, ast, daniel, andrii,
menglong8.dong, song, martin.lau, eddyz87, yonghong.song, clm,
ihor.solodrai
On Thu, Nov 13, 2025 at 01:02:18PM +0000, bot+bpf-ci@kernel.org wrote:
> > diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> > index e23e6a859..ded3a306a 100644
> > --- a/include/linux/ftrace.h
> > +++ b/include/linux/ftrace.h
>
> [ ... ]
>
> > @@ -568,6 +570,11 @@ static inline int modify_ftrace_direct_nolock(struct ftrace_ops *ops, unsigned l
> > return -ENODEV;
> > }
> >
> > +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
>
> Should this stub function have static inline qualifiers? All the other
> stub functions in this section (ftrace_find_rec_direct,
> register_ftrace_direct, unregister_ftrace_direct, modify_ftrace_direct,
> and modify_ftrace_direct_nolock) use static inline. Without it, this
> could cause multiple definition linker errors when the header is
> included in multiple compilation units.
yep, also the other ones are missing it, will add.. thanks
jirka
>
>
> ---
> 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
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCHv2 bpf-next 4/8] ftrace: Add update_ftrace_direct_del function
2025-11-13 12:37 [PATCHv2 bpf-next 0/9] ftrace,bpf: Use single direct ops for bpf trampolines Jiri Olsa
` (2 preceding siblings ...)
2025-11-13 12:37 ` [PATCHv2 bpf-next 3/8] ftrace: Add update_ftrace_direct_add function Jiri Olsa
@ 2025-11-13 12:37 ` Jiri Olsa
2025-11-13 13:02 ` bot+bpf-ci
2025-11-13 12:37 ` [PATCHv2 bpf-next 5/8] ftrace: Add update_ftrace_direct_mod function Jiri Olsa
` (3 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Jiri Olsa @ 2025-11-13 12:37 UTC (permalink / raw)
To: Steven Rostedt, Florent Revest, Mark Rutland
Cc: bpf, linux-kernel, linux-trace-kernel, linux-arm-kernel,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Menglong Dong, Song Liu
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 <jolsa@kernel.org>
---
include/linux/ftrace.h | 6 +++
kernel/trace/ftrace.c | 99 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 105 insertions(+)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index ded3a306a8b2..433c36c3af3b 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -543,6 +543,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);
@@ -575,6 +576,11 @@ int update_ftrace_direct_add(struct ftrace_ops *ops, struct ftrace_hash *hash)
return -ENODEV;
}
+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
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 0b6e1af8f922..3dd1e69aceca 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -6376,6 +6376,105 @@ int update_ftrace_direct_add(struct ftrace_ops *ops, struct ftrace_hash *hash)
return err;
}
+/**
+ * hash_sub - substracts @b from @a and returns the result
+ * @a: struct ftrace_hash object
+ * @b: struct ftrace_hash object
+ *
+ * Returns struct ftrace_hash object on success, NULL on error.
+ */
+static struct ftrace_hash *hash_sub(struct ftrace_hash *a, struct ftrace_hash *b)
+{
+ struct ftrace_func_entry *entry, *del;
+ struct ftrace_hash *sub;
+ int size, i;
+
+ sub = alloc_and_copy_ftrace_hash(a->size_bits, a);
+ if (!sub)
+ goto error;
+
+ size = 1 << b->size_bits;
+ for (i = 0; i < size; i++) {
+ hlist_for_each_entry(entry, &b->buckets[i], hlist) {
+ del = __ftrace_lookup_ip(sub, entry->ip);
+ if (WARN_ON_ONCE(!del))
+ goto error;
+ remove_hash_entry(sub, del);
+ kfree(del);
+ }
+ }
+ return sub;
+
+ error:
+ free_ftrace_hash(sub);
+ return NULL;
+}
+
+int update_ftrace_direct_del(struct ftrace_ops *ops, struct ftrace_hash *hash)
+{
+ struct ftrace_hash *new_hash = NULL, *filter_hash = NULL, *free_hash = NULL;
+ struct ftrace_func_entry *del, *entry;
+ unsigned long size, i;
+ int err = -EINVAL;
+
+ 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;
+
+ mutex_lock(&direct_mutex);
+
+ /* Make sure requested entries are already registered. */
+ size = 1 << hash->size_bits;
+ for (i = 0; i < size; i++) {
+ hlist_for_each_entry(entry, &hash->buckets[i], hlist) {
+ del = __ftrace_lookup_ip(direct_functions, entry->ip);
+ if (!del || del->direct != entry->direct)
+ goto out_unlock;
+ }
+ }
+
+ err = -ENOMEM;
+ filter_hash = hash_sub(ops->func_hash->filter_hash, hash);
+ if (!filter_hash)
+ goto out_unlock;
+
+ new_hash = hash_sub(direct_functions, hash);
+ if (!new_hash)
+ goto out_unlock;
+
+ /* If there's nothing left, we need to unregister the ops. */
+ if (ftrace_hash_empty(filter_hash)) {
+ err = unregister_ftrace_function(ops);
+ /* cleanup for possible another register call */
+ ops->func = NULL;
+ ops->trampoline = 0;
+ ftrace_free_filter(ops);
+ ops->func_hash->filter_hash = NULL;
+ } else {
+ err = ftrace_update_ops(ops, filter_hash, EMPTY_HASH);
+ }
+
+ if (!err) {
+ free_hash = direct_functions;
+ rcu_assign_pointer(direct_functions, new_hash);
+ }
+
+ out_unlock:
+ mutex_unlock(&direct_mutex);
+
+ if (free_hash && free_hash != EMPTY_HASH)
+ call_rcu_tasks(&free_hash->rcu, register_ftrace_direct_cb);
+ if (filter_hash)
+ free_ftrace_hash(filter_hash);
+
+ return err;
+}
+
#endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
/**
--
2.51.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCHv2 bpf-next 4/8] ftrace: Add update_ftrace_direct_del function
2025-11-13 12:37 ` [PATCHv2 bpf-next 4/8] ftrace: Add update_ftrace_direct_del function Jiri Olsa
@ 2025-11-13 13:02 ` bot+bpf-ci
2025-11-13 16:00 ` Jiri Olsa
0 siblings, 1 reply; 16+ messages in thread
From: bot+bpf-ci @ 2025-11-13 13:02 UTC (permalink / raw)
To: jolsa, rostedt, revest, mark.rutland
Cc: bpf, linux-kernel, linux-trace-kernel, linux-arm-kernel, ast,
daniel, andrii, menglong8.dong, song, ast, andrii, daniel,
martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 3213 bytes --]
> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index ded3a306a..433c36c3a 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
> @@ -543,6 +543,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);
[ ... ]
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 311ce6406..345f3fdd2 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
[ ... ]
> +int update_ftrace_direct_del(struct ftrace_ops *ops, struct ftrace_hash *hash)
> +{
> + struct ftrace_hash *new_hash = NULL, *filter_hash = NULL, *free_hash = NULL;
> + struct ftrace_func_entry *del, *entry;
> + unsigned long size, i;
> + int err = -EINVAL;
> +
> + 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;
> +
> + mutex_lock(&direct_mutex);
> +
> + /* Make sure requested entries are already registered. */
> + size = 1 << hash->size_bits;
> + for (i = 0; i < size; i++) {
> + hlist_for_each_entry(entry, &hash->buckets[i], hlist) {
> + del = __ftrace_lookup_ip(direct_functions, entry->ip);
> + if (!del || del->direct != entry->direct)
> + goto out_unlock;
> + }
> + }
> +
> + err = -ENOMEM;
> + filter_hash = hash_sub(ops->func_hash->filter_hash, hash);
> + if (!filter_hash)
> + goto out_unlock;
> +
> + new_hash = hash_sub(direct_functions, hash);
> + if (!new_hash)
> + goto out_unlock;
Can this leak new_hash? If hash_sub() succeeds for both filter_hash
and new_hash, but then either unregister_ftrace_function() or
ftrace_update_ops() fails below, we goto out_unlock without freeing
new_hash. The cleanup code only frees filter_hash:
> +
> + /* If there's nothing left, we need to unregister the ops. */
> + if (ftrace_hash_empty(filter_hash)) {
> + err = unregister_ftrace_function(ops);
> + /* cleanup for possible another register call */
> + ops->func = NULL;
> + ops->trampoline = 0;
> + ftrace_free_filter(ops);
> + ops->func_hash->filter_hash = NULL;
> + } else {
> + err = ftrace_update_ops(ops, filter_hash, EMPTY_HASH);
> + }
> +
> + if (!err) {
> + free_hash = direct_functions;
> + rcu_assign_pointer(direct_functions, new_hash);
> + }
> +
> + out_unlock:
> + mutex_unlock(&direct_mutex);
> +
> + if (free_hash && free_hash != EMPTY_HASH)
> + call_rcu_tasks(&free_hash->rcu, register_ftrace_direct_cb);
> + if (filter_hash)
> + free_ftrace_hash(filter_hash);
> +
> + return err;
> +}
If err != 0, new_hash remains allocated but is never freed. Should the
cleanup code add free_ftrace_hash(new_hash) when it's non-NULL?
---
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
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCHv2 bpf-next 4/8] ftrace: Add update_ftrace_direct_del function
2025-11-13 13:02 ` bot+bpf-ci
@ 2025-11-13 16:00 ` Jiri Olsa
0 siblings, 0 replies; 16+ messages in thread
From: Jiri Olsa @ 2025-11-13 16:00 UTC (permalink / raw)
To: bot+bpf-ci
Cc: rostedt, revest, mark.rutland, bpf, linux-kernel,
linux-trace-kernel, linux-arm-kernel, ast, daniel, andrii,
menglong8.dong, song, martin.lau, eddyz87, yonghong.song, clm,
ihor.solodrai
On Thu, Nov 13, 2025 at 01:02:21PM +0000, bot+bpf-ci@kernel.org wrote:
> > diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> > index ded3a306a..433c36c3a 100644
> > --- a/include/linux/ftrace.h
> > +++ b/include/linux/ftrace.h
> > @@ -543,6 +543,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);
>
> [ ... ]
>
> > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> > index 311ce6406..345f3fdd2 100644
> > --- a/kernel/trace/ftrace.c
> > +++ b/kernel/trace/ftrace.c
>
> [ ... ]
>
> > +int update_ftrace_direct_del(struct ftrace_ops *ops, struct ftrace_hash *hash)
> > +{
> > + struct ftrace_hash *new_hash = NULL, *filter_hash = NULL, *free_hash = NULL;
> > + struct ftrace_func_entry *del, *entry;
> > + unsigned long size, i;
> > + int err = -EINVAL;
> > +
> > + 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;
> > +
> > + mutex_lock(&direct_mutex);
> > +
> > + /* Make sure requested entries are already registered. */
> > + size = 1 << hash->size_bits;
> > + for (i = 0; i < size; i++) {
> > + hlist_for_each_entry(entry, &hash->buckets[i], hlist) {
> > + del = __ftrace_lookup_ip(direct_functions, entry->ip);
> > + if (!del || del->direct != entry->direct)
> > + goto out_unlock;
> > + }
> > + }
> > +
> > + err = -ENOMEM;
> > + filter_hash = hash_sub(ops->func_hash->filter_hash, hash);
> > + if (!filter_hash)
> > + goto out_unlock;
> > +
> > + new_hash = hash_sub(direct_functions, hash);
> > + if (!new_hash)
> > + goto out_unlock;
>
> Can this leak new_hash? If hash_sub() succeeds for both filter_hash
> and new_hash, but then either unregister_ftrace_function() or
> ftrace_update_ops() fails below, we goto out_unlock without freeing
> new_hash. The cleanup code only frees filter_hash:
>
> > +
> > + /* If there's nothing left, we need to unregister the ops. */
> > + if (ftrace_hash_empty(filter_hash)) {
> > + err = unregister_ftrace_function(ops);
> > + /* cleanup for possible another register call */
> > + ops->func = NULL;
> > + ops->trampoline = 0;
> > + ftrace_free_filter(ops);
> > + ops->func_hash->filter_hash = NULL;
> > + } else {
> > + err = ftrace_update_ops(ops, filter_hash, EMPTY_HASH);
> > + }
> > +
> > + if (!err) {
> > + free_hash = direct_functions;
> > + rcu_assign_pointer(direct_functions, new_hash);
> > + }
> > +
> > + out_unlock:
> > + mutex_unlock(&direct_mutex);
> > +
> > + if (free_hash && free_hash != EMPTY_HASH)
> > + call_rcu_tasks(&free_hash->rcu, register_ftrace_direct_cb);
> > + if (filter_hash)
> > + free_ftrace_hash(filter_hash);
> > +
> > + return err;
> > +}
>
> If err != 0, new_hash remains allocated but is never freed. Should the
> cleanup code add free_ftrace_hash(new_hash) when it's non-NULL?
yep, that's a miss, will fix
thanks,
jirka
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCHv2 bpf-next 5/8] ftrace: Add update_ftrace_direct_mod function
2025-11-13 12:37 [PATCHv2 bpf-next 0/9] ftrace,bpf: Use single direct ops for bpf trampolines Jiri Olsa
` (3 preceding siblings ...)
2025-11-13 12:37 ` [PATCHv2 bpf-next 4/8] ftrace: Add update_ftrace_direct_del function Jiri Olsa
@ 2025-11-13 12:37 ` Jiri Olsa
2025-11-13 13:02 ` bot+bpf-ci
2025-11-13 12:37 ` [PATCHv2 bpf-next 6/8] bpf: Add trampoline ip hash table Jiri Olsa
` (2 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Jiri Olsa @ 2025-11-13 12:37 UTC (permalink / raw)
To: Steven Rostedt, Florent Revest, Mark Rutland
Cc: bpf, linux-kernel, linux-trace-kernel, linux-arm-kernel,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Menglong Dong, Song Liu
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 <jolsa@kernel.org>
---
include/linux/ftrace.h | 6 ++++
kernel/trace/ftrace.c | 68 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 74 insertions(+)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 433c36c3af3b..bacb6d9ab426 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)
+{
+ return -ENODEV;
+}
+
/*
* This must be implemented by the architecture.
* It is the way the ftrace direct_ops helper, when called
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 3dd1e69aceca..e701516a4a65 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -6475,6 +6475,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);
+ 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);
+
+ 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 */
/**
--
2.51.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCHv2 bpf-next 5/8] ftrace: Add update_ftrace_direct_mod function
2025-11-13 12:37 ` [PATCHv2 bpf-next 5/8] ftrace: Add update_ftrace_direct_mod function Jiri Olsa
@ 2025-11-13 13:02 ` bot+bpf-ci
2025-11-13 16:00 ` Jiri Olsa
0 siblings, 1 reply; 16+ messages in thread
From: bot+bpf-ci @ 2025-11-13 13:02 UTC (permalink / raw)
To: jolsa, rostedt, revest, mark.rutland
Cc: bpf, linux-kernel, linux-trace-kernel, linux-arm-kernel, ast,
daniel, andrii, menglong8.dong, song, ast, andrii, daniel,
martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 3918 bytes --]
> 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
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCHv2 bpf-next 5/8] ftrace: Add update_ftrace_direct_mod function
2025-11-13 13:02 ` bot+bpf-ci
@ 2025-11-13 16:00 ` Jiri Olsa
2025-11-13 17:57 ` Alexei Starovoitov
0 siblings, 1 reply; 16+ messages in thread
From: Jiri Olsa @ 2025-11-13 16:00 UTC (permalink / raw)
To: bot+bpf-ci
Cc: rostedt, revest, mark.rutland, bpf, linux-kernel,
linux-trace-kernel, linux-arm-kernel, ast, daniel, andrii,
menglong8.dong, song, martin.lau, eddyz87, yonghong.song, clm,
ihor.solodrai
On Thu, Nov 13, 2025 at 01:02:17PM +0000, bot+bpf-ci@kernel.org wrote:
> > 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()?
it should, my bad
>
> > +{
> > + 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.
the whole patchset is based on bpf-next/master plus Song's livepatch
fixes which change modify_ftrace_direct_hash function, it's mentioned
in the cover letter
>
> > + 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.
I don't think this can happen because tmp_ops does not have
FTRACE_OPS_FL_IPMODIFY flag set
thanks,
jirka
>
> > +
> > +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
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCHv2 bpf-next 5/8] ftrace: Add update_ftrace_direct_mod function
2025-11-13 16:00 ` Jiri Olsa
@ 2025-11-13 17:57 ` Alexei Starovoitov
0 siblings, 0 replies; 16+ messages in thread
From: Alexei Starovoitov @ 2025-11-13 17:57 UTC (permalink / raw)
To: Jiri Olsa
Cc: bot+bpf-ci, rostedt, Florent Revest, Mark Rutland, bpf, LKML,
linux-trace-kernel, linux-arm-kernel, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Menglong Dong, Song Liu,
Martin KaFai Lau, Eduard, Yonghong Song, Chris Mason,
Ihor Solodrai
On Thu, Nov 13, 2025 at 8:00 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Thu, Nov 13, 2025 at 01:02:17PM +0000, bot+bpf-ci@kernel.org wrote:
> > > 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()?
>
> it should, my bad
>
> >
> > > +{
> > > + 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.
>
> the whole patchset is based on bpf-next/master plus Song's livepatch
> fixes which change modify_ftrace_direct_hash function, it's mentioned
> in the cover letter
Ohh. Will send bpf PR to Linus today and merge into bpf-next afterwards.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCHv2 bpf-next 6/8] bpf: Add trampoline ip hash table
2025-11-13 12:37 [PATCHv2 bpf-next 0/9] ftrace,bpf: Use single direct ops for bpf trampolines Jiri Olsa
` (4 preceding siblings ...)
2025-11-13 12:37 ` [PATCHv2 bpf-next 5/8] ftrace: Add update_ftrace_direct_mod function Jiri Olsa
@ 2025-11-13 12:37 ` Jiri Olsa
2025-11-13 12:37 ` [PATCHv2 bpf-next 7/8] ftrace: Factor ftrace_ops ops_func interface Jiri Olsa
2025-11-13 12:37 ` [PATCHv2 bpf-next 8/8] bpf, x86: Use single ftrace_ops for direct calls Jiri Olsa
7 siblings, 0 replies; 16+ messages in thread
From: Jiri Olsa @ 2025-11-13 12:37 UTC (permalink / raw)
To: Steven Rostedt, Florent Revest, Mark Rutland
Cc: bpf, linux-kernel, linux-trace-kernel, linux-arm-kernel,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Menglong Dong, Song Liu
Following changes need to lookup trampoline based on its ip address,
adding hash table for that.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
include/linux/bpf.h | 7 +++++--
kernel/bpf/trampoline.c | 30 +++++++++++++++++++-----------
2 files changed, 24 insertions(+), 13 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 09d5dc541d1c..ad86857a8562 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1295,14 +1295,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;
diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index f2cb0b097093..9ca75b22aae0 100644
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -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++)
@@ -799,7 +804,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;
@@ -819,7 +824,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;
@@ -855,7 +860,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);
@@ -1128,7 +1134,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);
--
2.51.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCHv2 bpf-next 7/8] ftrace: Factor ftrace_ops ops_func interface
2025-11-13 12:37 [PATCHv2 bpf-next 0/9] ftrace,bpf: Use single direct ops for bpf trampolines Jiri Olsa
` (5 preceding siblings ...)
2025-11-13 12:37 ` [PATCHv2 bpf-next 6/8] bpf: Add trampoline ip hash table Jiri Olsa
@ 2025-11-13 12:37 ` Jiri Olsa
2025-11-13 12:37 ` [PATCHv2 bpf-next 8/8] bpf, x86: Use single ftrace_ops for direct calls Jiri Olsa
7 siblings, 0 replies; 16+ messages in thread
From: Jiri Olsa @ 2025-11-13 12:37 UTC (permalink / raw)
To: Steven Rostedt, Florent Revest, Mark Rutland
Cc: bpf, linux-kernel, linux-trace-kernel, linux-arm-kernel,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Menglong Dong, Song Liu
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 <jolsa@kernel.org>
---
include/linux/ftrace.h | 2 +-
kernel/bpf/trampoline.c | 3 ++-
kernel/trace/ftrace.c | 6 +++---
3 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index bacb6d9ab426..d4246d02ea0d 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -394,7 +394,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
diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index 9ca75b22aae0..26887a0db955 100644
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -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;
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index e701516a4a65..03948ec81434 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -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) {
@@ -8965,7 +8965,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;
}
@@ -9012,7 +9012,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);
--
2.51.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCHv2 bpf-next 8/8] bpf, x86: Use single ftrace_ops for direct calls
2025-11-13 12:37 [PATCHv2 bpf-next 0/9] ftrace,bpf: Use single direct ops for bpf trampolines Jiri Olsa
` (6 preceding siblings ...)
2025-11-13 12:37 ` [PATCHv2 bpf-next 7/8] ftrace: Factor ftrace_ops ops_func interface Jiri Olsa
@ 2025-11-13 12:37 ` Jiri Olsa
7 siblings, 0 replies; 16+ messages in thread
From: Jiri Olsa @ 2025-11-13 12:37 UTC (permalink / raw)
To: Steven Rostedt, Florent Revest, Mark Rutland
Cc: bpf, linux-kernel, linux-trace-kernel, linux-arm-kernel,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Menglong Dong, Song Liu
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 <jolsa@kernel.org>
---
arch/x86/Kconfig | 1 +
kernel/bpf/trampoline.c | 166 ++++++++++++++++++++++++++++++++++++----
kernel/trace/Kconfig | 3 +
kernel/trace/ftrace.c | 7 +-
4 files changed, 160 insertions(+), 17 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index fa3b616af03a..65a2fc279b46 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -332,6 +332,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
diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index 26887a0db955..436d393591a5 100644
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -33,12 +33,40 @@ 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);
+#ifdef CONFIG_HAVE_SINGLE_FTRACE_DIRECT_OPS
+static struct bpf_trampoline *direct_ops_ip_lookup(struct ftrace_ops *ops, unsigned long ip)
+{
+ struct hlist_head *head_ip;
+ struct bpf_trampoline *tr;
+
+ mutex_lock(&trampoline_mutex);
+ head_ip = &trampoline_ip_table[hash_64(ip, TRAMPOLINE_HASH_BITS)];
+ hlist_for_each_entry(tr, head_ip, hlist_ip) {
+ if (tr->ip == ip)
+ goto out;
+ }
+ tr = NULL;
+out:
+ mutex_unlock(&trampoline_mutex);
+ return tr;
+}
+#else
+static struct bpf_trampoline *direct_ops_ip_lookup(struct ftrace_ops *ops, unsigned long ip)
+{
+ return ops->private;
+}
+#endif /* CONFIG_HAVE_SINGLE_FTRACE_DIRECT_OPS */
+
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;
+ struct bpf_trampoline *tr;
int ret = 0;
+ tr = direct_ops_ip_lookup(ops, ip);
+ if (!tr)
+ return -EINVAL;
+
if (cmd == FTRACE_OPS_CMD_ENABLE_SHARE_IPMODIFY_SELF) {
/* This is called inside register_ftrace_direct_multi(), so
* tr->mutex is already locked.
@@ -137,6 +165,122 @@ void bpf_image_ksym_del(struct bpf_ksym *ksym)
PAGE_SIZE, true, ksym->name);
}
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
+#ifdef CONFIG_HAVE_SINGLE_FTRACE_DIRECT_OPS
+/*
+ * We have only single direct_ops which contains all the direct call
+ * sites and is the only global ftrace_ops for all trampolines.
+ *
+ * We use 'update_ftrace_direct_*' api for attachment.
+ */
+struct ftrace_ops direct_ops = {
+ .ops_func = bpf_tramp_ftrace_ops_func,
+};
+
+static int direct_ops_alloc(struct bpf_trampoline *tr)
+{
+ tr->fops = &direct_ops;
+ return 0;
+}
+
+static void direct_ops_free(struct bpf_trampoline *tr) { }
+
+static struct ftrace_hash *hash_from(unsigned long ip, void *addr)
+{
+ struct ftrace_hash *hash;
+
+ ip = ftrace_location(ip);
+ if (!ip)
+ return NULL;
+ hash = alloc_ftrace_hash(FTRACE_HASH_DEFAULT_BITS);
+ if (!hash)
+ return NULL;
+ if (!add_hash_entry_direct(hash, ip, (unsigned long) addr)) {
+ free_ftrace_hash(hash);
+ return NULL;
+ }
+ return hash;
+}
+
+static int direct_ops_add(struct ftrace_ops *ops, unsigned long ip, void *addr)
+{
+ struct ftrace_hash *hash = hash_from(ip, addr);
+ int err = -ENOMEM;
+
+ if (hash)
+ err = update_ftrace_direct_add(ops, hash);
+ free_ftrace_hash(hash);
+ return err;
+}
+
+static int direct_ops_del(struct ftrace_ops *ops, unsigned long ip, void *addr)
+{
+ struct ftrace_hash *hash = hash_from(ip, addr);
+ int err = -ENOMEM;
+
+ if (hash)
+ err = update_ftrace_direct_del(ops, hash);
+ free_ftrace_hash(hash);
+ return err;
+}
+
+static int direct_ops_mod(struct ftrace_ops *ops, unsigned long ip, void *addr, bool lock_direct_mutex)
+{
+ struct ftrace_hash *hash = hash_from(ip, addr);
+ int err = -ENOMEM;
+
+ if (hash)
+ err = update_ftrace_direct_mod(ops, hash, lock_direct_mutex);
+ free_ftrace_hash(hash);
+ return err;
+}
+#else
+/*
+ * We allocate ftrace_ops object for each trampoline and it contains
+ * call site specific for that trampoline.
+ *
+ * We use *_ftrace_direct api for attachment.
+ */
+static int direct_ops_alloc(struct bpf_trampoline *tr)
+{
+ tr->fops = kzalloc(sizeof(struct ftrace_ops), GFP_KERNEL);
+ if (!tr->fops)
+ return -ENOMEM;
+ tr->fops->private = tr;
+ tr->fops->ops_func = bpf_tramp_ftrace_ops_func;
+ return 0;
+}
+
+static void direct_ops_free(struct bpf_trampoline *tr)
+{
+ if (tr->fops) {
+ ftrace_free_filter(tr->fops);
+ kfree(tr->fops);
+ }
+}
+
+static int direct_ops_add(struct ftrace_ops *ops, unsigned long ip, void *addr)
+{
+ ftrace_set_filter_ip(ops, (unsigned long)ip, 0, 1);
+ return register_ftrace_direct(ops, (long)addr);
+}
+
+static int direct_ops_del(struct ftrace_ops *ops, unsigned long ip, void *addr)
+{
+ return unregister_ftrace_direct(ops, (long)addr, false);
+}
+
+static int direct_ops_mod(struct ftrace_ops *ops, unsigned long ip, void *addr, bool lock_direct_mutex)
+{
+ if (lock_direct_mutex)
+ return modify_ftrace_direct(ops, (long)addr);
+ return modify_ftrace_direct_nolock(ops, (long)addr);
+}
+#endif /* CONFIG_HAVE_SINGLE_FTRACE_DIRECT_OPS */
+#else
+static void direct_ops_free(struct bpf_trampoline *tr) { }
+#endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
+
static struct bpf_trampoline *bpf_trampoline_lookup(u64 key, unsigned long ip)
{
struct bpf_trampoline *tr;
@@ -155,14 +299,11 @@ static struct bpf_trampoline *bpf_trampoline_lookup(u64 key, unsigned long ip)
if (!tr)
goto out;
#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
- tr->fops = kzalloc(sizeof(struct ftrace_ops), GFP_KERNEL);
- if (!tr->fops) {
+ if (direct_ops_alloc(tr)) {
kfree(tr);
tr = NULL;
goto out;
}
- tr->fops->private = tr;
- tr->fops->ops_func = bpf_tramp_ftrace_ops_func;
#endif
tr->key = key;
@@ -187,7 +328,7 @@ static int unregister_fentry(struct bpf_trampoline *tr, void *old_addr)
int ret;
if (tr->func.ftrace_managed)
- ret = unregister_ftrace_direct(tr->fops, (long)old_addr, false);
+ ret = direct_ops_del(tr->fops, tr->ip, old_addr);
else
ret = bpf_arch_text_poke(ip, BPF_MOD_CALL, old_addr, NULL);
@@ -201,10 +342,7 @@ static int modify_fentry(struct bpf_trampoline *tr, void *old_addr, void *new_ad
int ret;
if (tr->func.ftrace_managed) {
- if (lock_direct_mutex)
- ret = modify_ftrace_direct(tr->fops, (long)new_addr);
- else
- ret = modify_ftrace_direct_nolock(tr->fops, (long)new_addr);
+ ret = direct_ops_mod(tr->fops, tr->ip, new_addr, lock_direct_mutex);
} else {
ret = bpf_arch_text_poke(ip, BPF_MOD_CALL, old_addr, new_addr);
}
@@ -226,8 +364,7 @@ static int register_fentry(struct bpf_trampoline *tr, void *new_addr)
}
if (tr->func.ftrace_managed) {
- ftrace_set_filter_ip(tr->fops, (unsigned long)ip, 0, 1);
- ret = register_ftrace_direct(tr->fops, (long)new_addr);
+ ret = direct_ops_add(tr->fops, tr->ip, new_addr);
} else {
ret = bpf_arch_text_poke(ip, BPF_MOD_CALL, NULL, new_addr);
}
@@ -863,10 +1000,7 @@ void bpf_trampoline_put(struct bpf_trampoline *tr)
*/
hlist_del(&tr->hlist_key);
hlist_del(&tr->hlist_ip);
- if (tr->fops) {
- ftrace_free_filter(tr->fops);
- kfree(tr->fops);
- }
+ direct_ops_free(tr);
kfree(tr);
out:
mutex_unlock(&trampoline_mutex);
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index d2c79da81e4f..4bf5beb04a5b 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -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
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 03948ec81434..e223fc196567 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -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;
--
2.51.1
^ permalink raw reply related [flat|nested] 16+ messages in thread