Trivial fix since we now need to grab a reference to the fence we have added. Previously the dma_resv function where doing that for us.
Signed-off-by: Christian König christian.koenig@amd.com Fixes: 9c2ba265352a ("drm/scheduler: use new iterator in drm_sched_job_add_implicit_dependencies v2") Link: https://patchwork.freedesktop.org/patch/msgid/20211019112706.27769-1-christi... Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch Reported-by: Nicolas Frattaroli frattaroli.nicolas@gmail.com References: https://lore.kernel.org/dri-devel/2023306.UmlnhvANQh@archbook/ Tested-by: Nicolas Frattaroli frattaroli.nicolas@gmail.com Tested-by: Yassine Oudjana y.oudjana@protonmail.com --- drivers/gpu/drm/scheduler/sched_main.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c index 5bc5f775abe1..94fe51b3caa2 100644 --- a/drivers/gpu/drm/scheduler/sched_main.c +++ b/drivers/gpu/drm/scheduler/sched_main.c @@ -707,6 +707,9 @@ int drm_sched_job_add_implicit_dependencies(struct drm_sched_job *job, ret = drm_sched_job_add_dependency(job, fence); if (ret) return ret; + + /* Make sure to grab an additional ref on the added fence */ + dma_fence_get(fence); } return 0; }
We need to grab another ref before trying to add the fence to the sched job and not after.
Signed-off-by: Christian König christian.koenig@amd.com --- drivers/gpu/drm/scheduler/sched_main.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c index 94fe51b3caa2..400d201c3c28 100644 --- a/drivers/gpu/drm/scheduler/sched_main.c +++ b/drivers/gpu/drm/scheduler/sched_main.c @@ -704,12 +704,14 @@ int drm_sched_job_add_implicit_dependencies(struct drm_sched_job *job, int ret;
dma_resv_for_each_fence(&cursor, obj->resv, write, fence) { - ret = drm_sched_job_add_dependency(job, fence); - if (ret) - return ret; - /* Make sure to grab an additional ref on the added fence */ dma_fence_get(fence); + + ret = drm_sched_job_add_dependency(job, fence); + if (ret) { + dma_fence_put(fence); + return ret; + } } return 0; }
On Tue, Nov 16, 2021 at 10:25:19AM +0100, Christian König wrote:
We need to grab another ref before trying to add the fence to the sched job and not after.
Signed-off-by: Christian König christian.koenig@amd.com
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
I wondered first why this goes boom, but then I realized that in some cases add_dependency() drops the reference of the passed-in fence.
Please also add the Fixes: line like in the previous patch.
Cheers, Daniel
drivers/gpu/drm/scheduler/sched_main.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c index 94fe51b3caa2..400d201c3c28 100644 --- a/drivers/gpu/drm/scheduler/sched_main.c +++ b/drivers/gpu/drm/scheduler/sched_main.c @@ -704,12 +704,14 @@ int drm_sched_job_add_implicit_dependencies(struct drm_sched_job *job, int ret;
dma_resv_for_each_fence(&cursor, obj->resv, write, fence) {
ret = drm_sched_job_add_dependency(job, fence);if (ret)return ret;- /* Make sure to grab an additional ref on the added fence */ dma_fence_get(fence);
ret = drm_sched_job_add_dependency(job, fence);if (ret) {dma_fence_put(fence);return ret;}}
2.25.1
On Tue, Nov 16, 2021 at 8:37 AM Daniel Vetter daniel@ffwll.ch wrote:
On Tue, Nov 16, 2021 at 10:25:19AM +0100, Christian König wrote:
We need to grab another ref before trying to add the fence to the sched job and not after.
Signed-off-by: Christian König christian.koenig@amd.com
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
I wondered first why this goes boom, but then I realized that in some cases add_dependency() drops the reference of the passed-in fence.
Please also add the Fixes: line like in the previous patch.
oh, I sent https://patchwork.freedesktop.org/patch/463329/ before I saw this.. it already has the fixes tag, and IMO a better description, so I think you can just pick that one instead
BR, -R
Cheers, Daniel
drivers/gpu/drm/scheduler/sched_main.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c index 94fe51b3caa2..400d201c3c28 100644 --- a/drivers/gpu/drm/scheduler/sched_main.c +++ b/drivers/gpu/drm/scheduler/sched_main.c @@ -704,12 +704,14 @@ int drm_sched_job_add_implicit_dependencies(struct drm_sched_job *job, int ret;
dma_resv_for_each_fence(&cursor, obj->resv, write, fence) {
ret = drm_sched_job_add_dependency(job, fence);if (ret)return ret;/* Make sure to grab an additional ref on the added fence */ dma_fence_get(fence);
ret = drm_sched_job_add_dependency(job, fence);if (ret) {dma_fence_put(fence);return ret;} } return 0;}
2.25.1
-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Am 16.11.21 um 19:07 schrieb Rob Clark:
On Tue, Nov 16, 2021 at 8:37 AM Daniel Vetter daniel@ffwll.ch wrote:
On Tue, Nov 16, 2021 at 10:25:19AM +0100, Christian König wrote:
We need to grab another ref before trying to add the fence to the sched job and not after.
Signed-off-by: Christian König christian.koenig@amd.com
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
I wondered first why this goes boom, but then I realized that in some cases add_dependency() drops the reference of the passed-in fence.
Please also add the Fixes: line like in the previous patch.
oh, I sent https://patchwork.freedesktop.org/patch/463329/ before I saw this.. it already has the fixes tag, and IMO a better description, so I think you can just pick that one instead
Yeah, agree. You also have the missing Fixes line already.
Going to add Daniels rb to your patch as well since it is technically the same.
Thanks, Christian.
BR, -R
Cheers, Daniel
drivers/gpu/drm/scheduler/sched_main.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c index 94fe51b3caa2..400d201c3c28 100644 --- a/drivers/gpu/drm/scheduler/sched_main.c +++ b/drivers/gpu/drm/scheduler/sched_main.c @@ -704,12 +704,14 @@ int drm_sched_job_add_implicit_dependencies(struct drm_sched_job *job, int ret;
dma_resv_for_each_fence(&cursor, obj->resv, write, fence) {
ret = drm_sched_job_add_dependency(job, fence);if (ret)return ret;/* Make sure to grab an additional ref on the added fence */ dma_fence_get(fence);
ret = drm_sched_job_add_dependency(job, fence);if (ret) {dma_fence_put(fence);return ret;} } return 0;-- 2.25.1
-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
dri-devel@lists.freedesktop.org
-
Christian König -
Daniel Vetter -
Rob Clark