It looks like we still need to call dma_fence_put() on the man->move, otherwise we just end up leaking it, leading to fireworks later.
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5689 Fixes: 8bb31587820a ("drm/ttm: remove bo->moving") Cc: Christian König christian.koenig@amd.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Signed-off-by: Matthew Auld matthew.auld@intel.com --- drivers/gpu/drm/ttm/ttm_bo.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 015a94f766de..b15b77e10383 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -744,6 +744,8 @@ static int ttm_bo_add_move_fence(struct ttm_buffer_object *bo, dma_fence_put(fence); return ret; } + + dma_fence_put(fence); return 0; }
Am 13.04.22 um 10:21 schrieb Matthew Auld:
It looks like we still need to call dma_fence_put() on the man->move, otherwise we just end up leaking it, leading to fireworks later.
Closes: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.fre... Fixes: 8bb31587820a ("drm/ttm: remove bo->moving") Cc: Christian König christian.koenig@amd.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Signed-off-by: Matthew Auld matthew.auld@intel.com
Ah, yes! Previously we added the fence reference to bo->moving. I was already searching for that one as well.
Reviewed-by: Christian König christian.koenig@amd.com
Going to push it to drm-misc-next in a minute.
Thanks, Christian.
drivers/gpu/drm/ttm/ttm_bo.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 015a94f766de..b15b77e10383 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -744,6 +744,8 @@ static int ttm_bo_add_move_fence(struct ttm_buffer_object *bo, dma_fence_put(fence); return ret; }
- dma_fence_put(fence); return 0; }
On Wed, Apr 13, 2022 at 10:43:02AM +0200, Christian König wrote:
Am 13.04.22 um 10:21 schrieb Matthew Auld:
It looks like we still need to call dma_fence_put() on the man->move, otherwise we just end up leaking it, leading to fireworks later.
Closes: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.fre... Fixes: 8bb31587820a ("drm/ttm: remove bo->moving") Cc: Christian König christian.koenig@amd.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Signed-off-by: Matthew Auld matthew.auld@intel.com
Ah, yes! Previously we added the fence reference to bo->moving. I was already searching for that one as well.
Reviewed-by: Christian König christian.koenig@amd.com
Going to push it to drm-misc-next in a minute.
If you haven't pushed yet, pls also check my bikeshed, the code looks a bit funny with this patch applied :-) -Daniel
Thanks, Christian.
drivers/gpu/drm/ttm/ttm_bo.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 015a94f766de..b15b77e10383 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -744,6 +744,8 @@ static int ttm_bo_add_move_fence(struct ttm_buffer_object *bo, dma_fence_put(fence); return ret; }
- dma_fence_put(fence); return 0; }
On Wed, Apr 13, 2022 at 09:21:33AM +0100, Matthew Auld wrote:
It looks like we still need to call dma_fence_put() on the man->move, otherwise we just end up leaking it, leading to fireworks later.
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5689 Fixes: 8bb31587820a ("drm/ttm: remove bo->moving") Cc: Christian König christian.koenig@amd.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Signed-off-by: Matthew Auld matthew.auld@intel.com
drivers/gpu/drm/ttm/ttm_bo.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 015a94f766de..b15b77e10383 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -744,6 +744,8 @@ static int ttm_bo_add_move_fence(struct ttm_buffer_object *bo, dma_fence_put(fence); return ret; }
- dma_fence_put(fence);
Please delete the above if () and simplify the function tail to
dma_fence_put(fence); return ret;
With that Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
And thanks a lot for catching this, I guess I get a prize for most useless review ever :-/ Hopefully this one here is better. -Daniel
return 0; }
-- 2.34.1
Am 13.04.22 um 10:44 schrieb Daniel Vetter:
On Wed, Apr 13, 2022 at 09:21:33AM +0100, Matthew Auld wrote:
It looks like we still need to call dma_fence_put() on the man->move, otherwise we just end up leaking it, leading to fireworks later.
Closes: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.fre... Fixes: 8bb31587820a ("drm/ttm: remove bo->moving") Cc: Christian König christian.koenig@amd.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Signed-off-by: Matthew Auld matthew.auld@intel.com
drivers/gpu/drm/ttm/ttm_bo.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 015a94f766de..b15b77e10383 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -744,6 +744,8 @@ static int ttm_bo_add_move_fence(struct ttm_buffer_object *bo, dma_fence_put(fence); return ret; }
- dma_fence_put(fence);
Please delete the above if () and simplify the function tail to
dma_fence_put(fence); return ret;
With that Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
And thanks a lot for catching this, I guess I get a prize for most useless review ever :-/ Hopefully this one here is better.
Well so far we one inversion of min/max, one missing put and the incorrect handling of the return code in i915.
Considering how complex the patches have been I think we are still pretty good.
Christian.
-Daniel
return 0; }
-- 2.34.1
dri-devel@lists.freedesktop.org
-
Christian König -
Daniel Vetter -
Matthew Auld