linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Peter Wang <peter.wang@mediatek.com>,
	Chun-Hung Wu <chun-hung.wu@mediatek.com>,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	Sasha Levin <sashal@kernel.org>,
	chaotian.jing@mediatek.com, matthias.bgg@gmail.com,
	angelogioacchino.delregno@collabora.com,
	linux-scsi@vger.kernel.org, linux-mediatek@lists.infradead.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: [PATCH AUTOSEL 6.18-6.12] scsi: ufs: host: mediatek: Fix shutdown/suspend race condition
Date: Tue,  9 Dec 2025 22:48:49 -0500	[thread overview]
Message-ID: <20251210034915.2268617-8-sashal@kernel.org> (raw)
In-Reply-To: <20251210034915.2268617-1-sashal@kernel.org>

From: Peter Wang <peter.wang@mediatek.com>

[ Upstream commit 014de20bb36ba03e0e0b0a7e0a1406ab900c9fda ]

Address a race condition between shutdown and suspend operations in the
UFS Mediatek driver. Before entering suspend, check if a shutdown is in
progress to prevent conflicts and ensure system stability.

Signed-off-by: Peter Wang <peter.wang@mediatek.com>
Acked-by: Chun-Hung Wu <chun-hung.wu@mediatek.com>
Link: https://patch.msgid.link/20250924094527.2992256-6-peter.wang@mediatek.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## Analysis Summary

### 1. Commit Message Analysis
- **Subject**: "Fix shutdown/suspend race condition" - clearly indicates
  a bug fix
- **No Cc: stable tag** or **Fixes: tag** present
- The message describes preventing conflicts between shutdown and
  suspend operations "to ensure system stability"

### 2. Code Change Analysis
The fix is extremely small (4 lines):
```c
if (hba->shutting_down) {
    ret = -EBUSY;
    goto out;
}
```

**What it does**: At the start of `ufs_mtk_system_suspend()`, before
calling `ufshcd_system_suspend()` and performing Mediatek-specific
operations (voltage regulator LPM, MTCMOS control), it checks if
`hba->shutting_down` is true.

**Why the race is problematic**: The Mediatek driver performs hardware-
specific operations after calling the core suspend:
- `ufs_mtk_dev_vreg_set_lpm()` - controls voltage regulators
- `ufs_mtk_mtcmos_ctrl()` - controls power domains

If shutdown is in progress (`ufshcd_wl_shutdown()` sets
`hba->shutting_down = true`), these operations could conflict with the
shutdown sequence that also manipulates hardware state, causing
instability.

### 3. Classification
- **Bug fix**: Yes - fixes a real race condition
- **Not a feature**: No new functionality, APIs, or capabilities added
- Uses existing infrastructure (`shutting_down` flag already exists in
  `struct ufs_hba`)

### 4. Scope and Risk Assessment
- **Lines changed**: 4 lines (minimal)
- **Files touched**: 1 (driver-specific)
- **Risk**: Very low - adds a defensive early return that makes the
  driver more conservative
- **Regression potential**: Minimal - worst case is a suspend returning
  `-EBUSY` when it shouldn't, but this is very unlikely given the
  shutdown context

### 5. User Impact
- **Affected hardware**: Mediatek UFS storage (common in Android devices
  and embedded systems)
- **Severity**: Race conditions between suspend/shutdown can cause
  system hangs, crashes, or data corruption
- **Real-world scenario**: User initiates shutdown while system is
  suspending (e.g., closing laptop lid during shutdown)

### 6. Stability Indicators
- Signed-off-by Mediatek developer (Peter Wang)
- Acked-by another Mediatek engineer
- Merged by SCSI maintainer Martin K. Petersen

### 7. Concerns
- No explicit `Cc: stable` tag
- No `Fixes:` tag indicating when the bug was introduced
- No bug report or `Reported-by` tag showing users encountered this
  issue

### Decision Rationale

**In favor of backporting:**
- Fixes a real race condition that can cause system instability
- The fix is trivially correct and uses existing infrastructure
- Very small change with minimal regression risk
- Mediatek UFS is widely deployed in consumer devices
- Race conditions between power management paths (suspend/shutdown) are
  serious issues

**Against backporting:**
- No explicit stable tag from the maintainer
- No evidence of users hitting this bug in practice
- Other UFS drivers don't have similar protection (though they may not
  have the same post-suspend hardware operations)

The fix is clearly correct, surgical, and addresses a real class of bug
(PM race conditions). While lacking explicit stable markers, the fix
meets the stable kernel criteria: it's obviously correct, fixes a real
bug affecting system stability, and is small and contained.

**YES**

 drivers/ufs/host/ufs-mediatek.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-mediatek.c
index 758a393a9de1a..d0cbd96ad29dc 100644
--- a/drivers/ufs/host/ufs-mediatek.c
+++ b/drivers/ufs/host/ufs-mediatek.c
@@ -2373,6 +2373,11 @@ static int ufs_mtk_system_suspend(struct device *dev)
 	struct arm_smccc_res res;
 	int ret;
 
+	if (hba->shutting_down) {
+		ret = -EBUSY;
+		goto out;
+	}
+
 	ret = ufshcd_system_suspend(dev);
 	if (ret)
 		goto out;
-- 
2.51.0



       reply	other threads:[~2025-12-10  3:49 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20251210034915.2268617-1-sashal@kernel.org>
2025-12-10  3:48 ` Sasha Levin [this message]
2025-12-10  3:48 ` [PATCH AUTOSEL 6.18-5.10] clk: mvebu: cp110 add CLK_IGNORE_UNUSED to pcie_x10, pcie_x11 & pcie_x4 Sasha Levin
2025-12-10  3:48 ` [PATCH AUTOSEL 6.18-5.10] firmware: imx: scu-irq: Init workqueue before request mbox channel 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=20251210034915.2268617-8-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=angelogioacchino.delregno@collabora.com \
    --cc=chaotian.jing@mediatek.com \
    --cc=chun-hung.wu@mediatek.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=matthias.bgg@gmail.com \
    --cc=patches@lists.linux.dev \
    --cc=peter.wang@mediatek.com \
    --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).