From: George Kennedy george.kennedy@oracle.com
[ Upstream commit 866cf36bfee4fba6a492d2dcc5133f857e3446b0 ]
On AMD machines cpuc->events[idx] can become NULL in a subtle race condition with NMI->throttle->x86_pmu_stop().
Check event for NULL in amd_pmu_enable_all() before enable to avoid a GPF. This appears to be an AMD only issue.
Syzkaller reported a GPF in amd_pmu_enable_all.
INFO: NMI handler (perf_event_nmi_handler) took too long to run: 13.143 msecs Oops: general protection fault, probably for non-canonical address 0xdffffc0000000034: 0000 PREEMPT SMP KASAN NOPTI KASAN: null-ptr-deref in range [0x00000000000001a0-0x00000000000001a7] CPU: 0 UID: 0 PID: 328415 Comm: repro_36674776 Not tainted 6.12.0-rc1-syzk RIP: 0010:x86_pmu_enable_event (arch/x86/events/perf_event.h:1195 arch/x86/events/core.c:1430) RSP: 0018:ffff888118009d60 EFLAGS: 00010012 RAX: dffffc0000000000 RBX: 0000000000000000 RCX: 0000000000000000 RDX: 0000000000000034 RSI: 0000000000000000 RDI: 00000000000001a0 RBP: 0000000000000001 R08: 0000000000000000 R09: 0000000000000000 R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000002 R13: ffff88811802a440 R14: ffff88811802a240 R15: ffff8881132d8601 FS: 00007f097dfaa700(0000) GS:ffff888118000000(0000) GS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00000000200001c0 CR3: 0000000103d56000 CR4: 00000000000006f0 Call Trace: <IRQ> amd_pmu_enable_all (arch/x86/events/amd/core.c:760 (discriminator 2)) x86_pmu_enable (arch/x86/events/core.c:1360) event_sched_out (kernel/events/core.c:1191 kernel/events/core.c:1186 kernel/events/core.c:2346) __perf_remove_from_context (kernel/events/core.c:2435) event_function (kernel/events/core.c:259) remote_function (kernel/events/core.c:92 (discriminator 1) kernel/events/core.c:72 (discriminator 1)) __flush_smp_call_function_queue (./arch/x86/include/asm/jump_label.h:27 ./include/linux/jump_label.h:207 ./include/trace/events/csd.h:64 kernel/smp.c:135 kernel/smp.c:540) __sysvec_call_function_single (./arch/x86/include/asm/jump_label.h:27 ./include/linux/jump_label.h:207 ./arch/x86/include/asm/trace/irq_vectors.h:99 arch/x86/kernel/smp.c:272) sysvec_call_function_single (arch/x86/kernel/smp.c:266 (discriminator 47) arch/x86/kernel/smp.c:266 (discriminator 47)) </IRQ>
Reported-by: syzkaller syzkaller@googlegroups.com Signed-off-by: George Kennedy george.kennedy@oracle.com Signed-off-by: Peter Zijlstra (Intel) peterz@infradead.org Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
Now I have a complete understanding of this commit. Let me provide a comprehensive analysis.
---
## COMPREHENSIVE ANALYSIS: perf/x86/amd: Check event before enable to avoid GPF
### 1. COMMIT MESSAGE ANALYSIS
**Subject:** "perf/x86/amd: Check event before enable to avoid GPF"
**Key indicators in the commit message:** - **"GPF" (General Protection Fault)**: A serious kernel crash/oops - critical bug - **"race condition"**: Real bug, not theoretical - found and triggered - **"Syzkaller reported"**: Kernel fuzzer found a reproducible issue - **"Reported-by: syzkaller"**: Confirms the bug is triggerable - **"Signed-off-by: Peter Zijlstra (Intel)"**: Reviewed and signed by top perf maintainer
**Missing tags:** - No "Fixes:" tag pointing to the original buggy commit - No "Cc: stable@vger.kernel.org" tag
### 2. CODE CHANGE ANALYSIS
**The Bug Mechanism:**
The race condition occurs between two code paths:
**Path 1 - Normal enable path:** ```c // In amd_pmu_enable_all() for_each_set_bit(idx, x86_pmu.cntr_mask, X86_PMC_IDX_MAX) { if (!test_bit(idx, cpuc->active_mask)) // Check 1 continue; amd_pmu_enable_event(cpuc->events[idx]); // Dereference } ```
**Path 2 - NMI throttle path:** ```c // In x86_pmu_stop() called from NMI->throttle if (test_bit(hwc->idx, cpuc->active_mask)) { __clear_bit(hwc->idx, cpuc->active_mask); // Clear bit cpuc->events[hwc->idx] = NULL; // Set to NULL ... } ```
**The Race:** 1. CPU executes `amd_pmu_enable_all()` during an IPI (`remote_function`/`event_function`) 2. Code passes `test_bit(idx, cpuc->active_mask)` check ✓ 3. **NMI fires** before `cpuc->events[idx]` is read 4. NMI handler throttles an event → calls `x86_pmu_stop()` 5. `x86_pmu_stop()` clears `active_mask` bit AND sets `cpuc->events[idx] = NULL` 6. NMI returns 7. Original code tries to dereference `cpuc->events[idx]` → **NULL pointer dereference → GPF**
**The Fix:** ```c // After the fix if (!test_bit(idx, cpuc->active_mask)) continue;
/* FIXME: cpuc->events[idx] can become NULL in a subtle race - condition with NMI->throttle->x86_pmu_stop(). */ if (cpuc->events[idx]) amd_pmu_enable_event(cpuc->events[idx]); ```
The fix adds a simple NULL check before dereferencing the pointer. The "FIXME" comment acknowledges this is a workaround for a deeper architectural issue in the event lifecycle, but is correct and safe.
### 3. CLASSIFICATION
| Criterion | Assessment | |-----------|------------| | Type | **Bug fix** - NULL pointer dereference causing kernel crash | | Severity | **High** - Causes kernel oops/GPF | | Category | Not a new feature, device ID, quirk, or DT update | | Security | Not explicitly a CVE, but denial-of-service via crash |
### 4. SCOPE AND RISK ASSESSMENT
**Change Statistics:** - **Lines changed:** 6 added, 1 removed (+5 net) - **Files affected:** 1 (`arch/x86/events/amd/core.c`) - **Functions modified:** 1 (`amd_pmu_enable_all()`)
**Risk Assessment:** - **Very Low Risk**: Adding a NULL check is the most conservative possible fix - **No behavior change**: If event is valid, same behavior; if NULL, safely skipped - **Cannot cause regressions**: A NULL check cannot break working code - **Isolated to AMD**: Only affects AMD PMU code path, not Intel or other architectures
**Affected Subsystem:** - `arch/x86/events/amd/` - AMD Performance Monitoring Unit (PMU) - Mature subsystem, present since v5.19
### 5. USER IMPACT
**Who is affected:** - All AMD CPU users running performance monitoring (`perf`) - Enterprise users, cloud providers with AMD servers - Developers using perf for profiling
**Severity:** - **Kernel crash (oops)** - System becomes unstable or halts - Reproducible via syzkaller, meaning users can hit this in real workloads
**Trigger conditions:** - Using perf events on AMD CPUs - High frequency of events causing throttling - SMP systems with IPI-based event functions
### 6. STABILITY INDICATORS
| Indicator | Status | |-----------|--------| | Tested by syzkaller | Yes (reported and reproduced) | | Maintainer sign-off | Peter Zijlstra (top perf maintainer) | | Tested-by tag | Not present | | Time in mainline | In v6.18-rc1/v6.18 |
### 7. DEPENDENCY CHECK
**Dependencies:** None - The fix is self-contained - Does not require any other patches - The affected function `amd_pmu_enable_all()` exists in all kernels since v5.19
**Original buggy commit:** - `ada543459cab7f` "perf/x86/amd: Add AMD Fam19h Branch Sampling support" (March 2022) - Present in v5.19 and later - All stable trees from 5.19+ are affected
**Backport feasibility:** - Clean apply expected - the code structure is unchanged - The `for_each_set_bit` macro and surrounding code are stable - Minor adjustment may be needed for `x86_pmu.cntr_mask` vs older API
### 8. HISTORICAL CONTEXT
The original bug was introduced in commit `ada543459cab7f` dated April 2022 (v5.19-rc1), when the `amd_pmu_enable_all()` function was added as part of AMD Branch Sampling support. The race condition has been latent since then, only now caught by syzkaller fuzzing.
### 9. SUMMARY
| Criterion | Verdict | |-----------|---------| | Fixes real bug | ✅ Yes - NULL pointer dereference crash | | Affects users | ✅ Yes - Any AMD perf user | | Small and contained | ✅ Yes - 6 lines, 1 file | | Obviously correct | ✅ Yes - Simple NULL check | | No new features | ✅ Yes - Pure bug fix | | Low regression risk | ✅ Yes - Cannot break anything | | No dependencies | ✅ Yes - Self-contained | | Maintainer approved | ✅ Yes - Peter Zijlstra signed |
### CONCLUSION
This commit **should be backported** to stable kernel trees. It fixes a real, reproducible kernel crash (GPF/oops) caused by a NULL pointer dereference in AMD PMU code. The fix is minimal (6 lines), obviously correct (simple NULL check), and carries essentially zero regression risk. The bug affects all AMD users running perf since kernel v5.19.
The lack of explicit `Cc: stable@vger.kernel.org` and `Fixes:` tags appears to be an oversight given the clear bug-fix nature of this commit. The fix meets all stable kernel criteria: 1. Obviously correct and tested (syzkaller) 2. Fixes a real bug that affects users (kernel crash) 3. Fixes an important issue (system crash) 4. Small and contained (6 lines in 1 file) 5. Does not introduce new features
**YES**
arch/x86/events/amd/core.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c index b20661b8621d1..8868f5f5379ba 100644 --- a/arch/x86/events/amd/core.c +++ b/arch/x86/events/amd/core.c @@ -763,7 +763,12 @@ static void amd_pmu_enable_all(int added) if (!test_bit(idx, cpuc->active_mask)) continue;
- amd_pmu_enable_event(cpuc->events[idx]); + /* + * FIXME: cpuc->events[idx] can become NULL in a subtle race + * condition with NMI->throttle->x86_pmu_stop(). + */ + if (cpuc->events[idx]) + amd_pmu_enable_event(cpuc->events[idx]); } }
From: Pankaj Raghav p.raghav@samsung.com
[ Upstream commit ff5c0466486ba8d07ab2700380e8fd6d5344b4e9 ]
The run_readelf() function reads the entire output of readelf into a single shell variable. For large object files with extensive debug information, the size of this variable can exceed the system's command-line argument length limit.
When this variable is subsequently passed to sed via `echo "${out}"`, it triggers an "Argument list too long" error, causing the script to fail.
Fix this by redirecting the output of readelf to a temporary file instead of a variable. The sed commands are then modified to read from this file, avoiding the argument length limitation entirely.
Signed-off-by: Pankaj Raghav p.raghav@samsung.com Signed-off-by: Josh Poimboeuf jpoimboe@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
## Comprehensive Analysis of Commit ff5c0466486ba
### 1. COMMIT MESSAGE ANALYSIS
**Subject**: `scripts/faddr2line: Fix "Argument list too long" error`
The commit message clearly identifies this as a **bug fix** with explicit use of the word "Fix" and a specific error being addressed.
**Key elements**: - **Problem identified**: The `run_readelf()` function stores readelf output in a shell variable, which can exceed the system's `ARG_MAX` limit (typically 2MB on Linux) - **Failure mode**: When `echo "${out}"` is used to pipe to sed, the shell expansion triggers "Argument list too long" (E2BIG error) - **Solution**: Redirect readelf output to a temporary file, have sed read from the file directly
**Important missing tags**: - **No `Cc: stable@vger.kernel.org` tag** - maintainer did not explicitly request backport - **No `Fixes:` tag** - though the bug was clearly introduced in commit `b8d9d9496c1e7`
---
### 2. CODE CHANGE ANALYSIS
The change modifies the `run_readelf()` function in `scripts/faddr2line`:
**Before (buggy code):** ```bash run_readelf() { local objfile=$1 local out=$(${READELF} --file-header --section-headers --symbols --wide $objfile)
ELF_FILEHEADER=$(echo "${out}" | sed -n '...') ELF_SECHEADERS=$(echo "${out}" | sed -n '...') ELF_SYMS=$(echo "${out}" | sed -n '...') } ```
**After (fixed code):** ```bash run_readelf() { local objfile=$1 local tmpfile tmpfile=$(mktemp)
${READELF} --file-header --section-headers --symbols --wide "$objfile" > "$tmpfile"
ELF_FILEHEADER=$(sed -n '...' "$tmpfile") ELF_SECHEADERS=$(sed -n '...' "$tmpfile") ELF_SYMS=$(sed -n '...' "$tmpfile")
rm -f -- "$tmpfile" } ```
**Technical mechanism of the bug**: 1. `readelf` output for large vmlinux with `CONFIG_DEBUG_INFO` can easily exceed 2MB 2. When bash expands `${out}` in `echo "${out}"`, the expanded text becomes part of the argument vector 3. Linux enforces `ARG_MAX` limit (~2MB by default) on total argument size for `execve()` 4. When exceeded, the kernel returns `E2BIG` error, causing "Argument list too long"
**Why the fix works**: - Writing to a file bypasses `ARG_MAX` entirely - `sed` can read arbitrarily large files without limit - `mktemp` ensures safe temp file creation; `rm -f -- "$tmpfile"` properly cleans up
**Code quality**: - Proper quoting of `"$objfile"` (fixed unquoted variable) - Uses `--` in `rm` to handle files starting with `-` - Clean, minimal change
---
### 3. CLASSIFICATION
- **Bug type**: Real functional bug causing tool failure - **Exception category**: This is a **BUILD/TOOL FIX** - faddr2line is a developer tool used for analyzing stack traces from kernel oopses - **Security implications**: None
---
### 4. SCOPE AND RISK ASSESSMENT
| Metric | Value | |--------|-------| | Lines changed | +9/-4 (13 total) | | Files modified | 1 (`scripts/faddr2line`) | | Subsystem | Developer scripts (not kernel runtime code) | | Risk level | **VERY LOW** |
**Why risk is minimal**: - It's a shell script, not compiled kernel code - Cannot cause kernel crashes or data corruption - Logically equivalent transformation (temp file vs shell variable) - Well-understood shell programming pattern
---
### 5. USER IMPACT
**Who is affected**: - Kernel developers debugging oopses/crashes using faddr2line - Anyone using faddr2line on vmlinux with `CONFIG_DEBUG_INFO` enabled
**Impact severity**: - **HIGH within scope**: The tool fails completely on large object files - **Limited audience**: Developer tool, not end-user facing
**Real-world trigger**: - Building vmlinux with `CONFIG_DEBUG_INFO=y` (common for debugging) - Running `scripts/faddr2line vmlinux func+0x123` - If readelf output > 2MB, **script fails completely**
---
### 6. STABILITY INDICATORS
- **Author**: Pankaj Raghav (Samsung) - active kernel contributor - **Signed-off-by**: Josh Poimboeuf - **official MAINTAINER** for faddr2line per `MAINTAINERS` file - **No Tested-by tag**, but the fix is straightforward
---
### 7. DEPENDENCY CHECK
**Bug introduction**: Commit `b8d9d9496c1e7` ("scripts/faddr2line: Combine three readelf calls into one") first appeared in **v6.11-rc1**
**Affected stable versions**: - v6.11.y: **HAS BUGGY CODE** ✓ - v6.12.y: **HAS BUGGY CODE** ✓ - v6.6.y: Does NOT have buggy code (older implementation) - v6.1.y: Does NOT have buggy code (older implementation)
**Patch applicability**: - ✓ Applies cleanly to v6.11 - ✓ Applies cleanly to v6.12 - Not needed for v6.6 and earlier
**Dependencies**: None - completely self-contained fix
---
### 8. HISTORICAL CONTEXT
The bug was introduced as an unintended side effect of a performance optimization:
``` 39cf650d68289 (Apr 2024): Introduced run_readelf() with 3 separate readelf calls b8d9d9496c1e7 (Jul 2024): Combined into single readelf call, storing in variable ← BUG INTRODUCED HERE ff5c0466486ba (Oct 2025): Fix by using temp file instead of variable ```
The original optimization was well-intentioned (reduce readelf invocations from many to one), but the implementer didn't account for the `ARG_MAX` limit.
---
### FINAL ASSESSMENT
**Arguments FOR backporting**: 1. ✓ **Obviously correct**: The fix is a standard shell scripting pattern to avoid `ARG_MAX` limits 2. ✓ **Fixes real bug**: Tool fails completely on large vmlinux files (not theoretical) 3. ✓ **Small and contained**: Only 13 lines changed in one file 4. ✓ **No new features**: Pure bug fix 5. ✓ **Applies cleanly**: Tested on v6.11 and v6.12 6. ✓ **No dependencies**: Self-contained 7. ✓ **Zero runtime risk**: It's a shell script, not kernel code 8. ✓ **Maintainer approved**: Josh Poimboeuf signed off
**Arguments AGAINST backporting**: 1. ✗ No explicit `Cc: stable@vger.kernel.org` tag 2. ✗ No `Fixes:` tag pointing to the introducing commit 3. ✗ Developer tool, not end-user visible
**Verdict**: The lack of explicit stable tags is not disqualifying. This is a clear, surgical bug fix to a commonly-used developer tool. The fix has zero risk of causing regressions (it's a shell script), and it enables faddr2line to work correctly on large vmlinux files. For developers using stable kernels (6.11.y, 6.12.y) who need to debug kernel issues, having a functioning faddr2line is valuable.
The fix meets all the technical criteria for stable backporting: - Obviously correct - Fixes a real bug that affects users - Small and contained - No new features
**YES**
scripts/faddr2line | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/scripts/faddr2line b/scripts/faddr2line index 1fa6beef9f978..477b6d2aa3179 100755 --- a/scripts/faddr2line +++ b/scripts/faddr2line @@ -107,14 +107,19 @@ find_dir_prefix() {
run_readelf() { local objfile=$1 - local out=$(${READELF} --file-header --section-headers --symbols --wide $objfile) + local tmpfile + tmpfile=$(mktemp) + + ${READELF} --file-header --section-headers --symbols --wide "$objfile" > "$tmpfile"
# This assumes that readelf first prints the file header, then the section headers, then the symbols. # Note: It seems that GNU readelf does not prefix section headers with the "There are X section headers" # line when multiple options are given, so let's also match with the "Section Headers:" line. - ELF_FILEHEADER=$(echo "${out}" | sed -n '/There are [0-9]* section headers, starting at offset|Section Headers:/q;p') - ELF_SECHEADERS=$(echo "${out}" | sed -n '/There are [0-9]* section headers, starting at offset|Section Headers:/,$p' | sed -n '/Symbol table .* contains [0-9]* entries:/q;p') - ELF_SYMS=$(echo "${out}" | sed -n '/Symbol table .* contains [0-9]* entries:/,$p') + ELF_FILEHEADER=$(sed -n '/There are [0-9]* section headers, starting at offset|Section Headers:/q;p' "$tmpfile") + ELF_SECHEADERS=$(sed -n '/There are [0-9]* section headers, starting at offset|Section Headers:/,$p' "$tmpfile" | sed -n '/Symbol table .* contains [0-9]* entries:/q;p') + ELF_SYMS=$(sed -n '/Symbol table .* contains [0-9]* entries:/,$p' "$tmpfile") + + rm -f -- "$tmpfile" }
check_vmlinux() {
From: Doug Berger opendmb@gmail.com
[ Upstream commit 382748c05e58a9f1935f5a653c352422375566ea ]
Commit 16b269436b72 ("sched/deadline: Modify cpudl::free_cpus to reflect rd->online") introduced the cpudl_set/clear_freecpu functions to allow the cpu_dl::free_cpus mask to be manipulated by the deadline scheduler class rq_on/offline callbacks so the mask would also reflect this state.
Commit 9659e1eeee28 ("sched/deadline: Remove cpu_active_mask from cpudl_find()") removed the check of the cpu_active_mask to save some processing on the premise that the cpudl::free_cpus mask already reflected the runqueue online state.
Unfortunately, there are cases where it is possible for the cpudl_clear function to set the free_cpus bit for a CPU when the deadline runqueue is offline. When this occurs while a CPU is connected to the default root domain the flag may retain the bad state after the CPU has been unplugged. Later, a different CPU that is transitioning through the default root domain may push a deadline task to the powered down CPU when cpudl_find sees its free_cpus bit is set. If this happens the task will not have the opportunity to run.
One example is outlined here: https://lore.kernel.org/lkml/20250110233010.2339521-1-opendmb@gmail.com
Another occurs when the last deadline task is migrated from a CPU that has an offlined runqueue. The dequeue_task member of the deadline scheduler class will eventually call cpudl_clear and set the free_cpus bit for the CPU.
This commit modifies the cpudl_clear function to be aware of the online state of the deadline runqueue so that the free_cpus mask can be updated appropriately.
It is no longer necessary to manage the mask outside of the cpudl_set/clear functions so the cpudl_set/clear_freecpu functions are removed. In addition, since the free_cpus mask is now only updated under the cpudl lock the code was changed to use the non-atomic __cpumask functions.
Signed-off-by: Doug Berger opendmb@gmail.com Signed-off-by: Peter Zijlstra (Intel) peterz@infradead.org Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
Now I have all the information needed for a complete analysis. Let me summarize:
## DETAILED ANALYSIS
### 1. COMMIT MESSAGE ANALYSIS
**Subject**: `sched/deadline: only set free_cpus for online runqueues`
**Key points from message**: - References two previous commits that introduced the bug: - `16b269436b72` ("sched/deadline: Modify cpudl::free_cpus to reflect rd->online") - from v4.0-rc1 - `9659e1eeee28` ("sched/deadline: Remove cpu_active_mask from cpudl_find()") - from v4.0-rc1 - Describes a real bug where `cpudl_clear()` can incorrectly set `free_cpus` bit for an offline CPU - **Consequence**: Tasks can be pushed to powered-down CPUs and won't run
**Missing tags**: - **NO "Cc: stable@vger.kernel.org"** - maintainer didn't explicitly request stable backport - **NO "Fixes:" tag** - doesn't explicitly reference the buggy commit
### 2. CODE CHANGE ANALYSIS
**The Bug Mechanism**: 1. `cpudl::free_cpus` tracks which CPUs can receive deadline tasks 2. When a CPU goes offline, `rq_offline_dl()` calls `cpudl_clear()` + `cpudl_clear_freecpu()` to clear its bit 3. **BUT**: If the last DL task is later migrated away from that offline CPU, `dec_dl_deadline()` calls `cpudl_clear()` which **unconditionally sets the bit** in `free_cpus` 4. Now `cpudl_find()` sees this offline CPU as available and may push tasks to it 5. **Result**: Tasks pushed to offline CPUs won't run - **task starvation**
**The Fix**: - Adds `bool online` parameter to `cpudl_clear(struct cpudl *cp, int cpu, bool online)` - `dec_dl_deadline()`: passes `rq->online` - only sets bit if CPU is online - `rq_online_dl()`: passes `true` - `rq_offline_dl()`: passes `false` - ensures bit stays cleared - Removes now-unnecessary `cpudl_set_freecpu()` and `cpudl_clear_freecpu()` helpers - Uses non-atomic `__cpumask_*` functions since operations are under `cp->lock`
### 3. SCOPE AND RISK ASSESSMENT
**Files changed**: 3 (cpudeadline.c, cpudeadline.h, deadline.c) **Lines changed**: +14 / -32 (net code removal)
**Risk factors**: - Touches scheduler core code (high impact if wrong) - Changes function signature (API change) - BUT: Logic is straightforward - add online state check - BUT: Self-contained change with no dependencies
### 4. COMPATIBILITY CHECK
Verified the code structure is **identical** across all stable kernels (v5.4, v5.10, v5.15, v6.1, v6.6): - `rq->online` field exists in all versions - `rq_online_dl()` / `rq_offline_dl()` functions are identical - `cpudl_clear()` signature and callers are identical - **Fix will apply cleanly to all stable trees**
### 5. USER IMPACT
- **Who's affected**: Users running SCHED_DEADLINE tasks with CPU hotplug - **Severity**: HIGH - tasks may not run (starvation) - **Bug age**: 10 years (since v4.0-rc1 in 2015) - **Trigger conditions**: Requires specific timing with CPU hotplug + DL tasks - **Referenced LKML report**: https://lore.kernel.org/lkml/2025011023301 0.2339521-1-opendmb@gmail.com - shows real-world occurrence
### 6. STABILITY INDICATORS
**Positive**: - Signed-off by Peter Zijlstra (Intel) - the main scheduler maintainer - Logic is straightforward and correct - Self-contained fix
**Concerning**: - Very fresh commit (just merged to 6.18-rc cycle) - No Tested-by or Reviewed-by tags - No explicit stable request from maintainer
### 7. DECISION FACTORS
**FOR backporting**: 1. Fixes a real, severe bug (task starvation) 2. Bug has existed for 10 years affecting ALL stable kernels 3. Fix is small (~50 lines), surgical, and correct 4. Applies cleanly to all stable trees 5. Signed off by scheduler maintainer 6. The change is logically straightforward (add online state awareness)
**AGAINST backporting**: 1. No Cc: stable tag - maintainer didn't explicitly request it 2. No Fixes: tag 3. Brand new commit - hasn't soaked in mainline 4. Scheduler is sensitive - regressions impact all workloads 5. Bug requires specific conditions to trigger
### CONCLUSION
This is a genuine bug fix for a task starvation issue that has existed since 2015. While the commit lacks explicit stable markers (Cc: stable, Fixes:), the fix is: - **Obviously correct** - adds missing online state check - **Small and contained** - ~50 lines across 3 files - **Applies cleanly** - code structure identical in all stable kernels - **Signed off by the maintainer** - Peter Zijlstra reviewed and approved it - **Fixes a severe bug** - tasks pushed to offline CPUs won't run
The absence of explicit stable tagging is concerning and suggests the maintainer may want it to soak in mainline first. However, the severity of the bug (task starvation) and the correctness of the fix make it a reasonable candidate for stable backporting.
**YES**
kernel/sched/cpudeadline.c | 34 +++++++++------------------------- kernel/sched/cpudeadline.h | 4 +--- kernel/sched/deadline.c | 8 ++++---- 3 files changed, 14 insertions(+), 32 deletions(-)
diff --git a/kernel/sched/cpudeadline.c b/kernel/sched/cpudeadline.c index cdd740b3f7743..37b572cc8aca2 100644 --- a/kernel/sched/cpudeadline.c +++ b/kernel/sched/cpudeadline.c @@ -166,12 +166,13 @@ int cpudl_find(struct cpudl *cp, struct task_struct *p, * cpudl_clear - remove a CPU from the cpudl max-heap * @cp: the cpudl max-heap context * @cpu: the target CPU + * @online: the online state of the deadline runqueue * * Notes: assumes cpu_rq(cpu)->lock is locked * * Returns: (void) */ -void cpudl_clear(struct cpudl *cp, int cpu) +void cpudl_clear(struct cpudl *cp, int cpu, bool online) { int old_idx, new_cpu; unsigned long flags; @@ -184,7 +185,7 @@ void cpudl_clear(struct cpudl *cp, int cpu) if (old_idx == IDX_INVALID) { /* * Nothing to remove if old_idx was invalid. - * This could happen if a rq_offline_dl is + * This could happen if rq_online_dl or rq_offline_dl is * called for a CPU without -dl tasks running. */ } else { @@ -195,9 +196,12 @@ void cpudl_clear(struct cpudl *cp, int cpu) cp->elements[new_cpu].idx = old_idx; cp->elements[cpu].idx = IDX_INVALID; cpudl_heapify(cp, old_idx); - - cpumask_set_cpu(cpu, cp->free_cpus); } + if (likely(online)) + __cpumask_set_cpu(cpu, cp->free_cpus); + else + __cpumask_clear_cpu(cpu, cp->free_cpus); + raw_spin_unlock_irqrestore(&cp->lock, flags); }
@@ -228,7 +232,7 @@ void cpudl_set(struct cpudl *cp, int cpu, u64 dl) cp->elements[new_idx].cpu = cpu; cp->elements[cpu].idx = new_idx; cpudl_heapify_up(cp, new_idx); - cpumask_clear_cpu(cpu, cp->free_cpus); + __cpumask_clear_cpu(cpu, cp->free_cpus); } else { cp->elements[old_idx].dl = dl; cpudl_heapify(cp, old_idx); @@ -237,26 +241,6 @@ void cpudl_set(struct cpudl *cp, int cpu, u64 dl) raw_spin_unlock_irqrestore(&cp->lock, flags); }
-/* - * cpudl_set_freecpu - Set the cpudl.free_cpus - * @cp: the cpudl max-heap context - * @cpu: rd attached CPU - */ -void cpudl_set_freecpu(struct cpudl *cp, int cpu) -{ - cpumask_set_cpu(cpu, cp->free_cpus); -} - -/* - * cpudl_clear_freecpu - Clear the cpudl.free_cpus - * @cp: the cpudl max-heap context - * @cpu: rd attached CPU - */ -void cpudl_clear_freecpu(struct cpudl *cp, int cpu) -{ - cpumask_clear_cpu(cpu, cp->free_cpus); -} - /* * cpudl_init - initialize the cpudl structure * @cp: the cpudl max-heap context diff --git a/kernel/sched/cpudeadline.h b/kernel/sched/cpudeadline.h index 11c0f1faa7e11..d7699468eedd5 100644 --- a/kernel/sched/cpudeadline.h +++ b/kernel/sched/cpudeadline.h @@ -19,8 +19,6 @@ struct cpudl {
int cpudl_find(struct cpudl *cp, struct task_struct *p, struct cpumask *later_mask); void cpudl_set(struct cpudl *cp, int cpu, u64 dl); -void cpudl_clear(struct cpudl *cp, int cpu); +void cpudl_clear(struct cpudl *cp, int cpu, bool online); int cpudl_init(struct cpudl *cp); -void cpudl_set_freecpu(struct cpudl *cp, int cpu); -void cpudl_clear_freecpu(struct cpudl *cp, int cpu); void cpudl_cleanup(struct cpudl *cp); diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c index 7b7671060bf9e..19b1a8b81c76c 100644 --- a/kernel/sched/deadline.c +++ b/kernel/sched/deadline.c @@ -1811,7 +1811,7 @@ static void dec_dl_deadline(struct dl_rq *dl_rq, u64 deadline) if (!dl_rq->dl_nr_running) { dl_rq->earliest_dl.curr = 0; dl_rq->earliest_dl.next = 0; - cpudl_clear(&rq->rd->cpudl, rq->cpu); + cpudl_clear(&rq->rd->cpudl, rq->cpu, rq->online); cpupri_set(&rq->rd->cpupri, rq->cpu, rq->rt.highest_prio.curr); } else { struct rb_node *leftmost = rb_first_cached(&dl_rq->root); @@ -2883,9 +2883,10 @@ static void rq_online_dl(struct rq *rq) if (rq->dl.overloaded) dl_set_overload(rq);
- cpudl_set_freecpu(&rq->rd->cpudl, rq->cpu); if (rq->dl.dl_nr_running > 0) cpudl_set(&rq->rd->cpudl, rq->cpu, rq->dl.earliest_dl.curr); + else + cpudl_clear(&rq->rd->cpudl, rq->cpu, true); }
/* Assumes rq->lock is held */ @@ -2894,8 +2895,7 @@ static void rq_offline_dl(struct rq *rq) if (rq->dl.overloaded) dl_clear_overload(rq);
- cpudl_clear(&rq->rd->cpudl, rq->cpu); - cpudl_clear_freecpu(&rq->rd->cpudl, rq->cpu); + cpudl_clear(&rq->rd->cpudl, rq->cpu, false); }
void __init init_sched_dl_class(void)
From: Peter Zijlstra peterz@infradead.org
[ Upstream commit d206fbad9328ddb68ebabd7cf7413392acd38081 ]
Many people reported regressions on their database workloads due to:
155213a2aed4 ("sched/fair: Bump sd->max_newidle_lb_cost when newidle balance fails")
For instance Adam Li reported a 6% regression on SpecJBB.
Conversely this will regress schbench again; on my machine from 2.22 Mrps/s down to 2.04 Mrps/s.
Reported-by: Joseph Salisbury joseph.salisbury@oracle.com Reported-by: Adam Li adamli@os.amperecomputing.com Reported-by: Dietmar Eggemann dietmar.eggemann@arm.com Reported-by: Hazem Mohamed Abuelfotoh abuehaze@amazon.com Signed-off-by: Peter Zijlstra (Intel) peterz@infradead.org Reviewed-by: Dietmar Eggemann dietmar.eggemann@arm.com Tested-by: Dietmar Eggemann dietmar.eggemann@arm.com Tested-by: Chris Mason clm@meta.com Link: https://lkml.kernel.org/r/20250626144017.1510594-2-clm@fb.com Link: https://lkml.kernel.org/r/006c9df2-b691-47f1-82e6-e233c3f91faf@oracle.com Link: https://patch.msgid.link/20251107161739.406147760@infradead.org Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
Now I have all the information needed for a comprehensive analysis. Let me compile my findings.
---
## COMPREHENSIVE BACKPORT ANALYSIS
### 1. COMMIT MESSAGE ANALYSIS
**Subject:** `sched/fair: Revert max_newidle_lb_cost bump`
**Key signals:** - **"Revert"** - This is undoing a previous change that caused problems - **Multiple "Reported-by:" tags** from major organizations: - Joseph Salisbury (Oracle) - Adam Li (Ampere Computing) - Dietmar Eggemann (ARM) - Hazem Mohamed Abuelfotoh (Amazon) - **"Tested-by:" and "Reviewed-by:"** tags showing validation - **Explicit regression** mentioned: "6% regression on SpecJBB" - **No "Cc: stable" tag** explicitly, but this is fixing a regression in code that WAS backported
**Missing Fixes: tag:** There's no explicit `Fixes:` tag, but the commit message clearly states it reverts `155213a2aed4`.
### 2. CODE CHANGE ANALYSIS
The revert removes two related changes from the original commit:
**Change 1 - `update_newidle_cost()` function:** ```c // BEFORE (problematic - being reverted): sd->max_newidle_lb_cost = min(cost, sysctl_sched_migration_cost + 200);
// AFTER (restored original): sd->max_newidle_lb_cost = cost; ```
**Change 2 - `sched_balance_newidle()` function:** ```c // BEFORE (problematic - being reverted): domain_cost = t1 - t0; curr_cost += domain_cost; t0 = t1;
if (!pulled_task) domain_cost = (3 * sd->max_newidle_lb_cost) / 2;
update_newidle_cost(sd, domain_cost);
// AFTER (restored original): domain_cost = t1 - t0; update_newidle_cost(sd, domain_cost);
curr_cost += domain_cost; t0 = t1; ```
**Technical explanation:** The original commit 155213a2aed4 tried to fix a schbench regression by: 1. Capping `max_newidle_lb_cost` to `sysctl_sched_migration_cost + 200` 2. Artificially bumping the cost by 50% when newidle balance fails to find work
The intent was to make the scheduler stop doing expensive newidle balancing operations that weren't productive. However, this approach was too aggressive and caused database workloads to suffer because they rely on the scheduler's ability to quickly migrate tasks to idle CPUs.
### 3. CLASSIFICATION
| Category | Assessment | |----------|------------| | Bug fix | **YES** - Fixing a performance regression | | Security fix | NO | | Feature addition | NO - This removes code | | Cleanup/refactoring | NO | | Device ID/quirk | NO |
This is a **clear regression fix** - the original commit caused measurable performance degradation in real-world workloads.
### 4. SCOPE AND RISK ASSESSMENT
**Size:** 1 file, 19 lines changed (-16 lines removed, +3 lines added)
**Complexity:** LOW - This is a straight revert removing problematic logic
**Subsystem:** Scheduler (kernel/sched/fair.c) - Core but well-tested area
**Risk factors:** - ✅ Simple revert to known-good state - ✅ Tested by multiple parties - ✅ Well-understood change - ⚠️ Will cause schbench regression (~8%), but this is a synthetic benchmark
### 5. USER IMPACT
**Who is affected:** - **Database workloads** (SpecJBB) - MAJOR, 6% regression fixed - **Production systems** running Oracle, Amazon, ARM platforms - Any workload relying on fast task migration to idle CPUs
**Severity:** HIGH - This affects real production database workloads with measurable performance impact
### 6. STABILITY INDICATORS
| Tag | Present | |-----|---------| | Reviewed-by | ✅ Dietmar Eggemann (ARM) | | Tested-by | ✅ Dietmar Eggemann (ARM), Chris Mason (Meta) | | Signed-off-by | ✅ Peter Zijlstra (Intel) - scheduler maintainer |
**Maturity:** The revert was authored by Peter Zijlstra, the primary Linux scheduler maintainer, demonstrating careful consideration.
### 7. DEPENDENCY CHECK
I verified the commits between the original and the revert:
``` d206fbad9328d sched/fair: Revert max_newidle_lb_cost bump 522fb20fbdbe4 sched/fair: Have SD_SERIALIZE affect newidle balancing ... ```
The `SD_SERIALIZE` commit (522fb20fbdbe4) touches `sched_balance_rq()`, not the areas modified by the revert. **No dependencies or conflicts detected.**
### 8. STABLE TREE STATUS - CRITICAL FINDING
**The original problematic commit WAS backported to multiple stable branches:**
| Stable Branch | Original Commit Hash | Revert Status | |--------------|---------------------|---------------| | 6.12.y (LTS) | fc4289233e4b7 | ❌ NOT REVERTED | | 6.15.y | a4946f4b01421 | ❌ NOT REVERTED | | 6.16.y | c6e88242c06d4 | ❌ NOT REVERTED | | 6.17.y | 155213a2aed4 | ❌ NOT REVERTED | | 6.18.y | 155213a2aed4 | ❌ NOT REVERTED |
**This means all these stable branches currently have the regression- causing code!**
I verified the code in these branches - they all have: 1. The capped `min(cost, sysctl_sched_migration_cost + 200)` 2. The bump logic `(3 * sd->max_newidle_lb_cost) / 2`
The revert applies cleanly because the code structure is identical.
### 9. TRADE-OFF ANALYSIS
| Workload | Impact | |----------|--------| | SpecJBB (database) | **+6% restored** | | schbench (synthetic) | -8% regression |
The maintainers deliberately chose to prioritize real-world database workloads over a synthetic microbenchmark. This is the correct decision for stable trees.
---
## FINAL ASSESSMENT
**This commit SHOULD be backported to stable trees because:**
1. **Fixes a real performance regression** - 6% on database workloads is significant 2. **Multiple independent reporters** from Oracle, Ampere, ARM, and Amazon confirms real-world impact 3. **The problematic commit was already backported** to 6.12.y, 6.15.y, 6.16.y, 6.17.y, and 6.18.y - users of these stable trees are currently affected 4. **Simple, low-risk change** - It's a revert returning to previous stable behavior 5. **Small and contained** - 19 lines in one file, no architectural changes 6. **Well-tested** - Has Tested-by and Reviewed-by tags from ARM and Meta engineers 7. **Authored by maintainer** - Peter Zijlstra (scheduler maintainer) made this decision deliberately 8. **Clean backport** - Code matches exactly in stable trees, will apply without conflicts
**This is exactly the type of fix stable trees need:** fixing regressions introduced by prior backports, with minimal risk and maximum user benefit.
**YES**
kernel/sched/fair.c | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index 5b752324270b0..9492a1de2501a 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -12159,14 +12159,8 @@ static inline bool update_newidle_cost(struct sched_domain *sd, u64 cost) /* * Track max cost of a domain to make sure to not delay the * next wakeup on the CPU. - * - * sched_balance_newidle() bumps the cost whenever newidle - * balance fails, and we don't want things to grow out of - * control. Use the sysctl_sched_migration_cost as the upper - * limit, plus a litle extra to avoid off by ones. */ - sd->max_newidle_lb_cost = - min(cost, sysctl_sched_migration_cost + 200); + sd->max_newidle_lb_cost = cost; sd->last_decay_max_lb_cost = jiffies; } else if (time_after(jiffies, sd->last_decay_max_lb_cost + HZ)) { /* @@ -12858,17 +12852,10 @@ static int sched_balance_newidle(struct rq *this_rq, struct rq_flags *rf)
t1 = sched_clock_cpu(this_cpu); domain_cost = t1 - t0; + update_newidle_cost(sd, domain_cost); + curr_cost += domain_cost; t0 = t1; - - /* - * Failing newidle means it is not effective; - * bump the cost so we end up doing less of it. - */ - if (!pulled_task) - domain_cost = (3 * sd->max_newidle_lb_cost) / 2; - - update_newidle_cost(sd, domain_cost); }
/*
linux-stable-mirror@lists.linaro.org
-
Sasha Levin