* [PATCHv4 bpf-next 9/9] bpf,x86: Use single ftrace_ops for direct calls
From: Jiri Olsa @ 2025-12-03 8:24 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
In-Reply-To: <20251203082402.78816-1-jolsa@kernel.org>
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 | 195 ++++++++++++++++++++++++++++++++++------
kernel/trace/Kconfig | 3 +
kernel/trace/ftrace.c | 7 +-
4 files changed, 177 insertions(+), 29 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 462250a20311..737dff48342b 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -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
diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index 17af2aad8382..02371db3db3e 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,139 @@ 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_ip(struct bpf_trampoline *tr, void *ptr)
+{
+ unsigned long ip, addr = (unsigned long) ptr;
+ struct ftrace_hash *hash;
+
+ ip = ftrace_location(tr->ip);
+ if (!ip)
+ return NULL;
+ hash = alloc_ftrace_hash(FTRACE_HASH_DEFAULT_BITS);
+ if (!hash)
+ return NULL;
+ if (bpf_trampoline_use_jmp(tr->flags))
+ addr = ftrace_jmp_set(addr);
+ if (!add_hash_entry_direct(hash, ip, addr)) {
+ free_ftrace_hash(hash);
+ return NULL;
+ }
+ return hash;
+}
+
+static int direct_ops_add(struct bpf_trampoline *tr, void *addr)
+{
+ struct ftrace_hash *hash = hash_from_ip(tr, addr);
+ int err = -ENOMEM;
+
+ if (hash)
+ err = update_ftrace_direct_add(tr->fops, hash);
+ free_ftrace_hash(hash);
+ return err;
+}
+
+static int direct_ops_del(struct bpf_trampoline *tr, void *addr)
+{
+ struct ftrace_hash *hash = hash_from_ip(tr, addr);
+ int err = -ENOMEM;
+
+ if (hash)
+ err = update_ftrace_direct_del(tr->fops, hash);
+ free_ftrace_hash(hash);
+ return err;
+}
+
+static int direct_ops_mod(struct bpf_trampoline *tr, void *addr, bool lock_direct_mutex)
+{
+ struct ftrace_hash *hash = hash_from_ip(tr, addr);
+ int err = -ENOMEM;
+
+ if (hash)
+ err = update_ftrace_direct_mod(tr->fops, 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 bpf_trampoline *tr, void *ptr)
+{
+ unsigned long addr = (unsigned long) ptr;
+ struct ftrace_ops *ops = tr->fops;
+ int ret;
+
+ if (bpf_trampoline_use_jmp(tr->flags))
+ addr = ftrace_jmp_set(addr);
+
+ ret = ftrace_set_filter_ip(ops, tr->ip, 0, 1);
+ if (ret)
+ return ret;
+ return register_ftrace_direct(ops, addr);
+}
+
+static int direct_ops_del(struct bpf_trampoline *tr, void *addr)
+{
+ return unregister_ftrace_direct(tr->fops, (long)addr, false);
+}
+
+static int direct_ops_mod(struct bpf_trampoline *tr, void *ptr, bool lock_direct_mutex)
+{
+ unsigned long addr = (unsigned long) ptr;
+ struct ftrace_ops *ops = tr->fops;
+
+ if (bpf_trampoline_use_jmp(tr->flags))
+ addr = ftrace_jmp_set(addr);
+ if (lock_direct_mutex)
+ return modify_ftrace_direct(ops, addr);
+ return modify_ftrace_direct_nolock(ops, 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 +316,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;
@@ -206,7 +364,7 @@ static int unregister_fentry(struct bpf_trampoline *tr, u32 orig_flags,
int ret;
if (tr->func.ftrace_managed)
- ret = unregister_ftrace_direct(tr->fops, (long)old_addr, false);
+ ret = direct_ops_del(tr, old_addr);
else
ret = bpf_trampoline_update_fentry(tr, orig_flags, old_addr, NULL);
@@ -220,15 +378,7 @@ 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, addr);
- else
- ret = modify_ftrace_direct_nolock(tr->fops, addr);
+ ret = direct_ops_mod(tr, new_addr, lock_direct_mutex);
} else {
ret = bpf_trampoline_update_fentry(tr, orig_flags, old_addr,
new_addr);
@@ -251,15 +401,7 @@ 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, addr);
+ ret = direct_ops_add(tr, new_addr);
} else {
ret = bpf_trampoline_update_fentry(tr, 0, NULL, new_addr);
}
@@ -910,10 +1052,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 4661b9e606e0..1ad2e307c834 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 9582cb374d4f..0bdee1758fa1 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.52.0
^ permalink raw reply related
* [PATCHv4 bpf-next 8/9] ftrace: Factor ftrace_ops ops_func interface
From: Jiri Olsa @ 2025-12-03 8:24 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
In-Reply-To: <20251203082402.78816-1-jolsa@kernel.org>
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 c27b7381c5f1..515cf5b723dd 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -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
diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index f298bafab76e..17af2aad8382 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 c77f620b3eb3..9582cb374d4f 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) {
@@ -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);
--
2.52.0
^ permalink raw reply related
* [PATCHv4 bpf-next 7/9] bpf: Add trampoline ip hash table
From: Jiri Olsa @ 2025-12-03 8:24 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
In-Reply-To: <20251203082402.78816-1-jolsa@kernel.org>
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 6498be4c44f8..ef44653eae44 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -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;
diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index b9a358d7a78f..f298bafab76e 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++)
@@ -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);
--
2.52.0
^ permalink raw reply related
* [PATCHv4 bpf-next 6/9] ftrace: Add update_ftrace_direct_mod function
From: Jiri Olsa @ 2025-12-03 8:23 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
In-Reply-To: <20251203082402.78816-1-jolsa@kernel.org>
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 | 72 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 78 insertions(+)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index bac9dd784826..c27b7381c5f1 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -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
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 01e830be20e3..c77f620b3eb3 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -6490,6 +6490,78 @@ 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_func_entry *entry, *tmp;
+ static struct ftrace_ops tmp_ops = {
+ .func = ftrace_stub,
+ .flags = FTRACE_OPS_FL_STUB,
+ };
+ struct ftrace_hash *orig_hash;
+ unsigned long size, i;
+ int err = -EINVAL;
+
+ if (!hash_count(hash))
+ return -EINVAL;
+ 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);
+
+ orig_hash = ops->func_hash ? ops->func_hash->filter_hash : NULL;
+ if (!orig_hash)
+ goto unlock;
+
+ /* 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.52.0
^ permalink raw reply related
* [PATCHv4 bpf-next 5/9] ftrace: Add update_ftrace_direct_del function
From: Jiri Olsa @ 2025-12-03 8:23 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
In-Reply-To: <20251203082402.78816-1-jolsa@kernel.org>
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 | 113 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 119 insertions(+)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index eb60d971ec1d..bac9dd784826 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -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
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 1660a87547dd..01e830be20e3 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -6377,6 +6377,119 @@ 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 *old_direct_functions = NULL, *new_direct_functions = NULL;
+ struct ftrace_hash *old_filter_hash = NULL, *new_filter_hash = NULL;
+ struct ftrace_func_entry *del, *entry;
+ unsigned long size, i;
+ int err = -EINVAL;
+
+ if (!hash_count(hash))
+ return -EINVAL;
+ 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);
+
+ old_filter_hash = ops->func_hash ? ops->func_hash->filter_hash : NULL;
+ old_direct_functions = direct_functions;
+
+ if (!hash_count(old_filter_hash))
+ goto out_unlock;
+
+ /* 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;
+ new_filter_hash = hash_sub(old_filter_hash, hash);
+ if (!new_filter_hash)
+ goto out_unlock;
+
+ new_direct_functions = hash_sub(old_direct_functions, hash);
+ if (!new_direct_functions)
+ goto out_unlock;
+
+ /* If there's nothing left, we need to unregister the ops. */
+ if (ftrace_hash_empty(new_filter_hash)) {
+ err = unregister_ftrace_function(ops);
+ if (!err) {
+ /* 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, 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
+ */
+ }
+
+ if (err) {
+ /* reset direct_functions and free the new one */
+ old_direct_functions = new_direct_functions;
+ } else {
+ rcu_assign_pointer(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);
+ free_ftrace_hash(new_filter_hash);
+
+ return err;
+}
+
#endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
/**
--
2.52.0
^ permalink raw reply related
* [PATCHv4 bpf-next 4/9] ftrace: Add update_ftrace_direct_add function
From: Jiri Olsa @ 2025-12-03 8:23 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
In-Reply-To: <20251203082402.78816-1-jolsa@kernel.org>
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 8c034b028af4..eb60d971ec1d 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -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
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 84aee9096a9e..1660a87547dd 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -6249,6 +6249,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 -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;
+
+ 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.52.0
^ permalink raw reply related
* [PATCHv4 bpf-next 3/9] ftrace: Export some of hash related functions
From: Jiri Olsa @ 2025-12-03 8:23 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
In-Reply-To: <20251203082402.78816-1-jolsa@kernel.org>
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 505b7d3f5641..8c034b028af4 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -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;
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 7e3a81bd6f1e..84aee9096a9e 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.52.0
^ permalink raw reply related
* [PATCHv4 bpf-next 2/9] ftrace: Make alloc_and_copy_ftrace_hash direct friendly
From: Jiri Olsa @ 2025-12-03 8:23 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
In-Reply-To: <20251203082402.78816-1-jolsa@kernel.org>
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 b0dc911411f1..7e3a81bd6f1e 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.52.0
^ permalink raw reply related
* [PATCHv4 bpf-next 1/9] ftrace,bpf: Remove FTRACE_OPS_FL_JMP ftrace_ops flag
From: Jiri Olsa @ 2025-12-03 8:23 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
In-Reply-To: <20251203082402.78816-1-jolsa@kernel.org>
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 <jolsa@kernel.org>
---
include/linux/ftrace.h | 1 -
kernel/bpf/trampoline.c | 32 ++++++++++++++------------------
kernel/trace/ftrace.c | 14 --------------
3 files changed, 14 insertions(+), 33 deletions(-)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 015dd1049bea..505b7d3f5641 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -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
diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index 976d89011b15..b9a358d7a78f 100644
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -214,10 +214,15 @@ static int modify_fentry(struct bpf_trampoline *tr, u32 orig_flags,
int ret;
if (tr->func.ftrace_managed) {
+ unsigned long addr = (unsigned long) new_addr;
+
+ if (bpf_trampoline_use_jmp(tr->flags))
+ addr = ftrace_jmp_set(addr);
+
if (lock_direct_mutex)
- ret = modify_ftrace_direct(tr->fops, (long)new_addr);
+ ret = modify_ftrace_direct(tr->fops, addr);
else
- ret = modify_ftrace_direct_nolock(tr->fops, (long)new_addr);
+ ret = modify_ftrace_direct_nolock(tr->fops, addr);
} else {
ret = bpf_trampoline_update_fentry(tr, orig_flags, old_addr,
new_addr);
@@ -240,10 +245,15 @@ static int register_fentry(struct bpf_trampoline *tr, void *new_addr)
}
if (tr->func.ftrace_managed) {
+ unsigned long addr = (unsigned long) new_addr;
+
+ if (bpf_trampoline_use_jmp(tr->flags))
+ addr = ftrace_jmp_set(addr);
+
ret = ftrace_set_filter_ip(tr->fops, (unsigned long)ip, 0, 1);
if (ret)
return ret;
- ret = register_ftrace_direct(tr->fops, (long)new_addr);
+ ret = register_ftrace_direct(tr->fops, addr);
} else {
ret = bpf_trampoline_update_fentry(tr, 0, NULL, new_addr);
}
@@ -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;
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index bbb37c0f8c6c..b0dc911411f1 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -6017,15 +6017,8 @@ int register_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
if (ftrace_hash_empty(hash))
return -EINVAL;
- /* This is a "raw" address, and this should never happen. */
- if (WARN_ON_ONCE(ftrace_is_jmp(addr)))
- return -EINVAL;
-
mutex_lock(&direct_mutex);
- if (ops->flags & FTRACE_OPS_FL_JMP)
- addr = ftrace_jmp_set(addr);
-
/* Make sure requested entries are not already registered.. */
size = 1 << hash->size_bits;
for (i = 0; i < size; i++) {
@@ -6146,13 +6139,6 @@ __modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
lockdep_assert_held_once(&direct_mutex);
- /* This is a "raw" address, and this should never happen. */
- if (WARN_ON_ONCE(ftrace_is_jmp(addr)))
- return -EINVAL;
-
- if (ops->flags & FTRACE_OPS_FL_JMP)
- addr = ftrace_jmp_set(addr);
-
/* Enable the tmp_ops to have the same functions as the direct ops */
ftrace_ops_init(&tmp_ops);
tmp_ops.func_hash = ops->func_hash;
--
2.52.0
^ permalink raw reply related
* [PATCHv4 bpf-next 0/9] ftrace,bpf: Use single direct ops for bpf trampolines
From: Jiri Olsa @ 2025-12-03 8:23 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
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/20250729102813.1531457-1-jolsa@kernel.org/
v1: https://lore.kernel.org/bpf/20250923215147.1571952-1-jolsa@kernel.org/
v2: https://lore.kernel.org/bpf/20251113123750.2507435-1-jolsa@kernel.org/
v3: https://lore.kernel.org/bpf/20251120212402.466524-1-jolsa@kernel.org/
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/20251027175023.1521602-1-song@kernel.org/
[2] https://lore.kernel.org/bpf/20250924050415.4aefcb91@batman.local.home/
---
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(-)
^ permalink raw reply
* Re: [PATCH 1/3] kernel.h: drop STACK_MAGIC macro
From: Jani Nikula @ 2025-12-03 7:40 UTC (permalink / raw)
To: Yury Norov, Andi Shyti
Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Andy Shevchenko, Randy Dunlap, Ingo Molnar, Joonas Lahtinen,
Rodrigo Vivi, Tvrtko Ursulin, Petr Pavlu, Daniel Gomez,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Andrew Morton, linux-kernel, intel-gfx, dri-devel, linux-modules,
linux-trace-kernel
In-Reply-To: <aS9XpaiGXn2upVKB@yury>
On Tue, 02 Dec 2025, Yury Norov <yury.norov@gmail.com> wrote:
> On Tue, Dec 02, 2025 at 09:58:19PM +0100, Andi Shyti wrote:
>> Hi Jani,
>>
>> On Mon, Dec 01, 2025 at 09:46:47AM +0200, Jani Nikula wrote:
>> > On Sat, 29 Nov 2025, "Yury Norov (NVIDIA)" <yury.norov@gmail.com> wrote:
>> > > The macro is only used by i915. Move it to a local header and drop from
>> > > the kernel.h.
>> > >
>> > > Signed-off-by: Yury Norov (NVIDIA) <yury.norov@gmail.com>
>> > > ---
>> > > drivers/gpu/drm/i915/i915_utils.h | 2 ++
>> > > include/linux/kernel.h | 2 --
>> > > 2 files changed, 2 insertions(+), 2 deletions(-)
>> > >
>> > > diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h
>> > > index a0c892e4c40d..6c197e968305 100644
>> > > --- a/drivers/gpu/drm/i915/i915_utils.h
>> > > +++ b/drivers/gpu/drm/i915/i915_utils.h
>> >
>> > i915_utils.h is on a diet itself. STACK_MAGIC is only used in selftests,
>> > please put this in i915_selftest.h.
>> >
>> > I guess also need to include that from gt/selftest_ring_submission.c,
>> > the only one that uses STACK_MAGIC but doesn't include i915_selftest.h.
>>
>> Doing this cleanups is a bit out of the scope of this patch.
>> Given that the patch itself has quite a good consensus, let move
>> it forward and I can take care of the i915 cleanup once it gets
>> merged.
>
> I'm already testing it in my tree:
>
> https://github.com/norov/linux/tree/sm1
>
> If everything is fine, I'll submit v2 with this change, otherwise will
> schedule it for future improvements.
I had a look,
Acked-by: Jani Nikula <jani.nikula@intel.com>
--
Jani Nikula, Intel
^ permalink raw reply
* Re: [PATCH v6 3/6] perf record: Add --call-graph fp,defer option for deferred callchains
From: Namhyung Kim @ 2025-12-03 5:49 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Ian Rogers, James Clark
Cc: Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
linux-perf-users, Steven Rostedt, Josh Poimboeuf, Indu Bhagat,
Jens Remus, Mathieu Desnoyers, linux-trace-kernel, bpf
In-Reply-To: <20251120234804.156340-4-namhyung@kernel.org>
On Thu, Nov 20, 2025 at 03:48:01PM -0800, Namhyung Kim wrote:
> Add a new callchain record mode option for deferred callchains. For now
> it only works with FP (frame-pointer) mode.
>
> And add the missing feature detection logic to clear the flag on old
> kernels.
>
> $ perf record --call-graph fp,defer -vv true
> ...
> ------------------------------------------------------------
> perf_event_attr:
> type 0 (PERF_TYPE_HARDWARE)
> size 136
> config 0 (PERF_COUNT_HW_CPU_CYCLES)
> { sample_period, sample_freq } 4000
> sample_type IP|TID|TIME|CALLCHAIN|PERIOD
> read_format ID|LOST
> disabled 1
> inherit 1
> mmap 1
> comm 1
> freq 1
> enable_on_exec 1
> task 1
> sample_id_all 1
> mmap2 1
> comm_exec 1
> ksymbol 1
> bpf_event 1
> defer_callchain 1
> defer_output 1
> ------------------------------------------------------------
> sys_perf_event_open: pid 162755 cpu 0 group_fd -1 flags 0x8
> sys_perf_event_open failed, error -22
> switching off deferred callchain support
>
> Reviewed-by: Ian Rogers <irogers@google.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/Documentation/perf-config.txt | 3 +++
> tools/perf/Documentation/perf-record.txt | 4 ++++
> tools/perf/util/callchain.c | 16 +++++++++++++---
> tools/perf/util/callchain.h | 1 +
> tools/perf/util/evsel.c | 19 +++++++++++++++++++
> tools/perf/util/evsel.h | 1 +
> 6 files changed, 41 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/Documentation/perf-config.txt b/tools/perf/Documentation/perf-config.txt
> index c6f33565966735fe..642d1c490d9e3bcd 100644
> --- a/tools/perf/Documentation/perf-config.txt
> +++ b/tools/perf/Documentation/perf-config.txt
> @@ -452,6 +452,9 @@ Variables
> kernel space is controlled not by this option but by the
> kernel config (CONFIG_UNWINDER_*).
>
> + The 'defer' mode can be used with 'fp' mode to enable deferred
> + user callchains (like 'fp,defer').
> +
> call-graph.dump-size::
> The size of stack to dump in order to do post-unwinding. Default is 8192 (byte).
> When using dwarf into record-mode, the default size will be used if omitted.
> diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
> index 067891bd7da6edc8..e8b9aadbbfa50574 100644
> --- a/tools/perf/Documentation/perf-record.txt
> +++ b/tools/perf/Documentation/perf-record.txt
> @@ -325,6 +325,10 @@ OPTIONS
> by default. User can change the number by passing it after comma
> like "--call-graph fp,32".
>
> + Also "defer" can be used with "fp" (like "--call-graph fp,defer") to
> + enable deferred user callchain which will collect user-space callchains
> + when the thread returns to the user space.
> +
> -q::
> --quiet::
> Don't print any warnings or messages, useful for scripting.
> diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
> index d7b7eef740b9d6ed..2884187ccbbecfdc 100644
> --- a/tools/perf/util/callchain.c
> +++ b/tools/perf/util/callchain.c
> @@ -275,9 +275,13 @@ int parse_callchain_record(const char *arg, struct callchain_param *param)
> if (tok) {
> unsigned long size;
>
> - size = strtoul(tok, &name, 0);
> - if (size < (unsigned) sysctl__max_stack())
> - param->max_stack = size;
> + if (!strncmp(tok, "defer", sizeof("defer"))) {
> + param->defer = true;
> + } else {
> + size = strtoul(tok, &name, 0);
> + if (size < (unsigned) sysctl__max_stack())
> + param->max_stack = size;
> + }
> }
> break;
>
> @@ -314,6 +318,12 @@ int parse_callchain_record(const char *arg, struct callchain_param *param)
> } while (0);
>
> free(buf);
> +
> + if (param->defer && param->record_mode != CALLCHAIN_FP) {
> + pr_err("callchain: deferred callchain only works with FP\n");
> + return -EINVAL;
> + }
> +
> return ret;
> }
>
> diff --git a/tools/perf/util/callchain.h b/tools/perf/util/callchain.h
> index 86ed9e4d04f9ee7b..d5ae4fbb7ce5fa44 100644
> --- a/tools/perf/util/callchain.h
> +++ b/tools/perf/util/callchain.h
> @@ -98,6 +98,7 @@ extern bool dwarf_callchain_users;
>
> struct callchain_param {
> bool enabled;
> + bool defer;
> enum perf_call_graph_mode record_mode;
> u32 dump_size;
> enum chain_mode mode;
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index f1a311637694ac0a..887c6ac6c49cc415 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -1065,6 +1065,9 @@ static void __evsel__config_callchain(struct evsel *evsel, struct record_opts *o
> pr_info("Disabling user space callchains for function trace event.\n");
> attr->exclude_callchain_user = 1;
> }
> +
> + if (param->defer && !attr->exclude_callchain_user)
> + attr->defer_callchain = 1;
> }
>
> void evsel__config_callchain(struct evsel *evsel, struct record_opts *opts,
> @@ -1511,6 +1514,7 @@ void evsel__config(struct evsel *evsel, struct record_opts *opts,
> attr->mmap2 = track && !perf_missing_features.mmap2;
> attr->comm = track;
> attr->build_id = track && opts->build_id;
> + attr->defer_output = track && callchain->defer;
I need to update this part like below:
attr->defer_output = track && callchain && callchain->defer;
Thanks,
Namhyung
>
> /*
> * ksymbol is tracked separately with text poke because it needs to be
> @@ -2199,6 +2203,10 @@ static int __evsel__prepare_open(struct evsel *evsel, struct perf_cpu_map *cpus,
>
> static void evsel__disable_missing_features(struct evsel *evsel)
> {
> + if (perf_missing_features.defer_callchain && evsel->core.attr.defer_callchain)
> + evsel->core.attr.defer_callchain = 0;
> + if (perf_missing_features.defer_callchain && evsel->core.attr.defer_output)
> + evsel->core.attr.defer_output = 0;
> if (perf_missing_features.inherit_sample_read && evsel->core.attr.inherit &&
> (evsel->core.attr.sample_type & PERF_SAMPLE_READ))
> evsel->core.attr.inherit = 0;
> @@ -2473,6 +2481,13 @@ static bool evsel__detect_missing_features(struct evsel *evsel, struct perf_cpu
>
> /* Please add new feature detection here. */
>
> + attr.defer_callchain = true;
> + if (has_attr_feature(&attr, /*flags=*/0))
> + goto found;
> + perf_missing_features.defer_callchain = true;
> + pr_debug2("switching off deferred callchain support\n");
> + attr.defer_callchain = false;
> +
> attr.inherit = true;
> attr.sample_type = PERF_SAMPLE_READ | PERF_SAMPLE_TID;
> if (has_attr_feature(&attr, /*flags=*/0))
> @@ -2584,6 +2599,10 @@ static bool evsel__detect_missing_features(struct evsel *evsel, struct perf_cpu
> errno = old_errno;
>
> check:
> + if ((evsel->core.attr.defer_callchain || evsel->core.attr.defer_output) &&
> + perf_missing_features.defer_callchain)
> + return true;
> +
> if (evsel->core.attr.inherit &&
> (evsel->core.attr.sample_type & PERF_SAMPLE_READ) &&
> perf_missing_features.inherit_sample_read)
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> index 3ae4ac8f9a37e009..a08130ff2e47a887 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -221,6 +221,7 @@ struct perf_missing_features {
> bool branch_counters;
> bool aux_action;
> bool inherit_sample_read;
> + bool defer_callchain;
> };
>
> extern struct perf_missing_features perf_missing_features;
> --
> 2.52.0.rc2.455.g230fcf2819-goog
>
^ permalink raw reply
* [PATCH] kprobes: avoid crash when rmmod/insmod after ftrace killed
From: Masami Hiramatsu (Google) @ 2025-12-03 5:24 UTC (permalink / raw)
To: LKML, Linux Trace Kernel, Ye Bin
Cc: Steven Rostedt, Naveen N Rao, David S . Miller, Masami Hiramatsu,
yebin10
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
After we hit ftrace is killed by some errors, the kernel crash if
we remove modules in which kprobe probes.
BUG: unable to handle page fault for address: fffffbfff805000d
PGD 817fcc067 P4D 817fcc067 PUD 817fc8067 PMD 101555067 PTE 0
Oops: Oops: 0000 [#1] SMP KASAN PTI
CPU: 4 UID: 0 PID: 2012 Comm: rmmod Tainted: G W OE
Tainted: [W]=WARN, [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
RIP: 0010:kprobes_module_callback+0x89/0x790
RSP: 0018:ffff88812e157d30 EFLAGS: 00010a02
RAX: 1ffffffff805000d RBX: dffffc0000000000 RCX: ffffffff86a8de90
RDX: ffffed1025c2af9b RSI: 0000000000000008 RDI: ffffffffc0280068
RBP: 0000000000000000 R08: 0000000000000001 R09: ffffed1025c2af9a
R10: ffff88812e157cd7 R11: 205d323130325420 R12: 0000000000000002
R13: ffffffffc0290488 R14: 0000000000000002 R15: ffffffffc0280040
FS: 00007fbc450dd740(0000) GS:ffff888420331000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: fffffbfff805000d CR3: 000000010f624000 CR4: 00000000000006f0
Call Trace:
<TASK>
notifier_call_chain+0xc6/0x280
blocking_notifier_call_chain+0x60/0x90
__do_sys_delete_module.constprop.0+0x32a/0x4e0
do_syscall_64+0x5d/0xfa0
entry_SYSCALL_64_after_hwframe+0x76/0x7e
This is because the kprobe on ftrace does not correctly handles
the kprobe_ftrace_disabled flag set by ftrace_kill().
To prevent this error, check kprobe_ftrace_disabled in
__disarm_kprobe_ftrace() and skip all ftrace related operations.
Reported-by: Ye Bin <yebin10@huawei.com>
Closes: https://lore.kernel.org/all/20251125020536.2484381-1-yebin@huaweicloud.com/
Fixes: ae6aa16fdc16 ("kprobes: introduce ftrace based optimization")
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
kernel/kprobes.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index ab8f9fc1f0d1..17d451553389 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1104,6 +1104,10 @@ static int __disarm_kprobe_ftrace(struct kprobe *p, struct ftrace_ops *ops,
int ret;
lockdep_assert_held(&kprobe_mutex);
+ if (unlikely(kprobe_ftrace_disabled)) {
+ /* Now ftrace is disabled forever, disarm is already done. */
+ return 0;
+ }
if (*cnt == 1) {
ret = unregister_ftrace_function(ops);
^ permalink raw reply related
* Re: [PATCH] kprobes: avoid crash when rmmod/insmod modules after ftrace_disabled
From: Masami Hiramatsu @ 2025-12-03 4:51 UTC (permalink / raw)
To: yebin; +Cc: naveen, davem, linux-trace-kernel, yebin10, Steven Rostedt
In-Reply-To: <692FA251.3080100@huaweicloud.com>
On Wed, 3 Dec 2025 10:37:05 +0800
yebin <yebin@huaweicloud.com> wrote:
> >>> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> >>> index ab8f9fc1f0d1..17d451553389 100644
> >>> --- a/kernel/kprobes.c
> >>> +++ b/kernel/kprobes.c
> >>> @@ -1104,6 +1104,10 @@ static int __disarm_kprobe_ftrace(struct kprobe *p, struct ftrace_ops *ops,
> >>> int ret;
> >>>
> >>> lockdep_assert_held(&kprobe_mutex);
> >>> + if (unlikely(kprobe_ftrace_disabled)) {
> >>> + /* Now ftrace is disabled forever, disarm is already done. */
> >>> + return 0;
> >>> + }
> >>>
> >>> if (*cnt == 1) {
> >>> ret = unregister_ftrace_function(ops);
> >>>
> >
> > This one, it should fix simply.
> >
> I tested it and it can solve the problem. This is indeed a good solution
> that reuses the original code.
> Do you want me to send a new patch version according to your proposal?
OK, I'll send it with some tags which is required for this fix.
Please add your tested-by on that.
Thank you,
> > Thank you,
> >
> >>>
> >>
> >
> >
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH] kprobes: avoid crash when rmmod/insmod modules after ftrace_disabled
From: yebin @ 2025-12-03 3:21 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu (Google), naveen, davem, linux-trace-kernel,
yebin10
In-Reply-To: <20251202114358.5e46cbae@gandalf.local.home>
On 2025/12/3 0:43, Steven Rostedt wrote:
> On Fri, 28 Nov 2025 10:27:42 +0800
> yebin <yebin@huaweicloud.com> wrote:
>
>>>> right before this BUG, don't you?
>>>> If you reported with that line, it's more easier to understand.
>>>>
>> Yes, there is indeed a warning generated. I might not have expressed it
>> clearly enough. The issue below is related to the problem that occurs
>> when the second module is unloaded. When the first module was unloaded,
>> some nodes were left in the hash list, causing a use-after-free (UAF)
>> issue when traversing the hash list.
>> Therefore, this patch aims to resolve the UAF problem caused by residual
>> nodes in the hash list after unloading a module while ftrace is disabled.
>
> ftrace_disabled is equivalent to BUG(). But it doesn't crash the system
> immediately, but requires a reboot ASAP.
>
> I'm not interested in fixing residual bugs that happen because of a
> ftrace_disabled was triggered. The ftrace_disabled triggering is the cause
> of this. It should *never* happen. If it does, it needs to be fixed.
>
> Let's focus our attention on fixing the cause of ftrace_disabled. Anything
> else is just waste of effort.
>
> Do you have the output of the first warning when ftrace_disabled was
> triggered?
The reason for ftrace_disable is a conflict between a livepatch and a
module. This issue must be resolved. However, since this problem occurs
in the current network environment, it does not affect the normal
operation of services, so customers may not necessarily be willing to
restart their systems to fix it. The only thing that can be done is to
implement proper exception protection to ensure the system can continue
to function until the customer is willing to upgrade and restart.
WARNING: CPU: 0 PID: 2394 at kernel/trace/ftrace.c:2603
ftrace_modify_all_code+0x81/0x140
CPU: 0 PID: 2394 Comm: insmod Tainted: P OE K
RIP: 0010:ftrace_modify_all_code+0x81/0x140
Call Trace:
arch_ftrace_update_code+0xc/0x20
ftrace_run_update_code+0x13/0x70
ftrace_startup+0xc0/0x190
register_ftrace_function+0x44/0x60
arm_kprobe+0xc7/0x120
register_kprobe+0x5f0/0x6a0
kxxx_emerge_init+0x89/0xe0 [kxxx]
kxxx_region_init+0x16d/0x330 [kxxx]
kxxx_init+0xe3/0x1000 [kxxx]
do_one_initcall+0x46/0x1c8
do_init_module+0x5b/0x1fb
load_module+0x15a7/0x1e20
__do_sys_finit_module+0xe9/0x110
do_syscall_64+0x5b/0x1b0
entry_SYSCALL_64_after_hwframe+0x65/0xca
>
> -- Steve
>
^ permalink raw reply
* Re: [PATCH v13 mm-new 05/16] khugepaged: introduce is_mthp_order helper
From: Zi Yan @ 2025-12-03 3:13 UTC (permalink / raw)
To: Nico Pache
Cc: linux-kernel, linux-trace-kernel, linux-mm, linux-doc, david,
baolin.wang, lorenzo.stoakes, Liam.Howlett, ryan.roberts,
dev.jain, corbet, rostedt, mhiramat, mathieu.desnoyers, akpm,
baohua, willy, peterx, wangkefeng.wang, usamaarif642, sunnanyong,
vishal.moola, thomas.hellstrom, yang, kas, aarcange, raquini,
anshuman.khandual, catalin.marinas, tiwai, will, dave.hansen,
jack, cl, jglisse, surenb, zokeefe, hannes, rientjes, mhocko,
rdunlap, hughd, richard.weiyang, lance.yang, vbabka, rppt, jannh,
pfalcato
In-Reply-To: <20251201174627.23295-6-npache@redhat.com>
On 1 Dec 2025, at 12:46, Nico Pache wrote:
> In order to add mTHP support, we will often be checking if a given order
> is a mTHP or PMD order. Lets create a simple helper function to keep the
> code clean and readable.
>
> Suggested-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> Signed-off-by: Nico Pache <npache@redhat.com>
> ---
> mm/khugepaged.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 8599c7fa112e..9c041141b2e3 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -338,6 +338,11 @@ static bool pte_none_or_zero(pte_t pte)
> return pte_present(pte) && is_zero_pfn(pte_pfn(pte));
> }
>
> +static bool is_mthp_order(unsigned int order)
> +{
> + return order != HPAGE_PMD_ORDER;
> +}
> +
The code at the bottom of __folio_split() in mm/huge_memory.c
can use this helper.
The code in deferred_split_folio() uses folio_test_pmd_mappable()
instead of order == HPAGE_PMD_ORDER, but the code can be changed
to use the helper.
> int hugepage_madvise(struct vm_area_struct *vma,
> vm_flags_t *vm_flags, int advice)
> {
> @@ -1071,13 +1076,13 @@ static int alloc_charge_folio(struct folio **foliop, struct mm_struct *mm,
> folio = __folio_alloc(gfp, order, node, &cc->alloc_nmask);
> if (!folio) {
> *foliop = NULL;
> - if (order == HPAGE_PMD_ORDER)
> + if (!is_mthp_order(order))
> count_vm_event(THP_COLLAPSE_ALLOC_FAILED);
> count_mthp_stat(order, MTHP_STAT_COLLAPSE_ALLOC_FAILED);
> return SCAN_ALLOC_HUGE_PAGE_FAIL;
> }
>
> - if (order == HPAGE_PMD_ORDER)
> + if (!is_mthp_order(order))
> count_vm_event(THP_COLLAPSE_ALLOC);
> count_mthp_stat(order, MTHP_STAT_COLLAPSE_ALLOC);
>
> --
> 2.51.1
Best Regards,
Yan, Zi
^ permalink raw reply
* Re: [PATCH v13 mm-new 04/16] khugepaged: generalize alloc_charge_folio()
From: Zi Yan @ 2025-12-03 3:07 UTC (permalink / raw)
To: Nico Pache
Cc: linux-kernel, linux-trace-kernel, linux-mm, linux-doc, david,
baolin.wang, lorenzo.stoakes, Liam.Howlett, ryan.roberts,
dev.jain, corbet, rostedt, mhiramat, mathieu.desnoyers, akpm,
baohua, willy, peterx, wangkefeng.wang, usamaarif642, sunnanyong,
vishal.moola, thomas.hellstrom, yang, kas, aarcange, raquini,
anshuman.khandual, catalin.marinas, tiwai, will, dave.hansen,
jack, cl, jglisse, surenb, zokeefe, hannes, rientjes, mhocko,
rdunlap, hughd, richard.weiyang, lance.yang, vbabka, rppt, jannh,
pfalcato
In-Reply-To: <20251201174627.23295-5-npache@redhat.com>
On 1 Dec 2025, at 12:46, Nico Pache wrote:
> From: Dev Jain <dev.jain@arm.com>
>
> Pass order to alloc_charge_folio() and update mTHP statistics.
>
> Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
> Reviewed-by: Lance Yang <lance.yang@linux.dev>
> Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> Acked-by: David Hildenbrand <david@redhat.com>
> Co-developed-by: Nico Pache <npache@redhat.com>
> Signed-off-by: Nico Pache <npache@redhat.com>
> Signed-off-by: Dev Jain <dev.jain@arm.com>
> ---
> Documentation/admin-guide/mm/transhuge.rst | 8 ++++++++
> include/linux/huge_mm.h | 2 ++
> mm/huge_memory.c | 4 ++++
> mm/khugepaged.c | 17 +++++++++++------
> 4 files changed, 25 insertions(+), 6 deletions(-)
>
LGTM. Reviewed-by: Zi Yan <ziy@nvidia.com>
Best Regards,
Yan, Zi
^ permalink raw reply
* Re: [PATCH v13 mm-new 03/16] khugepaged: generalize hugepage_vma_revalidate for mTHP support
From: Zi Yan @ 2025-12-03 3:03 UTC (permalink / raw)
To: Nico Pache
Cc: linux-kernel, linux-trace-kernel, linux-mm, linux-doc, david,
baolin.wang, lorenzo.stoakes, Liam.Howlett, ryan.roberts,
dev.jain, corbet, rostedt, mhiramat, mathieu.desnoyers, akpm,
baohua, willy, peterx, wangkefeng.wang, usamaarif642, sunnanyong,
vishal.moola, thomas.hellstrom, yang, kas, aarcange, raquini,
anshuman.khandual, catalin.marinas, tiwai, will, dave.hansen,
jack, cl, jglisse, surenb, zokeefe, hannes, rientjes, mhocko,
rdunlap, hughd, richard.weiyang, lance.yang, vbabka, rppt, jannh,
pfalcato
In-Reply-To: <20251201174627.23295-4-npache@redhat.com>
On 1 Dec 2025, at 12:46, Nico Pache wrote:
> For khugepaged to support different mTHP orders, we must generalize this
> to check if the PMD is not shared by another VMA and that the order is
> enabled.
>
> No functional change in this patch. Also correct a comment about the
> functionality of the revalidation.
>
> Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
> Reviewed-by: Lance Yang <lance.yang@linux.dev>
> Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> Acked-by: David Hildenbrand <david@redhat.com>
> Co-developed-by: Dev Jain <dev.jain@arm.com>
> Signed-off-by: Dev Jain <dev.jain@arm.com>
> Signed-off-by: Nico Pache <npache@redhat.com>
> ---
> mm/khugepaged.c | 20 +++++++++++---------
> 1 file changed, 11 insertions(+), 9 deletions(-)
>
LGTM. Reviewed-by: Zi Yan <ziy@nvidia.com>
Best Regards,
Yan, Zi
^ permalink raw reply
* Re: [PATCH] kprobes: avoid crash when rmmod/insmod modules after ftrace_disabled
From: yebin @ 2025-12-03 2:37 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: naveen, davem, linux-trace-kernel, yebin10, Steven Rostedt
In-Reply-To: <20251202113248.e9e2e7c665bd3ee07b83e399@kernel.org>
On 2025/12/2 10:32, Masami Hiramatsu (Google) wrote:
> Cc: Steve,
>
> Since this is caused by ftrace_kill.
>
> On Fri, 28 Nov 2025 10:27:42 +0800
> yebin <yebin@huaweicloud.com> wrote:
>
>>
>>
>> On 2025/11/27 12:18, Masami Hiramatsu (Google) wrote:
>>> On Thu, 27 Nov 2025 12:52:48 +0900
>>> Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
>>>
>>>> Hi,
>>>>
>>>> Thanks for reporting!
>>>>
>>>>
>>>> On Tue, 25 Nov 2025 10:05:36 +0800
>>>> Ye Bin <yebin@huaweicloud.com> wrote:
>>>>
>>>>> From: Ye Bin <yebin10@huawei.com>
>>>>>
>>>>> There's a issue as follows when rmmod modules after ftrace disabled:
>>>>
>>>> You may see something like;
>>>>
>>>> Failed to unregister kprobe-ftrace (error -19)
>>>>
>>>> or
>>>>
>>>> Failed to disarm kprobe-ftrace at <function name> (error -19)
>>>>
>>>> right before this BUG, don't you?
>>>> If you reported with that line, it's more easier to understand.
>>>>
>> Yes, there is indeed a warning generated. I might not have expressed it
>> clearly enough. The issue below is related to the problem that occurs
>> when the second module is unloaded. When the first module was unloaded,
>> some nodes were left in the hash list, causing a use-after-free (UAF)
>> issue when traversing the hash list.
>> Therefore, this patch aims to resolve the UAF problem caused by residual
>> nodes in the hash list after unloading a module while ftrace is disabled.
>
> Yes, but I think your patch is redundant. Can you test the code
> which I suggested at the last?
>
Yes, indeed, your solution is simpler and doesn't require writing
repetitive code.
> BTW, can you explain why that ftrace_disabled was kicked?
> That usually means ftrace hits some bug.
>
Yes, there is a conflict between two modules causing ftrace_disabled.
>>>>
>>>>> BUG: unable to handle page fault for address: fffffbfff805000d
>>>>> PGD 817fcc067 P4D 817fcc067 PUD 817fc8067 PMD 101555067 PTE 0
>>>>> Oops: Oops: 0000 [#1] SMP KASAN PTI
>>>>> CPU: 4 UID: 0 PID: 2012 Comm: rmmod Tainted: G W OE
>>>>> Tainted: [W]=WARN, [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
>>>>> RIP: 0010:kprobes_module_callback+0x89/0x790
>>>>> RSP: 0018:ffff88812e157d30 EFLAGS: 00010a02
>>>>> RAX: 1ffffffff805000d RBX: dffffc0000000000 RCX: ffffffff86a8de90
>>>>> RDX: ffffed1025c2af9b RSI: 0000000000000008 RDI: ffffffffc0280068
>>>>> RBP: 0000000000000000 R08: 0000000000000001 R09: ffffed1025c2af9a
>>>>> R10: ffff88812e157cd7 R11: 205d323130325420 R12: 0000000000000002
>>>>> R13: ffffffffc0290488 R14: 0000000000000002 R15: ffffffffc0280040
>>>>> FS: 00007fbc450dd740(0000) GS:ffff888420331000(0000) knlGS:0000000000000000
>>>>> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>>>> CR2: fffffbfff805000d CR3: 000000010f624000 CR4: 00000000000006f0
>>>>> Call Trace:
>>>>> <TASK>
>>>>> notifier_call_chain+0xc6/0x280
>>>>> blocking_notifier_call_chain+0x60/0x90
>>>>> __do_sys_delete_module.constprop.0+0x32a/0x4e0
>>>>> do_syscall_64+0x5d/0xfa0
>>>>> entry_SYSCALL_64_after_hwframe+0x76/0x7e
>>>>>
>>>>> The above issue occurs because the kprobe was not removed from the hash
>>>>> list after ftrace_disable.
>>>>> To prevent the system from restarting unexpectedly after ftrace_disable,
>>>>> in such cases, unregister_kprobe() ensures that the probe is removed from
>>>>> the hash list, preventing subsequent access to already freed memory.
>>>>>
>>>>> Fixes: 6f0f1dd71953 ("kprobes: Cleanup disabling and unregistering path")
>>>>> Signed-off-by: Ye Bin <yebin10@huawei.com>
>>>>> ---
>>>>> kernel/kprobes.c | 26 ++++++++++++++++++++++++--
>>>>> 1 file changed, 24 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
>>>>> index ab8f9fc1f0d1..d735a608b810 100644
>>>>> --- a/kernel/kprobes.c
>>>>> +++ b/kernel/kprobes.c
>>>>> @@ -1731,8 +1731,30 @@ static int __unregister_kprobe_top(struct kprobe *p)
>>>>>
>>>>> /* Disable kprobe. This will disarm it if needed. */
>>>>> ap = __disable_kprobe(p);
>>>>> - if (IS_ERR(ap))
>>>>> - return PTR_ERR(ap);
>>>>> + if (IS_ERR(ap)) {
>>>>> + int ret = PTR_ERR(ap);
>>>>> +
>>>>> + /*
>>>>> + * If ftrace disabled we need to delete kprobe node from
>>>>> + * hlist or aggregation list. If nodes are not removed when
>>>>> + * modules are removed, the already released nodes will
>>>>> + * remain in the linked list. Subsequent access to the
>>>>> + * linked list may then trigger exceptions.
>>>>> + */
>>>>> + if (ret != -ENODEV)
>>>>> + return ret;
>>>>> +
>>>>> + ap = __get_valid_kprobe(p);
>>>>> + if (!ap)
>>>>> + return ret;
>>>>> +
>>>>> + if (ap == p)
>>>>> + hlist_del_rcu(&ap->hlist);
>>>>> + else
>>>>> + list_del_rcu(&p->list);
>>>>
>>>> Instead of repeating this process, we should ignore
>>>> -ENODEV error from ftrace directly. BTW, ftrace_disabled is set
>>>> when ftrace_kill() is called, that means ftrace is no more usable.
>>>> So I think we can just ignore ftrace operation in
>>>> __disarm_kprobe_ftrace().
>>>
>>> So, what we need is;
>>>
>>> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
>>> index ab8f9fc1f0d1..17d451553389 100644
>>> --- a/kernel/kprobes.c
>>> +++ b/kernel/kprobes.c
>>> @@ -1104,6 +1104,10 @@ static int __disarm_kprobe_ftrace(struct kprobe *p, struct ftrace_ops *ops,
>>> int ret;
>>>
>>> lockdep_assert_held(&kprobe_mutex);
>>> + if (unlikely(kprobe_ftrace_disabled)) {
>>> + /* Now ftrace is disabled forever, disarm is already done. */
>>> + return 0;
>>> + }
>>>
>>> if (*cnt == 1) {
>>> ret = unregister_ftrace_function(ops);
>>>
>
> This one, it should fix simply.
>
I tested it and it can solve the problem. This is indeed a good solution
that reuses the original code.
Do you want me to send a new patch version according to your proposal?
> Thank you,
>
>>>
>>
>
>
^ permalink raw reply
* Re: [PATCH bpf v3 2/2] selftests/bpf: fix and consolidate d_path LSM regression test
From: Alexei Starovoitov @ 2025-12-03 1:21 UTC (permalink / raw)
To: Shuran Liu
Cc: Song Liu, Matt Bobrowski, bpf, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Eduard,
Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
Hao Luo, Jiri Olsa, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, LKML, linux-trace-kernel, Daniel Xu,
open list:KERNEL SELFTEST FRAMEWORK, Shuah Khan, Zesen Liu,
Peili Gao, Haoran Ni
In-Reply-To: <20251202141944.2209-3-electronlsr@gmail.com>
On Tue, Dec 2, 2025 at 6:20 AM Shuran Liu <electronlsr@gmail.com> wrote:
>
> Add a regression test for bpf_d_path() when invoked from an LSM program.
> The test attaches to the bprm_check_security hook, calls bpf_d_path() on
> the binary being executed, and verifies that a simple prefix comparison on
> the returned pathname behaves correctly after the fix in patch 1.
>
> To avoid nondeterminism, the LSM program now filters based on the
> expected PID, which is populated from userspace before the test binary is
> executed. This prevents unrelated processes that also trigger the
> bprm_check_security LSM hook from overwriting test results. Parent and
> child processes are synchronized through a pipe to ensure the PID is set
> before the child execs the test binary.
>
> Per review feedback, the new LSM coverage is merged into the existing
> d_path selftest rather than adding new prog_tests/ or progs/ files. The
> loop that checks the pathname prefix now uses bpf_for(), which is a
> verifier-friendly way to express a small, fixed-iteration loop, and the
> temporary /tmp/bpf_d_path_test binary is removed in the test cleanup
> path.
>
> Co-developed-by: Zesen Liu <ftyg@live.com>
> Signed-off-by: Zesen Liu <ftyg@live.com>
> Co-developed-by: Peili Gao <gplhust955@gmail.com>
> Signed-off-by: Peili Gao <gplhust955@gmail.com>
> Co-developed-by: Haoran Ni <haoran.ni.cs@gmail.com>
> Signed-off-by: Haoran Ni <haoran.ni.cs@gmail.com>
> Signed-off-by: Shuran Liu <electronlsr@gmail.com>
> Reviewed-by: Matt Bobrowski <mattbobrowski@google.com>
> ---
> .../testing/selftests/bpf/prog_tests/d_path.c | 65 +++++++++++++++++++
> .../testing/selftests/bpf/progs/test_d_path.c | 33 ++++++++++
> 2 files changed, 98 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/d_path.c b/tools/testing/selftests/bpf/prog_tests/d_path.c
> index ccc768592e66..202b44e6f482 100644
> --- a/tools/testing/selftests/bpf/prog_tests/d_path.c
> +++ b/tools/testing/selftests/bpf/prog_tests/d_path.c
> @@ -195,6 +195,68 @@ static void test_d_path_check_types(void)
> test_d_path_check_types__destroy(skel);
> }
>
> +static void test_d_path_lsm(void)
> +{
> + struct test_d_path *skel;
> + int err;
> + int pipefd[2];
> + pid_t pid;
> +
> + skel = test_d_path__open_and_load();
> + if (!ASSERT_OK_PTR(skel, "d_path skeleton failed"))
> + return;
> +
> + err = test_d_path__attach(skel);
> + if (!ASSERT_OK(err, "attach failed"))
> + goto cleanup;
> +
> + /* Prepare the test binary */
> + system("cp /bin/true /tmp/bpf_d_path_test 2>/dev/null || :");
> +
> + if (!ASSERT_OK(pipe(pipefd), "pipe failed"))
> + goto cleanup;
> +
> + pid = fork();
> + if (!ASSERT_GE(pid, 0, "fork failed")) {
> + close(pipefd[0]);
> + close(pipefd[1]);
> + goto cleanup;
> + }
> +
> + if (pid == 0) {
> + /* Child */
> + char buf;
> +
> + close(pipefd[1]);
> + /* Wait for parent to set PID in BPF map */
> + if (read(pipefd[0], &buf, 1) != 1)
> + exit(1);
> + close(pipefd[0]);
> + execl("/tmp/bpf_d_path_test", "/tmp/bpf_d_path_test", NULL);
> + exit(1);
> + }
No forks please. They often make selftest to be flaky.
Use simples possible way to test it.
Without forks and pipes.
pw-bot: cr
^ permalink raw reply
* Re: [PATCH v6 5/6] perf tools: Merge deferred user callchains
From: Namhyung Kim @ 2025-12-03 0:01 UTC (permalink / raw)
To: Ian Rogers
Cc: Arnaldo Carvalho de Melo, James Clark, Jiri Olsa, Adrian Hunter,
Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users,
Steven Rostedt, Josh Poimboeuf, Indu Bhagat, Jens Remus,
Mathieu Desnoyers, linux-trace-kernel, bpf
In-Reply-To: <CAP-5=fUVgxHn-oxQNQBJKDo=k8VPXBKA5BkJ5LbUF-UOm9t8Xw@mail.gmail.com>
On Tue, Dec 02, 2025 at 03:14:31PM -0800, Ian Rogers wrote:
> On Thu, Nov 20, 2025 at 3:48 PM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > Save samples with deferred callchains in a separate list and deliver
> > them after merging the user callchains. If users don't want to merge
> > they can set tool->merge_deferred_callchains to false to prevent the
> > behavior.
> >
> > With previous result, now perf script will show the merged callchains.
> >
> > $ perf script
> > ...
> > pwd 2312 121.163435: 249113 cpu/cycles/P:
> > ffffffff845b78d8 __build_id_parse.isra.0+0x218 ([kernel.kallsyms])
> > ffffffff83bb5bf6 perf_event_mmap+0x2e6 ([kernel.kallsyms])
> > ffffffff83c31959 mprotect_fixup+0x1e9 ([kernel.kallsyms])
> > ffffffff83c31dc5 do_mprotect_pkey+0x2b5 ([kernel.kallsyms])
> > ffffffff83c3206f __x64_sys_mprotect+0x1f ([kernel.kallsyms])
> > ffffffff845e6692 do_syscall_64+0x62 ([kernel.kallsyms])
> > ffffffff8360012f entry_SYSCALL_64_after_hwframe+0x76 ([kernel.kallsyms])
> > 7f18fe337fa7 mprotect+0x7 (/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2)
> > 7f18fe330e0f _dl_sysdep_start+0x7f (/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2)
> > 7f18fe331448 _dl_start_user+0x0 (/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2)
> > ...
> >
> > The old output can be get using --no-merge-callchain option.
> > Also perf report can get the user callchain entry at the end.
> >
> > $ perf report --no-children --stdio -q -S __build_id_parse.isra.0
> > # symbol: __build_id_parse.isra.0
> > 8.40% pwd [kernel.kallsyms]
> > |
> > ---__build_id_parse.isra.0
> > perf_event_mmap
> > mprotect_fixup
> > do_mprotect_pkey
> > __x64_sys_mprotect
> > do_syscall_64
> > entry_SYSCALL_64_after_hwframe
> > mprotect
> > _dl_sysdep_start
> > _dl_start_user
> >
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
>
> Reviewed-by: Ian Rogers <irogers@google.com>
>
> > ---
> > tools/perf/Documentation/perf-script.txt | 5 ++
> > tools/perf/builtin-inject.c | 1 +
> > tools/perf/builtin-report.c | 1 +
> > tools/perf/builtin-script.c | 4 ++
> > tools/perf/util/callchain.c | 29 +++++++++
> > tools/perf/util/callchain.h | 3 +
> > tools/perf/util/evlist.c | 1 +
> > tools/perf/util/evlist.h | 2 +
> > tools/perf/util/session.c | 79 +++++++++++++++++++++++-
> > tools/perf/util/tool.c | 2 +
> > tools/perf/util/tool.h | 1 +
> > 11 files changed, 127 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
> > index 28bec7e78bc858ba..03d1129606328d6d 100644
> > --- a/tools/perf/Documentation/perf-script.txt
> > +++ b/tools/perf/Documentation/perf-script.txt
> > @@ -527,6 +527,11 @@ include::itrace.txt[]
> > The known limitations include exception handing such as
> > setjmp/longjmp will have calls/returns not match.
> >
> > +--merge-callchains::
> > + Enable merging deferred user callchains if available. This is the
> > + default behavior. If you want to see separate CALLCHAIN_DEFERRED
> > + records for some reason, use --no-merge-callchains explicitly.
> > +
> > :GMEXAMPLECMD: script
> > :GMEXAMPLESUBCMD:
> > include::guest-files.txt[]
> > diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> > index bd9245d2dd41aa48..51d2721b6db9dccb 100644
> > --- a/tools/perf/builtin-inject.c
> > +++ b/tools/perf/builtin-inject.c
> > @@ -2527,6 +2527,7 @@ int cmd_inject(int argc, const char **argv)
> > inject.tool.auxtrace = perf_event__repipe_auxtrace;
> > inject.tool.bpf_metadata = perf_event__repipe_op2_synth;
> > inject.tool.dont_split_sample_group = true;
> > + inject.tool.merge_deferred_callchains = false;
> > inject.session = __perf_session__new(&data, &inject.tool,
> > /*trace_event_repipe=*/inject.output.is_pipe,
> > /*host_env=*/NULL);
> > diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> > index 2bc269f5fcef8023..add6b1c2aaf04270 100644
> > --- a/tools/perf/builtin-report.c
> > +++ b/tools/perf/builtin-report.c
> > @@ -1614,6 +1614,7 @@ int cmd_report(int argc, const char **argv)
> > report.tool.event_update = perf_event__process_event_update;
> > report.tool.feature = process_feature_event;
> > report.tool.ordering_requires_timestamps = true;
> > + report.tool.merge_deferred_callchains = !dump_trace;
> >
> > session = perf_session__new(&data, &report.tool);
> > if (IS_ERR(session)) {
> > diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> > index 85b42205a71b3993..62e43d3c5ad731a0 100644
> > --- a/tools/perf/builtin-script.c
> > +++ b/tools/perf/builtin-script.c
> > @@ -4009,6 +4009,7 @@ int cmd_script(int argc, const char **argv)
> > bool header_only = false;
> > bool script_started = false;
> > bool unsorted_dump = false;
> > + bool merge_deferred_callchains = true;
> > char *rec_script_path = NULL;
> > char *rep_script_path = NULL;
> > struct perf_session *session;
> > @@ -4162,6 +4163,8 @@ int cmd_script(int argc, const char **argv)
> > "Guest code can be found in hypervisor process"),
> > OPT_BOOLEAN('\0', "stitch-lbr", &script.stitch_lbr,
> > "Enable LBR callgraph stitching approach"),
> > + OPT_BOOLEAN('\0', "merge-callchains", &merge_deferred_callchains,
> > + "Enable merge deferred user callchains"),
> > OPTS_EVSWITCH(&script.evswitch),
> > OPT_END()
> > };
> > @@ -4418,6 +4421,7 @@ int cmd_script(int argc, const char **argv)
> > script.tool.throttle = process_throttle_event;
> > script.tool.unthrottle = process_throttle_event;
> > script.tool.ordering_requires_timestamps = true;
> > + script.tool.merge_deferred_callchains = merge_deferred_callchains;
> > session = perf_session__new(&data, &script.tool);
> > if (IS_ERR(session))
> > return PTR_ERR(session);
> > diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
> > index 2884187ccbbecfdc..71dc5a070065dd2a 100644
> > --- a/tools/perf/util/callchain.c
> > +++ b/tools/perf/util/callchain.c
> > @@ -1838,3 +1838,32 @@ int sample__for_each_callchain_node(struct thread *thread, struct evsel *evsel,
> > }
> > return 0;
> > }
> > +
> > +int sample__merge_deferred_callchain(struct perf_sample *sample_orig,
>
> nit: We use the term deferred rather than original except in this
> context. I think deferred is a little more intention revealing than
> original. Perhaps add a comment capturing that the original sample is
> the deferred kernel sample.
Sure, will add.
Thanks,
Namhyung
^ permalink raw reply
* Re: [PATCH v6 6/6] perf tools: Flush remaining samples w/o deferred callchains
From: Ian Rogers @ 2025-12-02 23:15 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, James Clark, Jiri Olsa, Adrian Hunter,
Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users,
Steven Rostedt, Josh Poimboeuf, Indu Bhagat, Jens Remus,
Mathieu Desnoyers, linux-trace-kernel, bpf
In-Reply-To: <20251120234804.156340-7-namhyung@kernel.org>
On Thu, Nov 20, 2025 at 3:48 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> It's possible that some kernel samples don't have matching deferred
> callchain records when the profiling session was ended before the
> threads came back to userspace. Let's flush the samples before
> finish the session.
>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Reviewed-by: Ian Rogers <irogers@google.com>
Thanks,
Ian
> ---
> tools/perf/util/session.c | 50 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 50 insertions(+)
>
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index dc570ad47ccc2c63..4236503c8f6c1350 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -1295,6 +1295,10 @@ struct deferred_event {
> union perf_event *event;
> };
>
> +/*
> + * This is called when a deferred callchain record comes up. Find all matching
> + * samples, merge the callchains and process them.
> + */
> static int evlist__deliver_deferred_callchain(struct evlist *evlist,
> const struct perf_tool *tool,
> union perf_event *event,
> @@ -1345,6 +1349,42 @@ static int evlist__deliver_deferred_callchain(struct evlist *evlist,
> return ret;
> }
>
> +/*
> + * This is called at the end of the data processing for the session. Flush the
> + * remaining samples as there's no hope for matching deferred callchains.
> + */
> +static int session__flush_deferred_samples(struct perf_session *session,
> + const struct perf_tool *tool)
> +{
> + struct evlist *evlist = session->evlist;
> + struct machine *machine = &session->machines.host;
> + struct deferred_event *de, *tmp;
> + struct evsel *evsel;
> + int ret = 0;
> +
> + list_for_each_entry_safe(de, tmp, &evlist->deferred_samples, list) {
> + struct perf_sample sample;
> +
> + ret = evlist__parse_sample(evlist, de->event, &sample);
> + if (ret < 0) {
> + pr_err("failed to parse original sample\n");
> + break;
> + }
> +
> + evsel = evlist__id2evsel(evlist, sample.id);
> + ret = evlist__deliver_sample(evlist, tool, de->event,
> + &sample, evsel, machine);
> +
> + list_del(&de->list);
> + free(de->event);
> + free(de);
> +
> + if (ret)
> + break;
> + }
> + return ret;
> +}
> +
> static int machines__deliver_event(struct machines *machines,
> struct evlist *evlist,
> union perf_event *event,
> @@ -2038,6 +2078,9 @@ static int __perf_session__process_pipe_events(struct perf_session *session)
> done:
> /* do the final flush for ordered samples */
> err = ordered_events__flush(oe, OE_FLUSH__FINAL);
> + if (err)
> + goto out_err;
> + err = session__flush_deferred_samples(session, tool);
> if (err)
> goto out_err;
> err = auxtrace__flush_events(session, tool);
> @@ -2384,6 +2427,9 @@ static int __perf_session__process_events(struct perf_session *session)
> if (err)
> goto out_err;
> err = auxtrace__flush_events(session, tool);
> + if (err)
> + goto out_err;
> + err = session__flush_deferred_samples(session, tool);
> if (err)
> goto out_err;
> err = perf_session__flush_thread_stacks(session);
> @@ -2506,6 +2552,10 @@ static int __perf_session__process_dir_events(struct perf_session *session)
> if (ret)
> goto out_err;
>
> + ret = session__flush_deferred_samples(session, tool);
> + if (ret)
> + goto out_err;
> +
> ret = perf_session__flush_thread_stacks(session);
> out_err:
> ui_progress__finish();
> --
> 2.52.0.rc2.455.g230fcf2819-goog
>
^ permalink raw reply
* Re: [PATCH v6 5/6] perf tools: Merge deferred user callchains
From: Ian Rogers @ 2025-12-02 23:14 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, James Clark, Jiri Olsa, Adrian Hunter,
Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users,
Steven Rostedt, Josh Poimboeuf, Indu Bhagat, Jens Remus,
Mathieu Desnoyers, linux-trace-kernel, bpf
In-Reply-To: <20251120234804.156340-6-namhyung@kernel.org>
On Thu, Nov 20, 2025 at 3:48 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Save samples with deferred callchains in a separate list and deliver
> them after merging the user callchains. If users don't want to merge
> they can set tool->merge_deferred_callchains to false to prevent the
> behavior.
>
> With previous result, now perf script will show the merged callchains.
>
> $ perf script
> ...
> pwd 2312 121.163435: 249113 cpu/cycles/P:
> ffffffff845b78d8 __build_id_parse.isra.0+0x218 ([kernel.kallsyms])
> ffffffff83bb5bf6 perf_event_mmap+0x2e6 ([kernel.kallsyms])
> ffffffff83c31959 mprotect_fixup+0x1e9 ([kernel.kallsyms])
> ffffffff83c31dc5 do_mprotect_pkey+0x2b5 ([kernel.kallsyms])
> ffffffff83c3206f __x64_sys_mprotect+0x1f ([kernel.kallsyms])
> ffffffff845e6692 do_syscall_64+0x62 ([kernel.kallsyms])
> ffffffff8360012f entry_SYSCALL_64_after_hwframe+0x76 ([kernel.kallsyms])
> 7f18fe337fa7 mprotect+0x7 (/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2)
> 7f18fe330e0f _dl_sysdep_start+0x7f (/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2)
> 7f18fe331448 _dl_start_user+0x0 (/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2)
> ...
>
> The old output can be get using --no-merge-callchain option.
> Also perf report can get the user callchain entry at the end.
>
> $ perf report --no-children --stdio -q -S __build_id_parse.isra.0
> # symbol: __build_id_parse.isra.0
> 8.40% pwd [kernel.kallsyms]
> |
> ---__build_id_parse.isra.0
> perf_event_mmap
> mprotect_fixup
> do_mprotect_pkey
> __x64_sys_mprotect
> do_syscall_64
> entry_SYSCALL_64_after_hwframe
> mprotect
> _dl_sysdep_start
> _dl_start_user
>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Reviewed-by: Ian Rogers <irogers@google.com>
> ---
> tools/perf/Documentation/perf-script.txt | 5 ++
> tools/perf/builtin-inject.c | 1 +
> tools/perf/builtin-report.c | 1 +
> tools/perf/builtin-script.c | 4 ++
> tools/perf/util/callchain.c | 29 +++++++++
> tools/perf/util/callchain.h | 3 +
> tools/perf/util/evlist.c | 1 +
> tools/perf/util/evlist.h | 2 +
> tools/perf/util/session.c | 79 +++++++++++++++++++++++-
> tools/perf/util/tool.c | 2 +
> tools/perf/util/tool.h | 1 +
> 11 files changed, 127 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
> index 28bec7e78bc858ba..03d1129606328d6d 100644
> --- a/tools/perf/Documentation/perf-script.txt
> +++ b/tools/perf/Documentation/perf-script.txt
> @@ -527,6 +527,11 @@ include::itrace.txt[]
> The known limitations include exception handing such as
> setjmp/longjmp will have calls/returns not match.
>
> +--merge-callchains::
> + Enable merging deferred user callchains if available. This is the
> + default behavior. If you want to see separate CALLCHAIN_DEFERRED
> + records for some reason, use --no-merge-callchains explicitly.
> +
> :GMEXAMPLECMD: script
> :GMEXAMPLESUBCMD:
> include::guest-files.txt[]
> diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> index bd9245d2dd41aa48..51d2721b6db9dccb 100644
> --- a/tools/perf/builtin-inject.c
> +++ b/tools/perf/builtin-inject.c
> @@ -2527,6 +2527,7 @@ int cmd_inject(int argc, const char **argv)
> inject.tool.auxtrace = perf_event__repipe_auxtrace;
> inject.tool.bpf_metadata = perf_event__repipe_op2_synth;
> inject.tool.dont_split_sample_group = true;
> + inject.tool.merge_deferred_callchains = false;
> inject.session = __perf_session__new(&data, &inject.tool,
> /*trace_event_repipe=*/inject.output.is_pipe,
> /*host_env=*/NULL);
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 2bc269f5fcef8023..add6b1c2aaf04270 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -1614,6 +1614,7 @@ int cmd_report(int argc, const char **argv)
> report.tool.event_update = perf_event__process_event_update;
> report.tool.feature = process_feature_event;
> report.tool.ordering_requires_timestamps = true;
> + report.tool.merge_deferred_callchains = !dump_trace;
>
> session = perf_session__new(&data, &report.tool);
> if (IS_ERR(session)) {
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index 85b42205a71b3993..62e43d3c5ad731a0 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -4009,6 +4009,7 @@ int cmd_script(int argc, const char **argv)
> bool header_only = false;
> bool script_started = false;
> bool unsorted_dump = false;
> + bool merge_deferred_callchains = true;
> char *rec_script_path = NULL;
> char *rep_script_path = NULL;
> struct perf_session *session;
> @@ -4162,6 +4163,8 @@ int cmd_script(int argc, const char **argv)
> "Guest code can be found in hypervisor process"),
> OPT_BOOLEAN('\0', "stitch-lbr", &script.stitch_lbr,
> "Enable LBR callgraph stitching approach"),
> + OPT_BOOLEAN('\0', "merge-callchains", &merge_deferred_callchains,
> + "Enable merge deferred user callchains"),
> OPTS_EVSWITCH(&script.evswitch),
> OPT_END()
> };
> @@ -4418,6 +4421,7 @@ int cmd_script(int argc, const char **argv)
> script.tool.throttle = process_throttle_event;
> script.tool.unthrottle = process_throttle_event;
> script.tool.ordering_requires_timestamps = true;
> + script.tool.merge_deferred_callchains = merge_deferred_callchains;
> session = perf_session__new(&data, &script.tool);
> if (IS_ERR(session))
> return PTR_ERR(session);
> diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
> index 2884187ccbbecfdc..71dc5a070065dd2a 100644
> --- a/tools/perf/util/callchain.c
> +++ b/tools/perf/util/callchain.c
> @@ -1838,3 +1838,32 @@ int sample__for_each_callchain_node(struct thread *thread, struct evsel *evsel,
> }
> return 0;
> }
> +
> +int sample__merge_deferred_callchain(struct perf_sample *sample_orig,
nit: We use the term deferred rather than original except in this
context. I think deferred is a little more intention revealing than
original. Perhaps add a comment capturing that the original sample is
the deferred kernel sample.
Thanks,
Ian
> + struct perf_sample *sample_callchain)
> +{
> + u64 nr_orig = sample_orig->callchain->nr - 1;
> + u64 nr_deferred = sample_callchain->callchain->nr;
> + struct ip_callchain *callchain;
> +
> + if (sample_orig->callchain->nr < 2) {
> + sample_orig->deferred_callchain = false;
> + return -EINVAL;
> + }
> +
> + callchain = calloc(1 + nr_orig + nr_deferred, sizeof(u64));
> + if (callchain == NULL) {
> + sample_orig->deferred_callchain = false;
> + return -ENOMEM;
> + }
> +
> + callchain->nr = nr_orig + nr_deferred;
> + /* copy original including PERF_CONTEXT_USER_DEFERRED (but the cookie) */
> + memcpy(callchain->ips, sample_orig->callchain->ips, nr_orig * sizeof(u64));
> + /* copy deferred user callchains */
> + memcpy(&callchain->ips[nr_orig], sample_callchain->callchain->ips,
> + nr_deferred * sizeof(u64));
> +
> + sample_orig->callchain = callchain;
> + return 0;
> +}
> diff --git a/tools/perf/util/callchain.h b/tools/perf/util/callchain.h
> index d5ae4fbb7ce5fa44..2a52af8c80ace33c 100644
> --- a/tools/perf/util/callchain.h
> +++ b/tools/perf/util/callchain.h
> @@ -318,4 +318,7 @@ int sample__for_each_callchain_node(struct thread *thread, struct evsel *evsel,
> struct perf_sample *sample, int max_stack,
> bool symbols, callchain_iter_fn cb, void *data);
>
> +int sample__merge_deferred_callchain(struct perf_sample *sample_orig,
> + struct perf_sample *sample_callchain);
> +
> #endif /* __PERF_CALLCHAIN_H */
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index e8217efdda5323c6..03674d2cbd015e4f 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -85,6 +85,7 @@ void evlist__init(struct evlist *evlist, struct perf_cpu_map *cpus,
> evlist->ctl_fd.pos = -1;
> evlist->nr_br_cntr = -1;
> metricgroup__rblist_init(&evlist->metric_events);
> + INIT_LIST_HEAD(&evlist->deferred_samples);
> }
>
> struct evlist *evlist__new(void)
> diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
> index 5e71e3dc60423079..911834ae7c2a6f76 100644
> --- a/tools/perf/util/evlist.h
> +++ b/tools/perf/util/evlist.h
> @@ -92,6 +92,8 @@ struct evlist {
> * of struct metric_expr.
> */
> struct rblist metric_events;
> + /* samples with deferred_callchain would wait here. */
> + struct list_head deferred_samples;
> };
>
> struct evsel_str_handler {
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index 361e15c1f26a96d0..dc570ad47ccc2c63 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -1285,6 +1285,66 @@ static int evlist__deliver_sample(struct evlist *evlist, const struct perf_tool
> per_thread);
> }
>
> +/*
> + * Samples with deferred callchains should wait for the next matching
> + * PERF_RECORD_CALLCHAIN_RECORD entries. Keep the events in a list and
> + * deliver them once it finds the callchains.
> + */
> +struct deferred_event {
> + struct list_head list;
> + union perf_event *event;
> +};
> +
> +static int evlist__deliver_deferred_callchain(struct evlist *evlist,
> + const struct perf_tool *tool,
> + union perf_event *event,
> + struct perf_sample *sample,
> + struct machine *machine)
> +{
> + struct deferred_event *de, *tmp;
> + struct evsel *evsel;
> + int ret = 0;
> +
> + if (!tool->merge_deferred_callchains) {
> + evsel = evlist__id2evsel(evlist, sample->id);
> + return tool->callchain_deferred(tool, event, sample,
> + evsel, machine);
> + }
> +
> + list_for_each_entry_safe(de, tmp, &evlist->deferred_samples, list) {
> + struct perf_sample orig_sample;
> +
> + ret = evlist__parse_sample(evlist, de->event, &orig_sample);
> + if (ret < 0) {
> + pr_err("failed to parse original sample\n");
> + break;
> + }
> +
> + if (sample->tid != orig_sample.tid)
> + continue;
> +
> + if (event->callchain_deferred.cookie == orig_sample.deferred_cookie)
> + sample__merge_deferred_callchain(&orig_sample, sample);
> + else
> + orig_sample.deferred_callchain = false;
> +
> + evsel = evlist__id2evsel(evlist, orig_sample.id);
> + ret = evlist__deliver_sample(evlist, tool, de->event,
> + &orig_sample, evsel, machine);
> +
> + if (orig_sample.deferred_callchain)
> + free(orig_sample.callchain);
> +
> + list_del(&de->list);
> + free(de->event);
> + free(de);
> +
> + if (ret)
> + break;
> + }
> + return ret;
> +}
> +
> static int machines__deliver_event(struct machines *machines,
> struct evlist *evlist,
> union perf_event *event,
> @@ -1313,6 +1373,22 @@ static int machines__deliver_event(struct machines *machines,
> return 0;
> }
> dump_sample(evsel, event, sample, perf_env__arch(machine->env));
> + if (sample->deferred_callchain && tool->merge_deferred_callchains) {
> + struct deferred_event *de = malloc(sizeof(*de));
> + size_t sz = event->header.size;
> +
> + if (de == NULL)
> + return -ENOMEM;
> +
> + de->event = malloc(sz);
> + if (de->event == NULL) {
> + free(de);
> + return -ENOMEM;
> + }
> + memcpy(de->event, event, sz);
> + list_add_tail(&de->list, &evlist->deferred_samples);
> + return 0;
> + }
> return evlist__deliver_sample(evlist, tool, event, sample, evsel, machine);
> case PERF_RECORD_MMAP:
> return tool->mmap(tool, event, sample, machine);
> @@ -1372,7 +1448,8 @@ static int machines__deliver_event(struct machines *machines,
> return tool->aux_output_hw_id(tool, event, sample, machine);
> case PERF_RECORD_CALLCHAIN_DEFERRED:
> dump_deferred_callchain(evsel, event, sample);
> - return tool->callchain_deferred(tool, event, sample, evsel, machine);
> + return evlist__deliver_deferred_callchain(evlist, tool, event,
> + sample, machine);
> default:
> ++evlist->stats.nr_unknown_events;
> return -1;
> diff --git a/tools/perf/util/tool.c b/tools/perf/util/tool.c
> index e77f0e2ecc1f79db..27ba5849c74a2e7d 100644
> --- a/tools/perf/util/tool.c
> +++ b/tools/perf/util/tool.c
> @@ -266,6 +266,7 @@ void perf_tool__init(struct perf_tool *tool, bool ordered_events)
> tool->cgroup_events = false;
> tool->no_warn = false;
> tool->show_feat_hdr = SHOW_FEAT_NO_HEADER;
> + tool->merge_deferred_callchains = true;
>
> tool->sample = process_event_sample_stub;
> tool->mmap = process_event_stub;
> @@ -448,6 +449,7 @@ void delegate_tool__init(struct delegate_tool *tool, struct perf_tool *delegate)
> tool->tool.cgroup_events = delegate->cgroup_events;
> tool->tool.no_warn = delegate->no_warn;
> tool->tool.show_feat_hdr = delegate->show_feat_hdr;
> + tool->tool.merge_deferred_callchains = delegate->merge_deferred_callchains;
>
> tool->tool.sample = delegate_sample;
> tool->tool.read = delegate_read;
> diff --git a/tools/perf/util/tool.h b/tools/perf/util/tool.h
> index 9b9f0a8cbf3de4b5..e96b69d25a5b737d 100644
> --- a/tools/perf/util/tool.h
> +++ b/tools/perf/util/tool.h
> @@ -90,6 +90,7 @@ struct perf_tool {
> bool cgroup_events;
> bool no_warn;
> bool dont_split_sample_group;
> + bool merge_deferred_callchains;
> enum show_feature_header show_feat_hdr;
> };
>
> --
> 2.52.0.rc2.455.g230fcf2819-goog
>
^ permalink raw reply
* Re: [PATCH 1/3] kernel.h: drop STACK_MAGIC macro
From: Yury Norov @ 2025-12-02 21:18 UTC (permalink / raw)
To: Andi Shyti
Cc: Jani Nikula, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Andy Shevchenko, Randy Dunlap, Ingo Molnar, Joonas Lahtinen,
Rodrigo Vivi, Tvrtko Ursulin, Petr Pavlu, Daniel Gomez,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Andrew Morton, linux-kernel, intel-gfx, dri-devel, linux-modules,
linux-trace-kernel
In-Reply-To: <3m64k5fagw7hp2duo43t5fldyn6argdjripx3nn6onxbr6xu6w@iwiepyn5krf6>
On Tue, Dec 02, 2025 at 09:58:19PM +0100, Andi Shyti wrote:
> Hi Jani,
>
> On Mon, Dec 01, 2025 at 09:46:47AM +0200, Jani Nikula wrote:
> > On Sat, 29 Nov 2025, "Yury Norov (NVIDIA)" <yury.norov@gmail.com> wrote:
> > > The macro is only used by i915. Move it to a local header and drop from
> > > the kernel.h.
> > >
> > > Signed-off-by: Yury Norov (NVIDIA) <yury.norov@gmail.com>
> > > ---
> > > drivers/gpu/drm/i915/i915_utils.h | 2 ++
> > > include/linux/kernel.h | 2 --
> > > 2 files changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h
> > > index a0c892e4c40d..6c197e968305 100644
> > > --- a/drivers/gpu/drm/i915/i915_utils.h
> > > +++ b/drivers/gpu/drm/i915/i915_utils.h
> >
> > i915_utils.h is on a diet itself. STACK_MAGIC is only used in selftests,
> > please put this in i915_selftest.h.
> >
> > I guess also need to include that from gt/selftest_ring_submission.c,
> > the only one that uses STACK_MAGIC but doesn't include i915_selftest.h.
>
> Doing this cleanups is a bit out of the scope of this patch.
> Given that the patch itself has quite a good consensus, let move
> it forward and I can take care of the i915 cleanup once it gets
> merged.
I'm already testing it in my tree:
https://github.com/norov/linux/tree/sm1
If everything is fine, I'll submit v2 with this change, otherwise will
schedule it for future improvements.
^ permalink raw reply
* [PATCH] tracing: Fix enabling of tracing on file release
From: Steven Rostedt @ 2025-12-02 21:17 UTC (permalink / raw)
To: LKML, Linux Trace Kernel
Cc: Masami Hiramatsu, Mathieu Desnoyers, syzkaller-bugs
From: Steven Rostedt <rostedt@goodmis.org>
The trace file will pause tracing if the tracing instance has the
"pause-on-trace" option is set. This happens when the file is opened, and
it is unpaused when the file is closed. When this was first added, there
was only one user that paused tracing. On open, the check to pause was:
if (!iter->snapshot && (tr->trace_flags & TRACE_ITER(PAUSE_ON_TRACE)))
Where if it is not the snapshot tracer and the "pause-on-trace" option is
set, then it increments a "stop_count" of the trace instance.
On close, the check is:
if (!iter->snapshot && tr->stop_count)
That is, if it is not the snapshot buffer and it was stopped, it will
re-enable tracing.
Now there's more places that stop tracing. This means, if something else
stops tracing the tr->stop_count will be non-zero, and that means if the
trace file is closed, it will decrement the stop_count even though it
never incremented it. This causes a warning because when the user that
stopped tracing enables it again, the stop_count goes below zero.
Instead of relying on the stop_count being set to know if the close of
the trace file should enable tracing again, add a new flag to the trace
iterator. The trace iterator is unique per open of the trace file, and if
the open stops tracing set the trace iterator PAUSE flag. On close, if the
PAUSE flag is set, then re-enable it again.
Fixes: 06e0a548bad0f ("tracing: Do not disable tracing when reading the trace file")
Reported-by: syzbot+ccdec3bfe0beec58a38d@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/692f44a5.a70a0220.2ea503.00c8.GAE@google.com/
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
include/linux/trace_events.h | 1 +
kernel/trace/trace.c | 6 ++++--
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 04307a19cde3..3690221ba3d8 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -138,6 +138,7 @@ enum trace_iter_flags {
TRACE_FILE_LAT_FMT = 1,
TRACE_FILE_ANNOTATE = 2,
TRACE_FILE_TIME_IN_NS = 4,
+ TRACE_FILE_PAUSE = 8,
};
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 3d433a426e5f..7b9d93147d01 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -4709,8 +4709,10 @@ __tracing_open(struct inode *inode, struct file *file, bool snapshot)
* If pause-on-trace is enabled, then stop the trace while
* dumping, unless this is the "snapshot" file
*/
- if (!iter->snapshot && (tr->trace_flags & TRACE_ITER(PAUSE_ON_TRACE)))
+ if (!iter->snapshot && (tr->trace_flags & TRACE_ITER(PAUSE_ON_TRACE))) {
+ iter->iter_flags |= TRACE_FILE_PAUSE;
tracing_stop_tr(tr);
+ }
if (iter->cpu_file == RING_BUFFER_ALL_CPUS) {
for_each_tracing_cpu(cpu) {
@@ -4842,7 +4844,7 @@ static int tracing_release(struct inode *inode, struct file *file)
if (iter->trace && iter->trace->close)
iter->trace->close(iter);
- if (!iter->snapshot && tr->stop_count)
+ if (iter->iter_flags & TRACE_FILE_PAUSE)
/* reenable tracing if it was previously enabled */
tracing_start_tr(tr);
--
2.51.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox