| Message ID | 20250312134400.2176393-1-christian.koenig@amd.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/sched: revert "drm_sched_job_cleanup(): correct false doc" | expand |
On Wed, 2025-03-12 at 14:44 +0100, Christian König wrote: > This reverts commit 44d2f310f008613c1dbe5e234c2cf2be90cbbfab. > > 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. > > 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> Applied to drm-misc-next, thx P. > --- > 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); > } >
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); }
This reverts commit 44d2f310f008613c1dbe5e234c2cf2be90cbbfab. 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. 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(-)