drm/msm: Move fence put to where failure occurs
Subsystems (4)
PATCH RESEND v3 1/1 drm/msm: Move fence put to where failure occurs
If dma_fence_wait fails to wait for a supplied in-fence in msm_ioctl_gem_submit, make sure we release that in-fence.
Also remove this dma_fence_put() from the 'out' label.
--- drivers/gpu/drm/msm/msm_gem_submit.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)
drivers/gpu/drm/msm/msm_gem_submit.c +8 -7
@@ -411,7 +411,6 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,struct msm_file_private *ctx = file->driver_priv;struct msm_gem_submit *submit;struct msm_gpu *gpu = priv->gpu; -struct dma_fence *in_fence =NULL ;struct sync_file *sync_file =NULL ;struct msm_gpu_submitqueue *queue;struct msm_ringbuffer *ring;
@@ -444,6 +443,8 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data, ring = gpu->rb[queue->prio];if (args->flags &MSM_SUBMIT_FENCE_FD_IN ) { +struct dma_fence *in_fence; + in_fence =sync_file_get_fence (args->fence_fd);if (!in_fence)
@@ -453,11 +454,13 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,* Wait if the fence is from a foreign context, or if the fence * array contains any fence from a foreign context. */ -if (!dma_fence_match_context (in_fence, ring->fctx->context)) { + ret =0 ; +if (!dma_fence_match_context (in_fence, ring->fctx->context)) ret =dma_fence_wait (in_fence, true); -if (ret) -return ret; - } + +dma_fence_put (in_fence); +if (ret) +return ret; } ret =mutex_lock_interruptible (&dev->struct_mutex);
@@ -583,8 +586,6 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data, } out: -if (in_fence) -dma_fence_put (in_fence);submit_cleanup (submit);if (ret)msm_gem_submit_free (submit);
Signature · 1 line
2.17.1
Quoted 9 lines · Robert Foss
If
dma_fence_waitfails to wait for a supplied in-fence inmsm_ioctl_gem_submit, make sure we release that in-fence.Also remove this
dma_fence_put()from the 'out' label.Signed-off-by Robert Foss <[email protected]>Reviewed-by Chris Wilson <[email protected]>
Fyi, this is queued up in msm-next/fixes
BR,
-R
Quoted 55 lines
--- drivers/gpu/drm/msm/msm_gem_submit.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)drivers/gpu/drm/msm/msm_gem_submit.c +8
-8@@ -411,7 +411,6 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,struct msm_file_private *ctx = file->driver_priv;struct msm_gem_submit *submit;struct msm_gpu *gpu = priv->gpu; -struct dma_fence *in_fence =NULL ;struct sync_file *sync_file =NULL ;struct msm_gpu_submitqueue *queue;struct msm_ringbuffer *ring;@@ -444,6 +443,8 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data, ring = gpu->rb[queue->prio];if (args->flags &MSM_SUBMIT_FENCE_FD_IN ) { +struct dma_fence *in_fence; + in_fence =sync_file_get_fence (args->fence_fd);if (!in_fence)@@ -453,11 +454,13 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,* Wait if the fence is from a foreign context, or if the fence * array contains any fence from a foreign context. */ -if (!dma_fence_match_context (in_fence, ring->fctx->context)) { + ret =0 ; +if (!dma_fence_match_context (in_fence, ring->fctx->context)) ret =dma_fence_wait (in_fence, true); -if (ret) -return ret; - } + +dma_fence_put (in_fence); +if (ret) +return ret; } ret =mutex_lock_interruptible (&dev->struct_mutex);@@ -583,8 +586,6 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data, } out: -if (in_fence) -dma_fence_put (in_fence);submit_cleanup (submit);if (ret)msm_gem_submit_free (submit); --2.17.1
Quoted 5 lines · Rob Clark
Quoted 4 lines · Robert Foss
If
dma_fence_waitfails to wait for a supplied in-fence inmsm_ioctl_gem_submit, make sure we release that in-fence.
Quoted 7 lines
Quoted 5 lines
Also remove this
dma_fence_put()from the 'out' label.Signed-off-by Robert Foss <[email protected]>Reviewed-by Chris Wilson <[email protected]>Fyi, this is queued up in msm-next/fixes
Ah!
I had a look for it in drm=misc-next, but didn't find it. Thanks for the heads up!
Quoted 51 lines
BR,
-RQuoted 46 lines
--- drivers/gpu/drm/msm/msm_gem_submit.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)drivers/gpu/drm/msm/msm_gem_submit.c +8
-5@@ -411,7 +411,6 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,struct msm_file_private *ctx = file->driver_priv;struct msm_gem_submit *submit;struct msm_gpu *gpu = priv->gpu; -struct dma_fence *in_fence =NULL ;struct sync_file *sync_file =NULL ;struct msm_gpu_submitqueue *queue;struct msm_ringbuffer *ring;@@ -444,6 +443,8 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data, ring = gpu->rb[queue->prio];if (args->flags &MSM_SUBMIT_FENCE_FD_IN ) { +struct dma_fence *in_fence; + in_fence =sync_file_get_fence (args->fence_fd);if (!in_fence)@@ -453,11 +454,13 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,* Wait if the fence is from a foreign context, or if the fence * array contains any fence from a foreign context. */ -if (!dma_fence_match_context (in_fence, ring->fctx->context)) { + ret =0 ; +if (!dma_fence_match_context (in_fence, ring->fctx->context)) ret =dma_fence_wait (in_fence, true); -if (ret) -return ret; - } + +dma_fence_put (in_fence); +if (ret) +return ret; } ret =mutex_lock_interruptible (&dev->struct_mutex);@@ -583,8 +586,6 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data, }
Quoted 9 lines
Quoted 9 lines
drivers/gpu/drm/msm/msm_gem_submit.c +0
-3out: -if (in_fence) -dma_fence_put (in_fence);submit_cleanup (submit);if (ret)msm_gem_submit_free (submit); --2.17.1
_______________________________________________
Freedreno mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/freedreno
A message that is not in the archive (sent off-list or not yet received): [email protected]
Hey,
Quoted 7 lines · Sasha Levin
Hi,
[This is an automated email]
This commit has been processed because it contains a -stable tag. The stable tag indicates that it's relevant for the following trees: all
Quoted 8 lines
The bot has tested the following trees: v4.19.6, v4.14.85, v4.9.142, v4.4.166, v3.18.128,
v4.19.6: Build OK!
v4.14.85: Failed to apply! Possible dependencies:
cd414f3d9316("drm/msm: Move memptrs tomsm_gpu")eec874ce5ff1("drm/msm/adreno: load gpu at probe/bind time")f7de15450e90("drm/msm: Add per-instance submit queues")f97decac5f4c("drm/msm: Support multiple ringbuffers")
v4.14.85 can be rebased fairly cleanly:
https://gitlab.collabora.com/robertfoss/linux/commits/msm_in_fence_put_v3_v4.14.85
However v4.9.142 and earlier does not rebase very cleanly at all.
I don't know what the rule of thumb for backporting fixes is, but the feature that is fixed here does not seem to exist in v4.9.142 and earlier.
Rob.
Quoted 61 lines
v4.9.142: Failed to apply! Possible dependencies:
1cec20f0ea0e("dma-buf: Restartreservation_object_wait_timeout_rcu()after writes")78010cd9736e("dma-buf/fence: add anlockdep_assert_held()")f54d1867005c("dma-buf: Rename struct fence todma_fence")fedf54132d24("dma-buf: Restartreservation_object_get_fences_rcu()after writes")v4.4.166: Failed to apply! Possible dependencies:
0f477c6dea70("staging/android/sync: addsync_fence_create_dma")31954660a7a6("dma-buf/sync_file: improveKconfigdescription for Sync Files")375fb53ec1be("staging: android: replace explicit NULL comparison")395dec6f6bc5("Documentation: add doc forsync_file_get_fence()")62304fb1fc08("dma-buf/sync_file: de-stagesync_file")
696f74cf5439("staging/android: movesync_filefunctions comments tosync.c")699f68556943("android: unconditionally remove callbacks insync_fence_free()")73465f1c0825("staging/android/sync: Support sync points created from dma-fences")8a0044846115("staging/android: create a 'sync' dir for debugfs information")9b32381ca50f("staging/android: fix sync framework documentation")a44eb74cd413("staging/android: moveSW_SYNC_USERto a debugfs file")b55b54b5db33("staging/android: remove structsync_pt")
c01a958eca6e("drm/msm: reject submit ioctl if no gpu")
c784c82a3fd6("Documentation: add Sync File doc")
cd572182bd3d("Staging: android: Fix brace coding style warning insync_debug.c")d4cab38e153d("staging/android: preparesync_filefor de-staging")d7c3110c28de("staging/android: remove name arg fromsync_file_create()")d7fdb0ae9d11("staging/android: renamesync_fencetosync_file")f54d1867005c("dma-buf: Rename struct fence todma_fence")fac8434dab96("Documentation: Fix some grammar mistakes in sync_file.txt")v3.18.128: Failed to apply! Possible dependencies:
0541cdf585a8("Staging: android: fix line length coding style issue insync_debug.c")0f477c6dea70("staging/android/sync: addsync_fence_create_dma")31954660a7a6("dma-buf/sync_file: improveKconfigdescription for Sync Files")353fdf17060b("staging: android: Replace timeval with timespec64")375fb53ec1be("staging: android: replace explicit NULL comparison")395dec6f6bc5("Documentation: add doc forsync_file_get_fence()")62304fb1fc08("dma-buf/sync_file: de-stagesync_file")
699f68556943("android: unconditionally remove callbacks insync_fence_free()")73465f1c0825("staging/android/sync: Support sync points created from dma-fences")8a0044846115("staging/android: create a 'sync' dir for debugfs information")9b32381ca50f("staging/android: fix sync framework documentation")a44eb74cd413("staging/android: moveSW_SYNC_USERto a debugfs file")b8797affa142("staging: android: fix coding style checks insw_sync.c")c01a958eca6e("drm/msm: reject submit ioctl if no gpu")
c784c82a3fd6("Documentation: add Sync File doc")
cd572182bd3d("Staging: android: Fix brace coding style warning insync_debug.c")d30649a8b618("staging: android: Remove unnecessary externs")d4cab38e153d("staging/android: preparesync_filefor de-staging")d7c3110c28de("staging/android: remove name arg fromsync_file_create()")d7fdb0ae9d11("staging/android: renamesync_fencetosync_file")db1ad33bcfc4("drivers/staging: make androidsw_sync.cexplicitly non-modular")f54d1867005c("dma-buf: Rename struct fence todma_fence")fac8434dab96("Documentation: Fix some grammar mistakes in sync_file.txt")How should we proceed with this patch?
--
Thanks,
Sasha