* [RESEND PATCH V4] sched: psi: Add psi events trace point
From: Xuewen Yan @ 2025-11-21 7:32 UTC (permalink / raw)
To: peterz, surenb, rostedt, mhiramat, mathieu.desnoyers, hannes,
vincent.guittot
Cc: mingo, juri.lelli, dietmar.eggemann, bsegall, mgorman,
linux-kernel, linux-trace-kernel, yuming.han, xuewen.yan94,
ke.wang, jstultz
Add trace point to psi triggers. This is useful to
observe the psi events in the kernel space.
One use of this is to monitor memory pressure.
When the pressure is too high, we can kill the process
in the kernel space to prevent OOM.
Signed-off-by: Xuewen Yan <xuewen.yan@unisoc.com>
Acked-by: Suren Baghdasaryan <surenb@google.com>
---
V4:
-generate the event only after cmpxchg() passes the check
---
V3:
-export it in the tracefs;
---
v2:
-fix compilation error;
-export the tp;
-add more commit message;
---
include/trace/events/sched.h | 27 +++++++++++++++++++++++++++
kernel/sched/psi.c | 5 +++++
2 files changed, 32 insertions(+)
diff --git a/include/trace/events/sched.h b/include/trace/events/sched.h
index 7b2645b50e78..db8b8f25466e 100644
--- a/include/trace/events/sched.h
+++ b/include/trace/events/sched.h
@@ -826,6 +826,33 @@ TRACE_EVENT(sched_wake_idle_without_ipi,
TP_printk("cpu=%d", __entry->cpu)
);
+#ifdef CONFIG_PSI
+TRACE_EVENT(psi_event,
+
+ TP_PROTO(int aggregator, int state, u64 threshold, u64 win_size),
+
+ TP_ARGS(aggregator, state, threshold, win_size),
+
+ TP_STRUCT__entry(
+ __field(int, aggregator)
+ __field(int, state)
+ __field(u64, threshold)
+ __field(u64, win_size)
+ ),
+
+ TP_fast_assign(
+ __entry->aggregator = aggregator;
+ __entry->state = state;
+ __entry->threshold = threshold;
+ __entry->win_size = win_size;
+ ),
+
+ TP_printk("aggregator=%d state=%d threshold=%llu window_size=%llu",
+ __entry->aggregator, __entry->state, __entry->threshold,
+ __entry->win_size)
+);
+#endif /* CONFIG_PSI */
+
/*
* Following tracepoints are not exported in tracefs and provide hooking
* mechanisms only for testing and debugging purposes.
diff --git a/kernel/sched/psi.c b/kernel/sched/psi.c
index 59fdb7ebbf22..e8a7fd04ba9f 100644
--- a/kernel/sched/psi.c
+++ b/kernel/sched/psi.c
@@ -141,6 +141,8 @@
#include <linux/psi.h>
#include "sched.h"
+EXPORT_TRACEPOINT_SYMBOL_GPL(psi_event);
+
static int psi_bug __read_mostly;
DEFINE_STATIC_KEY_FALSE(psi_disabled);
@@ -515,6 +517,9 @@ static void update_triggers(struct psi_group *group, u64 now,
kernfs_notify(t->of->kn);
else
wake_up_interruptible(&t->event_wait);
+
+ trace_psi_event(aggregator, t->state, t->threshold,
+ t->win.size);
}
t->last_event_time = now;
/* Reset threshold breach flag once event got generated */
--
2.25.1
^ permalink raw reply related
* [PATCH] tracing: Add boot-time backup of persistent ring buffer
From: Masami Hiramatsu (Google) @ 2025-11-21 7:01 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Currently, the persistent ring buffer instance needs to be read before
using it. This means we have to wait for boot up user space and dump
the persistent ring buffer. However, in that case we can not start
tracing on it from the kernel cmdline.
To solve this limitation, this adds an option which allows to create
a trace instance as a backup of the persistent ring buffer at boot.
If user specifies trace_instance=<BACKUP>=<PERSIST_RB> then the
<BACKUP> instance is made as a copy of the <PERSIST_RB> instance.
For example, the below kernel cmdline records all syscalls, scheduler
and interrupt events on the persistent ring buffer `boot_map` but
before starting the tracing, it makes a `backup` instance from the
`boot_map`. Thus, the `backup` instance has the previous boot events.
'reserve_mem=12M:4M:trace trace_instance=boot_map@trace,syscalls:*,sched:*,irq:* trace_instance=backup=boot_map'
As you can see, this just make a copy of entire reserved area and
make a backup instance on it. So you can release (or shrink) the
backup instance after use it to save the memory usage.
/sys/kernel/tracing/instances # free
total used free shared buff/cache available
Mem: 1999284 55704 1930520 10132 13060 1914628
Swap: 0 0 0
/sys/kernel/tracing/instances # rmdir backup/
/sys/kernel/tracing/instances # free
total used free shared buff/cache available
Mem: 1999284 40640 1945584 10132 13060 1929692
Swap: 0 0 0
Note: since there is no reason to make a copy of empty buffer, this
backup only accepts a persistent ring buffer as the original instance.
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
kernel/trace/trace.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++---
kernel/trace/trace.h | 1 +
2 files changed, 55 insertions(+), 4 deletions(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 33fcde91924c..616d24580371 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -10523,6 +10523,8 @@ static int __remove_instance(struct trace_array *tr)
reserve_mem_release_by_name(tr->range_name);
kfree(tr->range_name);
}
+ if (tr->flags & TRACE_ARRAY_FL_VMALLOC)
+ vfree((void *)tr->range_addr_start);
for (i = 0; i < tr->nr_topts; i++) {
kfree(tr->topts[i].topts);
@@ -11328,6 +11330,41 @@ __init static void do_allocate_snapshot(const char *name)
static inline void do_allocate_snapshot(const char *name) { }
#endif
+__init static int backup_instance_area(const char *backup,
+ unsigned long *addr, phys_addr_t *size)
+{
+ struct trace_array *backup_tr;
+ void *allocated_vaddr = NULL;
+
+ backup_tr = trace_array_get_by_name(backup, NULL);
+ if (!backup_tr) {
+ pr_warn("Tracing: Instance %s is not found.\n", backup);
+ return -ENOENT;
+ }
+ if (!(backup_tr->flags & TRACE_ARRAY_FL_BOOT)) {
+ pr_warn("Tracing: Instance %s is not boot mapped.\n", backup);
+ trace_array_put(backup_tr);
+ return -EINVAL;
+ }
+
+ *size = backup_tr->range_addr_size;
+
+ allocated_vaddr = vzalloc(*size);
+ if (!allocated_vaddr) {
+ pr_warn("Tracing: Failed to allocate memory for copying instance %s (size 0x%lx)\n",
+ backup, (unsigned long)*size);
+ trace_array_put(backup_tr);
+ return -ENOMEM;
+ }
+
+ memcpy(allocated_vaddr,
+ (void *)backup_tr->range_addr_start, (size_t)*size);
+ *addr = (unsigned long)allocated_vaddr;
+
+ trace_array_put(backup_tr);
+ return 0;
+}
+
__init static void enable_instances(void)
{
struct trace_array *tr;
@@ -11350,11 +11387,15 @@ __init static void enable_instances(void)
char *flag_delim;
char *addr_delim;
char *rname __free(kfree) = NULL;
+ char *backup;
tok = strsep(&curr_str, ",");
- flag_delim = strchr(tok, '^');
- addr_delim = strchr(tok, '@');
+ name = strsep(&tok, "=");
+ backup = tok;
+
+ flag_delim = strchr(name, '^');
+ addr_delim = strchr(name, '@');
if (addr_delim)
*addr_delim++ = '\0';
@@ -11362,7 +11403,10 @@ __init static void enable_instances(void)
if (flag_delim)
*flag_delim++ = '\0';
- name = tok;
+ if (backup) {
+ if (backup_instance_area(backup, &addr, &size) < 0)
+ continue;
+ }
if (flag_delim) {
char *flag;
@@ -11458,7 +11502,13 @@ __init static void enable_instances(void)
tr->ref++;
}
- if (start) {
+ /*
+ * Backup buffers can be freed but need vfree().
+ */
+ if (backup)
+ tr->flags |= TRACE_ARRAY_FL_VMALLOC;
+
+ if (start || backup) {
tr->flags |= TRACE_ARRAY_FL_BOOT | TRACE_ARRAY_FL_LAST_BOOT;
tr->range_name = no_free_ptr(rname);
}
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 58be6d741d72..9e5186c96e9c 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -453,6 +453,7 @@ enum {
TRACE_ARRAY_FL_LAST_BOOT = BIT(2),
TRACE_ARRAY_FL_MOD_INIT = BIT(3),
TRACE_ARRAY_FL_MEMMAP = BIT(4),
+ TRACE_ARRAY_FL_VMALLOC = BIT(5),
};
#ifdef CONFIG_MODULES
^ permalink raw reply related
* Re: [PATCH V2 1/2] mm/khugepaged: do synchronous writeback for MADV_COLLAPSE
From: Garg, Shivank @ 2025-11-21 6:27 UTC (permalink / raw)
To: David Hildenbrand (Red Hat), Andrew Morton, Lorenzo Stoakes
Cc: Zi Yan, Baolin Wang, Liam R . Howlett, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Zach O'Keefe, linux-mm,
linux-kernel, linux-trace-kernel, Branden Moore
In-Reply-To: <be698a14-c292-42d1-9a85-109a9072a0b2@kernel.org>
On 11/20/2025 7:05 PM, David Hildenbrand (Red Hat) wrote:
> On 11/20/25 07:50, Shivank Garg wrote:
>> When MADV_COLLAPSE is called on file-backed mappings (e.g., executable
>> text sections), the pages may still be dirty from recent writes and
>> cause collapse to fail with -EINVAL. This is particularly problematic
>> for freshly copied executables on filesystems, where page cache folios
>> remain dirty until background writeback completes.
>>
>> The current code in collapse_file() triggers async writeback via
>> filemap_flush() and expects khugepaged to revisit the page later.
>> However, MADV_COLLAPSE is a synchronous operation where userspace
>> expects immediate results.
>>
>> Perform synchronous writeback in madvise_collapse() before attempting
>> collapse to avoid failing on first attempt.
>>
>> Reported-by: Branden Moore <Branden.Moore@amd.com>
>> Closes: https://lore.kernel.org/all/4e26fe5e-7374-467c-a333-9dd48f85d7cc@amd.com
>> Fixes: 34488399fa08 ("mm/madvise: add file and shmem support to MADV_COLLAPSE")
>> Suggested-by: David Hildenbrand <david@kernel.org>
>> Signed-off-by: Shivank Garg <shivankg@amd.com>
>> ---
>> mm/khugepaged.c | 26 ++++++++++++++++++++++++++
>> 1 file changed, 26 insertions(+)
>>
>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>> index 97d1b2824386..066a332c76ad 100644
>> --- a/mm/khugepaged.c
>> +++ b/mm/khugepaged.c
>> @@ -22,6 +22,7 @@
>> #include <linux/dax.h>
>> #include <linux/ksm.h>
>> #include <linux/pgalloc.h>
>> +#include <linux/backing-dev.h>
>> #include <asm/tlb.h>
>> #include "internal.h"
>> @@ -2784,6 +2785,31 @@ int madvise_collapse(struct vm_area_struct *vma, unsigned long start,
>> hstart = (start + ~HPAGE_PMD_MASK) & HPAGE_PMD_MASK;
>> hend = end & HPAGE_PMD_MASK;
>> + /*
>> + * For file-backed VMAs, perform synchronous writeback to ensure
>> + * dirty folios are flushed before attempting collapse. This avoids
>> + * failing on the first attempt when freshly-written executable text
>> + * is still dirty in the page cache.
>> + */
>> + if (!vma_is_anonymous(vma) && vma->vm_file) {
>> + struct address_space *mapping = vma->vm_file->f_mapping;
>> +
>> + if (mapping_can_writeback(mapping)) {
>> + pgoff_t pgoff_start = linear_page_index(vma, hstart);
>> + pgoff_t pgoff_end = linear_page_index(vma, hend);
>> + loff_t lstart = (loff_t)pgoff_start << PAGE_SHIFT;
>> + loff_t lend = ((loff_t)pgoff_end << PAGE_SHIFT) - 1;
>> +
>
> Hm, so we always do that, without any indication that there actually is something dirty there.
>
> Internally filemap_write_and_wait_range() uses something called mapping_needs_writeback(), but it also applies to the complete file, not a range.
>
> Wouldn't it be better do do that only if we detect that there is actually a dirty folio in the range?
>
> That is, if we find any dirty folio in hpage_collapse_scan_file() and we are in madvise, do that dance here and retry?
>
Good point! This makes sense to me.
I'll send V3 with this approach.
Thanks,
Shivank
^ permalink raw reply
* Re: [PATCH V2 1/2] mm/khugepaged: do synchronous writeback for MADV_COLLAPSE
From: Garg, Shivank @ 2025-11-21 6:27 UTC (permalink / raw)
To: Lance Yang
Cc: Zi Yan, Baolin Wang, Liam R . Howlett, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Steven Rostedt, David Hildenbrand,
Masami Hiramatsu, Mathieu Desnoyers, Zach O'Keefe, linux-mm,
Andrew Morton, linux-kernel, linux-trace-kernel, Branden Moore,
Lorenzo Stoakes
In-Reply-To: <f4652d26-a0f0-4ffe-ad53-74a3418c75e1@linux.dev>
On 11/20/2025 6:31 PM, Lance Yang wrote:
>
>
> On 2025/11/20 14:50, Shivank Garg wrote:
>> When MADV_COLLAPSE is called on file-backed mappings (e.g., executable
>> text sections), the pages may still be dirty from recent writes and
>> cause collapse to fail with -EINVAL. This is particularly problematic
>> for freshly copied executables on filesystems, where page cache folios
>> remain dirty until background writeback completes.
>>
>> The current code in collapse_file() triggers async writeback via
>> filemap_flush() and expects khugepaged to revisit the page later.
>> However, MADV_COLLAPSE is a synchronous operation where userspace
>> expects immediate results.
>>
>> Perform synchronous writeback in madvise_collapse() before attempting
>> collapse to avoid failing on first attempt.
>
> Thanks!
>
>>
>> Reported-by: Branden Moore <Branden.Moore@amd.com>
>> Closes: https://lore.kernel.org/all/4e26fe5e-7374-467c-a333-9dd48f85d7cc@amd.com
>> Fixes: 34488399fa08 ("mm/madvise: add file and shmem support to MADV_COLLAPSE")
>> Suggested-by: David Hildenbrand <david@kernel.org>
>> Signed-off-by: Shivank Garg <shivankg@amd.com>
>> ---
>> mm/khugepaged.c | 26 ++++++++++++++++++++++++++
>> 1 file changed, 26 insertions(+)
>>
>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>> index 97d1b2824386..066a332c76ad 100644
>> --- a/mm/khugepaged.c
>> +++ b/mm/khugepaged.c
>> @@ -22,6 +22,7 @@
>> #include <linux/dax.h>
>> #include <linux/ksm.h>
>> #include <linux/pgalloc.h>
>> +#include <linux/backing-dev.h>
>> #include <asm/tlb.h>
>> #include "internal.h"
>> @@ -2784,6 +2785,31 @@ int madvise_collapse(struct vm_area_struct *vma, unsigned long start,
>> hstart = (start + ~HPAGE_PMD_MASK) & HPAGE_PMD_MASK;
>> hend = end & HPAGE_PMD_MASK;
>> + /*
>> + * For file-backed VMAs, perform synchronous writeback to ensure
>> + * dirty folios are flushed before attempting collapse. This avoids
>> + * failing on the first attempt when freshly-written executable text
>> + * is still dirty in the page cache.
>> + */
>> + if (!vma_is_anonymous(vma) && vma->vm_file) {
>> + struct address_space *mapping = vma->vm_file->f_mapping;
>> +
>> + if (mapping_can_writeback(mapping)) {
>> + pgoff_t pgoff_start = linear_page_index(vma, hstart);
>> + pgoff_t pgoff_end = linear_page_index(vma, hend);
>> + loff_t lstart = (loff_t)pgoff_start << PAGE_SHIFT;
>> + loff_t lend = ((loff_t)pgoff_end << PAGE_SHIFT) - 1;
>
> It looks like we need to hold a reference to the file here before
> dropping the mmap lock :)
>
> file = get_file(vma->vm_file);
>
> Without it, the vma could be destroyed by a concurrent munmap() while
> we are waiting in filemap_write_and_wait_range(), leading to a UAF
> on mapping, IIUC ...
Excellent catch!
Thanks for saving me from this nasty bug. I'll be more careful on file ref
handling in next version.
Best Regards,
Shivank
>
>> +
>> + mmap_read_unlock(mm);
>> + mmap_locked = false;
>> +
>> + if (filemap_write_and_wait_range(mapping, lstart, lend)) {
>
> And drop the reference :)
>
> fput(file);
>
>
>> + last_fail = SCAN_FAIL;
>> + goto out_maybelock;
>> + }
>
> Same here :)
>
> fput(file);
>
>
>> + }
>> + }
>> +
>> for (addr = hstart; addr < hend; addr += HPAGE_PMD_SIZE) {
>> int result = SCAN_FAIL;
>>
>
> Cheers,
> Lance
^ permalink raw reply
* Re: [PATCH v6 3/6] perf record: Add --call-graph fp,defer option for deferred callchains
From: Thomas Richter @ 2025-11-21 6:26 UTC (permalink / raw)
To: Namhyung Kim, Arnaldo Carvalho de Melo, Ian Rogers, James Clark
Cc: Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
linux-perf-users, Steven Rostedt, Josh Poimboeuf, Indu Bhagat,
Jens Remus, Mathieu Desnoyers, linux-trace-kernel, bpf
In-Reply-To: <20251120234804.156340-4-namhyung@kernel.org>
On 11/21/25 00:48, Namhyung Kim wrote:
> Add a new callchain record mode option for deferred callchains. For now
> it only works with FP (frame-pointer) mode.
>
> And add the missing feature detection logic to clear the flag on old
> kernels.
>
> $ perf record --call-graph fp,defer -vv true
Does this also works for dwarf format?
# perf record --call-graph dwarf,defer ....
--
Thomas Richter, Dept 3303, IBM s390 Linux Development, Boeblingen, Germany
--
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Wolfgang Wendt
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294
^ permalink raw reply
* Re: [PATCH V2 2/2] mm/khugepaged: map dirty/writeback pages failures to EAGAIN
From: Garg, Shivank @ 2025-11-21 6:15 UTC (permalink / raw)
To: David Hildenbrand (Red Hat), Lance Yang, Dev Jain
Cc: Zi Yan, Baolin Wang, Liam R . Howlett, Nico Pache, Ryan Roberts,
Barry Song, Andrew Morton, Steven Rostedt, Lorenzo Stoakes,
Masami Hiramatsu, Mathieu Desnoyers, Zach O'Keefe, linux-mm,
linux-kernel, linux-trace-kernel
In-Reply-To: <81e34d93-9031-4840-a0d0-56d488f3bc17@kernel.org>
On 11/20/2025 6:59 PM, David Hildenbrand (Red Hat) wrote:
> On 11/20/25 13:24, Lance Yang wrote:
>>
>>
>> On 2025/11/20 16:17, Garg, Shivank wrote:
>>>
>>>
>>> On 11/20/2025 1:33 PM, Dev Jain wrote:
>>>>
>>>> On 20/11/25 12:20 pm, Shivank Garg wrote:
>>>
>>>> SCAN_PAGE_NOT_CLEAN is confusing - NOT_CLEAN literally means dirty, so why not SCAN_PAGE_DIRTY?
>>>> Or SCAN_PAGE_DIRTY_OR_UNDER_WRITEBACK? Since folio_test_writeback() is true as a result of
>>>> the folio being dirty, maybe just SCAN_PAGE_DIRTY can do.
>>>>
>>>> Reviewed-by: Dev Jain <dev.jain@arm.com>
>>>>
>>> Thanks for the review.
>>>
>>> I chose not to use SCAN_PAGE_DIRTY because dirty and writeback have different meanings[1]:
>>>
>>> Dirty: Memory that is waiting to be written back to disk
>>> Writeback: Memory that is actively being written back to disk
>>>
>>> [1] https://www.kernel.org/doc/Documentation/filesystems/proc.txt
>>>
>>> IIUC, a page under writeback is no longer dirty, so using SCAN_PAGE_DIRTY would be misleading
>>> for pages in the writeback state.
>>>
>>> I considered SCAN_PAGE_DIRTY_OR_WRITEBACK initially but felt it was too long.
>>
>> Nit: If SCAN_PAGE_DIRTY_OR_WRITEBACK
>
> I would prefer that here.
>
I agree on this.
If the consensus is SCAN_PAGE_DIRTY_OR_WRITEBACK, I'll use it in v3.
Thanks,
Shivank
^ permalink raw reply
* [PATCH v6 6/6] perf tools: Flush remaining samples w/o deferred callchains
From: Namhyung Kim @ 2025-11-20 23:48 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Ian Rogers, James Clark
Cc: Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
linux-perf-users, Steven Rostedt, Josh Poimboeuf, Indu Bhagat,
Jens Remus, Mathieu Desnoyers, linux-trace-kernel, bpf
In-Reply-To: <20251120234804.156340-1-namhyung@kernel.org>
It's possible that some kernel samples don't have matching deferred
callchain records when the profiling session was ended before the
threads came back to userspace. Let's flush the samples before
finish the session.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/session.c | 50 +++++++++++++++++++++++++++++++++++++++
1 file changed, 50 insertions(+)
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index dc570ad47ccc2c63..4236503c8f6c1350 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1295,6 +1295,10 @@ struct deferred_event {
union perf_event *event;
};
+/*
+ * This is called when a deferred callchain record comes up. Find all matching
+ * samples, merge the callchains and process them.
+ */
static int evlist__deliver_deferred_callchain(struct evlist *evlist,
const struct perf_tool *tool,
union perf_event *event,
@@ -1345,6 +1349,42 @@ static int evlist__deliver_deferred_callchain(struct evlist *evlist,
return ret;
}
+/*
+ * This is called at the end of the data processing for the session. Flush the
+ * remaining samples as there's no hope for matching deferred callchains.
+ */
+static int session__flush_deferred_samples(struct perf_session *session,
+ const struct perf_tool *tool)
+{
+ struct evlist *evlist = session->evlist;
+ struct machine *machine = &session->machines.host;
+ struct deferred_event *de, *tmp;
+ struct evsel *evsel;
+ int ret = 0;
+
+ list_for_each_entry_safe(de, tmp, &evlist->deferred_samples, list) {
+ struct perf_sample sample;
+
+ ret = evlist__parse_sample(evlist, de->event, &sample);
+ if (ret < 0) {
+ pr_err("failed to parse original sample\n");
+ break;
+ }
+
+ evsel = evlist__id2evsel(evlist, sample.id);
+ ret = evlist__deliver_sample(evlist, tool, de->event,
+ &sample, evsel, machine);
+
+ list_del(&de->list);
+ free(de->event);
+ free(de);
+
+ if (ret)
+ break;
+ }
+ return ret;
+}
+
static int machines__deliver_event(struct machines *machines,
struct evlist *evlist,
union perf_event *event,
@@ -2038,6 +2078,9 @@ static int __perf_session__process_pipe_events(struct perf_session *session)
done:
/* do the final flush for ordered samples */
err = ordered_events__flush(oe, OE_FLUSH__FINAL);
+ if (err)
+ goto out_err;
+ err = session__flush_deferred_samples(session, tool);
if (err)
goto out_err;
err = auxtrace__flush_events(session, tool);
@@ -2384,6 +2427,9 @@ static int __perf_session__process_events(struct perf_session *session)
if (err)
goto out_err;
err = auxtrace__flush_events(session, tool);
+ if (err)
+ goto out_err;
+ err = session__flush_deferred_samples(session, tool);
if (err)
goto out_err;
err = perf_session__flush_thread_stacks(session);
@@ -2506,6 +2552,10 @@ static int __perf_session__process_dir_events(struct perf_session *session)
if (ret)
goto out_err;
+ ret = session__flush_deferred_samples(session, tool);
+ if (ret)
+ goto out_err;
+
ret = perf_session__flush_thread_stacks(session);
out_err:
ui_progress__finish();
--
2.52.0.rc2.455.g230fcf2819-goog
^ permalink raw reply related
* [PATCH v6 5/6] perf tools: Merge deferred user callchains
From: Namhyung Kim @ 2025-11-20 23:48 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Ian Rogers, James Clark
Cc: Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
linux-perf-users, Steven Rostedt, Josh Poimboeuf, Indu Bhagat,
Jens Remus, Mathieu Desnoyers, linux-trace-kernel, bpf
In-Reply-To: <20251120234804.156340-1-namhyung@kernel.org>
Save samples with deferred callchains in a separate list and deliver
them after merging the user callchains. If users don't want to merge
they can set tool->merge_deferred_callchains to false to prevent the
behavior.
With previous result, now perf script will show the merged callchains.
$ perf script
...
pwd 2312 121.163435: 249113 cpu/cycles/P:
ffffffff845b78d8 __build_id_parse.isra.0+0x218 ([kernel.kallsyms])
ffffffff83bb5bf6 perf_event_mmap+0x2e6 ([kernel.kallsyms])
ffffffff83c31959 mprotect_fixup+0x1e9 ([kernel.kallsyms])
ffffffff83c31dc5 do_mprotect_pkey+0x2b5 ([kernel.kallsyms])
ffffffff83c3206f __x64_sys_mprotect+0x1f ([kernel.kallsyms])
ffffffff845e6692 do_syscall_64+0x62 ([kernel.kallsyms])
ffffffff8360012f entry_SYSCALL_64_after_hwframe+0x76 ([kernel.kallsyms])
7f18fe337fa7 mprotect+0x7 (/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2)
7f18fe330e0f _dl_sysdep_start+0x7f (/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2)
7f18fe331448 _dl_start_user+0x0 (/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2)
...
The old output can be get using --no-merge-callchain option.
Also perf report can get the user callchain entry at the end.
$ perf report --no-children --stdio -q -S __build_id_parse.isra.0
# symbol: __build_id_parse.isra.0
8.40% pwd [kernel.kallsyms]
|
---__build_id_parse.isra.0
perf_event_mmap
mprotect_fixup
do_mprotect_pkey
__x64_sys_mprotect
do_syscall_64
entry_SYSCALL_64_after_hwframe
mprotect
_dl_sysdep_start
_dl_start_user
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/Documentation/perf-script.txt | 5 ++
tools/perf/builtin-inject.c | 1 +
tools/perf/builtin-report.c | 1 +
tools/perf/builtin-script.c | 4 ++
tools/perf/util/callchain.c | 29 +++++++++
tools/perf/util/callchain.h | 3 +
tools/perf/util/evlist.c | 1 +
tools/perf/util/evlist.h | 2 +
tools/perf/util/session.c | 79 +++++++++++++++++++++++-
tools/perf/util/tool.c | 2 +
tools/perf/util/tool.h | 1 +
11 files changed, 127 insertions(+), 1 deletion(-)
diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
index 28bec7e78bc858ba..03d1129606328d6d 100644
--- a/tools/perf/Documentation/perf-script.txt
+++ b/tools/perf/Documentation/perf-script.txt
@@ -527,6 +527,11 @@ include::itrace.txt[]
The known limitations include exception handing such as
setjmp/longjmp will have calls/returns not match.
+--merge-callchains::
+ Enable merging deferred user callchains if available. This is the
+ default behavior. If you want to see separate CALLCHAIN_DEFERRED
+ records for some reason, use --no-merge-callchains explicitly.
+
:GMEXAMPLECMD: script
:GMEXAMPLESUBCMD:
include::guest-files.txt[]
diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index bd9245d2dd41aa48..51d2721b6db9dccb 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -2527,6 +2527,7 @@ int cmd_inject(int argc, const char **argv)
inject.tool.auxtrace = perf_event__repipe_auxtrace;
inject.tool.bpf_metadata = perf_event__repipe_op2_synth;
inject.tool.dont_split_sample_group = true;
+ inject.tool.merge_deferred_callchains = false;
inject.session = __perf_session__new(&data, &inject.tool,
/*trace_event_repipe=*/inject.output.is_pipe,
/*host_env=*/NULL);
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 2bc269f5fcef8023..add6b1c2aaf04270 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -1614,6 +1614,7 @@ int cmd_report(int argc, const char **argv)
report.tool.event_update = perf_event__process_event_update;
report.tool.feature = process_feature_event;
report.tool.ordering_requires_timestamps = true;
+ report.tool.merge_deferred_callchains = !dump_trace;
session = perf_session__new(&data, &report.tool);
if (IS_ERR(session)) {
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 85b42205a71b3993..62e43d3c5ad731a0 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -4009,6 +4009,7 @@ int cmd_script(int argc, const char **argv)
bool header_only = false;
bool script_started = false;
bool unsorted_dump = false;
+ bool merge_deferred_callchains = true;
char *rec_script_path = NULL;
char *rep_script_path = NULL;
struct perf_session *session;
@@ -4162,6 +4163,8 @@ int cmd_script(int argc, const char **argv)
"Guest code can be found in hypervisor process"),
OPT_BOOLEAN('\0', "stitch-lbr", &script.stitch_lbr,
"Enable LBR callgraph stitching approach"),
+ OPT_BOOLEAN('\0', "merge-callchains", &merge_deferred_callchains,
+ "Enable merge deferred user callchains"),
OPTS_EVSWITCH(&script.evswitch),
OPT_END()
};
@@ -4418,6 +4421,7 @@ int cmd_script(int argc, const char **argv)
script.tool.throttle = process_throttle_event;
script.tool.unthrottle = process_throttle_event;
script.tool.ordering_requires_timestamps = true;
+ script.tool.merge_deferred_callchains = merge_deferred_callchains;
session = perf_session__new(&data, &script.tool);
if (IS_ERR(session))
return PTR_ERR(session);
diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index 2884187ccbbecfdc..71dc5a070065dd2a 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -1838,3 +1838,32 @@ int sample__for_each_callchain_node(struct thread *thread, struct evsel *evsel,
}
return 0;
}
+
+int sample__merge_deferred_callchain(struct perf_sample *sample_orig,
+ struct perf_sample *sample_callchain)
+{
+ u64 nr_orig = sample_orig->callchain->nr - 1;
+ u64 nr_deferred = sample_callchain->callchain->nr;
+ struct ip_callchain *callchain;
+
+ if (sample_orig->callchain->nr < 2) {
+ sample_orig->deferred_callchain = false;
+ return -EINVAL;
+ }
+
+ callchain = calloc(1 + nr_orig + nr_deferred, sizeof(u64));
+ if (callchain == NULL) {
+ sample_orig->deferred_callchain = false;
+ return -ENOMEM;
+ }
+
+ callchain->nr = nr_orig + nr_deferred;
+ /* copy original including PERF_CONTEXT_USER_DEFERRED (but the cookie) */
+ memcpy(callchain->ips, sample_orig->callchain->ips, nr_orig * sizeof(u64));
+ /* copy deferred user callchains */
+ memcpy(&callchain->ips[nr_orig], sample_callchain->callchain->ips,
+ nr_deferred * sizeof(u64));
+
+ sample_orig->callchain = callchain;
+ return 0;
+}
diff --git a/tools/perf/util/callchain.h b/tools/perf/util/callchain.h
index d5ae4fbb7ce5fa44..2a52af8c80ace33c 100644
--- a/tools/perf/util/callchain.h
+++ b/tools/perf/util/callchain.h
@@ -318,4 +318,7 @@ int sample__for_each_callchain_node(struct thread *thread, struct evsel *evsel,
struct perf_sample *sample, int max_stack,
bool symbols, callchain_iter_fn cb, void *data);
+int sample__merge_deferred_callchain(struct perf_sample *sample_orig,
+ struct perf_sample *sample_callchain);
+
#endif /* __PERF_CALLCHAIN_H */
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index e8217efdda5323c6..03674d2cbd015e4f 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -85,6 +85,7 @@ void evlist__init(struct evlist *evlist, struct perf_cpu_map *cpus,
evlist->ctl_fd.pos = -1;
evlist->nr_br_cntr = -1;
metricgroup__rblist_init(&evlist->metric_events);
+ INIT_LIST_HEAD(&evlist->deferred_samples);
}
struct evlist *evlist__new(void)
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index 5e71e3dc60423079..911834ae7c2a6f76 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -92,6 +92,8 @@ struct evlist {
* of struct metric_expr.
*/
struct rblist metric_events;
+ /* samples with deferred_callchain would wait here. */
+ struct list_head deferred_samples;
};
struct evsel_str_handler {
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 361e15c1f26a96d0..dc570ad47ccc2c63 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1285,6 +1285,66 @@ static int evlist__deliver_sample(struct evlist *evlist, const struct perf_tool
per_thread);
}
+/*
+ * Samples with deferred callchains should wait for the next matching
+ * PERF_RECORD_CALLCHAIN_RECORD entries. Keep the events in a list and
+ * deliver them once it finds the callchains.
+ */
+struct deferred_event {
+ struct list_head list;
+ union perf_event *event;
+};
+
+static int evlist__deliver_deferred_callchain(struct evlist *evlist,
+ const struct perf_tool *tool,
+ union perf_event *event,
+ struct perf_sample *sample,
+ struct machine *machine)
+{
+ struct deferred_event *de, *tmp;
+ struct evsel *evsel;
+ int ret = 0;
+
+ if (!tool->merge_deferred_callchains) {
+ evsel = evlist__id2evsel(evlist, sample->id);
+ return tool->callchain_deferred(tool, event, sample,
+ evsel, machine);
+ }
+
+ list_for_each_entry_safe(de, tmp, &evlist->deferred_samples, list) {
+ struct perf_sample orig_sample;
+
+ ret = evlist__parse_sample(evlist, de->event, &orig_sample);
+ if (ret < 0) {
+ pr_err("failed to parse original sample\n");
+ break;
+ }
+
+ if (sample->tid != orig_sample.tid)
+ continue;
+
+ if (event->callchain_deferred.cookie == orig_sample.deferred_cookie)
+ sample__merge_deferred_callchain(&orig_sample, sample);
+ else
+ orig_sample.deferred_callchain = false;
+
+ evsel = evlist__id2evsel(evlist, orig_sample.id);
+ ret = evlist__deliver_sample(evlist, tool, de->event,
+ &orig_sample, evsel, machine);
+
+ if (orig_sample.deferred_callchain)
+ free(orig_sample.callchain);
+
+ list_del(&de->list);
+ free(de->event);
+ free(de);
+
+ if (ret)
+ break;
+ }
+ return ret;
+}
+
static int machines__deliver_event(struct machines *machines,
struct evlist *evlist,
union perf_event *event,
@@ -1313,6 +1373,22 @@ static int machines__deliver_event(struct machines *machines,
return 0;
}
dump_sample(evsel, event, sample, perf_env__arch(machine->env));
+ if (sample->deferred_callchain && tool->merge_deferred_callchains) {
+ struct deferred_event *de = malloc(sizeof(*de));
+ size_t sz = event->header.size;
+
+ if (de == NULL)
+ return -ENOMEM;
+
+ de->event = malloc(sz);
+ if (de->event == NULL) {
+ free(de);
+ return -ENOMEM;
+ }
+ memcpy(de->event, event, sz);
+ list_add_tail(&de->list, &evlist->deferred_samples);
+ return 0;
+ }
return evlist__deliver_sample(evlist, tool, event, sample, evsel, machine);
case PERF_RECORD_MMAP:
return tool->mmap(tool, event, sample, machine);
@@ -1372,7 +1448,8 @@ static int machines__deliver_event(struct machines *machines,
return tool->aux_output_hw_id(tool, event, sample, machine);
case PERF_RECORD_CALLCHAIN_DEFERRED:
dump_deferred_callchain(evsel, event, sample);
- return tool->callchain_deferred(tool, event, sample, evsel, machine);
+ return evlist__deliver_deferred_callchain(evlist, tool, event,
+ sample, machine);
default:
++evlist->stats.nr_unknown_events;
return -1;
diff --git a/tools/perf/util/tool.c b/tools/perf/util/tool.c
index e77f0e2ecc1f79db..27ba5849c74a2e7d 100644
--- a/tools/perf/util/tool.c
+++ b/tools/perf/util/tool.c
@@ -266,6 +266,7 @@ void perf_tool__init(struct perf_tool *tool, bool ordered_events)
tool->cgroup_events = false;
tool->no_warn = false;
tool->show_feat_hdr = SHOW_FEAT_NO_HEADER;
+ tool->merge_deferred_callchains = true;
tool->sample = process_event_sample_stub;
tool->mmap = process_event_stub;
@@ -448,6 +449,7 @@ void delegate_tool__init(struct delegate_tool *tool, struct perf_tool *delegate)
tool->tool.cgroup_events = delegate->cgroup_events;
tool->tool.no_warn = delegate->no_warn;
tool->tool.show_feat_hdr = delegate->show_feat_hdr;
+ tool->tool.merge_deferred_callchains = delegate->merge_deferred_callchains;
tool->tool.sample = delegate_sample;
tool->tool.read = delegate_read;
diff --git a/tools/perf/util/tool.h b/tools/perf/util/tool.h
index 9b9f0a8cbf3de4b5..e96b69d25a5b737d 100644
--- a/tools/perf/util/tool.h
+++ b/tools/perf/util/tool.h
@@ -90,6 +90,7 @@ struct perf_tool {
bool cgroup_events;
bool no_warn;
bool dont_split_sample_group;
+ bool merge_deferred_callchains;
enum show_feature_header show_feat_hdr;
};
--
2.52.0.rc2.455.g230fcf2819-goog
^ permalink raw reply related
* [PATCH v6 4/6] perf script: Display PERF_RECORD_CALLCHAIN_DEFERRED
From: Namhyung Kim @ 2025-11-20 23:48 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Ian Rogers, James Clark
Cc: Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
linux-perf-users, Steven Rostedt, Josh Poimboeuf, Indu Bhagat,
Jens Remus, Mathieu Desnoyers, linux-trace-kernel, bpf
In-Reply-To: <20251120234804.156340-1-namhyung@kernel.org>
Handle the deferred callchains in the script output.
$ perf script
...
pwd 2312 121.163435: 249113 cpu/cycles/P:
ffffffff845b78d8 __build_id_parse.isra.0+0x218 ([kernel.kallsyms])
ffffffff83bb5bf6 perf_event_mmap+0x2e6 ([kernel.kallsyms])
ffffffff83c31959 mprotect_fixup+0x1e9 ([kernel.kallsyms])
ffffffff83c31dc5 do_mprotect_pkey+0x2b5 ([kernel.kallsyms])
ffffffff83c3206f __x64_sys_mprotect+0x1f ([kernel.kallsyms])
ffffffff845e6692 do_syscall_64+0x62 ([kernel.kallsyms])
ffffffff8360012f entry_SYSCALL_64_after_hwframe+0x76 ([kernel.kallsyms])
b00000006 (cookie) ([unknown])
pwd 2312 121.163447: DEFERRED CALLCHAIN [cookie: b00000006]
7f18fe337fa7 mprotect+0x7 (/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2)
7f18fe330e0f _dl_sysdep_start+0x7f (/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2)
7f18fe331448 _dl_start_user+0x0 (/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2)
Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-script.c | 89 +++++++++++++++++++++++++++++++++
tools/perf/util/evsel_fprintf.c | 5 +-
2 files changed, 93 insertions(+), 1 deletion(-)
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 011962e1ee0f6898..85b42205a71b3993 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -2706,6 +2706,94 @@ static int process_sample_event(const struct perf_tool *tool,
return ret;
}
+static int process_deferred_sample_event(const struct perf_tool *tool,
+ union perf_event *event,
+ struct perf_sample *sample,
+ struct evsel *evsel,
+ struct machine *machine)
+{
+ struct perf_script *scr = container_of(tool, struct perf_script, tool);
+ struct perf_event_attr *attr = &evsel->core.attr;
+ struct evsel_script *es = evsel->priv;
+ unsigned int type = output_type(attr->type);
+ struct addr_location al;
+ FILE *fp = es->fp;
+ int ret = 0;
+
+ if (output[type].fields == 0)
+ return 0;
+
+ /* Set thread to NULL to indicate addr_al and al are not initialized */
+ addr_location__init(&al);
+
+ if (perf_time__ranges_skip_sample(scr->ptime_range, scr->range_num,
+ sample->time)) {
+ goto out_put;
+ }
+
+ if (debug_mode) {
+ if (sample->time < last_timestamp) {
+ pr_err("Samples misordered, previous: %" PRIu64
+ " this: %" PRIu64 "\n", last_timestamp,
+ sample->time);
+ nr_unordered++;
+ }
+ last_timestamp = sample->time;
+ goto out_put;
+ }
+
+ if (filter_cpu(sample))
+ goto out_put;
+
+ if (machine__resolve(machine, &al, sample) < 0) {
+ pr_err("problem processing %d event, skipping it.\n",
+ event->header.type);
+ ret = -1;
+ goto out_put;
+ }
+
+ if (al.filtered)
+ goto out_put;
+
+ if (!show_event(sample, evsel, al.thread, &al, NULL))
+ goto out_put;
+
+ if (evswitch__discard(&scr->evswitch, evsel))
+ goto out_put;
+
+ perf_sample__fprintf_start(scr, sample, al.thread, evsel,
+ PERF_RECORD_CALLCHAIN_DEFERRED, fp);
+ fprintf(fp, "DEFERRED CALLCHAIN [cookie: %llx]",
+ (unsigned long long)event->callchain_deferred.cookie);
+
+ if (PRINT_FIELD(IP)) {
+ struct callchain_cursor *cursor = NULL;
+
+ if (symbol_conf.use_callchain && sample->callchain) {
+ cursor = get_tls_callchain_cursor();
+ if (thread__resolve_callchain(al.thread, cursor, evsel,
+ sample, NULL, NULL,
+ scripting_max_stack)) {
+ pr_info("cannot resolve deferred callchains\n");
+ cursor = NULL;
+ }
+ }
+
+ fputc(cursor ? '\n' : ' ', fp);
+ sample__fprintf_sym(sample, &al, 0, output[type].print_ip_opts,
+ cursor, symbol_conf.bt_stop_list, fp);
+ }
+
+ fprintf(fp, "\n");
+
+ if (verbose > 0)
+ fflush(fp);
+
+out_put:
+ addr_location__exit(&al);
+ return ret;
+}
+
// Used when scr->per_event_dump is not set
static struct evsel_script es_stdout;
@@ -4303,6 +4391,7 @@ int cmd_script(int argc, const char **argv)
perf_tool__init(&script.tool, !unsorted_dump);
script.tool.sample = process_sample_event;
+ script.tool.callchain_deferred = process_deferred_sample_event;
script.tool.mmap = perf_event__process_mmap;
script.tool.mmap2 = perf_event__process_mmap2;
script.tool.comm = perf_event__process_comm;
diff --git a/tools/perf/util/evsel_fprintf.c b/tools/perf/util/evsel_fprintf.c
index 103984b29b1e10ae..10f1a03c28601e36 100644
--- a/tools/perf/util/evsel_fprintf.c
+++ b/tools/perf/util/evsel_fprintf.c
@@ -168,7 +168,10 @@ int sample__fprintf_callchain(struct perf_sample *sample, int left_alignment,
node_al.addr = addr;
node_al.map = map__get(map);
- if (print_symoffset) {
+ if (sample->deferred_callchain &&
+ sample->deferred_cookie == node->ip) {
+ printed += fprintf(fp, "(cookie)");
+ } else if (print_symoffset) {
printed += __symbol__fprintf_symname_offs(sym, &node_al,
print_unknown_as_addr,
true, fp);
--
2.52.0.rc2.455.g230fcf2819-goog
^ permalink raw reply related
* [PATCH v6 3/6] perf record: Add --call-graph fp,defer option for deferred callchains
From: Namhyung Kim @ 2025-11-20 23:48 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Ian Rogers, James Clark
Cc: Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
linux-perf-users, Steven Rostedt, Josh Poimboeuf, Indu Bhagat,
Jens Remus, Mathieu Desnoyers, linux-trace-kernel, bpf
In-Reply-To: <20251120234804.156340-1-namhyung@kernel.org>
Add a new callchain record mode option for deferred callchains. For now
it only works with FP (frame-pointer) mode.
And add the missing feature detection logic to clear the flag on old
kernels.
$ perf record --call-graph fp,defer -vv true
...
------------------------------------------------------------
perf_event_attr:
type 0 (PERF_TYPE_HARDWARE)
size 136
config 0 (PERF_COUNT_HW_CPU_CYCLES)
{ sample_period, sample_freq } 4000
sample_type IP|TID|TIME|CALLCHAIN|PERIOD
read_format ID|LOST
disabled 1
inherit 1
mmap 1
comm 1
freq 1
enable_on_exec 1
task 1
sample_id_all 1
mmap2 1
comm_exec 1
ksymbol 1
bpf_event 1
defer_callchain 1
defer_output 1
------------------------------------------------------------
sys_perf_event_open: pid 162755 cpu 0 group_fd -1 flags 0x8
sys_perf_event_open failed, error -22
switching off deferred callchain support
Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/Documentation/perf-config.txt | 3 +++
tools/perf/Documentation/perf-record.txt | 4 ++++
tools/perf/util/callchain.c | 16 +++++++++++++---
tools/perf/util/callchain.h | 1 +
tools/perf/util/evsel.c | 19 +++++++++++++++++++
tools/perf/util/evsel.h | 1 +
6 files changed, 41 insertions(+), 3 deletions(-)
diff --git a/tools/perf/Documentation/perf-config.txt b/tools/perf/Documentation/perf-config.txt
index c6f33565966735fe..642d1c490d9e3bcd 100644
--- a/tools/perf/Documentation/perf-config.txt
+++ b/tools/perf/Documentation/perf-config.txt
@@ -452,6 +452,9 @@ Variables
kernel space is controlled not by this option but by the
kernel config (CONFIG_UNWINDER_*).
+ The 'defer' mode can be used with 'fp' mode to enable deferred
+ user callchains (like 'fp,defer').
+
call-graph.dump-size::
The size of stack to dump in order to do post-unwinding. Default is 8192 (byte).
When using dwarf into record-mode, the default size will be used if omitted.
diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index 067891bd7da6edc8..e8b9aadbbfa50574 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -325,6 +325,10 @@ OPTIONS
by default. User can change the number by passing it after comma
like "--call-graph fp,32".
+ Also "defer" can be used with "fp" (like "--call-graph fp,defer") to
+ enable deferred user callchain which will collect user-space callchains
+ when the thread returns to the user space.
+
-q::
--quiet::
Don't print any warnings or messages, useful for scripting.
diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index d7b7eef740b9d6ed..2884187ccbbecfdc 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -275,9 +275,13 @@ int parse_callchain_record(const char *arg, struct callchain_param *param)
if (tok) {
unsigned long size;
- size = strtoul(tok, &name, 0);
- if (size < (unsigned) sysctl__max_stack())
- param->max_stack = size;
+ if (!strncmp(tok, "defer", sizeof("defer"))) {
+ param->defer = true;
+ } else {
+ size = strtoul(tok, &name, 0);
+ if (size < (unsigned) sysctl__max_stack())
+ param->max_stack = size;
+ }
}
break;
@@ -314,6 +318,12 @@ int parse_callchain_record(const char *arg, struct callchain_param *param)
} while (0);
free(buf);
+
+ if (param->defer && param->record_mode != CALLCHAIN_FP) {
+ pr_err("callchain: deferred callchain only works with FP\n");
+ return -EINVAL;
+ }
+
return ret;
}
diff --git a/tools/perf/util/callchain.h b/tools/perf/util/callchain.h
index 86ed9e4d04f9ee7b..d5ae4fbb7ce5fa44 100644
--- a/tools/perf/util/callchain.h
+++ b/tools/perf/util/callchain.h
@@ -98,6 +98,7 @@ extern bool dwarf_callchain_users;
struct callchain_param {
bool enabled;
+ bool defer;
enum perf_call_graph_mode record_mode;
u32 dump_size;
enum chain_mode mode;
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index f1a311637694ac0a..887c6ac6c49cc415 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1065,6 +1065,9 @@ static void __evsel__config_callchain(struct evsel *evsel, struct record_opts *o
pr_info("Disabling user space callchains for function trace event.\n");
attr->exclude_callchain_user = 1;
}
+
+ if (param->defer && !attr->exclude_callchain_user)
+ attr->defer_callchain = 1;
}
void evsel__config_callchain(struct evsel *evsel, struct record_opts *opts,
@@ -1511,6 +1514,7 @@ void evsel__config(struct evsel *evsel, struct record_opts *opts,
attr->mmap2 = track && !perf_missing_features.mmap2;
attr->comm = track;
attr->build_id = track && opts->build_id;
+ attr->defer_output = track && callchain->defer;
/*
* ksymbol is tracked separately with text poke because it needs to be
@@ -2199,6 +2203,10 @@ static int __evsel__prepare_open(struct evsel *evsel, struct perf_cpu_map *cpus,
static void evsel__disable_missing_features(struct evsel *evsel)
{
+ if (perf_missing_features.defer_callchain && evsel->core.attr.defer_callchain)
+ evsel->core.attr.defer_callchain = 0;
+ if (perf_missing_features.defer_callchain && evsel->core.attr.defer_output)
+ evsel->core.attr.defer_output = 0;
if (perf_missing_features.inherit_sample_read && evsel->core.attr.inherit &&
(evsel->core.attr.sample_type & PERF_SAMPLE_READ))
evsel->core.attr.inherit = 0;
@@ -2473,6 +2481,13 @@ static bool evsel__detect_missing_features(struct evsel *evsel, struct perf_cpu
/* Please add new feature detection here. */
+ attr.defer_callchain = true;
+ if (has_attr_feature(&attr, /*flags=*/0))
+ goto found;
+ perf_missing_features.defer_callchain = true;
+ pr_debug2("switching off deferred callchain support\n");
+ attr.defer_callchain = false;
+
attr.inherit = true;
attr.sample_type = PERF_SAMPLE_READ | PERF_SAMPLE_TID;
if (has_attr_feature(&attr, /*flags=*/0))
@@ -2584,6 +2599,10 @@ static bool evsel__detect_missing_features(struct evsel *evsel, struct perf_cpu
errno = old_errno;
check:
+ if ((evsel->core.attr.defer_callchain || evsel->core.attr.defer_output) &&
+ perf_missing_features.defer_callchain)
+ return true;
+
if (evsel->core.attr.inherit &&
(evsel->core.attr.sample_type & PERF_SAMPLE_READ) &&
perf_missing_features.inherit_sample_read)
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 3ae4ac8f9a37e009..a08130ff2e47a887 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -221,6 +221,7 @@ struct perf_missing_features {
bool branch_counters;
bool aux_action;
bool inherit_sample_read;
+ bool defer_callchain;
};
extern struct perf_missing_features perf_missing_features;
--
2.52.0.rc2.455.g230fcf2819-goog
^ permalink raw reply related
* [PATCH v6 2/6] perf tools: Minimal DEFERRED_CALLCHAIN support
From: Namhyung Kim @ 2025-11-20 23:48 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Ian Rogers, James Clark
Cc: Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
linux-perf-users, Steven Rostedt, Josh Poimboeuf, Indu Bhagat,
Jens Remus, Mathieu Desnoyers, linux-trace-kernel, bpf
In-Reply-To: <20251120234804.156340-1-namhyung@kernel.org>
Add a new event type for deferred callchains and a new callback for the
struct perf_tool. For now it doesn't actually handle the deferred
callchains but it just marks the sample if it has the PERF_CONTEXT_
USER_DEFFERED in the callchain array.
At least, perf report can dump the raw data with this change. Actually
this requires the next commit to enable attr.defer_callchain, but if you
already have a data file, it'll show the following result.
$ perf report -D
...
0x2158@perf.data [0x40]: event: 22
.
. ... raw event: size 64 bytes
. 0000: 16 00 00 00 02 00 40 00 06 00 00 00 0b 00 00 00 ......@.........
. 0010: 03 00 00 00 00 00 00 00 a7 7f 33 fe 18 7f 00 00 ..........3.....
. 0020: 0f 0e 33 fe 18 7f 00 00 48 14 33 fe 18 7f 00 00 ..3.....H.3.....
. 0030: 08 09 00 00 08 09 00 00 e6 7a e7 35 1c 00 00 00 .........z.5....
121163447014 0x2158 [0x40]: PERF_RECORD_CALLCHAIN_DEFERRED(IP, 0x2): 2312/2312: 0xb00000006
... FP chain: nr:3
..... 0: 00007f18fe337fa7
..... 1: 00007f18fe330e0f
..... 2: 00007f18fe331448
: unhandled!
Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/lib/perf/include/perf/event.h | 13 ++++++++++
tools/perf/util/event.c | 1 +
tools/perf/util/evsel.c | 31 +++++++++++++++++++++--
tools/perf/util/machine.c | 1 +
tools/perf/util/perf_event_attr_fprintf.c | 2 ++
tools/perf/util/sample.h | 2 ++
tools/perf/util/session.c | 20 +++++++++++++++
tools/perf/util/tool.c | 3 +++
tools/perf/util/tool.h | 3 ++-
9 files changed, 73 insertions(+), 3 deletions(-)
diff --git a/tools/lib/perf/include/perf/event.h b/tools/lib/perf/include/perf/event.h
index aa1e91c97a226e1a..43a8cb04994fa033 100644
--- a/tools/lib/perf/include/perf/event.h
+++ b/tools/lib/perf/include/perf/event.h
@@ -151,6 +151,18 @@ struct perf_record_switch {
__u32 next_prev_tid;
};
+struct perf_record_callchain_deferred {
+ struct perf_event_header header;
+ /*
+ * This is to match kernel and (deferred) user stacks together.
+ * The kernel part will be in the sample callchain array after
+ * the PERF_CONTEXT_USER_DEFERRED entry.
+ */
+ __u64 cookie;
+ __u64 nr;
+ __u64 ips[];
+};
+
struct perf_record_header_attr {
struct perf_event_header header;
struct perf_event_attr attr;
@@ -523,6 +535,7 @@ union perf_event {
struct perf_record_read read;
struct perf_record_throttle throttle;
struct perf_record_sample sample;
+ struct perf_record_callchain_deferred callchain_deferred;
struct perf_record_bpf_event bpf;
struct perf_record_ksymbol ksymbol;
struct perf_record_text_poke_event text_poke;
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index fcf44149feb20c35..4c92cc1a952c1d9f 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -61,6 +61,7 @@ static const char *perf_event__names[] = {
[PERF_RECORD_CGROUP] = "CGROUP",
[PERF_RECORD_TEXT_POKE] = "TEXT_POKE",
[PERF_RECORD_AUX_OUTPUT_HW_ID] = "AUX_OUTPUT_HW_ID",
+ [PERF_RECORD_CALLCHAIN_DEFERRED] = "CALLCHAIN_DEFERRED",
[PERF_RECORD_HEADER_ATTR] = "ATTR",
[PERF_RECORD_HEADER_EVENT_TYPE] = "EVENT_TYPE",
[PERF_RECORD_HEADER_TRACING_DATA] = "TRACING_DATA",
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index aee42666e882daab..f1a311637694ac0a 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -3089,6 +3089,20 @@ int evsel__parse_sample(struct evsel *evsel, union perf_event *event,
data->data_src = PERF_MEM_DATA_SRC_NONE;
data->vcpu = -1;
+ if (event->header.type == PERF_RECORD_CALLCHAIN_DEFERRED) {
+ const u64 max_callchain_nr = UINT64_MAX / sizeof(u64);
+
+ data->callchain = (struct ip_callchain *)&event->callchain_deferred.nr;
+ if (data->callchain->nr > max_callchain_nr)
+ return -EFAULT;
+
+ data->deferred_cookie = event->callchain_deferred.cookie;
+
+ if (evsel->core.attr.sample_id_all)
+ perf_evsel__parse_id_sample(evsel, event, data);
+ return 0;
+ }
+
if (event->header.type != PERF_RECORD_SAMPLE) {
if (!evsel->core.attr.sample_id_all)
return 0;
@@ -3213,12 +3227,25 @@ int evsel__parse_sample(struct evsel *evsel, union perf_event *event,
if (type & PERF_SAMPLE_CALLCHAIN) {
const u64 max_callchain_nr = UINT64_MAX / sizeof(u64);
+ u64 callchain_nr;
OVERFLOW_CHECK_u64(array);
data->callchain = (struct ip_callchain *)array++;
- if (data->callchain->nr > max_callchain_nr)
+ callchain_nr = data->callchain->nr;
+ if (callchain_nr > max_callchain_nr)
return -EFAULT;
- sz = data->callchain->nr * sizeof(u64);
+ sz = callchain_nr * sizeof(u64);
+ /*
+ * Save the cookie for the deferred user callchain. The last 2
+ * entries in the callchain should be the context marker and the
+ * cookie. The cookie will be used to match PERF_RECORD_
+ * CALLCHAIN_DEFERRED later.
+ */
+ if (evsel->core.attr.defer_callchain && callchain_nr >= 2 &&
+ data->callchain->ips[callchain_nr - 2] == PERF_CONTEXT_USER_DEFERRED) {
+ data->deferred_cookie = data->callchain->ips[callchain_nr - 1];
+ data->deferred_callchain = true;
+ }
OVERFLOW_CHECK(array, sz, max_size);
array = (void *)array + sz;
}
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index b5dd42588c916d91..841b711d970e9457 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -2124,6 +2124,7 @@ static int add_callchain_ip(struct thread *thread,
*cpumode = PERF_RECORD_MISC_KERNEL;
break;
case PERF_CONTEXT_USER:
+ case PERF_CONTEXT_USER_DEFERRED:
*cpumode = PERF_RECORD_MISC_USER;
break;
default:
diff --git a/tools/perf/util/perf_event_attr_fprintf.c b/tools/perf/util/perf_event_attr_fprintf.c
index 66b666d9ce649dd7..741c3d657a8b6ae7 100644
--- a/tools/perf/util/perf_event_attr_fprintf.c
+++ b/tools/perf/util/perf_event_attr_fprintf.c
@@ -343,6 +343,8 @@ int perf_event_attr__fprintf(FILE *fp, struct perf_event_attr *attr,
PRINT_ATTRf(inherit_thread, p_unsigned);
PRINT_ATTRf(remove_on_exec, p_unsigned);
PRINT_ATTRf(sigtrap, p_unsigned);
+ PRINT_ATTRf(defer_callchain, p_unsigned);
+ PRINT_ATTRf(defer_output, p_unsigned);
PRINT_ATTRn("{ wakeup_events, wakeup_watermark }", wakeup_events, p_unsigned, false);
PRINT_ATTRf(bp_type, p_unsigned);
diff --git a/tools/perf/util/sample.h b/tools/perf/util/sample.h
index fae834144ef42105..a8307b20a9ea8066 100644
--- a/tools/perf/util/sample.h
+++ b/tools/perf/util/sample.h
@@ -107,6 +107,8 @@ struct perf_sample {
/** @weight3: On x86 holds retire_lat, on powerpc holds p_stage_cyc. */
u16 weight3;
bool no_hw_idx; /* No hw_idx collected in branch_stack */
+ bool deferred_callchain; /* Has deferred user callchains */
+ u64 deferred_cookie;
char insn[MAX_INSN];
void *raw_data;
struct ip_callchain *callchain;
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 4b0236b2df2913e1..361e15c1f26a96d0 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -720,6 +720,7 @@ static perf_event__swap_op perf_event__swap_ops[] = {
[PERF_RECORD_CGROUP] = perf_event__cgroup_swap,
[PERF_RECORD_TEXT_POKE] = perf_event__text_poke_swap,
[PERF_RECORD_AUX_OUTPUT_HW_ID] = perf_event__all64_swap,
+ [PERF_RECORD_CALLCHAIN_DEFERRED] = perf_event__all64_swap,
[PERF_RECORD_HEADER_ATTR] = perf_event__hdr_attr_swap,
[PERF_RECORD_HEADER_EVENT_TYPE] = perf_event__event_type_swap,
[PERF_RECORD_HEADER_TRACING_DATA] = perf_event__tracing_data_swap,
@@ -854,6 +855,9 @@ static void callchain__printf(struct evsel *evsel,
for (i = 0; i < callchain->nr; i++)
printf("..... %2d: %016" PRIx64 "\n",
i, callchain->ips[i]);
+
+ if (sample->deferred_callchain)
+ printf("...... (deferred)\n");
}
static void branch_stack__printf(struct perf_sample *sample,
@@ -1123,6 +1127,19 @@ static void dump_sample(struct evsel *evsel, union perf_event *event,
sample_read__printf(sample, evsel->core.attr.read_format);
}
+static void dump_deferred_callchain(struct evsel *evsel, union perf_event *event,
+ struct perf_sample *sample)
+{
+ if (!dump_trace)
+ return;
+
+ printf("(IP, 0x%x): %d/%d: %#" PRIx64 "\n",
+ event->header.misc, sample->pid, sample->tid, sample->deferred_cookie);
+
+ if (evsel__has_callchain(evsel))
+ callchain__printf(evsel, sample);
+}
+
static void dump_read(struct evsel *evsel, union perf_event *event)
{
struct perf_record_read *read_event = &event->read;
@@ -1353,6 +1370,9 @@ static int machines__deliver_event(struct machines *machines,
return tool->text_poke(tool, event, sample, machine);
case PERF_RECORD_AUX_OUTPUT_HW_ID:
return tool->aux_output_hw_id(tool, event, sample, machine);
+ case PERF_RECORD_CALLCHAIN_DEFERRED:
+ dump_deferred_callchain(evsel, event, sample);
+ return tool->callchain_deferred(tool, event, sample, evsel, machine);
default:
++evlist->stats.nr_unknown_events;
return -1;
diff --git a/tools/perf/util/tool.c b/tools/perf/util/tool.c
index 22a8a4ffe05f778e..e77f0e2ecc1f79db 100644
--- a/tools/perf/util/tool.c
+++ b/tools/perf/util/tool.c
@@ -287,6 +287,7 @@ void perf_tool__init(struct perf_tool *tool, bool ordered_events)
tool->read = process_event_sample_stub;
tool->throttle = process_event_stub;
tool->unthrottle = process_event_stub;
+ tool->callchain_deferred = process_event_sample_stub;
tool->attr = process_event_synth_attr_stub;
tool->event_update = process_event_synth_event_update_stub;
tool->tracing_data = process_event_synth_tracing_data_stub;
@@ -335,6 +336,7 @@ bool perf_tool__compressed_is_stub(const struct perf_tool *tool)
}
CREATE_DELEGATE_SAMPLE(read);
CREATE_DELEGATE_SAMPLE(sample);
+CREATE_DELEGATE_SAMPLE(callchain_deferred);
#define CREATE_DELEGATE_ATTR(name) \
static int delegate_ ## name(const struct perf_tool *tool, \
@@ -468,6 +470,7 @@ void delegate_tool__init(struct delegate_tool *tool, struct perf_tool *delegate)
tool->tool.ksymbol = delegate_ksymbol;
tool->tool.bpf = delegate_bpf;
tool->tool.text_poke = delegate_text_poke;
+ tool->tool.callchain_deferred = delegate_callchain_deferred;
tool->tool.attr = delegate_attr;
tool->tool.event_update = delegate_event_update;
diff --git a/tools/perf/util/tool.h b/tools/perf/util/tool.h
index 88337cee1e3e2be3..9b9f0a8cbf3de4b5 100644
--- a/tools/perf/util/tool.h
+++ b/tools/perf/util/tool.h
@@ -44,7 +44,8 @@ enum show_feature_header {
struct perf_tool {
event_sample sample,
- read;
+ read,
+ callchain_deferred;
event_op mmap,
mmap2,
comm,
--
2.52.0.rc2.455.g230fcf2819-goog
^ permalink raw reply related
* [PATCH v6 1/6] tools headers UAPI: Sync linux/perf_event.h for deferred callchains
From: Namhyung Kim @ 2025-11-20 23:47 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Ian Rogers, James Clark
Cc: Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
linux-perf-users, Steven Rostedt, Josh Poimboeuf, Indu Bhagat,
Jens Remus, Mathieu Desnoyers, linux-trace-kernel, bpf
In-Reply-To: <20251120234804.156340-1-namhyung@kernel.org>
It needs to sync with the kernel to support user space changes for the
deferred callchains.
Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/include/uapi/linux/perf_event.h | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h
index 78a362b8002776e5..d292f96bc06f86bc 100644
--- a/tools/include/uapi/linux/perf_event.h
+++ b/tools/include/uapi/linux/perf_event.h
@@ -463,7 +463,9 @@ struct perf_event_attr {
inherit_thread : 1, /* children only inherit if cloned with CLONE_THREAD */
remove_on_exec : 1, /* event is removed from task on exec */
sigtrap : 1, /* send synchronous SIGTRAP on event */
- __reserved_1 : 26;
+ defer_callchain: 1, /* request PERF_RECORD_CALLCHAIN_DEFERRED records */
+ defer_output : 1, /* output PERF_RECORD_CALLCHAIN_DEFERRED records */
+ __reserved_1 : 24;
union {
__u32 wakeup_events; /* wake up every n events */
@@ -1239,6 +1241,22 @@ enum perf_event_type {
*/
PERF_RECORD_AUX_OUTPUT_HW_ID = 21,
+ /*
+ * This user callchain capture was deferred until shortly before
+ * returning to user space. Previous samples would have kernel
+ * callchains only and they need to be stitched with this to make full
+ * callchains.
+ *
+ * struct {
+ * struct perf_event_header header;
+ * u64 cookie;
+ * u64 nr;
+ * u64 ips[nr];
+ * struct sample_id sample_id;
+ * };
+ */
+ PERF_RECORD_CALLCHAIN_DEFERRED = 22,
+
PERF_RECORD_MAX, /* non-ABI */
};
@@ -1269,6 +1287,7 @@ enum perf_callchain_context {
PERF_CONTEXT_HV = (__u64)-32,
PERF_CONTEXT_KERNEL = (__u64)-128,
PERF_CONTEXT_USER = (__u64)-512,
+ PERF_CONTEXT_USER_DEFERRED = (__u64)-640,
PERF_CONTEXT_GUEST = (__u64)-2048,
PERF_CONTEXT_GUEST_KERNEL = (__u64)-2176,
--
2.52.0.rc2.455.g230fcf2819-goog
^ permalink raw reply related
* [PATCHSET v6 0/6] perf tools: Add deferred callchain support
From: Namhyung Kim @ 2025-11-20 23:47 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Ian Rogers, James Clark
Cc: Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
linux-perf-users, Steven Rostedt, Josh Poimboeuf, Indu Bhagat,
Jens Remus, Mathieu Desnoyers, linux-trace-kernel, bpf
Hello,
This is a new version of deferred callchain support as the kernel part
is merged to the tip tree. Actually this is based on Steve's work (v16).
https://lore.kernel.org/r/20250908175319.841517121@kernel.org
v6 changes)
* always copy the events (Ian)
* add flush in the pipe mode
v5: https://lore.kernel.org/r/20251120021046.94490-1-namhyung@kernel.org/
* update delegate tools (Ian)
* copy and flush remaining samples (Ian)
* add Ian's Reviewed-by tags
v4: https://lore.kernel.org/r/20251115234106.348571-1-namhyung@kernel.org
* add --call-graph fp,defer option (Ian, Steve)
* add more comment on the cookie (Ian)
* display cookie part in the deferred callchain (Ian)
v3: https://lore.kernel.org/r/20251114070018.160330-1-namhyung@kernel.org
* handle new attr.defer_output to generate deferred callchains
* fix crash when cookies don't match (Steven)
* disable merging for perf inject
* fix missing feature detection bug
* symbolize merged callchains properly
Here's an example session.
$ perf record --call-graph fp,defer pwd
/home/namhyung/project/linux
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.010 MB perf.data (29 samples) ]
$ perf evlist -v
cpu/cycles/P: type: 0 (PERF_TYPE_HARDWARE), size: 136, config: 0 (PERF_COUNT_HW_CPU_CYCLES),
{ sample_period, sample_freq }: 4000, sample_type: IP|TID|TIME|CALLCHAIN|PERIOD,
read_format: ID|LOST, disabled: 1, inherit: 1, mmap: 1, comm: 1, freq: 1, enable_on_exec: 1,
task: 1, sample_id_all: 1, mmap2: 1, comm_exec: 1, ksymbol: 1, bpf_event: 1, build_id: 1,
defer_callchain: 1, defer_output: 1
$ perf script
...
pwd 2312 121.163435: 249113 cpu/cycles/P:
ffffffff845b78d8 __build_id_parse.isra.0+0x218 ([kernel.kallsyms])
ffffffff83bb5bf6 perf_event_mmap+0x2e6 ([kernel.kallsyms])
ffffffff83c31959 mprotect_fixup+0x1e9 ([kernel.kallsyms])
ffffffff83c31dc5 do_mprotect_pkey+0x2b5 ([kernel.kallsyms])
ffffffff83c3206f __x64_sys_mprotect+0x1f ([kernel.kallsyms])
ffffffff845e6692 do_syscall_64+0x62 ([kernel.kallsyms])
ffffffff8360012f entry_SYSCALL_64_after_hwframe+0x76 ([kernel.kallsyms])
7f18fe337fa7 mprotect+0x7 (/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2)
7f18fe330e0f _dl_sysdep_start+0x7f (/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2)
7f18fe331448 _dl_start_user+0x0 (/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2)
...
$ perf script --no-merge-callchains
...
pwd 2312 121.163435: 249113 cpu/cycles/P:
ffffffff845b78d8 __build_id_parse.isra.0+0x218 ([kernel.kallsyms])
ffffffff83bb5bf6 perf_event_mmap+0x2e6 ([kernel.kallsyms])
ffffffff83c31959 mprotect_fixup+0x1e9 ([kernel.kallsyms])
ffffffff83c31dc5 do_mprotect_pkey+0x2b5 ([kernel.kallsyms])
ffffffff83c3206f __x64_sys_mprotect+0x1f ([kernel.kallsyms])
ffffffff845e6692 do_syscall_64+0x62 ([kernel.kallsyms])
ffffffff8360012f entry_SYSCALL_64_after_hwframe+0x76 ([kernel.kallsyms])
b00000006 (cookie) ([unknown])
pwd 2312 121.163447: DEFERRED CALLCHAIN [cookie: b00000006]
7f18fe337fa7 mprotect+0x7 (/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2)
7f18fe330e0f _dl_sysdep_start+0x7f (/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2)
7f18fe331448 _dl_start_user+0x0 (/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2)
...
The code is available at 'perf/defer-callchain-v6' branch in
git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
Thanks,
Namhyung
Namhyung Kim (6):
tools headers UAPI: Sync linux/perf_event.h for deferred callchains
perf tools: Minimal DEFERRED_CALLCHAIN support
perf record: Add --call-graph fp,defer option for deferred callchains
perf script: Display PERF_RECORD_CALLCHAIN_DEFERRED
perf tools: Merge deferred user callchains
perf tools: Flush remaining samples w/o deferred callchains
tools/include/uapi/linux/perf_event.h | 21 +++-
tools/lib/perf/include/perf/event.h | 13 ++
tools/perf/Documentation/perf-config.txt | 3 +
tools/perf/Documentation/perf-record.txt | 4 +
tools/perf/Documentation/perf-script.txt | 5 +
tools/perf/builtin-inject.c | 1 +
tools/perf/builtin-report.c | 1 +
tools/perf/builtin-script.c | 93 ++++++++++++++
tools/perf/util/callchain.c | 45 ++++++-
tools/perf/util/callchain.h | 4 +
tools/perf/util/event.c | 1 +
tools/perf/util/evlist.c | 1 +
tools/perf/util/evlist.h | 2 +
tools/perf/util/evsel.c | 50 +++++++-
tools/perf/util/evsel.h | 1 +
tools/perf/util/evsel_fprintf.c | 5 +-
tools/perf/util/machine.c | 1 +
tools/perf/util/perf_event_attr_fprintf.c | 2 +
tools/perf/util/sample.h | 2 +
tools/perf/util/session.c | 147 ++++++++++++++++++++++
tools/perf/util/tool.c | 5 +
tools/perf/util/tool.h | 4 +-
22 files changed, 403 insertions(+), 8 deletions(-)
--
2.52.0.rc2.455.g230fcf2819-goog
^ permalink raw reply
* [PATCH] ftrace: Allow tracing of some of the tracing code
From: Steven Rostedt @ 2025-11-20 23:15 UTC (permalink / raw)
To: LKML, Linux Trace Kernel
Cc: Masami Hiramatsu, Mathieu Desnoyers, Andrew Morton, Mark Rutland,
Tom Zanussi
From: Steven Rostedt <rostedt@goodmis.org>
There is times when tracing the tracing infrastructure can be useful for
debugging the tracing code. Currently all files in the tracing directory
are set to "notrace" the functions.
Add a new config option FUNCTION_SELF_TRACING that will allow some of the
files in the tracing infrastructure to be traced. It requires a config to
enable because it will add noise to the function tracer if events and
other tracing features are enabled. Tracing functions and events together
is quite common, so not tracing the event code should be the default.
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/Kconfig | 14 ++++++++++++++
kernel/trace/Makefile | 17 +++++++++++++++++
2 files changed, 31 insertions(+)
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index 99283b2dcfd6..e1214b9dc990 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -330,6 +330,20 @@ config DYNAMIC_FTRACE_WITH_ARGS
depends on DYNAMIC_FTRACE
depends on HAVE_DYNAMIC_FTRACE_WITH_ARGS
+config FUNCTION_SELF_TRACING
+ bool "Function trace tracing code"
+ depends on FUNCTION_TRACER
+ help
+ Normally all the tracing code is set to notrace, where the function
+ tracer will ignore all the tracing functions. Sometimes it is useful
+ for debugging to trace some of the tracing infratructure itself.
+ Enable this to allow some of the tracing infrastructure to be traced
+ by the function tracer. Note, this will likely add noise to function
+ tracing if events and other tracing features are enabled along with
+ function tracing.
+
+ If unsure, say N.
+
config FPROBE
bool "Kernel Function Probe (fprobe)"
depends on HAVE_FUNCTION_GRAPH_FREGS && HAVE_FTRACE_GRAPH_FUNC
diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
index dcb4e02afc5f..fc5dcc888e13 100644
--- a/kernel/trace/Makefile
+++ b/kernel/trace/Makefile
@@ -16,6 +16,23 @@ obj-y += trace_selftest_dynamic.o
endif
endif
+# Allow some files to be function traced
+ifdef CONFIG_FUNCTION_SELF_TRACING
+CFLAGS_trace_output.o = $(CC_FLAGS_FTRACE)
+CFLAGS_trace_seq.o = $(CC_FLAGS_FTRACE)
+CFLAGS_trace_stat.o = $(CC_FLAGS_FTRACE)
+CFLAGS_tracing_map.o = $(CC_FLAGS_FTRACE)
+CFLAGS_synth_event_gen_test.o = $(CC_FLAGS_FTRACE)
+CFLAGS_trace_events.o = $(CC_FLAGS_FTRACE)
+CFLAGS_trace_syscalls.o = $(CC_FLAGS_FTRACE)
+CFLAGS_trace_events_filter.o = $(CC_FLAGS_FTRACE)
+CFLAGS_trace_events_trigger.o = $(CC_FLAGS_FTRACE)
+CFLAGS_trace_events_synth.o = $(CC_FLAGS_FTRACE)
+CFLAGS_trace_events_hist.o = $(CC_FLAGS_FTRACE)
+CFLAGS_trace_events_user.o = $(CC_FLAGS_FTRACE)
+CFLAGS_trace_dynevent.o = $(CC_FLAGS_FTRACE)
+endif
+
ifdef CONFIG_FTRACE_STARTUP_TEST
CFLAGS_trace_kprobe_selftest.o = $(CC_FLAGS_FTRACE)
obj-$(CONFIG_KPROBE_EVENTS) += trace_kprobe_selftest.o
--
2.51.0
^ permalink raw reply related
* Re: [PATCH v12 mm-new 12/15] khugepaged: Introduce mTHP collapse support
From: Nico Pache @ 2025-11-20 22:32 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: linux-kernel, linux-trace-kernel, linux-mm, linux-doc, david, ziy,
baolin.wang, Liam.Howlett, ryan.roberts, dev.jain, corbet,
rostedt, mhiramat, mathieu.desnoyers, akpm, baohua, willy, peterx,
wangkefeng.wang, usamaarif642, sunnanyong, vishal.moola,
thomas.hellstrom, yang, kas, aarcange, raquini, anshuman.khandual,
catalin.marinas, tiwai, will, dave.hansen, jack, cl, jglisse,
surenb, zokeefe, hannes, rientjes, mhocko, rdunlap, hughd,
richard.weiyang, lance.yang, vbabka, rppt, jannh, pfalcato
In-Reply-To: <d03e079f-35c0-4fc1-8856-44fe33fabb2f@lucifer.local>
On Wed, Nov 19, 2025 at 4:56 AM Lorenzo Stoakes
<lorenzo.stoakes@oracle.com> wrote:
>
> On Wed, Oct 22, 2025 at 12:37:14PM -0600, Nico Pache wrote:
> > During PMD range scanning, track occupied pages in a bitmap. If mTHPs are
> > enabled we remove the restriction of max_ptes_none during the scan phase
> > to avoid missing potential mTHP candidates.
>
> It's a bit odd to open the commit message with a very specific
> implementation detail, I think we should instead open with a broad
> description of what we intend here, e.g. to permit mTHP collapse, before:
>
> - Discussing the algorithm used (in more detail than below!)
> - How and under what circumstances this algorithm is invoked
> - (Mention MADV_COLLAPSE not supporting mTHP as of yet)
> - THEN super-specific details like this
>
> >
> > Implement collapse_scan_bitmap() to perform binary recursion on the bitmap
> > and determine the best eligible order for the collapse. A stack struct is
> > used instead of traditional recursion. The algorithm splits the bitmap
> > into smaller chunks to find the best fit mTHP. max_ptes_none is scaled by
> > the attempted collapse order to determine how "full" an order must be
> > before being considered for collapse.
>
> I feel this is a _very_ brief description of a complicated algorithm. I
> think we should go into a lot more detail here. 'Binary recursion' is pretty
> hand-wavey, and you go from hand waving that to being super-specific about
> max_ptes_none before handwaving about 'fullness' of an order.
>
> All in all I find it super confusing - so I think you need to take a step
> back and 'explain it to me like I'm 5' here :)
Sounds good, I'll rework the commit message with your feedback in mind! Thanks!
>
> >
> > Once we determine what mTHP sizes fits best in that PMD range a collapse
> > is attempted. A minimum collapse order of 2 is used as this is the lowest
> > order supported by anon memory.
>
> I don't know what 'lowest order supported by anon memory' means?
>
> I guess really this is the minimum order contptes support for arm64 right?
Anonymous memory supports mTHP sizes of order 2 or greater.
>
> >
> > mTHP collapses reject regions containing swapped out or shared pages.
> > This is because adding new entries can lead to new none pages, and these
> > may lead to constant promotion into a higher order (m)THP. A similar
> > issue can occur with "max_ptes_none > HPAGE_PMD_NR/2" due to a collapse
> > introducing at least 2x the number of pages, and on a future scan will
> > satisfy the promotion condition once again. This issue is prevented via
> > the collapse_allowable_orders() function.
>
> Obviously this has been discussed to death, but you should update this to
> reflect the decided upon course (0, 511 + warning, etc.).
Yeah I wasnt sure whether to reference collapse_allowable_orders()
which should now dictate this limitation, or directly reference the
limitations here. Ill do both.
>
> >
> > Currently madv_collapse is not supported and will only attempt PMD
> > collapse.
>
> Good to highlight this.
>
> >
> > We can also remove the check for is_khugepaged inside the PMD scan as
> > the collapse_max_ptes_none() function handles this logic now.
>
> Again we're kind of leaping from mega handwaving to super-specific details
> :) let's make it all a lot more specific + clear, and then put the really
> niche details like this at the end of the commit msg (I mean this one is
> fine where it is ofc as a result :)
>
> >
> > Signed-off-by: Nico Pache <npache@redhat.com>
> > ---
> > include/linux/khugepaged.h | 2 +
> > mm/khugepaged.c | 128 ++++++++++++++++++++++++++++++++++---
> > 2 files changed, 122 insertions(+), 8 deletions(-)
> >
> > diff --git a/include/linux/khugepaged.h b/include/linux/khugepaged.h
> > index eb1946a70cff..179ce716e769 100644
> > --- a/include/linux/khugepaged.h
> > +++ b/include/linux/khugepaged.h
> > @@ -1,6 +1,8 @@
> > /* SPDX-License-Identifier: GPL-2.0 */
> > #ifndef _LINUX_KHUGEPAGED_H
> > #define _LINUX_KHUGEPAGED_H
> > +#define KHUGEPAGED_MIN_MTHP_ORDER 2
> > +#define MAX_MTHP_BITMAP_STACK (1UL << (ilog2(MAX_PTRS_PER_PTE) - KHUGEPAGED_MIN_MTHP_ORDER))
>
> This is an internal implementation detail, I don't think we need this in a
> header do we? I think this define should be in khugepaged.c.
sounds good!
>
> Also this is a really fiddly and confusing value, I don't think it's a good idea
> to just put this here without explanation.
This is sadly an outcome of ppc64. HPAGE_PMD_ORDER cannot be used due
to ppc defining this at runtime, so instead we use
lg(MAX_PTRS_PER_PTE).
>
> It's not even clear what it is. I'd probably rename it to MTHP_STACK_SIZE?
Yeah MTHP_STACK_SIZE is better!
>
> We need a comment that explains how you're deriving this, something like:
>
> /*
> * In order to determine mTHP order, we consider every possible mTHP size
> * starting with MAX_PTRS_PER_PTE PTE entries and stopping at
> * 2^KHUGEPAGED_MIN_THP_ORDER.
> *
> * We store (offset, order) pairs on the stack to do so, each describing a
> * candidate mTHP collapse.
> *
> * For each (offset, order) candidate mTHP range that we consider, we must
> * also consider candiate mTHPs at (offset, order - 1), and (offset + (1 <<
> * order), order - 1).
> *
> *
> * This is because each order can be split into two (an order expresses the
> * power-of-two size), so we examine each half of the next lower order
> * mTHP:
> *
> * offset mid_offset
> * . |
> * . v
> * |---------------.-------------------|
> * | PTE page table |
> * |---------------.-------------------|
> * <--------><-------->
> * order-1 order-1
> *
> * Given we must consider the range of KHUGEPAGED_MIN_MTHP_ORDER to
> * MAX_PTRS_PER_PTE number of PTE entries, this is the same as saying we
> * must consider KHUGEPAGED_MIN_MTHP_ORDER to lg2(MAX_PTRS_PER_PTE) mTHP
> * orders.
> *
> * As we must consider 2 possible mTHP ranges for each order, this requires
> * our stack to be 2^n, where n is the number of orders we must consider.
> *
> * And thus MTHP_STACK_SIZE is 2^(lg2(MAX_PTRS_PER_PTE) -
> * KHUGEPAGED_MIN_MTHP_ORDER).
> */
>
> This may seem (very) long-winded, but this is all really non-obvious.
>
> You can additionally rephrase this and utilise it in the commit message,
> description of the iterative recursion function and possibly elsewhere to
> describe the algorithm more clearly.
>
> In fact, since this should really be declared in khugepaged.c, and since
> you can place it just before the mthp collapse function, you could expand
> this to describe the algorithm as a whole and simply put the define and the
> function immediately next to each other after the comment?
Sounds good ill break this down in more detail and try to group the
functions/comments into one section. It may not be fully possible to
keep the code together as some things are needed early in the code (ie
MAX_STACK_SIZE would be needed at scan_control definition)
>
> >
> > #include <linux/mm.h>
> >
> > diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> > index 89a105124790..e2319bfd0065 100644
> > --- a/mm/khugepaged.c
> > +++ b/mm/khugepaged.c
> > @@ -93,6 +93,11 @@ static DEFINE_READ_MOSTLY_HASHTABLE(mm_slots_hash, MM_SLOTS_HASH_BITS);
> >
> > static struct kmem_cache *mm_slot_cache __ro_after_init;
> >
> > +struct scan_bit_state {
>
> Scan bit state is a bit of a weird name. Scanning what? What bit? State is
> kind of implied?
>
> struct order_offset_pair?
>
> struct mthp_range?
mthp_range sounds good to me!
>
> > + u8 order;
> > + u16 offset;
>
> Real mega nit, but feels more natural to put offset first here. As
> (position, size) seems more naturally the way to view this than (size,
> position).
ack!
>
> > +};
> > +
>
> Also needs comments...? Order of what? Offset in what?
>
> > struct collapse_control {
> > bool is_khugepaged;
> >
> > @@ -101,6 +106,13 @@ struct collapse_control {
> >
> > /* nodemask for allocation fallback */
> > nodemask_t alloc_nmask;
> > +
> > + /*
> > + * bitmap used to collapse mTHP sizes.
> > + */
>
> Nit but this should be on one line /* Bitmap used to collapse mTHP sizes */.
ACK, already handled this one!
>
> But we're not storing sizes though are we? And we're declaring two bitmaps?
>
> > + DECLARE_BITMAP(mthp_bitmap, HPAGE_PMD_NR);
>
> Really this is more of a PTE table bitmap but probably fine to call it this.
>
> > + DECLARE_BITMAP(mthp_bitmap_mask, HPAGE_PMD_NR);
>
> You've added random whitespace after the tab twice here? [tab][space]DECLARE_...
>
> > + struct scan_bit_state mthp_bitmap_stack[MAX_MTHP_BITMAP_STACK];
> > };
> >
> > /**
> > @@ -1357,6 +1369,85 @@ static int collapse_huge_page(struct mm_struct *mm, unsigned long pmd_address,
> > return result;
> > }
> >
> > +static void push_mthp_bitmap_stack(struct collapse_control *cc, int *top,
> > + u8 order, u16 offset)
>
> Not sure we need to say mthp_bitmap_stack everywhere. This is a local
> static function we can be a little more succinct.
>
> mthp_stack_push()?
looks good!
>
> > +{
> > + cc->mthp_bitmap_stack[++*top] = (struct scan_bit_state)
> > + { order, offset };
>
> This feels rather difficult to read, cc->mthp_bitmap_stack[++*top] in
> particular is rather too succinct.
>
> This would be better more broken out, e.g.:
>
> static void mthp_stack_push(struct collapse_control *cc, int *sizep,
> u8 order, u16 offset)
> {
> const int size = *sizep;
> struct scan_bit_state *stack = &cc->mthp_bitmap_stack[size];
>
> VM_WARN_ON_ONCE(idx >= MAX_MTHP_BITMAP_STACK);
> stack->order = order;
> stack->offset = offset;
> *sizep++;
> }
>
> (Note this requires the change I suggest below re: not defaulting top to -1
> but instead renaming it to stack_size and starting at 0, see below).
>
> Re: below comment having pop as a helper too, that can be:
>
> static struct scan_bit_state mthp_stack_pop(struct collapse_control *cc,
> int *sizep)
> {
> const int size = *sizep;
>
> VM_WARN_ON_ONCE(size <= 0);
> *sizep--;
> return cc->mthp_bitmap_stack[size - 1];
> }
ack sounds good. I implemented these more verbosely!
>
> > +}
> > +
> > +/*
> > + * collapse_scan_bitmap() consumes the bitmap that is generated during
> > + * collapse_scan_pmd() to determine what regions and mTHP orders fit best.
> > + *
> > + * Each bit in the bitmap represents a single occupied (!none/zero) page.
>
> In which bitmap? There are 2 that are declared. Be specific - cc->mthp_bitmap.
>
> > + * A stack structure cc->mthp_bitmap_stack is used to check different regions
>
> > + * of the bitmap for collapse eligibility. We start at the PMD order and
> > + * check if it is eligible for collapse; if not, we add two entries to the
>
> I questioned this since you start at HPAGE_PMD_ORDER -
> KHUGEPAGED_MIN_MTHP_ORDER, but then realised you're intentionally
> offsetting like that.
>
> See comments below about changing this.
>
> > + * stack at a lower order to represent the left and right halves of the region.
> > + *
> > + * For each region, we calculate the number of set bits and compare it
> > + * against a threshold derived from collapse_max_ptes_none(). A region is
> > + * eligible if the number of set bits exceeds this threshold.
> > + */
>
> I think we could be clearer here. Something like:
>
> ...
> * stack at a lower order to represent the left and right halves of the
> * portion of the PTE page table we are examining.
> *
>
> * For each of these, we determine how many PTE entries are occupied in the
> * range of PTE entries we propose to collapse, then compare this to the
> * number of PTE entries which would need to be set for a collapse to be
> * permitted at that order (accounting for max_ptes_none).
> *
> * If a collapse is permissible, we attempt to perform one. We do so for
> * every possible mTHP in the PTE page table.
> */
>
> > +static int collapse_scan_bitmap(struct mm_struct *mm, unsigned long address,
>
> Really inconsistent naming going on here, we're collapsing and scanning and
> what's the bitmap?
>
> How about mthp_collapse()?
ok sounds good
>
> > + int referenced, int unmapped, struct collapse_control *cc,
> > + bool *mmap_locked, unsigned long enabled_orders)
> > +{
> > + u8 order, next_order;
> > + u16 offset, mid_offset;
> > + int num_chunks;
> > + int bits_set, threshold_bits;
> > + int top = -1;
>
> This seems unnecessary and confusing. Just start at 0 and treat this as the
> exclusive end of the stack.
>
> You can rename this stack_size to make that clear. Have commented above
> about adjustments to push function and introducing pop helper.
>
>
> > + int collapsed = 0;
> > + int ret;
> > + struct scan_bit_state state;
> > + unsigned int max_none_ptes;
>
> Everywhere else we say max_ptes_none, let's maintain that convention here
> please.
ack, didn't realize it was different. Must have been tired eyes.
>
> > +
> > + push_mthp_bitmap_stack(cc, &top, HPAGE_PMD_ORDER - KHUGEPAGED_MIN_MTHP_ORDER, 0);
>
> See below re: order here, we should change this.
>
> > +
> > + while (top >= 0) {
> > + state = cc->mthp_bitmap_stack[top--];
>
> I hate that we have a push helper but then do pop manually. See above.
>
> > + order = state.order + KHUGEPAGED_MIN_MTHP_ORDER;
>
> OK so now the order isn't state.order but is instead state.order +
> KHUGEPAGED_MIN_MTHP_ORDER? :/ this is extremely confusing.
>
> We shouldn't call this field order if you're doing a hack where state.order
> isn't the order but instead is order - KHUGEPAGED_MIN_MTHP_ORDER.
>
> Just have state.order be the actual order? And change the below condition
> as mentioned there.
Sounds good, Wei already suggested something similar. This made more
sense when we were compressing the bitmap into 128 bits. Already
changed :)
>
> > + offset = state.offset;
> > + num_chunks = 1UL << order;
>
> What's a chunk? You do need to clarify these things. This is a new term not
> used elsewhere in THP code as far as I can tell?
>
> This is the number of pte entries no?
>
> nr_entries? nr_pte_entries?
Yeah that looks much cleaner. Some remnants from my RFC.
>
> > +
> > + /* Skip mTHP orders that are not enabled */
>
> Note we're also considering PMD here :) Probably we can just delete this
> comment, the code below makes it clear what you're doing.
>
> > + if (!test_bit(order, &enabled_orders))
> > + goto next_order;
> > +
> > + max_none_ptes = collapse_max_ptes_none(order, !cc->is_khugepaged);
>
> OK so this is going to be scaled to order.
>
> > +
> > + /* Calculate weight of the range */
>
> What's the weight of a range? This isn't a very helpful comment. You're
> counting the Hamming weight or much more clearly - the number of set bits.
>
> So it seems you're simply counting the number of bits you have accumulated
> so far in cc->mthp_bitmap, adding in the latest offset.
>
> So I'd say add a comment saying something like:
>
> /*
> * Determine how many PTE entries are populated in the range in which we
> * propose to collapse this mTHP.
> */
>
> > + bitmap_zero(cc->mthp_bitmap_mask, HPAGE_PMD_NR);
> > + bitmap_set(cc->mthp_bitmap_mask, offset, num_chunks);
> > + bits_set = bitmap_weight_and(cc->mthp_bitmap,
>
> I think this variable name is pretty horrible, we don't care that it's the
> number of bits set, we care about what it _means_ - that is the number of
> PTE occupied entries.
>
> So nr_occupied_pte_entries? Or nr_occupied_ptes?
ack, looks better!
>
> > + cc->mthp_bitmap_mask, HPAGE_PMD_NR);
>
> Frustrating there isn't a bitmap_weight_offset() or something, as you could
> do that in one go then...
Yeah it's a shame, my previous implementation was much worse. I found
a better solution (this one) a few versions ago.
>
> I think this could be made clearer by separating out the gnarly bitmap
> stuff into a helper function:
>
> static int mthp_nr_occupied_pte_entries(struct collapse_control *cc,
> struct scan_bit_state state)
> {
> const int nr_pte_entries = 1 << state.order;
>
> /* Setup cc->mthp_bitmap_mask to contain mask for candidate mTHP. */
> bitmap_zero(cc->mthp_bitmap_mask, HPAGE_PMD_NR);
> bitmap_set(cc->mthp_bitmap_mask, state.offset, nr_pte_entries);
> /* Mask against actually occupied PTE entries in PTE table. */
> return bitmap_weight_and(cc->mthp_bitmap,
> cc->mthp_bitmap_mask, HPAGE_PMD_NR);
> }
Done, this fits well with all the other helpers!
>
> > +
> > + threshold_bits = (1UL << order) - max_none_ptes - 1;
>
> We defined num_chunks to 1UL << order then don't use here? :)
Cleaned these up! thanks :)
>
> I'm not sure we need this to be a separate value, and I don't think we need
> the -1 either, which only confuses matter more.
>
> How about just changing the below conditional to (assuming we've renamed
> num_chunks to something sensible like nr_pte_entries and bits_set to
> nr_occupied_pte_entries):
>
> if (nr_occupied_pte_entries >= nr_pte_entries - max_none_ptes) {
> ...
> }
>
> > +
> > + /* Check if the region is eligible based on the threshold */
>
> Probalby don't need this comment with the change above.
ack, that does look cleaner. Although iirc there was some weird corner
case that required the -1. This was back when we were compressing the
bitmap. I reviewed the logic, I think we can go with this solution
now. Ill make sure to test the corner cases I have.
>
> > + if (bits_set > threshold_bits) {
> > + ret = collapse_huge_page(mm, address, referenced,
> > + unmapped, cc, mmap_locked,
> > + order, offset);
>
> We declare ret at the top of the function scope, then only use it
> here. That's confusing and unnecessary, just declare it in block scope
> here.
>
> > + if (ret == SCAN_SUCCEED) {
> > + collapsed += 1UL << order;
>
> Again we have defined num_chunks or rather nr_pte_entries but then
> open-code 1UL << order, let's just use the value we declared here.
>
> > + continue;
>
> This is kinda subtle, we don't bother considering lower orders any longer
> *in this range*, but do continue to consider mTHP collapse in other
> portions of the PTE page table.
>
> This shouldn't just be a 'continue' :) we need a comment here to explain
> that.
sounds good i'll separate and add more to the comments to help explain
the flow. (more applicable to patch 13)
>
> E.g.:
>
> /*
> * We have collapsed an mTHP in this range at the maximum order we
> * could, so we do not push lower orders on to the stack.
> */
> continue;
>
>
> > + }
> > + }
> > +
> > +next_order:
> > + if (state.order > 0) {
>
> This is a great example of how this is confusing by making state.order not
> actually be the order but the order - KHUGEPAGED_MIN_MTHP_ORDER.
>
> Just make the order correct and change this to > KHUGEPAGED_MIN_MTHP_ORDER.
>
> > + next_order = state.order - 1;
>
> Not sure we should have a label and a variable be the same thing.
>
> Also why are we decl'ing next_order at the top of the function but only using here?
ack.
>
> Just declare this here, like:
>
> if (state.order > KHUGEPAGED_MIN_MTHP_ORDER) {
> const u16 new_order = state.order - 1;
>
> ...
> }
>
> > + mid_offset = offset + (num_chunks / 2);
> > + push_mthp_bitmap_stack(cc, &top, next_order, mid_offset);
> > + push_mthp_bitmap_stack(cc, &top, next_order, offset);
>
> I guess one subtlety that wouldn't be obvious at first glance is that
> num_chunks (oh so badly needs a rename :) is a power-of-2 so we never get
> weird 'what if num_chunks is odd' scenarios to worry about.
>
> Probably doesn't really need a comment though. But this _does_ badly needs
> an ASCII diagram :):
>
> /*
> * The next lowest mTHP order possesses half the number of PTE
> * entries of the current one. We therefore must consider both
> * halves of the current mTHP:
> *
> * offset mid_offset
> * . |
> * . v
> * |---------------.-------------------|
> * | PTE page table |
> * |---------------.-------------------|
> * <--------><-------->
> * order-1 order-1
> */
>
yeah a diagram would help a lot!
> Since writing this I copied this above in another suggestion :P so you
> could always say 'see comment above for details' or something.
>
> > + }
> > + }
> > + return collapsed;
> > +}
>
> I've commented this function to death here, but a few more things to note.
>
> (BTW - I'm sorry I personally _hate_ repeated iterations of review when
> there's stuff you could have commented in prior iterations BUT I think I
> may end up having to once we respin due to the subtleties here.)
>
> - I wonder if we could just use a helper struct to make this all a little
> easier. Perhaps as it's realtively short code not so necesary, but a bit
> horrid to pass around all these paramters all the time. Maybe something
> for later THP rework.
>
> - Could we exit early if it's obvious that we won't be able to collapse due
> to max_ptes_none? I mean for one, we could at least check if the next
> lowest order is empty. If max_ptes_none was 511, then we would have
> already collapsed so can surely throw that out?
>
> I was thinking we could go 'upwards', starting with the lowest order and
> increasing order (essentially reverse things) then not collapsing until
> we can't collapse at a given order (so collapse at next lowest). That
> might be less efficient though.
>
> - Given that we're going to be only permitting max_ptes_none of 0 and 511
> for mTHP to start with, maybe things can be simplified - either all bits
> have to 1 or we don't care what they are we attempt colalpse anyway?
>
> But then again, maybe it's worth having the generic algorithm in place
> for future flexibility? Thoughts?
I'd prefer to leave the generic algorithm for future work. ie
eagerness, and Baolins shmem mthp collapse support.
>
> - How much have you tested this? This is pretty subtle stuff... it _seems_
> correct to me logically, but this is crying out for some userland testing
> that exhaustively throws every possible permutation of state at this
> function and asserts it's all correct.
Lots! check out https://gitlab.com/npache/khugepaged_mthp_test I use
this to test a number of edge cases, gather statistics, etc.
We've also run a number of our internal CI on this including
performance testing.
>
> - Are we not missing a bunch of stat counts? Didn't we add a bunch but now
> are actually setting them? E.g. if we reject mTHP candidates due to
> pte_max_none?
They should already be added in the generalization patches.
>
> > +
> > static int collapse_scan_pmd(struct mm_struct *mm,
> > struct vm_area_struct *vma,
> > unsigned long start_addr, bool *mmap_locked,
> > @@ -1364,11 +1455,15 @@ static int collapse_scan_pmd(struct mm_struct *mm,
> > {
> > pmd_t *pmd;
> > pte_t *pte, *_pte;
> > + int i;
> > int result = SCAN_FAIL, referenced = 0;
> > - int none_or_zero = 0, shared = 0;
> > + int none_or_zero = 0, shared = 0, nr_collapsed = 0;
> > struct page *page = NULL;
> > + unsigned int max_ptes_none;
>
> Correct spelling of this :)
>
> > struct folio *folio = NULL;
> > unsigned long addr;
> > + unsigned long enabled_orders;
> > + bool full_scan = true;
> > spinlock_t *ptl;
> > int node = NUMA_NO_NODE, unmapped = 0;
> >
> > @@ -1378,16 +1473,29 @@ static int collapse_scan_pmd(struct mm_struct *mm,
> > if (result != SCAN_SUCCEED)
> > goto out;
> >
> > + bitmap_zero(cc->mthp_bitmap, HPAGE_PMD_NR);
> > memset(cc->node_load, 0, sizeof(cc->node_load));
> > nodes_clear(cc->alloc_nmask);
> > +
> > + enabled_orders = collapse_allowable_orders(vma, vma->vm_flags, cc->is_khugepaged);
> > +
> > + /*
> > + * If PMD is the only enabled order, enforce max_ptes_none, otherwise
> > + * scan all pages to populate the bitmap for mTHP collapse.
> > + */
>
> Ugh this is quite ugly. I don't really love that we've converted this from
> doing the actual work to _mostly_ just populating the bitmap for the mthp
> logic.
>
> Then again it's only a couple places where this is checked, but it's pretty
> horrible that what once was _the logic that determines what is being
> considered for THP collapse' is now turned into 'the logic that populates a
> bitmap'.
>
> > + if (cc->is_khugepaged && enabled_orders == _BITUL(HPAGE_PMD_ORDER))
>
> I think this should be BIT(HPAGE_PMD_ORDER), I realise I reviewed the
> opposite before (or think I did) but as per David we prefer BIT() :)
>
> > + full_scan = false;
> > + max_ptes_none = collapse_max_ptes_none(HPAGE_PMD_ORDER, full_scan);
>
> Again really quite nasty, this may as well be:
>
> if (cc->is_khugepaged && enabled_orders == BIT(HPAGE_PMD_ORDER))
> max_ptes_none = khugepaged_max_ptes_none;
> else
> max_ptes_none = HPAGE_PMD_NR - 1;
>
> It makes this hack a lot more obvious.
The point of collapse_max_ptes_none was to centralize all this logic
into a helper function.
This check/toggle is mostly to preserve the original khugepaged
behavior (aborting during scan phase), if only PMD is enabled. ie)
full scan vs abort early.
>
> But also, what if !cc->is_khugepaged? We're going to scan everything even
> if we only have PMD? I thought we only considered PMD size for MADV_COLLAPSE?
MADV_COLLAPSE also ignores sysfs tunables. So if !khugepaged we still
do the full scan.
>
> > +
> > pte = pte_offset_map_lock(mm, pmd, start_addr, &ptl);
> > if (!pte) {
> > result = SCAN_PMD_NULL;
> > goto out;
> > }
> >
> > - for (addr = start_addr, _pte = pte; _pte < pte + HPAGE_PMD_NR;
> > - _pte++, addr += PAGE_SIZE) {
> > + for (i = 0; i < HPAGE_PMD_NR; i++) {
> > + _pte = pte + i;
> > + addr = start_addr + i * PAGE_SIZE;
>
> That's nicer. I still hate _pte...
>
> > pte_t pteval = ptep_get(_pte);
> > if (is_swap_pte(pteval)) {
> > ++unmapped;
> > @@ -1412,8 +1520,7 @@ static int collapse_scan_pmd(struct mm_struct *mm,
> > if (pte_none_or_zero(pteval)) {
> > ++none_or_zero;
> > if (!userfaultfd_armed(vma) &&
> > - (!cc->is_khugepaged ||
> > - none_or_zero <= khugepaged_max_ptes_none)) {
> > + none_or_zero <= max_ptes_none) {
>
> Why are we dropping !cc->is_khugepaged?
One of the nice things about using the collapse_max_ptes_none helper
is we simplify the logic here. if !cc->is_khugepaged (ie
madv_collapse) we ignore the max_ptes_none value. But the helper
already does this by returning HPAGE_PMD_NR in the case of
madv_collapse.
>
> > continue;
> > } else {
> > result = SCAN_EXCEED_NONE_PTE;
> > @@ -1461,6 +1568,8 @@ static int collapse_scan_pmd(struct mm_struct *mm,
> > }
> > }
> >
> > + /* Set bit for occupied pages */
> > + bitmap_set(cc->mthp_bitmap, i, 1);
>
> Maybe worth highlighting this is now _the entire point_ of the loop.
>
> I wonder if we shouldn't just separate this logic out and name it
> apppropriately? As we're into realms of real confusion here.
That is the clean up that conflicts with my series. We decided to wait
until after as with my changes the helper that was suggested needs to
be reworked.
>
> > /*
> > * Record which node the original page is from and save this
> > * information to cc->node_load[].
> > @@ -1517,9 +1626,12 @@ static int collapse_scan_pmd(struct mm_struct *mm,
> > out_unmap:
> > pte_unmap_unlock(pte, ptl);
> > if (result == SCAN_SUCCEED) {
> > - result = collapse_huge_page(mm, start_addr, referenced,
> > - unmapped, cc, mmap_locked,
> > - HPAGE_PMD_ORDER, 0);
>
> Hmm... what's actually enforcing that MADV_COLLAPSE isn't using this?
> You've not done any cc->khugepaged checks afaict?
The collapse_allowable_orders helper function handles this.
>
> It seems that you _are_ enabling this for MADV_COLLAPSE unless I've missed
> something?
>
> > + nr_collapsed = collapse_scan_bitmap(mm, start_addr, referenced, unmapped,
> > + cc, mmap_locked, enabled_orders);
> > + if (nr_collapsed > 0)
> > + result = SCAN_SUCCEED;
> > + else
> > + result = SCAN_FAIL;
> > }
> > out:
> > trace_mm_khugepaged_scan_pmd(mm, folio, referenced,
> > --
> > 2.51.0
> >
>
> Thanks, Lorenzo
Thanks for the very thorough review! Hopefully I didn't miss any of
your points. I'll get these changes in place before my next version :)
Cheers,
-- Nico
>
^ permalink raw reply
* Re: [PATCHv3 bpf-next 5/8] ftrace: Add update_ftrace_direct_mod function
From: bot+bpf-ci @ 2025-11-20 22:23 UTC (permalink / raw)
To: jolsa, rostedt, revest, mark.rutland
Cc: bpf, linux-kernel, linux-trace-kernel, linux-arm-kernel, ast,
daniel, andrii, menglong8.dong, song, ast, andrii, daniel,
martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai
In-Reply-To: <20251120212402.466524-6-jolsa@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 1662 bytes --]
> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index c571deeff..4e2c0ed76 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
[ ... ]
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index cc730a8fd..5243aefb6 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
[ ... ]
> @@ -6486,6 +6486,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;
^^^^^^^^^^^^^^^^
Can this crash if ops->func_hash is NULL? In update_ftrace_direct_add()
and update_ftrace_direct_del(), there's a defensive check:
old_filter_hash = ops->func_hash ? ops->func_hash->filter_hash : NULL;
But update_ftrace_direct_mod() dereferences ops->func_hash before any
validation checks occur. The check_direct_multi(ops) validation happens
later, but the dereference occurs first at function entry.
> + 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 -EINVAL;
> + if (check_direct_multi(ops))
> + return -EINVAL;
[ ... ]
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/19552032346
^ permalink raw reply
* Re: [PATCHv3 bpf-next 4/8] ftrace: Add update_ftrace_direct_del function
From: bot+bpf-ci @ 2025-11-20 22:23 UTC (permalink / raw)
To: jolsa, rostedt, revest, mark.rutland
Cc: bpf, linux-kernel, linux-trace-kernel, linux-arm-kernel, ast,
daniel, andrii, menglong8.dong, song, ast, andrii, daniel,
martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai
In-Reply-To: <20251120212402.466524-5-jolsa@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 2989 bytes --]
> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index 9cf4cd56d..c571deeff 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
> @@ -551,6 +551,7 @@ int modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr);
> int modify_ftrace_direct_nolock(struct ftrace_ops *ops, unsigned long addr);
>
> int update_ftrace_direct_add(struct ftrace_ops *ops, struct ftrace_hash *hash);
> +int update_ftrace_direct_del(struct ftrace_ops *ops, struct ftrace_hash *hash);
>
> void ftrace_stub_direct_tramp(void);
>
> @@ -583,6 +584,11 @@ static inline int update_ftrace_direct_add(struct ftrace_ops *ops, struct ftrace
> return -ENODEV;
> }
>
> +static inline int update_ftrace_direct_del(struct ftrace_ops *ops, struct ftrace_hash *hash)
> +{
> + return -ENODEV;
> +}
> +
[ ... ]
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 850ff55ff..cc730a8fd 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
[ ... ]
> +/**
> + * 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);
^^^^^^^^^^^^
Can hash_sub() handle a NULL hash pointer in parameter a? I notice the
parallel function hash_add() safely handles NULL via hash_count() and
alloc_and_copy_ftrace_hash() which checks ftrace_hash_empty().
However, hash_sub() directly dereferences a->size_bits before passing a
to alloc_and_copy_ftrace_hash(). This will crash if a is NULL.
This matters because update_ftrace_direct_del() calls hash_sub() with
old_filter_hash which can be NULL:
old_filter_hash = ops->func_hash ? ops->func_hash->filter_hash : NULL;
...
new_filter_hash = hash_sub(old_filter_hash, hash);
The existing update_ftrace_direct_add() has the same defensive check and
passes the potentially-NULL old_filter_hash to hash_add(), which handles
it correctly.
> + 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)
> +{
[ ... ]
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/19552032346
^ permalink raw reply
* Re: [PATCH v5 5/6] perf tools: Merge deferred user callchains
From: Namhyung Kim @ 2025-11-20 21:44 UTC (permalink / raw)
To: Ian Rogers
Cc: Arnaldo Carvalho de Melo, James Clark, Jiri Olsa, Adrian Hunter,
Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users,
Steven Rostedt, Josh Poimboeuf, Indu Bhagat, Jens Remus,
Mathieu Desnoyers, linux-trace-kernel, bpf
In-Reply-To: <CAP-5=fXO=QEOcFyRGbZ6AqmcMHwLke=KsDSDR3KcxCECJF3jNA@mail.gmail.com>
On Wed, Nov 19, 2025 at 09:13:50PM -0800, Ian Rogers wrote:
> On Wed, Nov 19, 2025 at 6:11 PM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > Save samples with deferred callchains in a separate list and deliver
> > them after merging the user callchains. If users don't want to merge
> > they can set tool->merge_deferred_callchains to false to prevent the
> > behavior.
> >
> > With previous result, now perf script will show the merged callchains.
> >
> > $ perf script
> > ...
> > pwd 2312 121.163435: 249113 cpu/cycles/P:
> > ffffffff845b78d8 __build_id_parse.isra.0+0x218 ([kernel.kallsyms])
> > ffffffff83bb5bf6 perf_event_mmap+0x2e6 ([kernel.kallsyms])
> > ffffffff83c31959 mprotect_fixup+0x1e9 ([kernel.kallsyms])
> > ffffffff83c31dc5 do_mprotect_pkey+0x2b5 ([kernel.kallsyms])
> > ffffffff83c3206f __x64_sys_mprotect+0x1f ([kernel.kallsyms])
> > ffffffff845e6692 do_syscall_64+0x62 ([kernel.kallsyms])
> > ffffffff8360012f entry_SYSCALL_64_after_hwframe+0x76 ([kernel.kallsyms])
> > 7f18fe337fa7 mprotect+0x7 (/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2)
> > 7f18fe330e0f _dl_sysdep_start+0x7f (/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2)
> > 7f18fe331448 _dl_start_user+0x0 (/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2)
> > ...
> >
> > The old output can be get using --no-merge-callchain option.
> > Also perf report can get the user callchain entry at the end.
> >
> > $ perf report --no-children --stdio -q -S __build_id_parse.isra.0
> > # symbol: __build_id_parse.isra.0
> > 8.40% pwd [kernel.kallsyms]
> > |
> > ---__build_id_parse.isra.0
> > perf_event_mmap
> > mprotect_fixup
> > do_mprotect_pkey
> > __x64_sys_mprotect
> > do_syscall_64
> > entry_SYSCALL_64_after_hwframe
> > mprotect
> > _dl_sysdep_start
> > _dl_start_user
> >
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> > tools/perf/Documentation/perf-script.txt | 5 ++
> > tools/perf/builtin-inject.c | 1 +
> > tools/perf/builtin-report.c | 1 +
> > tools/perf/builtin-script.c | 4 ++
> > tools/perf/util/callchain.c | 29 ++++++++++
> > tools/perf/util/callchain.h | 3 ++
> > tools/perf/util/evlist.c | 1 +
> > tools/perf/util/evlist.h | 2 +
> > tools/perf/util/session.c | 67 +++++++++++++++++++++++-
> > tools/perf/util/tool.c | 2 +
> > tools/perf/util/tool.h | 1 +
> > 11 files changed, 115 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
> > index 28bec7e78bc858ba..03d1129606328d6d 100644
> > --- a/tools/perf/Documentation/perf-script.txt
> > +++ b/tools/perf/Documentation/perf-script.txt
> > @@ -527,6 +527,11 @@ include::itrace.txt[]
> > The known limitations include exception handing such as
> > setjmp/longjmp will have calls/returns not match.
> >
> > +--merge-callchains::
> > + Enable merging deferred user callchains if available. This is the
> > + default behavior. If you want to see separate CALLCHAIN_DEFERRED
> > + records for some reason, use --no-merge-callchains explicitly.
> > +
> > :GMEXAMPLECMD: script
> > :GMEXAMPLESUBCMD:
> > include::guest-files.txt[]
> > diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> > index bd9245d2dd41aa48..51d2721b6db9dccb 100644
> > --- a/tools/perf/builtin-inject.c
> > +++ b/tools/perf/builtin-inject.c
> > @@ -2527,6 +2527,7 @@ int cmd_inject(int argc, const char **argv)
> > inject.tool.auxtrace = perf_event__repipe_auxtrace;
> > inject.tool.bpf_metadata = perf_event__repipe_op2_synth;
> > inject.tool.dont_split_sample_group = true;
> > + inject.tool.merge_deferred_callchains = false;
> > inject.session = __perf_session__new(&data, &inject.tool,
> > /*trace_event_repipe=*/inject.output.is_pipe,
> > /*host_env=*/NULL);
> > diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> > index 2bc269f5fcef8023..add6b1c2aaf04270 100644
> > --- a/tools/perf/builtin-report.c
> > +++ b/tools/perf/builtin-report.c
> > @@ -1614,6 +1614,7 @@ int cmd_report(int argc, const char **argv)
> > report.tool.event_update = perf_event__process_event_update;
> > report.tool.feature = process_feature_event;
> > report.tool.ordering_requires_timestamps = true;
> > + report.tool.merge_deferred_callchains = !dump_trace;
> >
> > session = perf_session__new(&data, &report.tool);
> > if (IS_ERR(session)) {
> > diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> > index 85b42205a71b3993..62e43d3c5ad731a0 100644
> > --- a/tools/perf/builtin-script.c
> > +++ b/tools/perf/builtin-script.c
> > @@ -4009,6 +4009,7 @@ int cmd_script(int argc, const char **argv)
> > bool header_only = false;
> > bool script_started = false;
> > bool unsorted_dump = false;
> > + bool merge_deferred_callchains = true;
> > char *rec_script_path = NULL;
> > char *rep_script_path = NULL;
> > struct perf_session *session;
> > @@ -4162,6 +4163,8 @@ int cmd_script(int argc, const char **argv)
> > "Guest code can be found in hypervisor process"),
> > OPT_BOOLEAN('\0', "stitch-lbr", &script.stitch_lbr,
> > "Enable LBR callgraph stitching approach"),
> > + OPT_BOOLEAN('\0', "merge-callchains", &merge_deferred_callchains,
> > + "Enable merge deferred user callchains"),
> > OPTS_EVSWITCH(&script.evswitch),
> > OPT_END()
> > };
> > @@ -4418,6 +4421,7 @@ int cmd_script(int argc, const char **argv)
> > script.tool.throttle = process_throttle_event;
> > script.tool.unthrottle = process_throttle_event;
> > script.tool.ordering_requires_timestamps = true;
> > + script.tool.merge_deferred_callchains = merge_deferred_callchains;
> > session = perf_session__new(&data, &script.tool);
> > if (IS_ERR(session))
> > return PTR_ERR(session);
> > diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
> > index 2884187ccbbecfdc..71dc5a070065dd2a 100644
> > --- a/tools/perf/util/callchain.c
> > +++ b/tools/perf/util/callchain.c
> > @@ -1838,3 +1838,32 @@ int sample__for_each_callchain_node(struct thread *thread, struct evsel *evsel,
> > }
> > return 0;
> > }
> > +
> > +int sample__merge_deferred_callchain(struct perf_sample *sample_orig,
> > + struct perf_sample *sample_callchain)
> > +{
> > + u64 nr_orig = sample_orig->callchain->nr - 1;
> > + u64 nr_deferred = sample_callchain->callchain->nr;
> > + struct ip_callchain *callchain;
> > +
> > + if (sample_orig->callchain->nr < 2) {
> > + sample_orig->deferred_callchain = false;
> > + return -EINVAL;
> > + }
> > +
> > + callchain = calloc(1 + nr_orig + nr_deferred, sizeof(u64));
> > + if (callchain == NULL) {
> > + sample_orig->deferred_callchain = false;
> > + return -ENOMEM;
> > + }
> > +
> > + callchain->nr = nr_orig + nr_deferred;
> > + /* copy original including PERF_CONTEXT_USER_DEFERRED (but the cookie) */
> > + memcpy(callchain->ips, sample_orig->callchain->ips, nr_orig * sizeof(u64));
> > + /* copy deferred user callchains */
> > + memcpy(&callchain->ips[nr_orig], sample_callchain->callchain->ips,
> > + nr_deferred * sizeof(u64));
> > +
> > + sample_orig->callchain = callchain;
> > + return 0;
> > +}
> > diff --git a/tools/perf/util/callchain.h b/tools/perf/util/callchain.h
> > index d5ae4fbb7ce5fa44..2a52af8c80ace33c 100644
> > --- a/tools/perf/util/callchain.h
> > +++ b/tools/perf/util/callchain.h
> > @@ -318,4 +318,7 @@ int sample__for_each_callchain_node(struct thread *thread, struct evsel *evsel,
> > struct perf_sample *sample, int max_stack,
> > bool symbols, callchain_iter_fn cb, void *data);
> >
> > +int sample__merge_deferred_callchain(struct perf_sample *sample_orig,
> > + struct perf_sample *sample_callchain);
> > +
> > #endif /* __PERF_CALLCHAIN_H */
> > diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> > index e8217efdda5323c6..03674d2cbd015e4f 100644
> > --- a/tools/perf/util/evlist.c
> > +++ b/tools/perf/util/evlist.c
> > @@ -85,6 +85,7 @@ void evlist__init(struct evlist *evlist, struct perf_cpu_map *cpus,
> > evlist->ctl_fd.pos = -1;
> > evlist->nr_br_cntr = -1;
> > metricgroup__rblist_init(&evlist->metric_events);
> > + INIT_LIST_HEAD(&evlist->deferred_samples);
> > }
> >
> > struct evlist *evlist__new(void)
> > diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
> > index 5e71e3dc60423079..911834ae7c2a6f76 100644
> > --- a/tools/perf/util/evlist.h
> > +++ b/tools/perf/util/evlist.h
> > @@ -92,6 +92,8 @@ struct evlist {
> > * of struct metric_expr.
> > */
> > struct rblist metric_events;
> > + /* samples with deferred_callchain would wait here. */
> > + struct list_head deferred_samples;
> > };
> >
> > struct evsel_str_handler {
> > diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> > index 361e15c1f26a96d0..2e777fd1bcf6707b 100644
> > --- a/tools/perf/util/session.c
> > +++ b/tools/perf/util/session.c
> > @@ -1285,6 +1285,60 @@ static int evlist__deliver_sample(struct evlist *evlist, const struct perf_tool
> > per_thread);
> > }
> >
> > +struct deferred_event {
> > + struct list_head list;
> > + union perf_event *event;
>
> Is this the old version of the patch? No comment and it seems the
> event's memory isn't copied. I'm worried as we have events in stack
> allocated memory such as:
> https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/session.c?h=perf-tools-next#n1618
> or copies:
> https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/builtin-inject.c?h=perf-tools-next#n336
> or just because the ring buffer overwrites itself. My belief is that
> we don't hold events, and the associated parsed sample, longer than
> the tool callback because it'll be wrong/corrupt after that. Here the
> deferred callchain events are all being held longer than a single tool
> event callback.
I added the conditional copy in the next patch, but as you said, it'd be
safer to copy always. The concern is the performance of perf report
which is already slow and I don't want to make it worse. But let's
start with the simple, safe version and optimize it later.
Thanks,
Namhyung
^ permalink raw reply
* [PATCHv3 bpf-next 8/8] bpf, x86: Use single ftrace_ops for direct calls
From: Jiri Olsa @ 2025-11-20 21:24 UTC (permalink / raw)
To: Steven Rostedt, Florent Revest, Mark Rutland
Cc: bpf, linux-kernel, linux-trace-kernel, linux-arm-kernel,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Menglong Dong, Song Liu
In-Reply-To: <20251120212402.466524-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 | 172 +++++++++++++++++++++++++++++++++++-----
kernel/trace/Kconfig | 3 +
kernel/trace/ftrace.c | 7 +-
4 files changed, 164 insertions(+), 19 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 d8762f7990ed..9cc4de215df7 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,126 @@ 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)
+{
+ int ret;
+
+ ret = ftrace_set_filter_ip(ops, (unsigned long)ip, 0, 1);
+ if (ret)
+ return ret;
+ 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 +303,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 +332,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 +346,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,10 +368,7 @@ static int register_fentry(struct bpf_trampoline *tr, void *new_addr)
}
if (tr->func.ftrace_managed) {
- ret = ftrace_set_filter_ip(tr->fops, (unsigned long)ip, 0, 1);
- if (ret)
- return ret;
- ret = register_ftrace_direct(tr->fops, (long)new_addr);
+ ret = direct_ops_add(tr->fops, tr->ip, new_addr);
} else {
ret = bpf_arch_text_poke(ip, BPF_MOD_CALL, NULL, new_addr);
}
@@ -865,10 +1004,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 b0e4d7a5d47c..4380839d2422 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
* [PATCHv3 bpf-next 7/8] ftrace: Factor ftrace_ops ops_func interface
From: Jiri Olsa @ 2025-11-20 21:24 UTC (permalink / raw)
To: Steven Rostedt, Florent Revest, Mark Rutland
Cc: bpf, linux-kernel, linux-trace-kernel, linux-arm-kernel,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Menglong Dong, Song Liu
In-Reply-To: <20251120212402.466524-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 4e2c0ed769fc..5aa37dec0081 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -402,7 +402,7 @@ enum ftrace_ops_cmd {
* Negative on failure. The return value is dependent on the
* callback.
*/
-typedef int (*ftrace_ops_func_t)(struct ftrace_ops *op, enum ftrace_ops_cmd cmd);
+typedef int (*ftrace_ops_func_t)(struct ftrace_ops *op, unsigned long ip, enum ftrace_ops_cmd cmd);
#ifdef CONFIG_DYNAMIC_FTRACE
diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index f33693061c2e..d8762f7990ed 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 5243aefb6096..b0e4d7a5d47c 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) {
@@ -8976,7 +8976,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;
}
@@ -9023,7 +9023,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
* [PATCHv3 bpf-next 6/8] bpf: Add trampoline ip hash table
From: Jiri Olsa @ 2025-11-20 21:24 UTC (permalink / raw)
To: Steven Rostedt, Florent Revest, Mark Rutland
Cc: bpf, linux-kernel, linux-trace-kernel, linux-arm-kernel,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Menglong Dong, Song Liu
In-Reply-To: <20251120212402.466524-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 04104397c432..f33693061c2e 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++)
@@ -801,7 +806,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;
@@ -821,7 +826,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;
@@ -857,7 +862,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);
@@ -1130,7 +1136,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
* [PATCHv3 bpf-next 5/8] ftrace: Add update_ftrace_direct_mod function
From: Jiri Olsa @ 2025-11-20 21:23 UTC (permalink / raw)
To: Steven Rostedt, Florent Revest, Mark Rutland
Cc: bpf, linux-kernel, linux-trace-kernel, linux-arm-kernel,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Menglong Dong, Song Liu
In-Reply-To: <20251120212402.466524-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 c571deeff840..4e2c0ed769fc 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -552,6 +552,7 @@ int modify_ftrace_direct_nolock(struct ftrace_ops *ops, unsigned long addr);
int update_ftrace_direct_add(struct ftrace_ops *ops, struct ftrace_hash *hash);
int update_ftrace_direct_del(struct ftrace_ops *ops, struct ftrace_hash *hash);
+int update_ftrace_direct_mod(struct ftrace_ops *ops, struct ftrace_hash *hash, bool do_direct_lock);
void ftrace_stub_direct_tramp(void);
@@ -589,6 +590,11 @@ static inline int update_ftrace_direct_del(struct ftrace_ops *ops, struct ftrace
return -ENODEV;
}
+static inline int update_ftrace_direct_mod(struct ftrace_ops *ops, struct ftrace_hash *hash, bool do_direct_lock)
+{
+ return -ENODEV;
+}
+
/*
* This must be implemented by the architecture.
* It is the way the ftrace direct_ops helper, when called
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index cc730a8fd087..5243aefb6096 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -6486,6 +6486,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 -EINVAL;
+ if (check_direct_multi(ops))
+ return -EINVAL;
+ if (!(ops->flags & FTRACE_OPS_FL_ENABLED))
+ return -EINVAL;
+ if (direct_functions == EMPTY_HASH)
+ return -EINVAL;
+
+ if (do_direct_lock)
+ mutex_lock(&direct_mutex);
+
+ /* 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
* [PATCHv3 bpf-next 4/8] ftrace: Add update_ftrace_direct_del function
From: Jiri Olsa @ 2025-11-20 21:23 UTC (permalink / raw)
To: Steven Rostedt, Florent Revest, Mark Rutland
Cc: bpf, linux-kernel, linux-trace-kernel, linux-arm-kernel,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Menglong Dong, Song Liu
In-Reply-To: <20251120212402.466524-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 | 110 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 116 insertions(+)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 9cf4cd56d982..c571deeff840 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -551,6 +551,7 @@ int modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr);
int modify_ftrace_direct_nolock(struct ftrace_ops *ops, unsigned long addr);
int update_ftrace_direct_add(struct ftrace_ops *ops, struct ftrace_hash *hash);
+int update_ftrace_direct_del(struct ftrace_ops *ops, struct ftrace_hash *hash);
void ftrace_stub_direct_tramp(void);
@@ -583,6 +584,11 @@ static inline int update_ftrace_direct_add(struct ftrace_ops *ops, struct ftrace
return -ENODEV;
}
+static inline int update_ftrace_direct_del(struct ftrace_ops *ops, struct ftrace_hash *hash)
+{
+ return -ENODEV;
+}
+
/*
* This must be implemented by the architecture.
* It is the way the ftrace direct_ops helper, when called
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 850ff55ff25e..cc730a8fd087 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -6376,6 +6376,116 @@ int update_ftrace_direct_add(struct ftrace_ops *ops, struct ftrace_hash *hash)
return err;
}
+/**
+ * hash_sub - substracts @b from @a and returns the result
+ * @a: struct ftrace_hash object
+ * @b: struct ftrace_hash object
+ *
+ * Returns struct ftrace_hash object on success, NULL on error.
+ */
+static struct ftrace_hash *hash_sub(struct ftrace_hash *a, struct ftrace_hash *b)
+{
+ struct ftrace_func_entry *entry, *del;
+ struct ftrace_hash *sub;
+ int size, i;
+
+ sub = alloc_and_copy_ftrace_hash(a->size_bits, a);
+ if (!sub)
+ goto error;
+
+ size = 1 << b->size_bits;
+ for (i = 0; i < size; i++) {
+ hlist_for_each_entry(entry, &b->buckets[i], hlist) {
+ del = __ftrace_lookup_ip(sub, entry->ip);
+ if (WARN_ON_ONCE(!del))
+ goto error;
+ remove_hash_entry(sub, del);
+ kfree(del);
+ }
+ }
+ return sub;
+
+ error:
+ free_ftrace_hash(sub);
+ return NULL;
+}
+
+int update_ftrace_direct_del(struct ftrace_ops *ops, struct ftrace_hash *hash)
+{
+ struct ftrace_hash *old_direct_functions = NULL, *new_direct_functions = NULL;
+ struct ftrace_hash *old_filter_hash = NULL, *new_filter_hash = NULL;
+ struct ftrace_func_entry *del, *entry;
+ unsigned long size, i;
+ int err = -EINVAL;
+
+ if (!hash_count(hash))
+ return -EINVAL;
+ if (check_direct_multi(ops))
+ return -EINVAL;
+ if (!(ops->flags & FTRACE_OPS_FL_ENABLED))
+ return -EINVAL;
+ if (direct_functions == EMPTY_HASH)
+ return -EINVAL;
+
+ mutex_lock(&direct_mutex);
+
+ old_filter_hash = ops->func_hash ? ops->func_hash->filter_hash : NULL;
+ old_direct_functions = direct_functions;
+
+ /* Make sure requested entries are already registered. */
+ size = 1 << hash->size_bits;
+ for (i = 0; i < size; i++) {
+ hlist_for_each_entry(entry, &hash->buckets[i], hlist) {
+ del = __ftrace_lookup_ip(direct_functions, entry->ip);
+ if (!del || del->direct != entry->direct)
+ goto out_unlock;
+ }
+ }
+
+ err = -ENOMEM;
+ new_filter_hash = hash_sub(old_filter_hash, hash);
+ if (!new_filter_hash)
+ goto out_unlock;
+
+ new_direct_functions = hash_sub(old_direct_functions, hash);
+ if (!new_direct_functions)
+ goto out_unlock;
+
+ /* If there's nothing left, we need to unregister the ops. */
+ if (ftrace_hash_empty(new_filter_hash)) {
+ err = unregister_ftrace_function(ops);
+ if (!err) {
+ /* cleanup for possible another register call */
+ ops->func = NULL;
+ ops->trampoline = 0;
+ ftrace_free_filter(ops);
+ ops->func_hash->filter_hash = NULL;
+ }
+ } else {
+ err = ftrace_update_ops(ops, new_filter_hash, EMPTY_HASH);
+ /*
+ * new_filter_hash is dup-ed, so we need to release it anyway,
+ * old_filter_hash either stays on error or is released already
+ */
+ }
+
+ if (err) {
+ /* reset direct_functions and free the new one */
+ old_direct_functions = new_direct_functions;
+ } else {
+ rcu_assign_pointer(direct_functions, new_direct_functions);
+ }
+
+ out_unlock:
+ mutex_unlock(&direct_mutex);
+
+ if (old_direct_functions && old_direct_functions != EMPTY_HASH)
+ call_rcu_tasks(&old_direct_functions->rcu, register_ftrace_direct_cb);
+ free_ftrace_hash(new_filter_hash);
+
+ return err;
+}
+
#endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
/**
--
2.51.1
^ permalink raw reply related
* [PATCHv3 bpf-next 3/8] ftrace: Add update_ftrace_direct_add function
From: Jiri Olsa @ 2025-11-20 21:23 UTC (permalink / raw)
To: Steven Rostedt, Florent Revest, Mark Rutland
Cc: bpf, linux-kernel, linux-trace-kernel, linux-arm-kernel,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Menglong Dong, Song Liu
In-Reply-To: <20251120212402.466524-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 5752553bff60..9cf4cd56d982 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -550,6 +550,8 @@ int unregister_ftrace_direct(struct ftrace_ops *ops, unsigned long addr,
int modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr);
int modify_ftrace_direct_nolock(struct ftrace_ops *ops, unsigned long addr);
+int update_ftrace_direct_add(struct ftrace_ops *ops, struct ftrace_hash *hash);
+
void ftrace_stub_direct_tramp(void);
#else
@@ -576,6 +578,11 @@ static inline int modify_ftrace_direct_nolock(struct ftrace_ops *ops, unsigned l
return -ENODEV;
}
+static inline int update_ftrace_direct_add(struct ftrace_ops *ops, struct ftrace_hash *hash)
+{
+ return -ENODEV;
+}
+
/*
* This must be implemented by the architecture.
* It is the way the ftrace direct_ops helper, when called
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index e6ccf572c5f6..850ff55ff25e 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 -EINVAL;
+
+ mutex_lock(&direct_mutex);
+
+ /* Make sure requested entries are not already registered. */
+ size = 1 << hash->size_bits;
+ for (i = 0; i < size; i++) {
+ hlist_for_each_entry(entry, &hash->buckets[i], hlist) {
+ if (__ftrace_lookup_ip(direct_functions, entry->ip))
+ goto out_unlock;
+ }
+ }
+
+ old_filter_hash = ops->func_hash ? ops->func_hash->filter_hash : NULL;
+ old_direct_functions = direct_functions;
+
+ /* If there's nothing in filter_hash we need to register the ops. */
+ reg = hash_count(old_filter_hash) == 0;
+ if (reg) {
+ if (ops->func || ops->trampoline)
+ goto out_unlock;
+ if (ops->flags & FTRACE_OPS_FL_ENABLED)
+ goto out_unlock;
+ }
+
+ err = -ENOMEM;
+ new_filter_hash = hash_add(old_filter_hash, hash);
+ if (!new_filter_hash)
+ goto out_unlock;
+
+ new_direct_functions = hash_add(old_direct_functions, hash);
+ if (!new_direct_functions)
+ goto out_unlock;
+
+ rcu_assign_pointer(direct_functions, new_direct_functions);
+
+ if (reg) {
+ ops->func = call_direct_funcs;
+ ops->flags |= MULTI_FLAGS;
+ ops->trampoline = FTRACE_REGS_ADDR;
+ ops->local_hash.filter_hash = new_filter_hash;
+
+ err = register_ftrace_function_nolock(ops);
+ if (err) {
+ /* restore old filter on error */
+ ops->local_hash.filter_hash = old_filter_hash;
+ old_filter_hash = new_filter_hash;
+
+ /* cleanup for possible another register call */
+ ops->func = NULL;
+ ops->trampoline = 0;
+ }
+ } else {
+ err = ftrace_update_ops(ops, new_filter_hash, EMPTY_HASH);
+ /*
+ * new_filter_hash is dup-ed, so we need to release it anyway,
+ * old_filter_hash either stays on error or is released already
+ */
+ old_filter_hash = new_filter_hash;
+ }
+
+ if (err) {
+ /* reset direct_functions and free the new one */
+ rcu_assign_pointer(direct_functions, old_direct_functions);
+ old_direct_functions = new_direct_functions;
+ }
+
+ out_unlock:
+ mutex_unlock(&direct_mutex);
+
+ if (old_direct_functions && old_direct_functions != EMPTY_HASH)
+ call_rcu_tasks(&old_direct_functions->rcu, register_ftrace_direct_cb);
+ if (old_filter_hash)
+ free_ftrace_hash(old_filter_hash);
+
+ return err;
+}
+
#endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
/**
--
2.51.1
^ permalink raw reply related
* [PATCHv3 bpf-next 2/8] ftrace: Export some of hash related functions
From: Jiri Olsa @ 2025-11-20 21:23 UTC (permalink / raw)
To: Steven Rostedt, Florent Revest, Mark Rutland
Cc: bpf, linux-kernel, linux-trace-kernel, linux-arm-kernel,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Menglong Dong, Song Liu
In-Reply-To: <20251120212402.466524-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 07f8c309e432..5752553bff60 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -405,6 +405,22 @@ enum ftrace_ops_cmd {
typedef int (*ftrace_ops_func_t)(struct ftrace_ops *op, enum ftrace_ops_cmd cmd);
#ifdef CONFIG_DYNAMIC_FTRACE
+
+#define FTRACE_HASH_DEFAULT_BITS 10
+
+struct ftrace_hash {
+ unsigned long size_bits;
+ struct hlist_head *buckets;
+ unsigned long count;
+ unsigned long flags;
+ struct rcu_head rcu;
+};
+
+struct ftrace_hash *alloc_ftrace_hash(int size_bits);
+void free_ftrace_hash(struct ftrace_hash *hash);
+struct ftrace_func_entry *add_hash_entry_direct(struct ftrace_hash *hash,
+ unsigned long ip, unsigned long direct);
+
/* The hash used to know what functions callbacks trace */
struct ftrace_ops_hash {
struct ftrace_hash __rcu *notrace_hash;
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 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
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