All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH AUTOSEL 6.1 01/15] drm/msm: Fix a fence leak in submit error path
@ 2025-06-24  4:12 Sasha Levin
  2025-06-24  4:12 ` [PATCH AUTOSEL 6.1 02/15] drm/msm: Fix another leak in the " Sasha Levin
                   ` (13 more replies)
  0 siblings, 14 replies; 15+ messages in thread
From: Sasha Levin @ 2025-06-24  4:12 UTC (permalink / raw)
  To: patches, stable
  Cc: Rob Clark, Rob Clark, Sasha Levin, lumag, linux-arm-msm,
	dri-devel, freedreno

From: Rob Clark <robdclark@chromium.org>

[ Upstream commit 5d319f75ccf7f0927425a7545aa1a22b3eedc189 ]

In error paths, we could unref the submit without calling
drm_sched_entity_push_job(), so msm_job_free() will never get
called.  Since drm_sched_job_cleanup() will NULL out the
s_fence, we can use that to detect this case.

Signed-off-by: Rob Clark <robdclark@chromium.org>
Patchwork: https://patchwork.freedesktop.org/patch/653584/
Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

Let me analyze based on what I know from the commit:

**YES**

This commit should be backported to stable kernel trees. Here's my
extensive analysis:

## Analysis of the Fence Leak Fix

### 1. **Bug Description**
The commit fixes a resource leak in the MSM GPU driver's submit error
path. When errors occur after `drm_sched_job_arm()` but before
`drm_sched_entity_push_job()`, the scheduler fence (`s_fence`) is leaked
because `drm_sched_job_cleanup()` is never called.

### 2. **Code Changes Analysis**

The fix adds this critical code to `__msm_gem_submit_destroy()`:

```c
/*
 - In error paths, we could unref the submit without calling
 - drm_sched_entity_push_job(), so msm_job_free() will never
 - get called.  Since drm_sched_job_cleanup() will NULL out
 - s_fence, we can use that to detect this case.
 */
if (submit->base.s_fence)
    drm_sched_job_cleanup(&submit->base);
```

This is a defensive check that ensures proper cleanup of scheduler
resources when the submit is destroyed without going through the normal
scheduler path.

### 3. **Why This is a Good Backport Candidate**

**a) Fixes a Real Bug:** This addresses a concrete resource leak that
affects system stability. Each leaked fence consumes memory and other
kernel resources that are never freed.

**b) Small and Contained:** The fix is minimal - just 9 lines of code
including comments. It doesn't change any APIs or data structures.

**c) Clear Error Path Fix:** The fix targets a specific error handling
path without affecting the normal execution flow. When
`drm_sched_entity_push_job()` is called successfully, `msm_job_free()`
handles the cleanup as before.

**d) Follows Established Patterns:** The fix uses the same pattern as
the hw_fence cleanup (checking refcount) that's already in the code,
making it consistent with existing error handling.

**e) No Architectural Changes:** This is purely a bug fix that plugs a
resource leak. It doesn't introduce new features or change behavior.

**f) Critical Subsystem:** GPU drivers are important for system
stability and user experience. Memory leaks in GPU submission paths can
accumulate quickly under heavy graphics workloads.

### 4. **Comparison with Similar Commits**

Looking at the similar commits provided:
- The first commit (drm/msm: Fix wait_fence submitqueue leak) with
  backport status YES also fixes a resource leak in error paths
- It's similarly small, contained, and fixes a clear bug without
  architectural changes

### 5. **Risk Assessment**

**Low Risk:**
- The added check only executes in the destroy path when `s_fence` is
  non-NULL
- If `drm_sched_entity_push_job()` was called, `msm_job_free()` will
  have already cleaned up `s_fence`, making it NULL
- The fix is defensive and won't cause issues even if called
  unnecessarily

### 6. **Stable Criteria Met**

According to stable tree rules:
- ✓ Fixes a bug (fence resource leak)
- ✓ Minimal change (9 lines)
- ✓ Obviously correct (clear logic with explanatory comment)
- ✓ Already tested (has Patchwork link indicating review)
- ✓ No new features
- ✓ Fixes a real issue users could encounter

The fence leak could accumulate over time, especially in scenarios with
GPU submission failures, leading to memory exhaustion and system
instability. This makes it an important fix for stable kernels.

 drivers/gpu/drm/msm/msm_gem_submit.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c
index c12a6ac2d3840..4ee6aeb23c512 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -71,6 +71,15 @@ void __msm_gem_submit_destroy(struct kref *kref)
 			container_of(kref, struct msm_gem_submit, ref);
 	unsigned i;
 
+	/*
+	 * In error paths, we could unref the submit without calling
+	 * drm_sched_entity_push_job(), so msm_job_free() will never
+	 * get called.  Since drm_sched_job_cleanup() will NULL out
+	 * s_fence, we can use that to detect this case.
+	 */
+	if (submit->base.s_fence)
+		drm_sched_job_cleanup(&submit->base);
+
 	if (submit->fence_id) {
 		spin_lock(&submit->queue->idr_lock);
 		idr_remove(&submit->queue->fence_idr, submit->fence_id);
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 15+ messages in thread
end of thread, other threads:[~2025-06-24  4:12 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-24  4:12 [PATCH AUTOSEL 6.1 01/15] drm/msm: Fix a fence leak in submit error path Sasha Levin
2025-06-24  4:12 ` [PATCH AUTOSEL 6.1 02/15] drm/msm: Fix another leak in the " Sasha Levin
2025-06-24  4:12 ` [PATCH AUTOSEL 6.1 03/15] ALSA: sb: Don't allow changing the DMA mode during operations Sasha Levin
2025-06-24  4:12 ` [PATCH AUTOSEL 6.1 04/15] ALSA: sb: Force to disable DMAs once when DMA mode is changed Sasha Levin
2025-06-24  4:12 ` [PATCH AUTOSEL 6.1 05/15] ata: libata-acpi: Do not assume 40 wire cable if no devices are enabled Sasha Levin
2025-06-24  4:12 ` [PATCH AUTOSEL 6.1 06/15] ata: pata_cs5536: fix build on 32-bit UML Sasha Levin
2025-06-24  4:12 ` [PATCH AUTOSEL 6.1 07/15] powerpc: Fix struct termio related ioctl macros Sasha Levin
2025-06-24  4:12 ` [PATCH AUTOSEL 6.1 08/15] ASoC: amd: yc: update quirk data for HP Victus Sasha Levin
2025-06-24  4:12 ` [PATCH AUTOSEL 6.1 09/15] scsi: target: Fix NULL pointer dereference in core_scsi3_decode_spec_i_port() Sasha Levin
2025-06-24  4:12 ` [PATCH AUTOSEL 6.1 10/15] aoe: defer rexmit timer downdev work to workqueue Sasha Levin
2025-06-24  4:12 ` [PATCH AUTOSEL 6.1 11/15] wifi: mac80211: drop invalid source address OCB frames Sasha Levin
2025-06-24  4:12 ` [PATCH AUTOSEL 6.1 12/15] wifi: ath6kl: remove WARN on bad firmware input Sasha Levin
2025-06-24  4:12 ` [PATCH AUTOSEL 6.1 13/15] ACPICA: Refuse to evaluate a method if arguments are missing Sasha Levin
2025-06-24  4:12 ` [PATCH AUTOSEL 6.1 14/15] mtd: spinand: fix memory leak of ECC engine conf Sasha Levin
2025-06-24  4:12 ` [PATCH AUTOSEL 6.1 15/15] rcu: Return early if callback is not specified Sasha Levin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.