From: Ziqi Chen quic_ziqichen@quicinc.com
[ Upstream commit e97633492f5a3eca7b3ff03b4ef6f993017f7955 ]
When preparing for UFS clock scaling, the UFS driver will quiesce all sdevs queues in the UFS SCSI host tagset list and then unquiesce them in ufshcd_clock_scaling_unprepare(). If the UFS SCSI host async scan is in progress at this time, some LUs may be added to the tagset list between UFS clkscale prepare and unprepare. This can cause two issues:
1. During clock scaling, there may be I/O requests issued through new added queues that have not been quiesced, leading to task abort issue.
2. These new added queues that have not been quiesced will be unquiesced as well when UFS clkscale is unprepared, resulting in warning prints.
Therefore, use the mutex lock scan_mutex in ufshcd_clock_scaling_prepare() and ufshcd_clock_scaling_unprepare() to protect it.
Co-developed-by: Can Guo quic_cang@quicinc.com Signed-off-by: Can Guo quic_cang@quicinc.com Signed-off-by: Ziqi Chen quic_ziqichen@quicinc.com Link: https://lore.kernel.org/r/20250522081233.2358565-1-quic_ziqichen@quicinc.com Suggested-by: Bart Van Assche bvanassche@acm.org Reviewed-by: Bart Van Assche bvanassche@acm.org Signed-off-by: Martin K. Petersen martin.petersen@oracle.com Signed-off-by: Sasha Levin sashal@kernel.org ---
Based on my thorough analysis of the code and the race condition, here's my assessment:
**YES**
This commit should be backported to stable kernel trees. Here's my extensive analysis:
## Bug Analysis
The commit fixes a **critical race condition** between UFS clock scaling operations and async SCSI scanning that can lead to two serious issues:
### 1. **The Race Condition Explained**
Looking at the code changes, the issue occurs when:
**Before the fix**: `ufshcd_clock_scaling_prepare()` performs operations in this order: ```c blk_mq_quiesce_tagset(&hba->host->tag_set); // Quiesce existing queues mutex_lock(&hba->wb_mutex); down_write(&hba->clk_scaling_lock); ```
**The race**: If `ufshcd_async_scan()` is running concurrently, it calls `scsi_scan_host()` at line 8789, which: 1. Discovers new LUNs and adds them to the tagset 2. Creates new block queues for these LUNs 3. These new queues are **not quiesced** by the earlier `blk_mq_quiesce_tagset()` call
**After the fix**: The addition of `mutex_lock(&hba->host->scan_mutex)` **before** `blk_mq_quiesce_tagset()` ensures: ```c mutex_lock(&hba->host->scan_mutex); // NEW: Serialize with scanning blk_mq_quiesce_tagset(&hba->host->tag_set); // Now quiesces ALL queues ```
### 2. **Specific Problems This Fixes**
**Issue #1 - Task Abort**: Non-quiesced new queues can continue issuing I/O during clock scaling, leading to task aborts when the UFS controller changes power states mid-transaction.
**Issue #2 - Warning Messages**: In `ufshcd_clock_scaling_unprepare()`, `blk_mq_unquiesce_tagset()` attempts to unquiesce ALL queues in the tagset, including newly added ones that were never quiesced, triggering warning messages.
### 3. **Why This Should Be Backported**
**Critical System Stability**: This fixes a race that can cause: - I/O errors and potential data corruption during clock scaling - System warnings and potential instability - Unpredictable behavior during device discovery
**High Impact**: The race affects: - **UFS storage devices** (widely used in mobile phones, tablets, servers) - **Clock scaling operations** (essential for power management) - **Device discovery** (happens during boot and hotplug)
**Low Risk Change**: The fix is minimal and surgical: - Only adds `scan_mutex` synchronization around existing code - No functional changes to the clock scaling or scanning logic - Follows established kernel locking patterns (similar to other SCSI drivers)
**Timing-Dependent Bug**: This is exactly the type of race condition that: - May not be easily reproducible in testing - Can appear randomly in production under load - Becomes more likely with faster systems or specific timing conditions
### 4. **Code Quality Assessment**
The fix demonstrates good kernel programming practices: - **Proper lock ordering**: Acquires `scan_mutex` first to serialize with scanning - **Complete error handling**: Properly releases `scan_mutex` in error paths - **Symmetric operations**: Both `prepare` and `unprepare` handle the mutex consistently - **Minimal scope**: Changes only what's necessary to fix the race
### 5. **Comparison with Similar Commits**
Looking at the historical commits provided: - This is similar to commit #4 (marked YES) which also fixes synchronization during async operations - Unlike commits #1, #2, #3, #5 (marked NO) which are optimization/refactoring changes - This is a **bug fix** addressing a real race condition, not a feature addition
**Conclusion**: This commit fixes a legitimate race condition that can cause system instability in UFS-based systems. It's a small, well- contained fix that addresses a critical synchronization issue without introducing new features or architectural changes. This makes it an ideal candidate for stable kernel backporting.
drivers/ufs/core/ufshcd.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index 47ec4e4e4a2a0..c68c868d112bb 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -1368,6 +1368,7 @@ static int ufshcd_clock_scaling_prepare(struct ufs_hba *hba, u64 timeout_us) * make sure that there are no outstanding requests when * clock scaling is in progress */ + mutex_lock(&hba->host->scan_mutex); blk_mq_quiesce_tagset(&hba->host->tag_set); mutex_lock(&hba->wb_mutex); down_write(&hba->clk_scaling_lock); @@ -1378,6 +1379,7 @@ static int ufshcd_clock_scaling_prepare(struct ufs_hba *hba, u64 timeout_us) up_write(&hba->clk_scaling_lock); mutex_unlock(&hba->wb_mutex); blk_mq_unquiesce_tagset(&hba->host->tag_set); + mutex_unlock(&hba->host->scan_mutex); goto out; }
@@ -1399,6 +1401,7 @@ static void ufshcd_clock_scaling_unprepare(struct ufs_hba *hba, int err, bool sc mutex_unlock(&hba->wb_mutex);
blk_mq_unquiesce_tagset(&hba->host->tag_set); + mutex_unlock(&hba->host->scan_mutex); ufshcd_release(hba); }