linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Josh Poimboeuf <jpoimboe@kernel.org>,
	"Steven Rostedt (Google)" <rostedt@goodmis.org>,
	"Peter Zijlstra (Intel)" <peterz@infradead.org>,
	Sasha Levin <sashal@kernel.org>,
	mingo@redhat.com, acme@kernel.org, namhyung@kernel.org,
	linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH AUTOSEL 6.17-6.6] perf: Skip user unwind if the task is a kernel thread
Date: Mon,  6 Oct 2025 14:17:45 -0400	[thread overview]
Message-ID: <20251006181835.1919496-13-sashal@kernel.org> (raw)
In-Reply-To: <20251006181835.1919496-1-sashal@kernel.org>

From: Josh Poimboeuf <jpoimboe@kernel.org>

[ Upstream commit 16ed389227651330879e17bd83d43bd234006722 ]

If the task is not a user thread, there's no user stack to unwind.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lore.kernel.org/r/20250820180428.930791978@kernel.org
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## Backport Recommendation: **YES**

### Comprehensive Analysis

#### What the Bug Fixes

This commit addresses a **correctness and robustness bug** in the perf
subsystem's callchain unwinding logic. The issue is that
`perf_callchain()` incorrectly attempts to unwind user stacks for kernel
threads that have a memory descriptor (mm) field, specifically io_uring
helpers and other `PF_USER_WORKER` tasks.

**The Core Problem:**
- Line 8195 in kernel/events/core.c:8195: `bool user =
  !event->attr.exclude_callchain_user;`
- Line 8201-8202: Only checks `if (!current->mm) user = false;`
- **However**, io_uring helpers (marked with `PF_USER_WORKER`) are
  kernel threads that **do have** `current->mm` set
- This causes the code to incorrectly attempt user stack unwinding for
  these kernel threads

**The Fix:**
The commit adds an explicit check for kernel thread flags when
determining whether to unwind user stacks:
```c
bool user = !event->attr.exclude_callchain_user &&
    !(current->flags & (PF_KTHREAD | PF_USER_WORKER));
```

This provides defense-in-depth alongside the later `!current->mm` check
at line 8201.

#### Context from Related Commits

This is part of a coordinated patch series (commits e649bcda25b5a →
16ed389227651) that improves perf's handling of kernel threads:

1. **Commit 90942f9fac057** (Steven Rostedt): Fixed
   `get_perf_callchain()` and other locations in
   kernel/events/callchain.c and kernel/events/core.c with the same
   PF_KTHREAD|PF_USER_WORKER check
2. **Commit 16ed389227651** (this commit, Josh Poimboeuf): Completes the
   fix by applying the same logic to `perf_callchain()`

The commit message from 90942f9fac057 explains the rationale clearly:
> "To determine if a task is a kernel thread or not, it is more reliable
to use (current->flags & (PF_KTHREAD|PF_USER_WORKER)) than to rely on
current->mm being NULL. That is because some kernel tasks (io_uring
helpers) may have a mm field."

#### Historical Context

- **PF_USER_WORKER** was introduced in v6.4 (commit 54e6842d0775, March
  2023) as part of moving common PF_IO_WORKER behavior
- The bug has existed since v6.4 when io_uring helpers started having mm
  fields set
- This fix is from **August 2025** (very recent)

#### Impact Assessment

**1. Correctness Issues:**
- Perf events collecting callchains will have **incorrect/garbage data**
  when profiling workloads using io_uring
- This affects production systems using io_uring with performance
  profiling

**2. Performance Impact:**
- Unnecessary CPU cycles wasted attempting to unwind non-existent user
  stacks
- Could be significant in workloads with heavy io_uring usage and perf
  sampling

**3. Potential Stability Issues:**
- Attempting to walk a non-existent user stack could access invalid
  memory
- Architecture-specific `perf_callchain_user()` implementations may not
  handle this gracefully
- While no explicit crash reports are in the commit, the potential
  exists

**4. Affected Systems:**
- Any system using io_uring + perf profiling (common in modern high-
  performance applications)
- Affects all architectures that support perf callchain unwinding

#### Why This Should Be Backported

✅ **Fixes important bug**: Corrects fundamental logic error in
determining user vs kernel threads

✅ **Small and contained**: Only adds a single condition check - 2 lines
changed in kernel/events/core.c:8195-8196

✅ **Minimal regression risk**: The check is conservative - it only
prevents incorrect behavior, doesn't change valid cases

✅ **Affects real workloads**: io_uring is widely used in databases, web
servers, and high-performance applications

✅ **Part of coordinated fix series**: Works together with commit
90942f9fac057 that's likely already being backported

✅ **Follows stable rules**:
- Important correctness fix
- No architectural changes
- Confined to perf subsystem
- Low risk

✅ **No dependencies**: Clean application on top of existing code

#### Evidence from Code Analysis

Looking at kernel/events/core.c:8191-8209, the current code flow for a
`PF_USER_WORKER` task:
1. `user = !event->attr.exclude_callchain_user` → likely true
2. `if (!current->mm)` → **false** for io_uring helpers (they have mm)
3. `user` remains true
4. Calls `get_perf_callchain(..., user=true, ...)` → **INCORRECT**

After the fix:
1. `user = !event->attr.exclude_callchain_user && !(current->flags &
   PF_USER_WORKER)` → **correctly false**
2. Returns empty callchain or kernel-only callchain → **CORRECT**

This is clearly a bug that needs fixing in stable kernels.

 kernel/events/core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index ea9ff856770be..6f01304a73f63 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -8192,7 +8192,8 @@ 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->flags & (PF_KTHREAD | PF_USER_WORKER));
 	/* Disallow cross-task user callchains. */
 	bool crosstask = event->ctx->task && event->ctx->task != current;
 	const u32 max_stack = event->attr.sample_max_stack;
-- 
2.51.0


  parent reply	other threads:[~2025-10-06 18:19 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20251006181835.1919496-1-sashal@kernel.org>
2025-10-06 18:17 ` [PATCH AUTOSEL 6.17-6.12] perf/x86/intel: Add ICL_FIXED_0_ADAPTIVE bit into INTEL_FIXED_BITS_MASK Sasha Levin
2025-10-06 18:17 ` Sasha Levin [this message]
2025-10-06 18:17 ` [PATCH AUTOSEL 6.17-6.1] perf: Have get_perf_callchain() return NULL if crosstask and user are set Sasha Levin
2025-10-06 18:17 ` [PATCH AUTOSEL 6.17-6.6] perf: Use current->flags & PF_KTHREAD|PF_USER_WORKER instead of current->mm == NULL Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251006181835.1919496-13-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=acme@kernel.org \
    --cc=jpoimboe@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=patches@lists.linux.dev \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).