Linux ARM-MSM sub-architecture
 help / color / mirror / Atom feed
* [PATCH 0/2] drm/msm: Fix a couple submit leaks
@ 2025-05-14 16:33 Rob Clark
  2025-05-14 16:33 ` [PATCH 1/2] drm/msm: Fix a fence leak in submit error path Rob Clark
  2025-05-14 16:33 ` [PATCH 2/2] drm/msm: Fix another leak in the " Rob Clark
  0 siblings, 2 replies; 3+ messages in thread
From: Rob Clark @ 2025-05-14 16:33 UTC (permalink / raw)
  To: dri-devel
  Cc: linux-arm-msm, freedreno, Rob Clark, open list, Marijn Suijten,
	Sean Paul

From: Rob Clark <robdclark@chromium.org>

A couple things I found in the process of debugging VM_BIND, with all
the kernel debug knobs turned on.

Rob Clark (2):
  drm/msm: Fix a fence leak in submit error path
  drm/msm: Fix another leak in the submit error path

 drivers/gpu/drm/msm/msm_gem_submit.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

-- 
2.49.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH 1/2] drm/msm: Fix a fence leak in submit error path
  2025-05-14 16:33 [PATCH 0/2] drm/msm: Fix a couple submit leaks Rob Clark
@ 2025-05-14 16:33 ` Rob Clark
  2025-05-14 16:33 ` [PATCH 2/2] drm/msm: Fix another leak in the " Rob Clark
  1 sibling, 0 replies; 3+ messages in thread
From: Rob Clark @ 2025-05-14 16:33 UTC (permalink / raw)
  To: dri-devel
  Cc: linux-arm-msm, freedreno, Rob Clark, Rob Clark, Abhinav Kumar,
	Dmitry Baryshkov, Sean Paul, Marijn Suijten, David Airlie,
	Simona Vetter, open list

From: Rob Clark <robdclark@chromium.org>

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>
---
 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 3e9aa2cc38ef..b2aeaecaa39b 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -85,6 +85,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.49.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH 2/2] drm/msm: Fix another leak in the submit error path
  2025-05-14 16:33 [PATCH 0/2] drm/msm: Fix a couple submit leaks Rob Clark
  2025-05-14 16:33 ` [PATCH 1/2] drm/msm: Fix a fence leak in submit error path Rob Clark
@ 2025-05-14 16:33 ` Rob Clark
  1 sibling, 0 replies; 3+ messages in thread
From: Rob Clark @ 2025-05-14 16:33 UTC (permalink / raw)
  To: dri-devel
  Cc: linux-arm-msm, freedreno, Rob Clark, Rob Clark, Abhinav Kumar,
	Dmitry Baryshkov, Sean Paul, Marijn Suijten, David Airlie,
	Simona Vetter, open list

From: Rob Clark <robdclark@chromium.org>

put_unused_fd() doesn't free the installed file, if we've already done
fd_install().  So we need to also free the sync_file.

Signed-off-by: Rob Clark <robdclark@chromium.org>
---
 drivers/gpu/drm/msm/msm_gem_submit.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c
index b2aeaecaa39b..d4f71bb54e84 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -658,6 +658,7 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
 	struct msm_ringbuffer *ring;
 	struct msm_submit_post_dep *post_deps = NULL;
 	struct drm_syncobj **syncobjs_to_reset = NULL;
+	struct sync_file *sync_file = NULL;
 	int out_fence_fd = -1;
 	unsigned i;
 	int ret;
@@ -867,7 +868,7 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
 	}
 
 	if (ret == 0 && args->flags & MSM_SUBMIT_FENCE_FD_OUT) {
-		struct sync_file *sync_file = sync_file_create(submit->user_fence);
+		sync_file = sync_file_create(submit->user_fence);
 		if (!sync_file) {
 			ret = -ENOMEM;
 		} else {
@@ -901,8 +902,11 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
 out_unlock:
 	mutex_unlock(&queue->lock);
 out_post_unlock:
-	if (ret && (out_fence_fd >= 0))
+	if (ret && (out_fence_fd >= 0)) {
 		put_unused_fd(out_fence_fd);
+		if (sync_file)
+			fput(sync_file->file);
+	}
 
 	if (!IS_ERR_OR_NULL(submit)) {
 		msm_gem_submit_put(submit);
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-05-14 16:33 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-14 16:33 [PATCH 0/2] drm/msm: Fix a couple submit leaks Rob Clark
2025-05-14 16:33 ` [PATCH 1/2] drm/msm: Fix a fence leak in submit error path Rob Clark
2025-05-14 16:33 ` [PATCH 2/2] drm/msm: Fix another leak in the " Rob Clark

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox