[PATCH AUTOSEL 6.1 01/15] drm/msm: Fix a fence leak in submit error path

[Sasha Levin]

From: Rob Clark <robdclark at 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 at chromium.org>
Patchwork: https://patchwork.freedesktop.org/patch/653584/
Signed-off-by: Rob Clark <robin.clark at oss.qualcomm.com>
Signed-off-by: Sasha Levin <sashal at 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