From: "Christian König" <ckoenig.leichtzumerken@gmail.com>
To: matthew.brost@intel.com, dakr@kernel.org, phasta@kernel.org,
tvrtko.ursulin@igalia.com, dri-devel@lists.freedesktop.org,
linux-kernel@vger.kernel.org
Cc: "Christian König" <christian.koenig@amd.com>
Subject: [PATCH] drm/sched: revert "drm_sched_job_cleanup(): correct false doc"
Date: Mon, 10 Mar 2025 08:44:14 +0100 [thread overview]
Message-ID: <20250310074414.2129157-1-christian.koenig@amd.com> (raw)
This reverts commit 44d2f310f008613c1dbe5e234c2cf2be90cbbfab.
Sorry for the delayed response, I only stumbled over this now while going
over old mails and then re-thinking my reviewed by for this change.
The function drm_sched_job_arm() is indeed the point of no return. The
background is that it is nearly impossible for the driver to correctly
retract the fence and signal it in the order enforced by the dma_fence
framework.
The code in drm_sched_job_cleanup() is for the purpose to cleanup after
the job was armed through drm_sched_job_arm() *and* processed by the
scheduler.
The correct approach for error handling in this situation is to set the
error on the fences and then push to the entity anyway. We can certainly
improve the documentation, but removing the warning is clearly not a good
idea.
Signed-off-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/scheduler/sched_main.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c
index 53e6aec37b46..4d4219fbe49d 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -1015,13 +1015,11 @@ EXPORT_SYMBOL(drm_sched_job_has_dependency);
* Cleans up the resources allocated with drm_sched_job_init().
*
* Drivers should call this from their error unwind code if @job is aborted
- * before it was submitted to an entity with drm_sched_entity_push_job().
+ * before drm_sched_job_arm() is called.
*
- * Since calling drm_sched_job_arm() causes the job's fences to be initialized,
- * it is up to the driver to ensure that fences that were exposed to external
- * parties get signaled. drm_sched_job_cleanup() does not ensure this.
- *
- * This function must also be called in &struct drm_sched_backend_ops.free_job
+ * After that point of no return @job is committed to be executed by the
+ * scheduler, and this function should be called from the
+ * &drm_sched_backend_ops.free_job callback.
*/
void drm_sched_job_cleanup(struct drm_sched_job *job)
{
@@ -1032,7 +1030,7 @@ void drm_sched_job_cleanup(struct drm_sched_job *job)
/* drm_sched_job_arm() has been called */
dma_fence_put(&job->s_fence->finished);
} else {
- /* aborted job before arming */
+ /* aborted job before committing to run it */
drm_sched_fence_free(job->s_fence);
}
--
2.34.1
next reply other threads:[~2025-03-10 7:44 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-10 7:44 Christian König [this message]
2025-03-10 8:41 ` Philipp Stanner
2025-03-10 9:30 ` Tvrtko Ursulin
2025-03-10 9:56 ` Christian König
2025-03-10 12:11 ` Philipp Stanner
2025-03-10 12:25 ` Christian König
2025-03-10 12:37 ` Philipp Stanner
2025-03-10 12:27 ` Tvrtko Ursulin
2025-03-10 14:28 ` Christian König
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=20250310074414.2129157-1-christian.koenig@amd.com \
--to=ckoenig.leichtzumerken@gmail.com \
--cc=christian.koenig@amd.com \
--cc=dakr@kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=linux-kernel@vger.kernel.org \
--cc=matthew.brost@intel.com \
--cc=phasta@kernel.org \
--cc=tvrtko.ursulin@igalia.com \
/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
all inboxes | Powered by JetHome®