* [PATCHv2 bpf-next 8/8] bpf, x86: Use single ftrace_ops for direct calls
From: Jiri Olsa @ 2025-11-13 12:37 UTC (permalink / raw)
To: Steven Rostedt, Florent Revest, Mark Rutland
Cc: bpf, linux-kernel, linux-trace-kernel, linux-arm-kernel,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Menglong Dong, Song Liu
In-Reply-To: <20251113123750.2507435-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 | 166 ++++++++++++++++++++++++++++++++++++----
kernel/trace/Kconfig | 3 +
kernel/trace/ftrace.c | 7 +-
4 files changed, 160 insertions(+), 17 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index fa3b616af03a..65a2fc279b46 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -332,6 +332,7 @@ config X86
select SCHED_SMT if SMP
select ARCH_SUPPORTS_SCHED_CLUSTER if SMP
select ARCH_SUPPORTS_SCHED_MC if SMP
+ select HAVE_SINGLE_FTRACE_DIRECT_OPS if X86_64 && DYNAMIC_FTRACE_WITH_DIRECT_CALLS
config INSTRUCTION_DECODER
def_bool y
diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index 26887a0db955..436d393591a5 100644
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -33,12 +33,40 @@ static DEFINE_MUTEX(trampoline_mutex);
#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
static int bpf_trampoline_update(struct bpf_trampoline *tr, bool lock_direct_mutex);
+#ifdef CONFIG_HAVE_SINGLE_FTRACE_DIRECT_OPS
+static struct bpf_trampoline *direct_ops_ip_lookup(struct ftrace_ops *ops, unsigned long ip)
+{
+ struct hlist_head *head_ip;
+ struct bpf_trampoline *tr;
+
+ mutex_lock(&trampoline_mutex);
+ head_ip = &trampoline_ip_table[hash_64(ip, TRAMPOLINE_HASH_BITS)];
+ hlist_for_each_entry(tr, head_ip, hlist_ip) {
+ if (tr->ip == ip)
+ goto out;
+ }
+ tr = NULL;
+out:
+ mutex_unlock(&trampoline_mutex);
+ return tr;
+}
+#else
+static struct bpf_trampoline *direct_ops_ip_lookup(struct ftrace_ops *ops, unsigned long ip)
+{
+ return ops->private;
+}
+#endif /* CONFIG_HAVE_SINGLE_FTRACE_DIRECT_OPS */
+
static int bpf_tramp_ftrace_ops_func(struct ftrace_ops *ops, unsigned long ip,
enum ftrace_ops_cmd cmd)
{
- struct bpf_trampoline *tr = ops->private;
+ struct bpf_trampoline *tr;
int ret = 0;
+ tr = direct_ops_ip_lookup(ops, ip);
+ if (!tr)
+ return -EINVAL;
+
if (cmd == FTRACE_OPS_CMD_ENABLE_SHARE_IPMODIFY_SELF) {
/* This is called inside register_ftrace_direct_multi(), so
* tr->mutex is already locked.
@@ -137,6 +165,122 @@ void bpf_image_ksym_del(struct bpf_ksym *ksym)
PAGE_SIZE, true, ksym->name);
}
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
+#ifdef CONFIG_HAVE_SINGLE_FTRACE_DIRECT_OPS
+/*
+ * We have only single direct_ops which contains all the direct call
+ * sites and is the only global ftrace_ops for all trampolines.
+ *
+ * We use 'update_ftrace_direct_*' api for attachment.
+ */
+struct ftrace_ops direct_ops = {
+ .ops_func = bpf_tramp_ftrace_ops_func,
+};
+
+static int direct_ops_alloc(struct bpf_trampoline *tr)
+{
+ tr->fops = &direct_ops;
+ return 0;
+}
+
+static void direct_ops_free(struct bpf_trampoline *tr) { }
+
+static struct ftrace_hash *hash_from(unsigned long ip, void *addr)
+{
+ struct ftrace_hash *hash;
+
+ ip = ftrace_location(ip);
+ if (!ip)
+ return NULL;
+ hash = alloc_ftrace_hash(FTRACE_HASH_DEFAULT_BITS);
+ if (!hash)
+ return NULL;
+ if (!add_hash_entry_direct(hash, ip, (unsigned long) addr)) {
+ free_ftrace_hash(hash);
+ return NULL;
+ }
+ return hash;
+}
+
+static int direct_ops_add(struct ftrace_ops *ops, unsigned long ip, void *addr)
+{
+ struct ftrace_hash *hash = hash_from(ip, addr);
+ int err = -ENOMEM;
+
+ if (hash)
+ err = update_ftrace_direct_add(ops, hash);
+ free_ftrace_hash(hash);
+ return err;
+}
+
+static int direct_ops_del(struct ftrace_ops *ops, unsigned long ip, void *addr)
+{
+ struct ftrace_hash *hash = hash_from(ip, addr);
+ int err = -ENOMEM;
+
+ if (hash)
+ err = update_ftrace_direct_del(ops, hash);
+ free_ftrace_hash(hash);
+ return err;
+}
+
+static int direct_ops_mod(struct ftrace_ops *ops, unsigned long ip, void *addr, bool lock_direct_mutex)
+{
+ struct ftrace_hash *hash = hash_from(ip, addr);
+ int err = -ENOMEM;
+
+ if (hash)
+ err = update_ftrace_direct_mod(ops, hash, lock_direct_mutex);
+ free_ftrace_hash(hash);
+ return err;
+}
+#else
+/*
+ * We allocate ftrace_ops object for each trampoline and it contains
+ * call site specific for that trampoline.
+ *
+ * We use *_ftrace_direct api for attachment.
+ */
+static int direct_ops_alloc(struct bpf_trampoline *tr)
+{
+ tr->fops = kzalloc(sizeof(struct ftrace_ops), GFP_KERNEL);
+ if (!tr->fops)
+ return -ENOMEM;
+ tr->fops->private = tr;
+ tr->fops->ops_func = bpf_tramp_ftrace_ops_func;
+ return 0;
+}
+
+static void direct_ops_free(struct bpf_trampoline *tr)
+{
+ if (tr->fops) {
+ ftrace_free_filter(tr->fops);
+ kfree(tr->fops);
+ }
+}
+
+static int direct_ops_add(struct ftrace_ops *ops, unsigned long ip, void *addr)
+{
+ ftrace_set_filter_ip(ops, (unsigned long)ip, 0, 1);
+ return register_ftrace_direct(ops, (long)addr);
+}
+
+static int direct_ops_del(struct ftrace_ops *ops, unsigned long ip, void *addr)
+{
+ return unregister_ftrace_direct(ops, (long)addr, false);
+}
+
+static int direct_ops_mod(struct ftrace_ops *ops, unsigned long ip, void *addr, bool lock_direct_mutex)
+{
+ if (lock_direct_mutex)
+ return modify_ftrace_direct(ops, (long)addr);
+ return modify_ftrace_direct_nolock(ops, (long)addr);
+}
+#endif /* CONFIG_HAVE_SINGLE_FTRACE_DIRECT_OPS */
+#else
+static void direct_ops_free(struct bpf_trampoline *tr) { }
+#endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
+
static struct bpf_trampoline *bpf_trampoline_lookup(u64 key, unsigned long ip)
{
struct bpf_trampoline *tr;
@@ -155,14 +299,11 @@ static struct bpf_trampoline *bpf_trampoline_lookup(u64 key, unsigned long ip)
if (!tr)
goto out;
#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
- tr->fops = kzalloc(sizeof(struct ftrace_ops), GFP_KERNEL);
- if (!tr->fops) {
+ if (direct_ops_alloc(tr)) {
kfree(tr);
tr = NULL;
goto out;
}
- tr->fops->private = tr;
- tr->fops->ops_func = bpf_tramp_ftrace_ops_func;
#endif
tr->key = key;
@@ -187,7 +328,7 @@ static int unregister_fentry(struct bpf_trampoline *tr, void *old_addr)
int ret;
if (tr->func.ftrace_managed)
- ret = unregister_ftrace_direct(tr->fops, (long)old_addr, false);
+ ret = direct_ops_del(tr->fops, tr->ip, old_addr);
else
ret = bpf_arch_text_poke(ip, BPF_MOD_CALL, old_addr, NULL);
@@ -201,10 +342,7 @@ static int modify_fentry(struct bpf_trampoline *tr, void *old_addr, void *new_ad
int ret;
if (tr->func.ftrace_managed) {
- if (lock_direct_mutex)
- ret = modify_ftrace_direct(tr->fops, (long)new_addr);
- else
- ret = modify_ftrace_direct_nolock(tr->fops, (long)new_addr);
+ ret = direct_ops_mod(tr->fops, tr->ip, new_addr, lock_direct_mutex);
} else {
ret = bpf_arch_text_poke(ip, BPF_MOD_CALL, old_addr, new_addr);
}
@@ -226,8 +364,7 @@ static int register_fentry(struct bpf_trampoline *tr, void *new_addr)
}
if (tr->func.ftrace_managed) {
- ftrace_set_filter_ip(tr->fops, (unsigned long)ip, 0, 1);
- ret = register_ftrace_direct(tr->fops, (long)new_addr);
+ ret = direct_ops_add(tr->fops, tr->ip, new_addr);
} else {
ret = bpf_arch_text_poke(ip, BPF_MOD_CALL, NULL, new_addr);
}
@@ -863,10 +1000,7 @@ void bpf_trampoline_put(struct bpf_trampoline *tr)
*/
hlist_del(&tr->hlist_key);
hlist_del(&tr->hlist_ip);
- if (tr->fops) {
- ftrace_free_filter(tr->fops);
- kfree(tr->fops);
- }
+ direct_ops_free(tr);
kfree(tr);
out:
mutex_unlock(&trampoline_mutex);
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index d2c79da81e4f..4bf5beb04a5b 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -50,6 +50,9 @@ config HAVE_DYNAMIC_FTRACE_WITH_REGS
config HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
bool
+config HAVE_SINGLE_FTRACE_DIRECT_OPS
+ bool
+
config HAVE_DYNAMIC_FTRACE_WITH_CALL_OPS
bool
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 03948ec81434..e223fc196567 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -2605,8 +2605,13 @@ unsigned long ftrace_find_rec_direct(unsigned long ip)
static void call_direct_funcs(unsigned long ip, unsigned long pip,
struct ftrace_ops *ops, struct ftrace_regs *fregs)
{
- unsigned long addr = READ_ONCE(ops->direct_call);
+ unsigned long addr;
+#ifdef CONFIG_HAVE_SINGLE_FTRACE_DIRECT_OPS
+ addr = ftrace_find_rec_direct(ip);
+#else
+ addr = READ_ONCE(ops->direct_call);
+#endif
if (!addr)
return;
--
2.51.1
^ permalink raw reply related
* [PATCHv2 bpf-next 7/8] ftrace: Factor ftrace_ops ops_func interface
From: Jiri Olsa @ 2025-11-13 12:37 UTC (permalink / raw)
To: Steven Rostedt, Florent Revest, Mark Rutland
Cc: bpf, linux-kernel, linux-trace-kernel, linux-arm-kernel,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Menglong Dong, Song Liu
In-Reply-To: <20251113123750.2507435-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 bacb6d9ab426..d4246d02ea0d 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -394,7 +394,7 @@ enum ftrace_ops_cmd {
* Negative on failure. The return value is dependent on the
* callback.
*/
-typedef int (*ftrace_ops_func_t)(struct ftrace_ops *op, enum ftrace_ops_cmd cmd);
+typedef int (*ftrace_ops_func_t)(struct ftrace_ops *op, unsigned long ip, enum ftrace_ops_cmd cmd);
#ifdef CONFIG_DYNAMIC_FTRACE
diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index 9ca75b22aae0..26887a0db955 100644
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -33,7 +33,8 @@ static DEFINE_MUTEX(trampoline_mutex);
#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
static int bpf_trampoline_update(struct bpf_trampoline *tr, bool lock_direct_mutex);
-static int bpf_tramp_ftrace_ops_func(struct ftrace_ops *ops, enum ftrace_ops_cmd cmd)
+static int bpf_tramp_ftrace_ops_func(struct ftrace_ops *ops, unsigned long ip,
+ enum ftrace_ops_cmd cmd)
{
struct bpf_trampoline *tr = ops->private;
int ret = 0;
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index e701516a4a65..03948ec81434 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -2049,7 +2049,7 @@ static int __ftrace_hash_update_ipmodify(struct ftrace_ops *ops,
*/
if (!ops->ops_func)
return -EBUSY;
- ret = ops->ops_func(ops, FTRACE_OPS_CMD_ENABLE_SHARE_IPMODIFY_SELF);
+ ret = ops->ops_func(ops, rec->ip, FTRACE_OPS_CMD_ENABLE_SHARE_IPMODIFY_SELF);
if (ret)
return ret;
} else if (is_ipmodify) {
@@ -8965,7 +8965,7 @@ static int prepare_direct_functions_for_ipmodify(struct ftrace_ops *ops)
if (!op->ops_func)
return -EBUSY;
- ret = op->ops_func(op, FTRACE_OPS_CMD_ENABLE_SHARE_IPMODIFY_PEER);
+ ret = op->ops_func(op, ip, FTRACE_OPS_CMD_ENABLE_SHARE_IPMODIFY_PEER);
if (ret)
return ret;
}
@@ -9012,7 +9012,7 @@ static void cleanup_direct_functions_after_ipmodify(struct ftrace_ops *ops)
/* The cleanup is optional, ignore any errors */
if (found_op && op->ops_func)
- op->ops_func(op, FTRACE_OPS_CMD_DISABLE_SHARE_IPMODIFY_PEER);
+ op->ops_func(op, ip, FTRACE_OPS_CMD_DISABLE_SHARE_IPMODIFY_PEER);
}
}
mutex_unlock(&direct_mutex);
--
2.51.1
^ permalink raw reply related
* [PATCHv2 bpf-next 6/8] bpf: Add trampoline ip hash table
From: Jiri Olsa @ 2025-11-13 12:37 UTC (permalink / raw)
To: Steven Rostedt, Florent Revest, Mark Rutland
Cc: bpf, linux-kernel, linux-trace-kernel, linux-arm-kernel,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Menglong Dong, Song Liu
In-Reply-To: <20251113123750.2507435-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 09d5dc541d1c..ad86857a8562 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1295,14 +1295,17 @@ struct bpf_tramp_image {
};
struct bpf_trampoline {
- /* hlist for trampoline_table */
- struct hlist_node hlist;
+ /* hlist for trampoline_key_table */
+ struct hlist_node hlist_key;
+ /* hlist for trampoline_ip_table */
+ struct hlist_node hlist_ip;
struct ftrace_ops *fops;
/* serializes access to fields of this trampoline */
struct mutex mutex;
refcount_t refcnt;
u32 flags;
u64 key;
+ unsigned long ip;
struct {
struct btf_func_model model;
void *addr;
diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index f2cb0b097093..9ca75b22aae0 100644
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -24,9 +24,10 @@ const struct bpf_prog_ops bpf_extension_prog_ops = {
#define TRAMPOLINE_HASH_BITS 10
#define TRAMPOLINE_TABLE_SIZE (1 << TRAMPOLINE_HASH_BITS)
-static struct hlist_head trampoline_table[TRAMPOLINE_TABLE_SIZE];
+static struct hlist_head trampoline_key_table[TRAMPOLINE_TABLE_SIZE];
+static struct hlist_head trampoline_ip_table[TRAMPOLINE_TABLE_SIZE];
-/* serializes access to trampoline_table */
+/* serializes access to trampoline tables */
static DEFINE_MUTEX(trampoline_mutex);
#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
@@ -135,15 +136,15 @@ void bpf_image_ksym_del(struct bpf_ksym *ksym)
PAGE_SIZE, true, ksym->name);
}
-static struct bpf_trampoline *bpf_trampoline_lookup(u64 key)
+static struct bpf_trampoline *bpf_trampoline_lookup(u64 key, unsigned long ip)
{
struct bpf_trampoline *tr;
struct hlist_head *head;
int i;
mutex_lock(&trampoline_mutex);
- head = &trampoline_table[hash_64(key, TRAMPOLINE_HASH_BITS)];
- hlist_for_each_entry(tr, head, hlist) {
+ head = &trampoline_key_table[hash_64(key, TRAMPOLINE_HASH_BITS)];
+ hlist_for_each_entry(tr, head, hlist_key) {
if (tr->key == key) {
refcount_inc(&tr->refcnt);
goto out;
@@ -164,8 +165,12 @@ static struct bpf_trampoline *bpf_trampoline_lookup(u64 key)
#endif
tr->key = key;
- INIT_HLIST_NODE(&tr->hlist);
- hlist_add_head(&tr->hlist, head);
+ tr->ip = ftrace_location(ip);
+ INIT_HLIST_NODE(&tr->hlist_key);
+ INIT_HLIST_NODE(&tr->hlist_ip);
+ hlist_add_head(&tr->hlist_key, head);
+ head = &trampoline_ip_table[hash_64(tr->ip, TRAMPOLINE_HASH_BITS)];
+ hlist_add_head(&tr->hlist_ip, head);
refcount_set(&tr->refcnt, 1);
mutex_init(&tr->mutex);
for (i = 0; i < BPF_TRAMP_MAX; i++)
@@ -799,7 +804,7 @@ void bpf_trampoline_unlink_cgroup_shim(struct bpf_prog *prog)
prog->aux->attach_btf_id);
bpf_lsm_find_cgroup_shim(prog, &bpf_func);
- tr = bpf_trampoline_lookup(key);
+ tr = bpf_trampoline_lookup(key, 0);
if (WARN_ON_ONCE(!tr))
return;
@@ -819,7 +824,7 @@ struct bpf_trampoline *bpf_trampoline_get(u64 key,
{
struct bpf_trampoline *tr;
- tr = bpf_trampoline_lookup(key);
+ tr = bpf_trampoline_lookup(key, tgt_info->tgt_addr);
if (!tr)
return NULL;
@@ -855,7 +860,8 @@ void bpf_trampoline_put(struct bpf_trampoline *tr)
* fexit progs. The fentry-only trampoline will be freed via
* multiple rcu callbacks.
*/
- hlist_del(&tr->hlist);
+ hlist_del(&tr->hlist_key);
+ hlist_del(&tr->hlist_ip);
if (tr->fops) {
ftrace_free_filter(tr->fops);
kfree(tr->fops);
@@ -1128,7 +1134,9 @@ static int __init init_trampolines(void)
int i;
for (i = 0; i < TRAMPOLINE_TABLE_SIZE; i++)
- INIT_HLIST_HEAD(&trampoline_table[i]);
+ INIT_HLIST_HEAD(&trampoline_key_table[i]);
+ for (i = 0; i < TRAMPOLINE_TABLE_SIZE; i++)
+ INIT_HLIST_HEAD(&trampoline_ip_table[i]);
return 0;
}
late_initcall(init_trampolines);
--
2.51.1
^ permalink raw reply related
* [PATCHv2 bpf-next 5/8] ftrace: Add update_ftrace_direct_mod function
From: Jiri Olsa @ 2025-11-13 12:37 UTC (permalink / raw)
To: Steven Rostedt, Florent Revest, Mark Rutland
Cc: bpf, linux-kernel, linux-trace-kernel, linux-arm-kernel,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Menglong Dong, Song Liu
In-Reply-To: <20251113123750.2507435-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 | 68 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 74 insertions(+)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 433c36c3af3b..bacb6d9ab426 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -544,6 +544,7 @@ int modify_ftrace_direct_nolock(struct ftrace_ops *ops, unsigned long addr);
int update_ftrace_direct_add(struct ftrace_ops *ops, struct ftrace_hash *hash);
int update_ftrace_direct_del(struct ftrace_ops *ops, struct ftrace_hash *hash);
+int update_ftrace_direct_mod(struct ftrace_ops *ops, struct ftrace_hash *hash, bool do_direct_lock);
void ftrace_stub_direct_tramp(void);
@@ -581,6 +582,11 @@ int update_ftrace_direct_del(struct ftrace_ops *ops, struct ftrace_hash *hash)
return -ENODEV;
}
+int modify_ftrace_direct_hash(struct ftrace_ops *ops, struct ftrace_hash *hash, bool do_direct_lock)
+{
+ return -ENODEV;
+}
+
/*
* This must be implemented by the architecture.
* It is the way the ftrace direct_ops helper, when called
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 3dd1e69aceca..e701516a4a65 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -6475,6 +6475,74 @@ int update_ftrace_direct_del(struct ftrace_ops *ops, struct ftrace_hash *hash)
return err;
}
+int update_ftrace_direct_mod(struct ftrace_ops *ops, struct ftrace_hash *hash, bool do_direct_lock)
+{
+ struct ftrace_hash *orig_hash = ops->func_hash->filter_hash;
+ struct ftrace_func_entry *entry, *tmp;
+ static struct ftrace_ops tmp_ops = {
+ .func = ftrace_stub,
+ .flags = FTRACE_OPS_FL_STUB,
+ };
+ unsigned long size, i;
+ int err;
+
+ if (!hash_count(hash))
+ return 0;
+ if (check_direct_multi(ops))
+ return -EINVAL;
+ if (!(ops->flags & FTRACE_OPS_FL_ENABLED))
+ return -EINVAL;
+ if (direct_functions == EMPTY_HASH)
+ return -EINVAL;
+
+ if (do_direct_lock)
+ mutex_lock(&direct_mutex);
+
+ /* Enable the tmp_ops to have the same functions as the direct ops */
+ ftrace_ops_init(&tmp_ops);
+ tmp_ops.func_hash = ops->func_hash;
+
+ err = register_ftrace_function_nolock(&tmp_ops);
+ if (err)
+ goto unlock;
+
+ /*
+ * Call __ftrace_hash_update_ipmodify() here, so that we can call
+ * ops->ops_func for the ops. This is needed because the above
+ * register_ftrace_function_nolock() worked on tmp_ops.
+ */
+ err = __ftrace_hash_update_ipmodify(ops, orig_hash, orig_hash, true);
+ if (err)
+ goto out;
+
+ /*
+ * Now the ftrace_ops_list_func() is called to do the direct callers.
+ * We can safely change the direct functions attached to each entry.
+ */
+ mutex_lock(&ftrace_lock);
+
+ size = 1 << hash->size_bits;
+ for (i = 0; i < size; i++) {
+ hlist_for_each_entry(entry, &hash->buckets[i], hlist) {
+ tmp = __ftrace_lookup_ip(direct_functions, entry->ip);
+ if (!tmp)
+ continue;
+ tmp->direct = entry->direct;
+ }
+ }
+
+ mutex_unlock(&ftrace_lock);
+
+out:
+ /* Removing the tmp_ops will add the updated direct callers to the functions */
+ unregister_ftrace_function(&tmp_ops);
+
+unlock:
+ if (do_direct_lock)
+ mutex_unlock(&direct_mutex);
+ return err;
+}
+
#endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
/**
--
2.51.1
^ permalink raw reply related
* [PATCHv2 bpf-next 4/8] ftrace: Add update_ftrace_direct_del function
From: Jiri Olsa @ 2025-11-13 12:37 UTC (permalink / raw)
To: Steven Rostedt, Florent Revest, Mark Rutland
Cc: bpf, linux-kernel, linux-trace-kernel, linux-arm-kernel,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Menglong Dong, Song Liu
In-Reply-To: <20251113123750.2507435-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 | 99 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 105 insertions(+)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index ded3a306a8b2..433c36c3af3b 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -543,6 +543,7 @@ int modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr);
int modify_ftrace_direct_nolock(struct ftrace_ops *ops, unsigned long addr);
int update_ftrace_direct_add(struct ftrace_ops *ops, struct ftrace_hash *hash);
+int update_ftrace_direct_del(struct ftrace_ops *ops, struct ftrace_hash *hash);
void ftrace_stub_direct_tramp(void);
@@ -575,6 +576,11 @@ int update_ftrace_direct_add(struct ftrace_ops *ops, struct ftrace_hash *hash)
return -ENODEV;
}
+int update_ftrace_direct_del(struct ftrace_ops *ops, struct ftrace_hash *hash)
+{
+ return -ENODEV;
+}
+
/*
* This must be implemented by the architecture.
* It is the way the ftrace direct_ops helper, when called
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 0b6e1af8f922..3dd1e69aceca 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -6376,6 +6376,105 @@ int update_ftrace_direct_add(struct ftrace_ops *ops, struct ftrace_hash *hash)
return err;
}
+/**
+ * hash_sub - substracts @b from @a and returns the result
+ * @a: struct ftrace_hash object
+ * @b: struct ftrace_hash object
+ *
+ * Returns struct ftrace_hash object on success, NULL on error.
+ */
+static struct ftrace_hash *hash_sub(struct ftrace_hash *a, struct ftrace_hash *b)
+{
+ struct ftrace_func_entry *entry, *del;
+ struct ftrace_hash *sub;
+ int size, i;
+
+ sub = alloc_and_copy_ftrace_hash(a->size_bits, a);
+ if (!sub)
+ goto error;
+
+ size = 1 << b->size_bits;
+ for (i = 0; i < size; i++) {
+ hlist_for_each_entry(entry, &b->buckets[i], hlist) {
+ del = __ftrace_lookup_ip(sub, entry->ip);
+ if (WARN_ON_ONCE(!del))
+ goto error;
+ remove_hash_entry(sub, del);
+ kfree(del);
+ }
+ }
+ return sub;
+
+ error:
+ free_ftrace_hash(sub);
+ return NULL;
+}
+
+int update_ftrace_direct_del(struct ftrace_ops *ops, struct ftrace_hash *hash)
+{
+ struct ftrace_hash *new_hash = NULL, *filter_hash = NULL, *free_hash = NULL;
+ struct ftrace_func_entry *del, *entry;
+ unsigned long size, i;
+ int err = -EINVAL;
+
+ if (!hash_count(hash))
+ return 0;
+ if (check_direct_multi(ops))
+ return -EINVAL;
+ if (!(ops->flags & FTRACE_OPS_FL_ENABLED))
+ return -EINVAL;
+ if (direct_functions == EMPTY_HASH)
+ return -EINVAL;
+
+ mutex_lock(&direct_mutex);
+
+ /* Make sure requested entries are already registered. */
+ size = 1 << hash->size_bits;
+ for (i = 0; i < size; i++) {
+ hlist_for_each_entry(entry, &hash->buckets[i], hlist) {
+ del = __ftrace_lookup_ip(direct_functions, entry->ip);
+ if (!del || del->direct != entry->direct)
+ goto out_unlock;
+ }
+ }
+
+ err = -ENOMEM;
+ filter_hash = hash_sub(ops->func_hash->filter_hash, hash);
+ if (!filter_hash)
+ goto out_unlock;
+
+ new_hash = hash_sub(direct_functions, hash);
+ if (!new_hash)
+ goto out_unlock;
+
+ /* If there's nothing left, we need to unregister the ops. */
+ if (ftrace_hash_empty(filter_hash)) {
+ err = unregister_ftrace_function(ops);
+ /* cleanup for possible another register call */
+ ops->func = NULL;
+ ops->trampoline = 0;
+ ftrace_free_filter(ops);
+ ops->func_hash->filter_hash = NULL;
+ } else {
+ err = ftrace_update_ops(ops, filter_hash, EMPTY_HASH);
+ }
+
+ if (!err) {
+ free_hash = direct_functions;
+ rcu_assign_pointer(direct_functions, new_hash);
+ }
+
+ out_unlock:
+ mutex_unlock(&direct_mutex);
+
+ if (free_hash && free_hash != EMPTY_HASH)
+ call_rcu_tasks(&free_hash->rcu, register_ftrace_direct_cb);
+ if (filter_hash)
+ free_ftrace_hash(filter_hash);
+
+ return err;
+}
+
#endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
/**
--
2.51.1
^ permalink raw reply related
* [PATCHv2 bpf-next 3/8] ftrace: Add update_ftrace_direct_add function
From: Jiri Olsa @ 2025-11-13 12:37 UTC (permalink / raw)
To: Steven Rostedt, Florent Revest, Mark Rutland
Cc: bpf, linux-kernel, linux-trace-kernel, linux-arm-kernel,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Menglong Dong, Song Liu
In-Reply-To: <20251113123750.2507435-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 e23e6a859e08..ded3a306a8b2 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -542,6 +542,8 @@ int unregister_ftrace_direct(struct ftrace_ops *ops, unsigned long addr,
int modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr);
int modify_ftrace_direct_nolock(struct ftrace_ops *ops, unsigned long addr);
+int update_ftrace_direct_add(struct ftrace_ops *ops, struct ftrace_hash *hash);
+
void ftrace_stub_direct_tramp(void);
#else
@@ -568,6 +570,11 @@ static inline int modify_ftrace_direct_nolock(struct ftrace_ops *ops, unsigned l
return -ENODEV;
}
+int update_ftrace_direct_add(struct ftrace_ops *ops, struct ftrace_hash *hash)
+{
+ return -ENODEV;
+}
+
/*
* This must be implemented by the architecture.
* It is the way the ftrace direct_ops helper, when called
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index e6ccf572c5f6..0b6e1af8f922 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -6248,6 +6248,134 @@ int modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
return err;
}
EXPORT_SYMBOL_GPL(modify_ftrace_direct);
+
+static unsigned long hash_count(struct ftrace_hash *hash)
+{
+ return hash ? hash->count : 0;
+}
+
+/**
+ * hash_add - adds two struct ftrace_hash and returns the result
+ * @a: struct ftrace_hash object
+ * @b: struct ftrace_hash object
+ *
+ * Returns struct ftrace_hash object on success, NULL on error.
+ */
+static struct ftrace_hash *hash_add(struct ftrace_hash *a, struct ftrace_hash *b)
+{
+ struct ftrace_func_entry *entry;
+ struct ftrace_hash *add;
+ int size, i;
+
+ size = hash_count(a) + hash_count(b);
+ if (size > 32)
+ size = 32;
+
+ add = alloc_and_copy_ftrace_hash(fls(size), a);
+ if (!add)
+ goto error;
+
+ size = 1 << b->size_bits;
+ for (i = 0; i < size; i++) {
+ hlist_for_each_entry(entry, &b->buckets[i], hlist) {
+ if (add_hash_entry_direct(add, entry->ip, entry->direct) == NULL)
+ goto error;
+ }
+ }
+ return add;
+
+ error:
+ free_ftrace_hash(add);
+ return NULL;
+}
+
+int update_ftrace_direct_add(struct ftrace_ops *ops, struct ftrace_hash *hash)
+{
+ struct ftrace_hash *old_direct_functions = NULL, *new_direct_functions = NULL;
+ struct ftrace_hash *old_filter_hash = NULL, *new_filter_hash = NULL;
+ struct ftrace_func_entry *entry;
+ int i, size, err = -EINVAL;
+ bool reg;
+
+ if (!hash_count(hash))
+ return 0;
+
+ mutex_lock(&direct_mutex);
+
+ /* Make sure requested entries are not already registered. */
+ size = 1 << hash->size_bits;
+ for (i = 0; i < size; i++) {
+ hlist_for_each_entry(entry, &hash->buckets[i], hlist) {
+ if (__ftrace_lookup_ip(direct_functions, entry->ip))
+ goto out_unlock;
+ }
+ }
+
+ old_filter_hash = ops->func_hash ? ops->func_hash->filter_hash : NULL;
+ old_direct_functions = direct_functions;
+
+ /* If there's nothing in filter_hash we need to register the ops. */
+ reg = hash_count(old_filter_hash) == 0;
+ if (reg) {
+ if (ops->func || ops->trampoline)
+ goto out_unlock;
+ if (ops->flags & FTRACE_OPS_FL_ENABLED)
+ goto out_unlock;
+ }
+
+ err = -ENOMEM;
+ new_filter_hash = hash_add(old_filter_hash, hash);
+ if (!new_filter_hash)
+ goto out_unlock;
+
+ new_direct_functions = hash_add(old_direct_functions, hash);
+ if (!new_direct_functions)
+ goto out_unlock;
+
+ rcu_assign_pointer(direct_functions, new_direct_functions);
+
+ if (reg) {
+ ops->func = call_direct_funcs;
+ ops->flags |= MULTI_FLAGS;
+ ops->trampoline = FTRACE_REGS_ADDR;
+ ops->local_hash.filter_hash = new_filter_hash;
+
+ err = register_ftrace_function_nolock(ops);
+ if (err) {
+ /* restore old filter on error */
+ ops->local_hash.filter_hash = old_filter_hash;
+ old_filter_hash = new_filter_hash;
+
+ /* cleanup for possible another register call */
+ ops->func = NULL;
+ ops->trampoline = 0;
+ }
+ } else {
+ err = ftrace_update_ops(ops, new_filter_hash, EMPTY_HASH);
+ /*
+ * new_filter_hash is dup-ed, so we need to release it anyway,
+ * old_filter_hash either stays on error or is released already
+ */
+ old_filter_hash = new_filter_hash;
+ }
+
+ if (err) {
+ /* reset direct_functions and free the new one */
+ rcu_assign_pointer(direct_functions, old_direct_functions);
+ old_direct_functions = new_direct_functions;
+ }
+
+ out_unlock:
+ mutex_unlock(&direct_mutex);
+
+ if (old_direct_functions && old_direct_functions != EMPTY_HASH)
+ call_rcu_tasks(&old_direct_functions->rcu, register_ftrace_direct_cb);
+ if (old_filter_hash)
+ free_ftrace_hash(old_filter_hash);
+
+ return err;
+}
+
#endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
/**
--
2.51.1
^ permalink raw reply related
* [PATCHv2 bpf-next 2/8] ftrace: Export some of hash related functions
From: Jiri Olsa @ 2025-11-13 12:37 UTC (permalink / raw)
To: Steven Rostedt, Florent Revest, Mark Rutland
Cc: bpf, linux-kernel, linux-trace-kernel, linux-arm-kernel,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Menglong Dong, Song Liu
In-Reply-To: <20251113123750.2507435-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 7ded7df6e9b5..e23e6a859e08 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -397,6 +397,22 @@ enum ftrace_ops_cmd {
typedef int (*ftrace_ops_func_t)(struct ftrace_ops *op, enum ftrace_ops_cmd cmd);
#ifdef CONFIG_DYNAMIC_FTRACE
+
+#define FTRACE_HASH_DEFAULT_BITS 10
+
+struct ftrace_hash {
+ unsigned long size_bits;
+ struct hlist_head *buckets;
+ unsigned long count;
+ unsigned long flags;
+ struct rcu_head rcu;
+};
+
+struct ftrace_hash *alloc_ftrace_hash(int size_bits);
+void free_ftrace_hash(struct ftrace_hash *hash);
+struct ftrace_func_entry *add_hash_entry_direct(struct ftrace_hash *hash,
+ unsigned long ip, unsigned long direct);
+
/* The hash used to know what functions callbacks trace */
struct ftrace_ops_hash {
struct ftrace_hash __rcu *notrace_hash;
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 0e3714b796d9..e6ccf572c5f6 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -68,7 +68,6 @@
})
/* hash bits for specific function selection */
-#define FTRACE_HASH_DEFAULT_BITS 10
#define FTRACE_HASH_MAX_BITS 12
#ifdef CONFIG_DYNAMIC_FTRACE
@@ -1185,7 +1184,7 @@ static void __add_hash_entry(struct ftrace_hash *hash,
hash->count++;
}
-static struct ftrace_func_entry *
+struct ftrace_func_entry *
add_hash_entry_direct(struct ftrace_hash *hash, unsigned long ip, unsigned long direct)
{
struct ftrace_func_entry *entry;
@@ -1265,7 +1264,7 @@ static void clear_ftrace_mod_list(struct list_head *head)
mutex_unlock(&ftrace_lock);
}
-static void free_ftrace_hash(struct ftrace_hash *hash)
+void free_ftrace_hash(struct ftrace_hash *hash)
{
if (!hash || hash == EMPTY_HASH)
return;
@@ -1305,7 +1304,7 @@ void ftrace_free_filter(struct ftrace_ops *ops)
}
EXPORT_SYMBOL_GPL(ftrace_free_filter);
-static struct ftrace_hash *alloc_ftrace_hash(int size_bits)
+struct ftrace_hash *alloc_ftrace_hash(int size_bits)
{
struct ftrace_hash *hash;
int size;
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 85eabb454bee..62e0ac625f65 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -899,14 +899,6 @@ enum {
FTRACE_HASH_FL_MOD = (1 << 0),
};
-struct ftrace_hash {
- unsigned long size_bits;
- struct hlist_head *buckets;
- unsigned long count;
- unsigned long flags;
- struct rcu_head rcu;
-};
-
struct ftrace_func_entry *
ftrace_lookup_ip(struct ftrace_hash *hash, unsigned long ip);
--
2.51.1
^ permalink raw reply related
* [PATCHv2 bpf-next 1/8] ftrace: Make alloc_and_copy_ftrace_hash direct friendly
From: Jiri Olsa @ 2025-11-13 12:37 UTC (permalink / raw)
To: Steven Rostedt, Florent Revest, Mark Rutland
Cc: bpf, linux-kernel, linux-trace-kernel, linux-arm-kernel,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Menglong Dong, Song Liu
In-Reply-To: <20251113123750.2507435-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 59cfacb8a5bb..0e3714b796d9 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -1186,7 +1186,7 @@ static void __add_hash_entry(struct ftrace_hash *hash,
}
static struct ftrace_func_entry *
-add_hash_entry(struct ftrace_hash *hash, unsigned long ip)
+add_hash_entry_direct(struct ftrace_hash *hash, unsigned long ip, unsigned long direct)
{
struct ftrace_func_entry *entry;
@@ -1195,11 +1195,18 @@ add_hash_entry(struct ftrace_hash *hash, unsigned long ip)
return NULL;
entry->ip = ip;
+ entry->direct = direct;
__add_hash_entry(hash, entry);
return entry;
}
+static struct ftrace_func_entry *
+add_hash_entry(struct ftrace_hash *hash, unsigned long ip)
+{
+ return add_hash_entry_direct(hash, ip, 0);
+}
+
static void
free_hash_entry(struct ftrace_hash *hash,
struct ftrace_func_entry *entry)
@@ -1372,7 +1379,7 @@ alloc_and_copy_ftrace_hash(int size_bits, struct ftrace_hash *hash)
size = 1 << hash->size_bits;
for (i = 0; i < size; i++) {
hlist_for_each_entry(entry, &hash->buckets[i], hlist) {
- if (add_hash_entry(new_hash, entry->ip) == NULL)
+ if (add_hash_entry_direct(new_hash, entry->ip, entry->direct) == NULL)
goto free_hash;
}
}
--
2.51.1
^ permalink raw reply related
* [PATCHv2 bpf-next 0/9] ftrace,bpf: Use single direct ops for bpf trampolines
From: Jiri Olsa @ 2025-11-13 12:37 UTC (permalink / raw)
To: Steven Rostedt, Florent Revest, Mark Rutland
Cc: bpf, linux-kernel, linux-trace-kernel, linux-arm-kernel,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Menglong Dong, Song Liu
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 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 (8):
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 | 37 ++++++++++++++-
kernel/bpf/trampoline.c | 199 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------
kernel/trace/Kconfig | 3 ++
kernel/trace/ftrace.c | 326 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
kernel/trace/trace.h | 8 ----
7 files changed, 532 insertions(+), 49 deletions(-)
^ permalink raw reply
* Re: [RFC PATCH 2/8] mm: Add PG_atomic
From: David Hildenbrand (Red Hat) @ 2025-11-13 12:34 UTC (permalink / raw)
To: Matthew Wilcox, Ojaswin Mujoo
Cc: Christian Brauner, djwong, ritesh.list, john.g.garry, tytso,
dchinner, hch, linux-xfs, linux-kernel, linux-ext4, linux-fsdevel,
linux-mm, jack, nilay, martin.petersen, rostedt, axboe,
linux-block, linux-trace-kernel
In-Reply-To: <aRSuH82gM-8BzPCU@casper.infradead.org>
On 12.11.25 16:56, Matthew Wilcox wrote:
> On Wed, Nov 12, 2025 at 04:36:05PM +0530, Ojaswin Mujoo wrote:
>> From: John Garry <john.g.garry@oracle.com>
>>
>> Add page flag PG_atomic, meaning that a folio needs to be written back
>> atomically. This will be used by for handling RWF_ATOMIC buffered IO
>> in upcoming patches.
>
> Page flags are a precious resource. I'm not thrilled about allocating one
> to this rather niche usecase.
Fully agreed.
--
Cheers
David
^ permalink raw reply
* Re: [trace:latency/for-next 3/5] kernel/trace/rv/monitors/sleep/sleep.c:219:26: error: static declaration of 'rv_sleep' follows non-static declaration
From: Gabriele Monaco @ 2025-11-13 12:09 UTC (permalink / raw)
To: Thomas Weißschuh, Nam Cao; +Cc: oe-kbuild-all, linux-trace-kernel
In-Reply-To: <202511131948.vxi5mdjU-lkp@intel.com>
On Thu, 2025-11-13 at 19:41 +0800, kernel test robot wrote:
> tree:
> https://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace latency/for-
> next
> head: 69d8895cb9a9f6450374577af8584c2e21cb5a9f
> commit: 4f739ed19d222de33b19ca639a34523fbbec20d0 [3/5] rv: Pass va_list to
> reactors
> config: arc-randconfig-r071-20251113
> (https://download.01.org/0day-ci/archive/20251113/202511131948.vxi5mdjU-lkp@in
> tel.com/config)
> compiler: arc-linux-gcc (GCC) 8.5.0
> reproduce (this is a W=1 build):
> (https://download.01.org/0day-ci/archive/20251113/202511131948.vxi5mdjU-lkp@in
> tel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version
> of
> the same patch/commit), kindly add following tags
> > Reported-by: kernel test robot <lkp@intel.com>
> > Closes:
> > https://lore.kernel.org/oe-kbuild-all/202511131948.vxi5mdjU-lkp@intel.com/
>
> All errors (new ones prefixed by >>):
>
> > > kernel/trace/rv/monitors/sleep/sleep.c:219:26: error: static declaration
> > > of 'rv_sleep' follows non-static declaration
> static struct rv_monitor rv_sleep = {
> ^~~~~~~~
My bad I didn't test with !CONFIG_RV_REACTORS, is there a reason why we'd want
this non-static?
#ifdef CONFIG_RV_REACTORS
static struct rv_monitor RV_MONITOR_NAME;
#else
extern struct rv_monitor RV_MONITOR_NAME;
#endif
ltl_monitor is included only by the files defining the rv_MONITOR_NAME, that's
definitely static as far as I see it.
I'd go with a Fix like this:
diff --git a/include/rv/ltl_monitor.h b/include/rv/ltl_monitor.h
index 00c42b36f961..eff60cd61106 100644
--- a/include/rv/ltl_monitor.h
+++ b/include/rv/ltl_monitor.h
@@ -17,12 +17,7 @@
#endif
#define RV_MONITOR_NAME CONCATENATE(rv_, MONITOR_NAME)
-
-#ifdef CONFIG_RV_REACTORS
static struct rv_monitor RV_MONITOR_NAME;
-#else
-extern struct rv_monitor RV_MONITOR_NAME;
-#endif
static int ltl_monitor_slot = RV_PER_TASK_MONITOR_INIT;
Any thought?
Thanks,
Gabriele
> In file included from include/linux/cleanup.h:7,
> from include/linux/interrupt.h:8,
> from include/linux/trace_recursion.h:5,
> from include/linux/ftrace.h:10,
> from kernel/trace/rv/monitors/sleep/sleep.c:2:
> include/rv/ltl_monitor.h:19:37: note: previous declaration of 'rv_sleep'
> was here
> #define RV_MONITOR_NAME CONCATENATE(rv_, MONITOR_NAME)
> ^~~
> include/linux/args.h:25:24: note: in definition of macro '__CONCAT'
> #define __CONCAT(a, b) a ## b
> ^
> include/rv/ltl_monitor.h:19:25: note: in expansion of macro 'CONCATENATE'
> #define RV_MONITOR_NAME CONCATENATE(rv_, MONITOR_NAME)
> ^~~~~~~~~~~
> include/rv/ltl_monitor.h:24:26: note: in expansion of macro
> 'RV_MONITOR_NAME'
> extern struct rv_monitor RV_MONITOR_NAME;
> ^~~~~~~~~~~~~~~
>
>
> vim +/rv_sleep +219 kernel/trace/rv/monitors/sleep/sleep.c
>
> f74f8bb246cf22 Nam Cao 2025-07-09 218
> f74f8bb246cf22 Nam Cao 2025-07-09 @219 static struct rv_monitor rv_sleep = {
> f74f8bb246cf22 Nam Cao 2025-07-09 220 .name = "sleep",
> f74f8bb246cf22 Nam Cao 2025-07-09 221 .description = "Monitor that
> RT tasks do not undesirably sleep",
> f74f8bb246cf22 Nam Cao 2025-07-09 222 .enable = enable_sleep,
> f74f8bb246cf22 Nam Cao 2025-07-09 223 .disable = disable_sleep,
> f74f8bb246cf22 Nam Cao 2025-07-09 224 };
> f74f8bb246cf22 Nam Cao 2025-07-09 225
>
> :::::: The code at line 219 was first introduced by commit
> :::::: f74f8bb246cf22f27752977da62079cb615f55b2 rv: Add rtapp_sleep monitor
>
> :::::: TO: Nam Cao <namcao@linutronix.de>
> :::::: CC: Steven Rostedt (Google) <rostedt@goodmis.org>
^ permalink raw reply related
* Re: [PATCH v3] trace/pid_list: optimize pid_list->lock contention
From: Yongliang Gao @ 2025-11-13 11:13 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: rostedt, mhiramat, mathieu.desnoyers, linux-kernel,
linux-trace-kernel, frankjpliu, Yongliang Gao, Huang Cun
In-Reply-To: <20251113073420.yko6jYcI@linutronix.de>
Hi Sebastian,
Thank you for your review and the thoughtful questions.
1. Performance Data
We encountered this issue in a production environment with 288 cores
where enabling set_ftrace_pid caused system CPU usage (sys%) to
increase from 10% to over 90%. In our 92-core VM test environment:
Before patch (spinlock):
- Without filtering: cs=2395401/s, sys%=7%
- With filtering: cs=1828261/s, sys%=40%
After patch (seqlock):
- Without filtering: cs=2397032/s, sys%=6%
- With filtering: cs=2398922/s, sys%=6%
The seqlock approach eliminates the pid_list->lock contention that was
previously causing sys% to increase from 7% to 40%.
2. Reader Retry Behavior
Yes, if the write side is continuously busy, the reader might spin and
retry. However, in practice:
- Writes are infrequent (only when setting ftrace_pid filter or during
task fork/exit with function-fork enabled)
- For readers, trace_pid_list_is_set() is called on every task switch,
which can occur at a very high frequency.
3. Result Accuracy
You're correct that the result might change immediately after the
read. For trace_ignore_this_task(), we don't require absolute
accuracy. Slight race conditions (where a task might be traced or not
in borderline cases) are acceptable.
Best regards,
Yongliang
On Thu, Nov 13, 2025 at 3:34 PM Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
>
> On 2025-11-13 08:02:52 [+0800], Yongliang Gao wrote:
> > --- a/kernel/trace/pid_list.c
> > +++ b/kernel/trace/pid_list.c
> > @@ -138,14 +139,16 @@ bool trace_pid_list_is_set(struct trace_pid_list *pid_list, unsigned int pid)
> > if (pid_split(pid, &upper1, &upper2, &lower) < 0)
> > return false;
> >
> > - raw_spin_lock_irqsave(&pid_list->lock, flags);
> > - upper_chunk = pid_list->upper[upper1];
> > - if (upper_chunk) {
> > - lower_chunk = upper_chunk->data[upper2];
> > - if (lower_chunk)
> > - ret = test_bit(lower, lower_chunk->data);
> > - }
> > - raw_spin_unlock_irqrestore(&pid_list->lock, flags);
> > + do {
> > + seq = read_seqcount_begin(&pid_list->seqcount);
> > + ret = false;
> > + upper_chunk = pid_list->upper[upper1];
> > + if (upper_chunk) {
> > + lower_chunk = upper_chunk->data[upper2];
> > + if (lower_chunk)
> > + ret = test_bit(lower, lower_chunk->data);
> > + }
> > + } while (read_seqcount_retry(&pid_list->seqcount, seq));
>
> How is this better? Any numbers?
> If the write side is busy and the lock is handed over from one CPU to
> another then it is possible that the reader spins here and does several
> loops, right?
> And in this case, how accurate would it be? I mean the result could
> change right after the sequence here is completed because the write side
> got active again. How bad would it be if there would be no locking and
> RCU ensures that the chunks (and data) don't disappear while looking at
> it?
>
> > return ret;
> > }
>
> Sebastian
^ permalink raw reply
* Re: [RFC PATCH 0/8] xfs: single block atomic writes for buffered IO
From: Dave Chinner @ 2025-11-13 10:32 UTC (permalink / raw)
To: Ritesh Harjani
Cc: Christoph Hellwig, Ojaswin Mujoo, Christian Brauner, djwong,
john.g.garry, tytso, willy, dchinner, linux-xfs, linux-kernel,
linux-ext4, linux-fsdevel, linux-mm, jack, nilay, martin.petersen,
rostedt, axboe, linux-block, linux-trace-kernel
In-Reply-To: <87frai8p46.ritesh.list@gmail.com>
On Thu, Nov 13, 2025 at 11:12:49AM +0530, Ritesh Harjani wrote:
> Christoph Hellwig <hch@lst.de> writes:
>
> > On Thu, Nov 13, 2025 at 08:56:56AM +1100, Dave Chinner wrote:
> >> On Wed, Nov 12, 2025 at 04:36:03PM +0530, Ojaswin Mujoo wrote:
> >> > This patch adds support to perform single block RWF_ATOMIC writes for
> >> > iomap xfs buffered IO. This builds upon the inital RFC shared by John
> >> > Garry last year [1]. Most of the details are present in the respective
> >> > commit messages but I'd mention some of the design points below:
> >>
> >> What is the use case for this functionality? i.e. what is the
> >> reason for adding all this complexity?
> >
> > Seconded. The atomic code has a lot of complexity, and further mixing
> > it with buffered I/O makes this even worse. We'd need a really important
> > use case to even consider it.
>
> I agree this should have been in the cover letter itself.
>
> I believe the reason for adding this functionality was also discussed at
> LSFMM too...
>
> For e.g. https://lwn.net/Articles/974578/ goes in depth and talks about
> Postgres folks looking for this, since PostgreSQL databases uses
> buffered I/O for their database writes.
Pointing at a discussion about how "this application has some ideas
on how it can maybe use it someday in the future" isn't a
particularly good justification. This still sounds more like a
research project than something a production system needs right now.
Why didn't you use the existing COW buffered write IO path to
implement atomic semantics for buffered writes? The XFS
functionality is already all there, and it doesn't require any
changes to the page cache or iomap to support...
-Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply
* Re: [BUG/RFC 1/2] arm64/ftrace,bpf: Fix partial regs after bpf_prog_run
From: Jiri Olsa @ 2025-11-13 7:51 UTC (permalink / raw)
To: Masami Hiramatsu, Steven Rostedt, Will Deacon
Cc: Peter Zijlstra, bpf, linux-trace-kernel, linux-arm-kernel, x86,
Yonghong Song, Song Liu, Andrii Nakryiko, Mark Rutland,
Mahe Tardy
In-Reply-To: <20251105125924.365205-1-jolsa@kernel.org>
ping, thanks
jirka
On Wed, Nov 05, 2025 at 01:59:23PM +0100, Jiri Olsa wrote:
> hi,
> Mahe reported issue with bpf_override_return helper not working
> when executed from kprobe.multi bpf program on arm.
>
> The problem seems to be that on arm we use alternate storage for
> pt_regs object that is passed to bpf_prog_run and if any register
> is changed (which is the case of bpf_override_return) it's not
> propagated back to actual pt_regs object.
>
> The change below seems to fix the issue, but I have no idea if
> that's proper fix for arm, thoughts?
>
> I'm attaching selftest to actually test bpf_override_return helper
> functionality, because currently we only test that we are able to
> attach a program with it, but not the override itself.
>
> thanks,
> jirka
>
>
> ---
> arch/arm64/include/asm/ftrace.h | 11 +++++++++++
> include/linux/ftrace.h | 3 +++
> kernel/trace/bpf_trace.c | 1 +
> 3 files changed, 15 insertions(+)
>
> diff --git a/arch/arm64/include/asm/ftrace.h b/arch/arm64/include/asm/ftrace.h
> index ba7cf7fec5e9..ad6cf587885c 100644
> --- a/arch/arm64/include/asm/ftrace.h
> +++ b/arch/arm64/include/asm/ftrace.h
> @@ -157,6 +157,17 @@ ftrace_partial_regs(const struct ftrace_regs *fregs, struct pt_regs *regs)
> return regs;
> }
>
> +static __always_inline void
> +ftrace_partial_regs_fix(const struct ftrace_regs *fregs, struct pt_regs *regs)
> +{
> + struct __arch_ftrace_regs *afregs = arch_ftrace_regs(fregs);
> +
> + if (afregs->pc != regs->pc) {
> + afregs->pc = regs->pc;
> + afregs->regs[0] = regs->regs[0];
> + }
> +}
> +
> #define arch_ftrace_fill_perf_regs(fregs, _regs) do { \
> (_regs)->pc = arch_ftrace_regs(fregs)->pc; \
> (_regs)->regs[29] = arch_ftrace_regs(fregs)->fp; \
> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index 7ded7df6e9b5..4cb1315522bb 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
> @@ -205,6 +205,9 @@ ftrace_partial_regs(struct ftrace_regs *fregs, struct pt_regs *regs)
> return &arch_ftrace_regs(fregs)->regs;
> }
>
> +static __always_inline void
> +ftrace_partial_regs_fix(struct ftrace_regs *fregs, struct pt_regs *regs) { }
> +
> #endif /* !CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS || CONFIG_HAVE_FTRACE_REGS_HAVING_PT_REGS */
>
> #ifdef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index a795f7afbf3d..7b5768ced9b3 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -2564,6 +2564,7 @@ kprobe_multi_link_prog_run(struct bpf_kprobe_multi_link *link,
> old_run_ctx = bpf_set_run_ctx(&run_ctx.session_ctx.run_ctx);
> err = bpf_prog_run(link->link.prog, regs);
> bpf_reset_run_ctx(old_run_ctx);
> + ftrace_partial_regs_fix(fregs, bpf_kprobe_multi_pt_regs_ptr());
> rcu_read_unlock();
>
> out:
> --
> 2.51.1
>
^ permalink raw reply
* Re: [PATCH v3] trace/pid_list: optimize pid_list->lock contention
From: Sebastian Andrzej Siewior @ 2025-11-13 7:34 UTC (permalink / raw)
To: Yongliang Gao
Cc: rostedt, mhiramat, mathieu.desnoyers, linux-kernel,
linux-trace-kernel, frankjpliu, Yongliang Gao, Huang Cun
In-Reply-To: <20251113000252.1058144-1-leonylgao@gmail.com>
On 2025-11-13 08:02:52 [+0800], Yongliang Gao wrote:
> --- a/kernel/trace/pid_list.c
> +++ b/kernel/trace/pid_list.c
> @@ -138,14 +139,16 @@ bool trace_pid_list_is_set(struct trace_pid_list *pid_list, unsigned int pid)
> if (pid_split(pid, &upper1, &upper2, &lower) < 0)
> return false;
>
> - raw_spin_lock_irqsave(&pid_list->lock, flags);
> - upper_chunk = pid_list->upper[upper1];
> - if (upper_chunk) {
> - lower_chunk = upper_chunk->data[upper2];
> - if (lower_chunk)
> - ret = test_bit(lower, lower_chunk->data);
> - }
> - raw_spin_unlock_irqrestore(&pid_list->lock, flags);
> + do {
> + seq = read_seqcount_begin(&pid_list->seqcount);
> + ret = false;
> + upper_chunk = pid_list->upper[upper1];
> + if (upper_chunk) {
> + lower_chunk = upper_chunk->data[upper2];
> + if (lower_chunk)
> + ret = test_bit(lower, lower_chunk->data);
> + }
> + } while (read_seqcount_retry(&pid_list->seqcount, seq));
How is this better? Any numbers?
If the write side is busy and the lock is handed over from one CPU to
another then it is possible that the reader spins here and does several
loops, right?
And in this case, how accurate would it be? I mean the result could
change right after the sequence here is completed because the write side
got active again. How bad would it be if there would be no locking and
RCU ensures that the chunks (and data) don't disappear while looking at
it?
> return ret;
> }
Sebastian
^ permalink raw reply
* [PATCH v5] function_graph: Enable funcgraph-args and funcgraph-retaddr to work simultaneously
From: Donglin Peng @ 2025-11-13 7:29 UTC (permalink / raw)
To: rostedt
Cc: linux-trace-kernel, linux-kernel, Donglin Peng, Sven Schnelle,
Masami Hiramatsu
From: Donglin Peng <pengdonglin@xiaomi.com>
Currently, the funcgraph-args and funcgraph-retaddr features are
mutually exclusive. This patch resolves this limitation by modifying
funcgraph-retaddr to adopt the same implementation approach as
funcgraph-args, specifically by storing the return address in the
first entry of the args array.
As a result, both features now coexist seamlessly and function as
intended.
To verify the change, use perf to trace vfs_write with both options
enabled:
Before:
# perf ftrace -G vfs_write --graph-opts args,retaddr
......
down_read() { /* <-n_tty_write+0xa3/0x540 */
__cond_resched(); /* <-down_read+0x12/0x160 */
preempt_count_add(); /* <-down_read+0x3b/0x160 */
preempt_count_sub(); /* <-down_read+0x8b/0x160 */
}
After:
# perf ftrace -G vfs_write --graph-opts args,retaddr
......
down_read(sem=0xffff8880100bea78) { /* <-n_tty_write+0xa3/0x540 */
__cond_resched(); /* <-down_read+0x12/0x160 */
preempt_count_add(val=1); /* <-down_read+0x3b/0x160 */
preempt_count_sub(val=1); /* <-down_read+0x8b/0x160 */
}
Cc: Steven Rostedt (Google) <rostedt@goodmis.org>
Cc: Sven Schnelle <svens@linux.ibm.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Donglin Peng <pengdonglin@xiaomi.com>
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
v5:
- Avoid unnecessary branch overhead by first verifying
CONFIG_FUNCTION_GRAPH_RETADDR is enabled prior to testing
the TRACE_GRAPH_PRINT_RETADDR flag
v4:
- Remove redundant TRACE_GRAPH_ARGS flag check
- Eliminate unnecessary retaddr initialization
- Resend to add missing add Acked-by tags
v3:
- Replace min() with min_t() to improve type safety and maintainability
- Keep only one Signed-off-by for cleaner attribution
- Code refactoring for improved readability
v2:
- Preserve retaddr event functionality (suggested by Steven)
- Store the retaddr in args[0] (suggested by Steven)
- Refactor implementation logic and commit message clarity
---
include/linux/ftrace.h | 11 ---
kernel/trace/trace.h | 4 -
kernel/trace/trace_entries.h | 6 +-
kernel/trace/trace_functions_graph.c | 142 ++++++++++++---------------
4 files changed, 66 insertions(+), 97 deletions(-)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 7ded7df6e9b5..88cb54d73bdb 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -1129,17 +1129,6 @@ struct ftrace_graph_ent {
int depth;
} __packed;
-/*
- * Structure that defines an entry function trace with retaddr.
- * It's already packed but the attribute "packed" is needed
- * to remove extra padding at the end.
- */
-struct fgraph_retaddr_ent {
- unsigned long func; /* Current function */
- int depth;
- unsigned long retaddr; /* Return address */
-} __packed;
-
/*
* Structure that defines a return function trace.
* It's already packed but the attribute "packed" is needed
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 85eabb454bee..9fac291b913a 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -955,10 +955,6 @@ extern void graph_trace_close(struct trace_iterator *iter);
extern int __trace_graph_entry(struct trace_array *tr,
struct ftrace_graph_ent *trace,
unsigned int trace_ctx);
-extern int __trace_graph_retaddr_entry(struct trace_array *tr,
- struct ftrace_graph_ent *trace,
- unsigned int trace_ctx,
- unsigned long retaddr);
extern void __trace_graph_return(struct trace_array *tr,
struct ftrace_graph_ret *trace,
unsigned int trace_ctx,
diff --git a/kernel/trace/trace_entries.h b/kernel/trace/trace_entries.h
index de294ae2c5c5..593a74663c65 100644
--- a/kernel/trace/trace_entries.h
+++ b/kernel/trace/trace_entries.h
@@ -95,14 +95,14 @@ FTRACE_ENTRY_PACKED(fgraph_retaddr_entry, fgraph_retaddr_ent_entry,
TRACE_GRAPH_RETADDR_ENT,
F_STRUCT(
- __field_struct( struct fgraph_retaddr_ent, graph_ent )
+ __field_struct( struct ftrace_graph_ent, graph_ent )
__field_packed( unsigned long, graph_ent, func )
__field_packed( unsigned int, graph_ent, depth )
- __field_packed( unsigned long, graph_ent, retaddr )
+ __dynamic_array(unsigned long, args )
),
F_printk("--> %ps (%u) <- %ps", (void *)__entry->func, __entry->depth,
- (void *)__entry->retaddr)
+ (void *)__entry->args[0])
);
#else
diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index a7f4b9a47a71..fa2fb9a0a8ba 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -16,6 +16,15 @@
#include "trace.h"
#include "trace_output.h"
+#ifdef CONFIG_FUNCTION_GRAPH_RETADDR
+#define HAVE_RETADDR 1
+#define ARGS_OFFS(args_size) \
+ ((args_size) > FTRACE_REGS_MAX_ARGS * sizeof(long) ? 1 : 0)
+#else
+#define HAVE_RETADDR 0
+#define ARGS_OFFS(args_size) 0
+#endif
+
/* When set, irq functions will be ignored */
static int ftrace_graph_skip_irqs;
@@ -27,21 +36,25 @@ struct fgraph_cpu_data {
unsigned long enter_funcs[FTRACE_RETFUNC_DEPTH];
};
+/*
+ * fgraph_retaddr_ent_entry and ftrace_graph_ent_entry share layout, ent
+ * member repurposed for storage
+ */
struct fgraph_ent_args {
struct ftrace_graph_ent_entry ent;
- /* Force the sizeof of args[] to have FTRACE_REGS_MAX_ARGS entries */
- unsigned long args[FTRACE_REGS_MAX_ARGS];
+ /*
+ * Force the sizeof of args[] to have (FTRACE_REGS_MAX_ARGS + HAVE_RETADDR)
+ * entries with the first entry storing the return address for
+ * TRACE_GRAPH_RETADDR_ENT.
+ */
+ unsigned long args[FTRACE_REGS_MAX_ARGS + HAVE_RETADDR];
};
struct fgraph_data {
struct fgraph_cpu_data __percpu *cpu_data;
/* Place to preserve last processed entry. */
- union {
- struct fgraph_ent_args ent;
- /* TODO allow retaddr to have args */
- struct fgraph_retaddr_ent_entry rent;
- };
+ struct fgraph_ent_args ent;
struct ftrace_graph_ret_entry ret;
int failed;
int cpu;
@@ -127,22 +140,40 @@ static int __graph_entry(struct trace_array *tr, struct ftrace_graph_ent *trace,
struct ring_buffer_event *event;
struct trace_buffer *buffer = tr->array_buffer.buffer;
struct ftrace_graph_ent_entry *entry;
- int size;
+ unsigned long retaddr;
+ int size = sizeof(*entry);
+ int type = TRACE_GRAPH_ENT;
+ int nr_args = 0, args_offs = 0;
+
+ if (IS_ENABLED(CONFIG_FUNCTION_GRAPH_RETADDR) &&
+ tracer_flags_is_set(TRACE_GRAPH_PRINT_RETADDR)) {
+ retaddr = ftrace_graph_top_ret_addr(current);
+ type = TRACE_GRAPH_RETADDR_ENT;
+ nr_args += 1;
+ }
/* If fregs is defined, add FTRACE_REGS_MAX_ARGS long size words */
- size = sizeof(*entry) + (FTRACE_REGS_MAX_ARGS * !!fregs * sizeof(long));
+ if (!!fregs)
+ nr_args += FTRACE_REGS_MAX_ARGS;
- event = trace_buffer_lock_reserve(buffer, TRACE_GRAPH_ENT, size, trace_ctx);
+ size += nr_args * sizeof(long);
+ event = trace_buffer_lock_reserve(buffer, type, size, trace_ctx);
if (!event)
return 0;
entry = ring_buffer_event_data(event);
entry->graph_ent = *trace;
+ /* Store the retaddr in args[0] */
+ if (type == TRACE_GRAPH_RETADDR_ENT) {
+ entry->args[0] = retaddr;
+ args_offs += 1;
+ }
+
#ifdef CONFIG_HAVE_FUNCTION_ARG_ACCESS_API
- if (fregs) {
+ if (nr_args >= FTRACE_REGS_MAX_ARGS) {
for (int i = 0; i < FTRACE_REGS_MAX_ARGS; i++)
- entry->args[i] = ftrace_regs_get_argument(fregs, i);
+ entry->args[i + args_offs] = ftrace_regs_get_argument(fregs, i);
}
#endif
@@ -158,38 +189,6 @@ int __trace_graph_entry(struct trace_array *tr,
return __graph_entry(tr, trace, trace_ctx, NULL);
}
-#ifdef CONFIG_FUNCTION_GRAPH_RETADDR
-int __trace_graph_retaddr_entry(struct trace_array *tr,
- struct ftrace_graph_ent *trace,
- unsigned int trace_ctx,
- unsigned long retaddr)
-{
- struct ring_buffer_event *event;
- struct trace_buffer *buffer = tr->array_buffer.buffer;
- struct fgraph_retaddr_ent_entry *entry;
-
- event = trace_buffer_lock_reserve(buffer, TRACE_GRAPH_RETADDR_ENT,
- sizeof(*entry), trace_ctx);
- if (!event)
- return 0;
- entry = ring_buffer_event_data(event);
- entry->graph_ent.func = trace->func;
- entry->graph_ent.depth = trace->depth;
- entry->graph_ent.retaddr = retaddr;
- trace_buffer_unlock_commit_nostack(buffer, event);
-
- return 1;
-}
-#else
-int __trace_graph_retaddr_entry(struct trace_array *tr,
- struct ftrace_graph_ent *trace,
- unsigned int trace_ctx,
- unsigned long retaddr)
-{
- return 1;
-}
-#endif
-
static inline int ftrace_graph_ignore_irqs(void)
{
if (!ftrace_graph_skip_irqs || trace_recursion_test(TRACE_IRQ_BIT))
@@ -211,7 +210,6 @@ static int graph_entry(struct ftrace_graph_ent *trace,
struct trace_array *tr = gops->private;
struct fgraph_times *ftimes;
unsigned int trace_ctx;
- int ret = 0;
if (*task_var & TRACE_GRAPH_NOTRACE)
return 0;
@@ -262,15 +260,7 @@ static int graph_entry(struct ftrace_graph_ent *trace,
return 1;
trace_ctx = tracing_gen_ctx();
- if (IS_ENABLED(CONFIG_FUNCTION_GRAPH_RETADDR) &&
- tracer_flags_is_set(TRACE_GRAPH_PRINT_RETADDR)) {
- unsigned long retaddr = ftrace_graph_top_ret_addr(current);
- ret = __trace_graph_retaddr_entry(tr, trace, trace_ctx, retaddr);
- } else {
- ret = __graph_entry(tr, trace, trace_ctx, fregs);
- }
-
- return ret;
+ return __graph_entry(tr, trace, trace_ctx, fregs);
}
int trace_graph_entry(struct ftrace_graph_ent *trace,
@@ -634,13 +624,9 @@ get_return_for_leaf(struct trace_iterator *iter,
* Save current and next entries for later reference
* if the output fails.
*/
- if (unlikely(curr->ent.type == TRACE_GRAPH_RETADDR_ENT)) {
- data->rent = *(struct fgraph_retaddr_ent_entry *)curr;
- } else {
- int size = min((int)sizeof(data->ent), (int)iter->ent_size);
+ int size = min_t(int, sizeof(data->ent), iter->ent_size);
- memcpy(&data->ent, curr, size);
- }
+ memcpy(&data->ent, curr, size);
/*
* If the next event is not a return type, then
* we only care about what type it is. Otherwise we can
@@ -811,21 +797,21 @@ print_graph_duration(struct trace_array *tr, unsigned long long duration,
#ifdef CONFIG_FUNCTION_GRAPH_RETADDR
#define __TRACE_GRAPH_PRINT_RETADDR TRACE_GRAPH_PRINT_RETADDR
-static void print_graph_retaddr(struct trace_seq *s, struct fgraph_retaddr_ent_entry *entry,
- u32 trace_flags, bool comment)
+static void print_graph_retaddr(struct trace_seq *s, unsigned long retaddr, u32 trace_flags,
+ bool comment)
{
if (comment)
trace_seq_puts(s, " /*");
trace_seq_puts(s, " <-");
- seq_print_ip_sym(s, entry->graph_ent.retaddr, trace_flags | TRACE_ITER_SYM_OFFSET);
+ seq_print_ip_sym(s, retaddr, trace_flags | TRACE_ITER_SYM_OFFSET);
if (comment)
trace_seq_puts(s, " */");
}
#else
#define __TRACE_GRAPH_PRINT_RETADDR 0
-#define print_graph_retaddr(_seq, _entry, _tflags, _comment) do { } while (0)
+#define print_graph_retaddr(_seq, _retaddr, _tflags, _comment) do { } while (0)
#endif
#if defined(CONFIG_FUNCTION_GRAPH_RETVAL) || defined(CONFIG_FUNCTION_GRAPH_RETADDR)
@@ -869,10 +855,12 @@ static void print_graph_retval(struct trace_seq *s, struct ftrace_graph_ent_entr
trace_seq_printf(s, "%ps", func);
if (args_size >= FTRACE_REGS_MAX_ARGS * sizeof(long)) {
- print_function_args(s, entry->args, (unsigned long)func);
+ print_function_args(s, entry->args + ARGS_OFFS(args_size),
+ (unsigned long)func);
trace_seq_putc(s, ';');
- } else
+ } else {
trace_seq_puts(s, "();");
+ }
if (print_retval || print_retaddr)
trace_seq_puts(s, " /*");
@@ -882,8 +870,7 @@ static void print_graph_retval(struct trace_seq *s, struct ftrace_graph_ent_entr
}
if (print_retaddr)
- print_graph_retaddr(s, (struct fgraph_retaddr_ent_entry *)entry,
- trace_flags, false);
+ print_graph_retaddr(s, entry->args[0], trace_flags, false);
if (print_retval) {
if (hex_format || (err_code == 0))
@@ -964,10 +951,12 @@ print_graph_entry_leaf(struct trace_iterator *iter,
trace_seq_printf(s, "%ps", (void *)ret_func);
if (args_size >= FTRACE_REGS_MAX_ARGS * sizeof(long)) {
- print_function_args(s, entry->args, ret_func);
+ print_function_args(s, entry->args + ARGS_OFFS(args_size),
+ ret_func);
trace_seq_putc(s, ';');
- } else
+ } else {
trace_seq_puts(s, "();");
+ }
}
trace_seq_putc(s, '\n');
@@ -1016,7 +1005,7 @@ print_graph_entry_nested(struct trace_iterator *iter,
args_size = iter->ent_size - offsetof(struct ftrace_graph_ent_entry, args);
if (args_size >= FTRACE_REGS_MAX_ARGS * sizeof(long))
- print_function_args(s, entry->args, func);
+ print_function_args(s, entry->args + ARGS_OFFS(args_size), func);
else
trace_seq_puts(s, "()");
@@ -1024,8 +1013,7 @@ print_graph_entry_nested(struct trace_iterator *iter,
if (flags & __TRACE_GRAPH_PRINT_RETADDR &&
entry->ent.type == TRACE_GRAPH_RETADDR_ENT)
- print_graph_retaddr(s, (struct fgraph_retaddr_ent_entry *)entry,
- tr->trace_flags, true);
+ print_graph_retaddr(s, entry->args[0], tr->trace_flags, true);
trace_seq_putc(s, '\n');
if (trace_seq_has_overflowed(s))
@@ -1202,7 +1190,7 @@ print_graph_entry(struct ftrace_graph_ent_entry *field, struct trace_seq *s,
* it can be safely saved at the stack.
*/
struct ftrace_graph_ent_entry *entry;
- u8 save_buf[sizeof(*entry) + FTRACE_REGS_MAX_ARGS * sizeof(long)];
+ u8 save_buf[sizeof(*entry) + (FTRACE_REGS_MAX_ARGS + HAVE_RETADDR) * sizeof(long)];
/* The ent_size is expected to be as big as the entry */
if (iter->ent_size > sizeof(save_buf))
@@ -1429,16 +1417,12 @@ print_graph_function_flags(struct trace_iterator *iter, u32 flags)
trace_assign_type(field, entry);
return print_graph_entry(field, s, iter, flags);
}
-#ifdef CONFIG_FUNCTION_GRAPH_RETADDR
case TRACE_GRAPH_RETADDR_ENT: {
- struct fgraph_retaddr_ent_entry saved;
struct fgraph_retaddr_ent_entry *rfield;
trace_assign_type(rfield, entry);
- saved = *rfield;
- return print_graph_entry((struct ftrace_graph_ent_entry *)&saved, s, iter, flags);
+ return print_graph_entry((typeof(field))rfield, s, iter, flags);
}
-#endif
case TRACE_GRAPH_RET: {
struct ftrace_graph_ret_entry *field;
trace_assign_type(field, entry);
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v1] cpufreq: Add policy_frequency trace event
From: Viresh Kumar @ 2025-11-13 6:44 UTC (permalink / raw)
To: Samuel Wu
Cc: Rafael J. Wysocki, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, kernel-team, linux-pm, linux-kernel,
linux-trace-kernel
In-Reply-To: <20251112235154.2974902-1-wusamuel@google.com>
On 12-11-25, 15:51, Samuel Wu wrote:
> The existing cpu_frequency trace_event can be verbose, emitting an event
> for every CPU in the policy even when their frequencies are identical.
>
> This patch adds a new policy_frequency trace event, which provides a
> more efficient alternative to cpu_frequency trace event. This option
> allows users who only need frequency at a policy level more concise logs
> with simpler analysis.
>
> Signed-off-by: Samuel Wu <wusamuel@google.com>
> ---
> drivers/cpufreq/cpufreq.c | 2 ++
> include/trace/events/power.h | 21 +++++++++++++++++++++
> 2 files changed, 23 insertions(+)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 4472bb1ec83c..b65534a4fd9a 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -345,6 +345,7 @@ static void cpufreq_notify_transition(struct cpufreq_policy *policy,
> pr_debug("FREQ: %u - CPUs: %*pbl\n", freqs->new,
> cpumask_pr_args(policy->cpus));
>
> + trace_policy_frequency(freqs->new, policy->cpu);
> for_each_cpu(cpu, policy->cpus)
> trace_cpu_frequency(freqs->new, cpu);
I don't see much value in almost duplicate trace events. If we feel that a
per-policy event is a better fit (which makes sens), then we can just drop the
trace_cpu_frequency() events and print policy->cpus (or related_cpus)
information along with the per-policy events.
--
viresh
^ permalink raw reply
* Re: [RFC PATCH 0/8] xfs: single block atomic writes for buffered IO
From: Christoph Hellwig @ 2025-11-13 5:57 UTC (permalink / raw)
To: Ritesh Harjani
Cc: Christoph Hellwig, Dave Chinner, Ojaswin Mujoo, Christian Brauner,
djwong, john.g.garry, tytso, willy, dchinner, linux-xfs,
linux-kernel, linux-ext4, linux-fsdevel, linux-mm, jack, nilay,
martin.petersen, rostedt, axboe, linux-block, linux-trace-kernel
In-Reply-To: <87frai8p46.ritesh.list@gmail.com>
On Thu, Nov 13, 2025 at 11:12:49AM +0530, Ritesh Harjani wrote:
> For e.g. https://lwn.net/Articles/974578/ goes in depth and talks about
> Postgres folks looking for this, since PostgreSQL databases uses
> buffered I/O for their database writes.
Honestly, a database stubbornly using the wrong I/O path should not be
a reaѕon for adding this complexity.
^ permalink raw reply
* [PATCH linux-next] fgraph: Fix and tighten PID filtering support
From: wang.yaxin @ 2025-11-13 5:56 UTC (permalink / raw)
To: rostedt, mhiramat, mark.rutland, mathieu.desnoyers
Cc: linux-kernel, linux-trace-kernel, hu.shengming, hang.run,
yang.yang29
From: Shengming Hu <hu.shengming@zte.com.cn>
Function graph tracing did not honor set_ftrace_pid() rules properly.
The root cause is that for fgraph_ops, the underlying ftrace_ops->private
was left uninitialized. As a result, ftrace_pids_enabled(op) always
returned false, effectively disabling PID filtering in the function graph
tracer.
PID filtering seemed to "work" only because graph_entry() performed an
extra coarse-grained check via ftrace_trace_task(). Specifically,
ftrace_ignore_pid is updated by ftrace_filter_pid_sched_switch_probe
during sched_switch events. Under the original logic, when the intent
is to trace only PID A, a context switch from task B to A sets
ftrace_ignore_pid to A’s PID. However, there remains a window
where B’s functions are still captured by the function-graph tracer.
The following trace demonstrates this leakage
(B = haveged-213, A = test.sh-385):
4) test.sh-385 | | set_next_entity() {
4) test.sh-385 | 2.108 us | __dequeue_entity();
4) test.sh-385 | 1.526 us | __update_load_avg_se();
4) test.sh-385 | 1.363 us | __update_load_avg_cfs_rq();
4) test.sh-385 | + 14.144 us | }
4) test.sh-385 | 1.417 us | __set_next_task_fair.part.0();
4) test.sh-385 | + 77.877 us | }
4) test.sh-385 | | __traceiter_sched_switch() {
4) test.sh-385 | 2.097 us | _raw_spin_lock_irqsave();
4) test.sh-385 | 1.483 us | _raw_spin_unlock_irqrestore();
4) test.sh-385 | + 14.014 us | }
------------------------------------------
4) test.sh-385 => haveged-213
------------------------------------------
4) haveged-213 | | switch_mm_irqs_off() {
4) haveged-213 | 2.387 us | choose_new_asid();
4) haveged-213 | + 12.462 us | switch_ldt();
4) haveged-213 | + 45.288 us | }
4) haveged-213 | 1.639 us | x86_task_fpu();
4) haveged-213 | | save_fpregs_to_fpstate() {
4) haveged-213 | 1.888 us | xfd_validate_state();
4) haveged-213 | 6.995 us | }
------------------------------------------
4) haveged-213 => test.sh-385
------------------------------------------
4) test.sh-385 | | finish_task_switch.isra.0() {
4) test.sh-385 | 1.857 us | _raw_spin_unlock();
4) test.sh-385 | 5.618 us | }
4) test.sh-385 | ! 446.226 us | }
4) test.sh-385 | | __pte_offset_map_lock() {
4) test.sh-385 | | ___pte_offset_map() {
4) test.sh-385 | 1.933 us | __rcu_read_lock();
4) test.sh-385 | 6.271 us | }
4) test.sh-385 | 2.056 us | _raw_spin_lock();
4) test.sh-385 | + 14.281 us | }
Fix this by:
1. Properly initializing gops->ops->private so that
ftrace_pids_enabled() works as expected.
2. Removing the imprecise fallback check in graph_entry().
3. Updating register_ftrace_graph() to set gops->entryfunc =
fgraph_pid_func whenever PID filtering is active, so the correct
per-task filtering is enforced at entry time.
With this change, function graph tracing will respect the configured
PID filter list consistently, and the redundant coarse check is no
longer needed.
Signed-off-by: Shengming Hu <hu.shengming@zte.com.cn>
---
kernel/trace/fgraph.c | 9 +++++++--
kernel/trace/trace.h | 9 ---------
kernel/trace/trace_functions_graph.c | 3 ---
3 files changed, 7 insertions(+), 14 deletions(-)
diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
index 484ad7a18..00df3d4ac 100644
--- a/kernel/trace/fgraph.c
+++ b/kernel/trace/fgraph.c
@@ -1019,6 +1019,7 @@ void fgraph_init_ops(struct ftrace_ops *dst_ops,
mutex_init(&dst_ops->local_hash.regex_lock);
INIT_LIST_HEAD(&dst_ops->subop_list);
dst_ops->flags |= FTRACE_OPS_FL_INITIALIZED;
+ dst_ops->private = src_ops->private;
}
#endif
}
@@ -1375,6 +1376,12 @@ int register_ftrace_graph(struct fgraph_ops *gops)
gops->idx = i;
ftrace_graph_active++;
+ /* Always save the function, and reset at unregistering */
+ gops->saved_func = gops->entryfunc;
+#ifdef CONFIG_DYNAMIC_FTRACE
+ if (ftrace_pids_enabled(&gops->ops))
+ gops->entryfunc = fgraph_pid_func;
+#endif
if (ftrace_graph_active == 2)
ftrace_graph_disable_direct(true);
@@ -1395,8 +1402,6 @@ int register_ftrace_graph(struct fgraph_ops *gops)
} else {
init_task_vars(gops->idx);
}
- /* Always save the function, and reset at unregistering */
- gops->saved_func = gops->entryfunc;
gops->ops.flags |= FTRACE_OPS_FL_GRAPH;
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index a3a15cfab..048a53282 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1162,11 +1162,6 @@ struct ftrace_func_command {
char *params, int enable);
};
extern bool ftrace_filter_param __initdata;
-static inline int ftrace_trace_task(struct trace_array *tr)
-{
- return this_cpu_read(tr->array_buffer.data->ftrace_ignore_pid) !=
- FTRACE_PID_IGNORE;
-}
extern int ftrace_is_dead(void);
int ftrace_create_function_files(struct trace_array *tr,
struct dentry *parent);
@@ -1184,10 +1179,6 @@ void ftrace_clear_pids(struct trace_array *tr);
int init_function_trace(void);
void ftrace_pid_follow_fork(struct trace_array *tr, bool enable);
#else
-static inline int ftrace_trace_task(struct trace_array *tr)
-{
- return 1;
-}
static inline int ftrace_is_dead(void) { return 0; }
static inline int
ftrace_create_function_files(struct trace_array *tr,
diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index fe9607edc..0efe831e4 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -232,9 +232,6 @@ static int graph_entry(struct ftrace_graph_ent *trace,
return 1;
}
- if (!ftrace_trace_task(tr))
- return 0;
-
if (ftrace_graph_ignore_func(gops, trace))
return 0;
--
2.25.1
^ permalink raw reply related
* Re: [RFC PATCH 0/8] xfs: single block atomic writes for buffered IO
From: Ritesh Harjani @ 2025-11-13 5:42 UTC (permalink / raw)
To: Christoph Hellwig, Dave Chinner
Cc: Ojaswin Mujoo, Christian Brauner, djwong, john.g.garry, tytso,
willy, dchinner, hch, linux-xfs, linux-kernel, linux-ext4,
linux-fsdevel, linux-mm, jack, nilay, martin.petersen, rostedt,
axboe, linux-block, linux-trace-kernel
In-Reply-To: <20251113052337.GA28533@lst.de>
Christoph Hellwig <hch@lst.de> writes:
> On Thu, Nov 13, 2025 at 08:56:56AM +1100, Dave Chinner wrote:
>> On Wed, Nov 12, 2025 at 04:36:03PM +0530, Ojaswin Mujoo wrote:
>> > This patch adds support to perform single block RWF_ATOMIC writes for
>> > iomap xfs buffered IO. This builds upon the inital RFC shared by John
>> > Garry last year [1]. Most of the details are present in the respective
>> > commit messages but I'd mention some of the design points below:
>>
>> What is the use case for this functionality? i.e. what is the
>> reason for adding all this complexity?
>
> Seconded. The atomic code has a lot of complexity, and further mixing
> it with buffered I/O makes this even worse. We'd need a really important
> use case to even consider it.
I agree this should have been in the cover letter itself.
I believe the reason for adding this functionality was also discussed at
LSFMM too...
For e.g. https://lwn.net/Articles/974578/ goes in depth and talks about
Postgres folks looking for this, since PostgreSQL databases uses
buffered I/O for their database writes.
-ritesh
^ permalink raw reply
* Re: [RFC PATCH 0/8] xfs: single block atomic writes for buffered IO
From: Christoph Hellwig @ 2025-11-13 5:23 UTC (permalink / raw)
To: Dave Chinner
Cc: Ojaswin Mujoo, Christian Brauner, djwong, ritesh.list,
john.g.garry, tytso, willy, dchinner, hch, linux-xfs,
linux-kernel, linux-ext4, linux-fsdevel, linux-mm, jack, nilay,
martin.petersen, rostedt, axboe, linux-block, linux-trace-kernel
In-Reply-To: <aRUCqA_UpRftbgce@dread.disaster.area>
On Thu, Nov 13, 2025 at 08:56:56AM +1100, Dave Chinner wrote:
> On Wed, Nov 12, 2025 at 04:36:03PM +0530, Ojaswin Mujoo wrote:
> > This patch adds support to perform single block RWF_ATOMIC writes for
> > iomap xfs buffered IO. This builds upon the inital RFC shared by John
> > Garry last year [1]. Most of the details are present in the respective
> > commit messages but I'd mention some of the design points below:
>
> What is the use case for this functionality? i.e. what is the
> reason for adding all this complexity?
Seconded. The atomic code has a lot of complexity, and further mixing
it with buffered I/O makes this even worse. We'd need a really important
use case to even consider it.
^ permalink raw reply
* Re: [PATCH v8 00/27] mm/ksw: Introduce KStackWatch debugging tool
From: Jinchao Wang @ 2025-11-13 4:40 UTC (permalink / raw)
To: Matthew Wilcox
Cc: kasan-dev, linux-arm-kernel, linux-doc, linux-kernel, linux-mm,
linux-perf-users, linux-trace-kernel, llvm, workflows, x86
In-Reply-To: <aRTv0eHfX0j8vJOW@casper.infradead.org>
On Wed, Nov 12, 2025 at 08:36:33PM +0000, Matthew Wilcox wrote:
> [dropping all the individual email addresses; leaving only the
> mailing lists]
>
> On Wed, Nov 12, 2025 at 10:14:29AM +0800, Jinchao Wang wrote:
> > On Mon, Nov 10, 2025 at 05:33:22PM +0000, Matthew Wilcox wrote:
> > > On Tue, Nov 11, 2025 at 12:35:55AM +0800, Jinchao Wang wrote:
> > > > Earlier this year, I debugged a stack corruption panic that revealed the
> > > > limitations of existing debugging tools. The bug persisted for 739 days
> > > > before being fixed (CVE-2025-22036), and my reproduction scenario
> > > > differed from the CVE report—highlighting how unpredictably these bugs
> > > > manifest.
> > >
> > > Well, this demonstrates the dangers of keeping this problem siloed
> > > within your own exfat group. The fix made in 1bb7ff4204b6 is wrong!
> > > It was fixed properly in 7375f22495e7 which lists its Fixes: as
> > > Linux-2.6.12-rc2, but that's simply the beginning of git history.
> > > It's actually been there since v2.4.6.4 where it's documented as simply:
> > >
> > > - some subtle fs/buffer.c race conditions (Andrew Morton, me)
> > >
> > > As far as I can tell the changes made in 1bb7ff4204b6 should be
> > > reverted.
> >
> > Thank you for the correction and the detailed history. I wasn't aware this
> > dated back to v2.4.6.4. I'm not part of the exfat group; I simply
> > encountered a bug that 1bb7ff4204b6 happened to resolve in my scenario.
> > The timeline actually illustrates the exact problem KStackWatch addresses:
> > a bug introduced in 2001, partially addressed in 2025, then properly fixed
> > months later. The 24-year gap suggests these silent stack corruptions are
> > extremely difficult to locate.
>
> I think that's a misdiagnosis caused by not understanding the limited
> circumstances in which the problem occurs. To hit this problem, you
> have to have a buffer_head allocated on the stack. That doesn't happen
> in many places:
>
> fs/buffer.c: struct buffer_head tmp = {
> fs/direct-io.c: struct buffer_head map_bh = { 0, };
> fs/ext2/super.c: struct buffer_head tmp_bh;
> fs/ext2/super.c: struct buffer_head tmp_bh;
> fs/ext4/mballoc-test.c: struct buffer_head bitmap_bh;
> fs/ext4/mballoc-test.c: struct buffer_head gd_bh;
> fs/gfs2/bmap.c: struct buffer_head bh;
> fs/gfs2/bmap.c: struct buffer_head bh;
> fs/isofs/inode.c: struct buffer_head dummy;
> fs/jfs/super.c: struct buffer_head tmp_bh;
> fs/jfs/super.c: struct buffer_head tmp_bh;
> fs/mpage.c: struct buffer_head map_bh;
> fs/mpage.c: struct buffer_head map_bh;
>
> It's far more common for buffer_heads to be allocated from slab and
> attached to folios. The other necessary condition to hit this problem
> is that get_block() has to actually read the data from disk. That's
> not normal either! Most filesystems just fill in the metadata about
> the block and defer the actual read to when the data is wanted. That's
> the high-performance way to do it.
>
> So our opportunity to catch this bug was highly limited by the fact that
> we just don't run the codepaths that would allow it to trigger.
>
> > > > Initially, I enabled KASAN, but the bug did not reproduce. Reviewing the
> > > > code in __blk_flush_plug(), I found it difficult to trace all logic
> > > > paths due to indirect function calls through function pointers.
> > >
> > > So why is the solution here not simply to fix KASAN instead of this
> > > giant patch series?
> >
> > KASAN caught 7375f22495e7 because put_bh() accessed bh->b_count after
> > wait_on_buffer() of another thread returned—the stack was invalid.
> > In 1bb7ff4204b6 and my case, corruption occurred before the victim
> > function of another thread returned. The stack remained valid to KASAN,
> > so no warning triggered. This is timing-dependent, not a KASAN deficiency.
>
> I agree that it's a narrow race window, but nevertheless KASAN did catch
> it with ntfs and not with exfat. The KASAN documentation states that
> it can catch this kind of bug:
>
> Generic KASAN supports finding bugs in all of slab, page_alloc, vmap, vmalloc,
> stack, and global memory.
>
> Software Tag-Based KASAN supports slab, page_alloc, vmalloc, and stack memory.
>
> Hardware Tag-Based KASAN supports slab, page_alloc, and non-executable vmalloc
> memory.
>
> (hm, were you using hwkasan instead of swkasan, and that's why you
> couldn't see it?)
>
You're right that these conditions are narrow. However, when these bugs
hit, they're severe and extremely difficult to debug. This year alone,
this specific buffer_head bug was hit at least twice: 1bb7ff4204b6 and my
case. Over 24 years, others likely encountered it but lacked tools to
pinpoint the root cause.
I used software KASAN for the exfat case, but the bug didn't reproduce,
likely due to timing changes from the overhead. More fundamentally, the
corruption was in-bounds within active stack frames, which KASAN cannot
detect by design.
Beyond buffer_head, I encountered another stack corruption bug in network
drivers this year. Without KStackWatch, I had to manually instrument the
code to locate where corruption occurred.
These issues may be more common than they appear. Given Linux's massive
user base combined with the kernel's huge codebase and the large volume of
driver code, both in-tree and out-of-tree, even narrow conditions will be
hit.
Since posting earlier versions, several developers have contacted me about
using KStackWatch for their own issues. KStackWatch fills a gap: it can
pinpoint in-bounds stack corruption with much lower overhead than KASAN.
^ permalink raw reply
* Re: [PATCH v3 RESEND] function_graph: Enable funcgraph-args and funcgraph-retaddr to work simultaneously
From: Steven Rostedt @ 2025-11-13 3:21 UTC (permalink / raw)
To: Donglin Peng
Cc: Masami Hiramatsu, linux-trace-kernel, linux-kernel, Donglin Peng,
Sven Schnelle
In-Reply-To: <CAErzpmuPY3_b9LCdeG_8H4hCm_r+4rhzxfwVf04H0jnxBdE5nQ@mail.gmail.com>
On Thu, 13 Nov 2025 11:01:15 +0800
Donglin Peng <dolinux.peng@gmail.com> wrote:
> > Thanks, I agree and it indeed introduces extra branch instruction and
> > may introduce more overhead when using indirection lookups.
>
> Is it necessary to do the same thing for funcgraph-retaddr considering
> that it checks the global tracer_flags in the graph_entry?
Not yet. There's still other flags checked. There's other methods to
help with this (at least for some flags). Let's wait on it for now.
Thanks,
-- Steve
^ permalink raw reply
* Re: [RESEND][PATCH 3/5] perf: Use current->flags & PF_KTHREAD|PF_USER_WORKER instead of current->mm == NULL
From: Guenter Roeck @ 2025-11-13 3:11 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-kernel, linux-trace-kernel, linux-perf-users,
Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Alexander Shishkin, Jiri Olsa, Ian Rogers,
Adrian Hunter, Liang, Kan, Thomas Gleixner
In-Reply-To: <20250820180428.592367294@kernel.org>
Hi Steven,
On Wed, Aug 20, 2025 at 02:03:41PM -0400, Steven Rostedt wrote:
> From: Steven Rostedt <rostedt@goodmis.org>
>
> To determine if a task is a kernel thread or not, it is more reliable to
> use (current->flags & (PF_KTHREAD|PF_USER_WORKERi)) than to rely on
> current->mm being NULL. That is because some kernel tasks (io_uring
> helpers) may have a mm field.
>
> Link: https://lore.kernel.org/linux-trace-kernel/20250424163607.GE18306@noisy.programming.kicks-ass.net/
> Link: https://lore.kernel.org/all/20250624130744.602c5b5f@batman.local.home/
>
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
> kernel/events/callchain.c | 6 +++---
> kernel/events/core.c | 4 ++--
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/events/callchain.c b/kernel/events/callchain.c
> index cd0e3fc7ed05..5982d18f169b 100644
> --- a/kernel/events/callchain.c
> +++ b/kernel/events/callchain.c
> @@ -246,10 +246,10 @@ get_perf_callchain(struct pt_regs *regs, bool kernel, bool user,
>
> if (user && !crosstask) {
> if (!user_mode(regs)) {
> - if (current->mm)
> - regs = task_pt_regs(current);
> - else
> + if (current->flags & (PF_KTHREAD | PF_USER_WORKER))
> regs = NULL;
> + else
> + regs = task_pt_regs(current);
> }
>
> if (regs) {
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index bade8e0fced7..f880cec0c980 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -7446,7 +7446,7 @@ static void perf_sample_regs_user(struct perf_regs *regs_user,
> if (user_mode(regs)) {
> regs_user->abi = perf_reg_abi(current);
> regs_user->regs = regs;
> - } else if (!(current->flags & PF_KTHREAD)) {
> + } else if (!(current->flags & (PF_KTHREAD | PF_USER_WORKER))) {
> perf_get_regs_user(regs_user, regs);
> } else {
> regs_user->abi = PERF_SAMPLE_REGS_ABI_NONE;
> @@ -8086,7 +8086,7 @@ static u64 perf_virt_to_phys(u64 virt)
> * Try IRQ-safe get_user_page_fast_only first.
> * If failed, leave phys_addr as 0.
> */
> - if (current->mm != NULL) {
> + if (!(current->flags & (PF_KTHREAD | PF_USER_WORKER))) {
Subsequent code uses current->mm. This triggers a crash when running a page
table stress test. See below for details. I have seen the crash in 6.12.57
and 6.18-rc5.
Guenter
---
[ 120.334908] BUG: kernel NULL pointer dereference, address: 0000000000000078
[ 120.341901] #PF: supervisor read access in kernel mode
[ 120.347055] #PF: error_code(0x0000) - not-present page
[ 120.352208] PGD 0 P4D 0
[ 120.354750] Oops: Oops: 0000 [#1] SMP NOPTI
[ 120.358946] CPU: 36 UID: 0 PID: 14127 Comm: page_table_stre Tainted: G S O 6.18.0-smp-DEV #2 NONE
[ 120.369242] Tainted: [S]=CPU_OUT_OF_SPEC, [O]=OOT_MODULE
[ 120.374568] Hardware name: Google LLC Indus/Indus_QC_03, BIOS 30.116.4 08/29/2025
[ 120.382075] RIP: 0010:gup_fast_fallback+0x150/0xb60
[ 120.386977] Code: d0 c9 8b 48 89 84 24 a0 00 00 00 48 8b 80 30 05 00 00 0f b6 0d 0d 6b 1a 01 49 89 f8 49 d3 e8 41 81 e0 ff 01 00 00 41 c1 e0
03 <4c> 03 40 78 4c 8d 5b ff 44 89 c8 83 e0 01 48 8d 04 45 05 00 00 00
[ 120.405809] RSP: 0018:ffffa32be5f9b7a0 EFLAGS: 00010006
[ 120.411051] RAX: 0000000000000000 RBX: 00007f0f57dfd000 RCX: 0000000000000027
[ 120.418210] RDX: 0000000000000046 RSI: 0000000000000001 RDI: 00007f0f57dfc000
[ 120.425368] RBP: 0000000000000000 R08: 00000000000007f0 R09: 0000000000100002
[ 120.432526] R10: ffffa32be5f9b8c8 R11: 0000000000000000 R12: 00007f0f57dfc6c0
[ 120.439683] R13: ffff99b44dd7c800 R14: 00000000fffffff2 R15: 00000000000800c3
[ 120.446842] FS: 0000000000000000(0000) GS:ffff9a127357b000(0000) knlGS:0000000000000000
[ 120.454956] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 120.460721] CR2: 0000000000000078 CR3: 000000512d03e006 CR4: 00000000007706f0
[ 120.467879] PKRU: 55555554
[ 120.470592] Call Trace:
[ 120.473045] <TASK>
[ 120.475152] perf_prepare_sample+0x77b/0x910
[ 120.479445] perf_event_output+0x35/0x100
[ 120.483467] intel_pmu_drain_pebs_nhm+0x570/0x750
[ 120.488198] intel_pmu_pebs_sched_task+0x74/0x80
[ 120.492839] ? __put_partials+0xd6/0x130
[ 120.496775] ? __mt_destroy+0x3f/0x80
[ 120.500451] ? put_cpu_partial+0x9b/0xc0
[ 120.504384] ? __slab_free+0x249/0x320
[ 120.508144] ? refill_obj_stock+0x120/0x1a0
[ 120.512341] ? __mt_destroy+0x3f/0x80
[ 120.516013] ? kfree+0x2ca/0x390
[ 120.519254] ? update_load_avg+0x1c8/0x7d0
[ 120.523364] ? update_entity_lag+0xf6/0x110
[ 120.527560] intel_pmu_sched_task+0x1d/0x30
[ 120.531755] perf_pmu_sched_task+0xf2/0x1a0
[ 120.535952] __perf_event_task_sched_out+0x3f/0x1f0
[ 120.540844] ? pick_next_task_fair+0x3e/0x2a0
[ 120.545214] __schedule+0xad0/0xb40
[ 120.548715] do_task_dead+0x48/0xa0
[ 120.552215] do_exit+0x734/0x920
[ 120.555463] ? do_exit+0x9/0x920
[ 120.558699] do_group_exit+0x85/0x90
[ 120.562284] __x64_sys_exit_group+0x17/0x20
[ 120.566478] x64_sys_call+0x21f7/0x2200
[ 120.570327] do_syscall_64+0x6f/0x940
[ 120.574001] ? clear_bhb_loop+0x50/0xa0
[ 120.577849] entry_SYSCALL_64_after_hwframe+0x76/0x7e
[ 120.582915] RIP: 0033:0x7f0f5a0d2c48
[ 120.586501] Code: Unable to access opcode bytes at 0x7f0f5a0d2c1e.
[ 120.592700] RSP: 002b:00007f0f57dfcec8 EFLAGS: 00000207 ORIG_RAX: 00000000000000e7
[ 120.600294] RAX: ffffffffffffffda RBX: 00007f0f57dfd700 RCX: 00007f0f5a0d2c48
[ 120.607452] RDX: 00007f0f57dfd660 RSI: 0000000000000000 RDI: 0000000000000000
[ 120.614607] RBP: 00007f0f57dfcef0 R08: 00007f0f57dfd700 R09: 00007f0f57dfd700
[ 120.621765] R10: 00007f0f5a17a6c0 R11: 0000000000000207 R12: 00007f0f57dfd9d0
[ 120.628923] R13: 00007ffc64840aa6 R14: 00007f0f57dfdd1c R15: 00007f0f57dfcfc0
[ 120.636081] </TASK>
[ 120.638272] Modules linked in: vfat fat i2c_mux_pca954x i2c_mux spidev cdc_acm xhci_pci xhci_hcd gq(O) sha3_generic
[ 120.649976] gsmi: Log Shutdown Reason 0x03
[ 120.654086] CR2: 0000000000000078
[ 120.657409] ---[ end trace 0000000000000000 ]---
Stack decode:
[ 120.334908] BUG: kernel NULL pointer dereference, address: 0000000000000078
[ 120.341901] #PF: supervisor read access in kernel mode
[ 120.347055] #PF: error_code(0x0000) - not-present page
[ 120.352208] PGD 0 P4D 0
[ 120.354750] Oops: Oops: 0000 [#1] SMP NOPTI
[ 120.358946] CPU: 36 UID: 0 PID: 14127 Comm: page_table_stre Tainted: G S O 6.18.0-smp-DEV #2 NONE
[ 120.369242] Tainted: [S]=CPU_OUT_OF_SPEC, [O]=OOT_MODULE
[ 120.374568] Hardware name: Google LLC Indus/Indus_QC_03, BIOS 30.116.4 08/29/2025
[ 120.382075] RIP: 0010:gup_fast_fallback (./include/linux/pgtable.h:140 mm/gup.c:3795 mm/gup.c:3899 mm/gup.c:3946)
[ 120.386977] Code: d0 c9 8b 48 89 84 24 a0 00 00 00 48 8b 80 30 05 00 00 0f b6 0d 0d 6b 1a 01 49 89 f8 49 d3 e8 41 81 e0 ff 01 00 00 41 c1 e0
03 <4c> 03 40 78 4c 8d 5b ff 44 89 c8 83 e0 01 48 8d 04 45 05 00 00 00
All code
========
0: d0 c9 ror $1,%cl
2: 8b 48 89 mov -0x77(%rax),%ecx
5: 84 24 a0 test %ah,(%rax,%riz,4)
8: 00 00 add %al,(%rax)
a: 00 48 8b add %cl,-0x75(%rax)
d: 80 30 05 xorb $0x5,(%rax)
10: 00 00 add %al,(%rax)
12: 0f b6 0d 0d 6b 1a 01 movzbl 0x11a6b0d(%rip),%ecx # 0x11a6b26
19: 49 89 f8 mov %rdi,%r8
1c: 49 d3 e8 shr %cl,%r8
1f: 41 81 e0 ff 01 00 00 and $0x1ff,%r8d
26: 41 c1 e0 03 shl $0x3,%r8d
2a:* 4c 03 40 78 add 0x78(%rax),%r8 <-- trapping instruction
2e: 4c 8d 5b ff lea -0x1(%rbx),%r11
32: 44 89 c8 mov %r9d,%eax
35: 83 e0 01 and $0x1,%eax
38: 48 8d 04 45 05 00 00 lea 0x5(,%rax,2),%rax
3f: 00
Code starting with the faulting instruction
===========================================
0: 4c 03 40 78 add 0x78(%rax),%r8
4: 4c 8d 5b ff lea -0x1(%rbx),%r11
8: 44 89 c8 mov %r9d,%eax
b: 83 e0 01 and $0x1,%eax
e: 48 8d 04 45 05 00 00 lea 0x5(,%rax,2),%rax
15: 00
[ 120.405809] RSP: 0018:ffffa32be5f9b7a0 EFLAGS: 00010006
[ 120.411051] RAX: 0000000000000000 RBX: 00007f0f57dfd000 RCX: 0000000000000027
[ 120.418210] RDX: 0000000000000046 RSI: 0000000000000001 RDI: 00007f0f57dfc000
[ 120.425368] RBP: 0000000000000000 R08: 00000000000007f0 R09: 0000000000100002
[ 120.432526] R10: ffffa32be5f9b8c8 R11: 0000000000000000 R12: 00007f0f57dfc6c0
[ 120.439683] R13: ffff99b44dd7c800 R14: 00000000fffffff2 R15: 00000000000800c3
[ 120.446842] FS: 0000000000000000(0000) GS:ffff9a127357b000(0000) knlGS:0000000000000000
[ 120.454956] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 120.460721] CR2: 0000000000000078 CR3: 000000512d03e006 CR4: 00000000007706f0
[ 120.467879] PKRU: 55555554
[ 120.470592] Call Trace:
[ 120.473045] <TASK>
[ 120.475152] perf_prepare_sample (kernel/events/core.c:7490 kernel/events/core.c:8302)
[ 120.479445] perf_event_output (kernel/events/core.c:8389 kernel/events/core.c:8426)
[ 120.483467] intel_pmu_drain_pebs_nhm (arch/x86/events/intel/ds.c:? arch/x86/events/intel/ds.c:2182 arch/x86/events/intel/ds.c:2372)
[ 120.488198] intel_pmu_pebs_sched_task (arch/x86/events/intel/ds.c:939 arch/x86/events/intel/ds.c:1248)
[ 120.492839] ? __put_partials (mm/slub.c:3195)
[ 120.496775] ? __mt_destroy (lib/maple_tree.c:? lib/maple_tree.c:6883)
[ 120.500451] ? put_cpu_partial (mm/slub.c:3278)
[ 120.504384] ? __slab_free (mm/slub.c:4521)
[ 120.508144] ? refill_obj_stock (./include/linux/percpu-refcount.h:335 ./include/linux/percpu-refcount.h:351 ./include/linux/memcontrol.h:988
mm/memcontrol.c:3732)
[ 120.512341] ? __mt_destroy (lib/maple_tree.c:? lib/maple_tree.c:6883)
[ 120.516013] ? kfree (mm/slab.h:681 mm/slub.c:4649 mm/slub.c:4797)
[ 120.519254] ? update_load_avg (kernel/sched/fair.c:5376 kernel/sched/fair.c:5601 kernel/sched/fair.c:5720)
[ 120.523364] ? update_entity_lag (kernel/sched/fair.c:?)
[ 120.527560] intel_pmu_sched_task (arch/x86/events/intel/core.c:5231)
[ 120.531755] perf_pmu_sched_task (kernel/events/core.c:1219 kernel/events/core.c:1231 kernel/events/core.c:3739 kernel/events/core.c:3755)
[ 120.535952] __perf_event_task_sched_out (kernel/events/core.c:3776)
[ 120.540844] ? pick_next_task_fair (kernel/sched/sched.h:4660 kernel/sched/sched.h:4666 kernel/sched/fair.c:9593 kernel/sched/fair.c:15504)
[ 120.545214] __schedule (kernel/sched/core.c:7405 kernel/sched/core.c:8080)
[ 120.548715] do_task_dead (??:?)
[ 120.552215] do_exit (./include/linux/list.h:364 kernel/exit.c:810 kernel/exit.c:1030)
[ 120.555463] ? do_exit (kernel/exit.c:934)
[ 120.558699] do_group_exit (kernel/exit.c:1161)
[ 120.562284] __x64_sys_exit_group (kernel/exit.c:1172)
[ 120.566478] x64_sys_call (arch/x86/entry/syscall_64.c:32)
[ 120.570327] do_syscall_64 (arch/x86/entry/common.c:57 arch/x86/entry/common.c:100)
[ 120.574001] ? clear_bhb_loop (arch/x86/entry/entry_64.S:1598)
^ permalink raw reply
* Re: [PATCH v3 RESEND] function_graph: Enable funcgraph-args and funcgraph-retaddr to work simultaneously
From: Donglin Peng @ 2025-11-13 3:01 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu, linux-trace-kernel, linux-kernel, Donglin Peng,
Sven Schnelle
In-Reply-To: <CAErzpmsn-uys3VdchF_CKBPUQT+328wtaKnbufoDi-o9x7eHaA@mail.gmail.com>
On Thu, Nov 13, 2025 at 10:38 AM Donglin Peng <dolinux.peng@gmail.com> wrote:
>
> On Thu, Nov 13, 2025 at 10:02 AM Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > On Thu, 13 Nov 2025 09:48:52 +0800
> > Donglin Peng <dolinux.peng@gmail.com> wrote:
> >
> > > I noticed that when funcgraph-args is enabled, it registers
> > > trace_graph_entry_args
> > > to replace trace_graph_entry. The only difference is whether a valid
> > > fregs pointer
> > > is passed.
> > >
> > > To reduce overhead, I propose consolidating the two entry functions. We could
> > > maintain only trace_graph_entry and pass the fregs parameter to graph_entry.
> > > Within __graph_entry, we can then conditionally record arguments based on
> > > 'TRACE_GRAPH_ARGS && !!fregs'.
> >
> > What overhead are you reducing? Why add a branch statement in a critical path?
>
> Thanks. During testing, I found that enabling funcgraph-args incurs significant
> overhead (344ms) compared to other trace options (near-zero).
>
> # time echo 1 > options/funcgraph-retaddr
>
> real 0m0.000s
> user 0m0.000s
> sys 0m0.000s
>
> # time echo 1 > options/funcgraph-args
>
> real 0m0.344s
> user 0m0.000s
> sys 0m0.344s
>
> So I thought it may need to be optimized.
>
> >
> > The graph_entry() should not be looking at the flags argument. It's called
> > by *every function*. Also note, I recently fixed the flags to be per
> > instance and not global. Which means testing the flags would require
> > indirection lookups.
>
> Thanks, I agree and it indeed introduces extra branch instruction and
> may introduce more overhead when using indirection lookups.
Is it necessary to do the same thing for funcgraph-retaddr considering
that it checks the global tracer_flags in the graph_entry?
>
> >
> > -- Steve
^ permalink raw reply
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