| Message ID | 24cde1381495cd637f0d5663ab7629572bc0eca6.1737511963.git.jpoimboe@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | unwind, perf: sframe user space unwinding | expand |
On Tue, Jan 21, 2025 at 06:31:26PM -0800, Josh Poimboeuf wrote: > If the task doesn't have any memory, there's no stack to unwind. > > Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org> > --- > kernel/events/core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel/events/core.c b/kernel/events/core.c > index 99f0f28feeb5..a886bb83f4d0 100644 > --- a/kernel/events/core.c > +++ b/kernel/events/core.c > @@ -7792,7 +7792,7 @@ struct perf_callchain_entry * > perf_callchain(struct perf_event *event, struct pt_regs *regs) > { > bool kernel = !event->attr.exclude_callchain_kernel; > - bool user = !event->attr.exclude_callchain_user; > + bool user = !event->attr.exclude_callchain_user && current->mm; What about things like io_uring helpers, don't they keep ->mm but are never in userspace?
On Wed, Jan 22, 2025 at 03:29:10PM +0100, Peter Zijlstra wrote: > On Tue, Jan 21, 2025 at 06:31:26PM -0800, Josh Poimboeuf wrote: > > If the task doesn't have any memory, there's no stack to unwind. > > > > Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org> > > --- > > kernel/events/core.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/kernel/events/core.c b/kernel/events/core.c > > index 99f0f28feeb5..a886bb83f4d0 100644 > > --- a/kernel/events/core.c > > +++ b/kernel/events/core.c > > @@ -7792,7 +7792,7 @@ struct perf_callchain_entry * > > perf_callchain(struct perf_event *event, struct pt_regs *regs) > > { > > bool kernel = !event->attr.exclude_callchain_kernel; > > - bool user = !event->attr.exclude_callchain_user; > > + bool user = !event->attr.exclude_callchain_user && current->mm; > > What about things like io_uring helpers, don't they keep ->mm but are > never in userspace? Hm, that's news to me. At least this patch doesn't make things worse in that regard. IIRC, user_regs(task_pt_regs(current)) returns false for kthreads because the regs area is cleared. We could add in a user_regs() check?
On Wed, Jan 22, 2025 at 03:08:38PM -0800, Josh Poimboeuf wrote: > On Wed, Jan 22, 2025 at 03:29:10PM +0100, Peter Zijlstra wrote: > > On Tue, Jan 21, 2025 at 06:31:26PM -0800, Josh Poimboeuf wrote: > > > If the task doesn't have any memory, there's no stack to unwind. > > > > > > Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org> > > > --- > > > kernel/events/core.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/kernel/events/core.c b/kernel/events/core.c > > > index 99f0f28feeb5..a886bb83f4d0 100644 > > > --- a/kernel/events/core.c > > > +++ b/kernel/events/core.c > > > @@ -7792,7 +7792,7 @@ struct perf_callchain_entry * > > > perf_callchain(struct perf_event *event, struct pt_regs *regs) > > > { > > > bool kernel = !event->attr.exclude_callchain_kernel; > > > - bool user = !event->attr.exclude_callchain_user; > > > + bool user = !event->attr.exclude_callchain_user && current->mm; > > > > What about things like io_uring helpers, don't they keep ->mm but are > > never in userspace? > > Hm, that's news to me. At least this patch doesn't make things worse in > that regard. > > IIRC, user_regs(task_pt_regs(current)) returns false for kthreads > because the regs area is cleared. We could add in a user_regs() check? Possibly, I've not yet dug into the io_worker thing in much detail yet.
diff --git a/kernel/events/core.c b/kernel/events/core.c index 99f0f28feeb5..a886bb83f4d0 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -7792,7 +7792,7 @@ struct perf_callchain_entry * perf_callchain(struct perf_event *event, struct pt_regs *regs) { bool kernel = !event->attr.exclude_callchain_kernel; - bool user = !event->attr.exclude_callchain_user; + bool user = !event->attr.exclude_callchain_user && current->mm; const u32 max_stack = event->attr.sample_max_stack; struct perf_callchain_entry *callchain;
If the task doesn't have any memory, there's no stack to unwind. Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org> --- kernel/events/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)