[PATCH AUTOSEL 6.18-5.10] char/nvram: Remove redundant nvram_mutex

[Sasha Levin]

From: Venkat Rao Bagalkote <venkat88@xxxxxxxxxxxxx>

[ Upstream commit e8c715f3a7dae43fabae261493a26474fec11863 ]

The global nvram_mutex in drivers/char/nvram.c is redundant and unused,
and this triggers compiler warnings on some configurations.

All platform-specific nvram operations already provide their own internal
synchronization, meaning the wrapper-level mutex does not provide any
additional safety.

Remove the nvram_mutex definition along with all remaining lock/unlock
users across PPC32, x86, and m68k code paths, and rely entirely on the
per-architecture nvram implementations for locking.

Reviewed-by: Arnd Bergmann <arnd@xxxxxxxx>
Suggested-by: Arnd Bergmann <arnd@xxxxxxxx>
Tested-by: Tellakula Yeswanth Krishna <yeswanth@xxxxxxxxxxxxx>
Signed-off-by: Venkat Rao Bagalkote <venkat88@xxxxxxxxxxxxx>
Tested-by: yeswanth <yeswanth@xxxxxxxxxxxxx>
Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@xxxxxxxxx>
Link: https://patch.msgid.link/20260428061540.73668-1-venkat88@xxxxxxxxxxxxx
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
---

LLM Generated explanations, may be completely bogus:

Connection lost, reconnecting to https://agentn.us.api5.cursor.sh
(attempt 1)...
Retry attempt 1...
## Phase 1: Commit Message Forensics

### Step 1.1: Subject line
**Record:** `[char/nvram]` `[Remove]` — Remove redundant `nvram_mutex`
that is unused on some configs and unnecessary on others.

### Step 1.2: Tags
**Record:**
- **Reviewed-by:** Arnd Bergmann (original author of `nvram_mutex` in
  2010)
- **Suggested-by:** Arnd Bergmann
- **Tested-by:** Tellakula Yeswanth Krishna (IBM), yeswanth
- **Reviewed-by:** Ritesh Harjani (IBM)
- **Link:**
  https://patch.msgid.link/20260428061540.73668-1-venkat88@xxxxxxxxxxxxx
- **Signed-off-by:** Venkat Rao Bagalkote, Greg Kroah-Hartman
- No `Fixes:`, no `Reported-by:`, no `Cc: stable@xxxxxxxxxxxxxxx`
- Notable: subsystem expert (Arnd) both suggested and reviewed; IBM
  tested on Power

### Step 1.3: Body analysis
**Record:**
- **Bug described:** Global `nvram_mutex` is redundant/unused; causes
  compiler warnings on some configurations.
- **Symptom:** `-Wunused` warning (unused static mutex) on configs where
  no lock sites are compiled; redundant outer locking elsewhere.
- **Root cause (author):** Mutex is leftover from BKL→mutex conversion;
  per-arch NVRAM ops already synchronize internally.
- **Version info:** None in message.

### Step 1.4: Hidden bug fix?
**Record:** Not a hidden crash/corruption fix. This is **build hygiene +
dead-code removal**. Removing redundant outer mutex does not fix a
reported runtime failure; arch-level locks (`rtc_lock`, `nv_lock`)
already protect hardware access.

---

## Phase 2: Diff Analysis

### Step 2.1: Inventory
**Record:**
- **File:** `drivers/char/nvram.c` only (~19 lines removed, 0 added)
- **Functions touched:** `nvram_misc_ioctl()` ioctl cases only; removes
  `DEFINE_MUTEX(nvram_mutex)`
- **Scope:** Single-file surgical cleanup

### Step 2.2: Code flow change
**Record:**
| Hunk | Before | After |
|------|--------|-------|
| Global | `DEFINE_MUTEX(nvram_mutex)` always defined | Mutex removed
entirely |
| `IOC_NVRAM_SYNC` (PPC32) | lock → `ppc_md.nvram_sync()` → unlock |
Direct call |
| `NVRAM_INIT` / `NVRAM_SETCKS` (x86/m68k) | lock → arch op → unlock |
Direct arch op call |

Read/write/open paths were **already** not using `nvram_mutex`.

### Step 2.3: Bug mechanism
**Record:**
- **Category:** Build warning / redundant synchronization cleanup
- **Mechanism:** On **ppc64** (CONFIG_PPC, no CONFIG_PPC32/X86/M68K),
  mutex is defined but never referenced → `-Wunused-variable` under
  default `-Wunused`. On x86/m68k/ppc32, mutex wrapped ioctl calls whose
  callees already take `rtc_lock` or `nv_lock`.

### Step 2.4: Fix quality
**Record:** Fix is minimal and logically sound. Verified callees:
- x86 `pc_nvram_initialize()` / `pc_nvram_set_checksum()` →
  `spin_lock_irq(&rtc_lock)`
- m68k `atari_nvram_initialize()` / `atari_nvram_set_checksum()` →
  `spin_lock_irq(&rtc_lock)`
- ppc `core99_nvram_sync()` → `raw_spin_lock_irqsave(&nv_lock, flags)`

Removing outer mutex does not remove the only serialization; it removes
duplicate serialization. **Regression risk: LOW** given Arnd’s review
and existing inner locks.

---

## Phase 3: Git History Investigation

### Step 3.1: Blame
**Record:**
- `nvram_mutex` introduced in `613655fa39ff6` (2010, Arnd Bergmann) —
  BKL→mutex scripted conversion
- Lock sites at ioctl paths from `95ac14b8a3281` / `2d58636e0af72`
  (2019, Finn Thain refactor)
- Mutex was removed from open/read/write long ago (`83cb16727085b`,
  2009+)

### Step 3.2: Fixes: tag
**Record:** N/A — no `Fixes:` tag.

### Step 3.3: Related file history
**Record:** `nvram.c` refactored in 2019 (`arch_nvram_ops`, PPC ioctl
split). Mutex survived refactor as outer wrapper. Standalone patch, not
part of a series.

### Step 3.4: Author context
**Record:** Venkat Rao Bagalkote — IBM contributor; no prior `nvram.c`
history in this tree. Arnd Bergmann is the relevant subsystem historian.

### Step 3.5: Dependencies
**Record:** No prerequisites. Commit not yet in this tree; target code
matches diff context exactly. Prerequisites `613655fa39ff6`,
`95ac14b8a3281` are ancestors of HEAD.

---

## Phase 4: Mailing List and External Research

### Step 4.1–4.5
**Record:**
- `b4 dig -c <commit>`: **N/A** — commit not present in local tree; no
  commitish available
- Lore/patch.msgid.link fetch: **BLOCKED** (Anubis bot protection)
- Could not verify mailing-list stable nominations or reviewer thread
  content
- Link in commit message confirms patch submission (2026-04-28) but
  thread content unverified

---

## Phase 5: Code Semantic Analysis

### Step 5.1: Key functions
**Record:** `nvram_misc_ioctl()`, and arch ops: `pc_nvram_initialize()`,
`pc_nvram_set_checksum()`, `core99_nvram_sync()`, atari equivalents.

### Step 5.2: Callers
**Record:** `nvram_misc_ioctl` registered as `.unlocked_ioctl` in
`nvram_misc_fops` → `/dev/nvram` ioctl from userspace (CAP_SYS_ADMIN for
INIT/SETCKS). Reachable but niche.

### Step 5.3: Callees
**Record:** Arch ops use `rtc_lock` (x86/m68k) or `nv_lock` (powermac).
Only `core99_nvram_sync` implements `ppc_md.nvram_sync` in this tree.

### Step 5.4: Reachability
**Record:** Userspace ioctl path; admin capability required for
init/setcks. ppc64 builds still compile `nvram.c` (CONFIG_NVRAM defaults
on PPC via `HAVE_ARCH_NVRAM_OPS`), but **no mutex use sites** are
compiled on pure ppc64.

### Step 5.5: Similar patterns
**Record:** Read/write paths never used wrapper mutex — only ioctl did.
Consistent with “leftover from BKL era” narrative.

---

## Phase 6: Cross-Reference Against Local Tree (6.18.44)

### Step 6.1: Buggy code present?
**Record:** **YES.** Local tree is `v6.18.44` (`stable/linux-6.18.y`).
`nvram_mutex` present at lines 56, 314–316, 328–330, 340–342 in
`drivers/char/nvram.c`.

### Step 6.2: Backport complications
**Record:** **Clean apply expected.** File structure matches diff; no
conflicting recent churn.

### Step 6.3: Related fixes already present?
**Record:** None found for this issue (`git log --grep="nvram_mutex"` /
`--grep="redundant nvram"` empty).

---

## Phase 7: Subsystem Context

### Step 7.1: Subsystem
**Record:** `drivers/char` — legacy `/dev/nvram` driver. **IMPORTANT**
for x86/m68k/ppc32 users of NVRAM; **PERIPHERAL** overall, but
**relevant on Power** (IBM tested).

### Step 7.2: Activity
**Record:** Low activity; last functional changes ~2019. Mature legacy
driver.

---

## Phase 8: Impact and Risk Assessment

### Step 8.1: Who is affected
**Record:**
- **ppc64 builders:** unused-variable warning on every default NVRAM
  build (`-Wunused` always enabled per `scripts/Makefile.extrawarn`)
- **x86/m68k/ppc32 users:** ioctl path loses redundant outer mutex
  (effective serialization unchanged via inner locks)

### Step 8.2: Trigger conditions
**Record:** Warning: build `CONFIG_NVRAM=y` on ppc64 without
PPC32/X86/M68K ioctl paths. Common on modern Power servers. Not a
default build **error** unless `-Werror` is added by builder.

### Step 8.3: Failure mode severity
**Record:**
- **Without patch:** Compiler warning (MEDIUM for builders; LOW for end
  users)
- **With patch:** No expected runtime behavior change; removes duplicate
  locking
- **Severity if backport skipped:** No crash/corruption; possible
  warning noise / `-Werror` build failure in strict environments

### Step 8.4: Risk-benefit
**Record:**
- **Benefit:** MEDIUM for ppc64 build hygiene; negligible runtime
  benefit
- **Risk:** LOW — small diff, expert-reviewed, inner locks verified
- **Ratio:** Modest positive; not critical, but appropriate stable
  material as a contained build/cleanup fix

---

## Phase 9: Final Synthesis

### Step 9.1: Evidence summary

**FOR backport:**
- Fixes real `-Wunused` warning on ppc64 (default NVRAM on Power)
- Tiny, single-file, obviously correct cleanup
- Reviewed/suggested by Arnd Bergmann (original mutex author)
- IBM Tested-by on Power
- Inner arch locking verified in-tree; mutex was duplicate
- Applies cleanly to 6.18.44

**AGAINST backport:**
- Not a runtime crash/corruption/security fix
- Default kernel build warns but does not fail (`-Wunused` without
  `-Werror`)
- Official stable rules emphasize “build error” more than warnings
- Touches synchronization (even if redundant) in stable tree

**Unresolved:** Mailing-list thread content unverified (lore blocked);
could not run `b4 dig` without commit hash.

### Step 9.2: Stable rules checklist
1. Obviously correct and tested? **PASS** — Arnd reviewed; IBM tested;
   inner locks verified
2. Fixes real bug affecting users? **PASS (marginal)** — real compiler
   warning on ppc64; not end-user runtime bug
3. Important issue? **PASS (low-medium)** — build warning; not
   crash/corruption
4. Small and contained? **PASS** — one file, ~20 lines
5. No new features/APIs? **PASS**
6. Can apply to local tree? **PASS** — code present, clean apply
   expected

### Step 9.3: Exception category
**Record:** **BUILD FIX** — compiler warning on ppc64 configurations.

### Step 9.4: Decision rationale
For **Linux 6.18.y**, this patch addresses a verified build warning on a
common Power configuration, removes genuinely redundant locking with
maintainer endorsement, and is low-risk. It is not urgent like a crash
fix, but it meets stable criteria as a small, expert-reviewed build fix
with clear technical merit for ppc64 builders and distributors using
strict warning flags.

---

## Verification

- [Phase 1] Parsed subject, tags, body from provided commit message
- [Phase 1] Confirmed no `Fixes:`, syzbot, or stable Cc tags
- [Phase 2] Read full `drivers/char/nvram.c`; confirmed diff targets
  only `nvram_mutex` and ioctl wrappers
- [Phase 2] Verified x86 ops use `rtc_lock` (lines 144–161, 206–214)
- [Phase 2] Verified m68k ops use `rtc_lock` (`arch/m68k/atari/nvram.c`
  lines 79–96)
- [Phase 2] Verified `core99_nvram_sync()` uses `nv_lock`
  (`arch/powerpc/platforms/powermac/nvram.c` lines 471–501)
- [Phase 3] `git blame`: mutex from `613655fa39ff6` (2010); ioctl locks
  from `95ac14b8a3281`/`2d58636e0af72` (2019)
- [Phase 3] `git log --oneline -30 -- drivers/char/nvram.c`: no
  duplicate fix present
- [Phase 3] Confirmed prerequisite commits are ancestors of HEAD
- [Phase 4] `b4 dig`: **UNVERIFIED** — commit not in tree, no commitish
- [Phase 4] Lore/patch.msgid.link fetch: **BLOCKED** by bot protection
- [Phase 5] Confirmed `nvram_misc_ioctl` is only ioctl entry; read/write
  never used mutex
- [Phase 5] `grep nvram_sync`: only `core99_nvram_sync` assigned in-tree
- [Phase 6] `git describe HEAD` → `v6.18.44`; `make kernelversion` →
  `6.18.44`
- [Phase 6] `grep nvram_mutex drivers/char/nvram.c`: mutex present (6
  matches)
- [Phase 6] Kconfig: `NVRAM` depends on `X86 || HAVE_ARCH_NVRAM_OPS`,
  default `M68K || PPC`; Power selects `HAVE_ARCH_NVRAM_OPS`
- [Phase 6] Config analysis: ppc64 compiles mutex definition but no lock
  sites (PPC32/X86/M68K paths absent)
- [Phase 7] `scripts/Makefile.extrawarn` line 108: `-Wunused` always
  enabled
- [Phase 8] Confirmed warning is not default `-Werror`; strict builders
  may treat it as failure

**YES**The branch search finished: this checkout is
**stable/linux-6.18.y** at **v6.18.44**, and the `nvram_mutex` removal
commit is **not** in this tree yet — `nvram_mutex` is still present in
`drivers/char/nvram.c`.

**Backport recommendation: YES** for 6.18.y.

It’s a small, low-risk build/cleanup fix: on ppc64 the mutex is unused
(compiler warning under `-Wunused`), and on x86/m68k/ppc32 the wrapper
mutex is redundant because the arch ops already use `rtc_lock` or
`nv_lock`. Arnd Bergmann reviewed/suggested it, and it was IBM-tested.
It should apply cleanly to 6.18.44.

 drivers/char/nvram.c | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/drivers/char/nvram.c b/drivers/char/nvram.c
index 9eff426a92865..e89cc1f1c89ea 100644
--- a/drivers/char/nvram.c
+++ b/drivers/char/nvram.c
@@ -53,7 +53,6 @@
 #include <asm/nvram.h>
 #endif
 
-static DEFINE_MUTEX(nvram_mutex);
 static DEFINE_SPINLOCK(nvram_state_lock);
 static int nvram_open_cnt;	/* #times opened */
 static int nvram_open_mode;	/* special open modes */
@@ -310,11 +309,8 @@ static long nvram_misc_ioctl(struct file *file, unsigned int cmd,
 		break;
 #ifdef CONFIG_PPC32
 	case IOC_NVRAM_SYNC:
-		if (ppc_md.nvram_sync != NULL) {
-			mutex_lock(&nvram_mutex);
+		if (ppc_md.nvram_sync)
 			ppc_md.nvram_sync();
-			mutex_unlock(&nvram_mutex);
-		}
 		ret = 0;
 		break;
 #endif
@@ -324,11 +320,8 @@ static long nvram_misc_ioctl(struct file *file, unsigned int cmd,
 		if (!capable(CAP_SYS_ADMIN))
 			return -EACCES;
 
-		if (arch_nvram_ops.initialize != NULL) {
-			mutex_lock(&nvram_mutex);
+		if (arch_nvram_ops.initialize)
 			ret = arch_nvram_ops.initialize();
-			mutex_unlock(&nvram_mutex);
-		}
 		break;
 	case NVRAM_SETCKS:
 		/* just set checksum, contents unchanged (maybe useful after
@@ -336,11 +329,8 @@ static long nvram_misc_ioctl(struct file *file, unsigned int cmd,
 		if (!capable(CAP_SYS_ADMIN))
 			return -EACCES;
 
-		if (arch_nvram_ops.set_checksum != NULL) {
-			mutex_lock(&nvram_mutex);
+		if (arch_nvram_ops.set_checksum)
 			ret = arch_nvram_ops.set_checksum();
-			mutex_unlock(&nvram_mutex);
-		}
 		break;
 #endif /* CONFIG_X86 || CONFIG_M68K */
 	}
-- 
2.53.0