| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 5 Oct 2020 08:23:25 -0600
From: Jordan Crouse <jcrouse@...eaurora.org>
To: Rob Clark <robdclark@...il.com>
Cc: dri-devel@...ts.freedesktop.org,
Rob Clark <robdclark@...omium.org>,
"open list:DRM DRIVER FOR MSM ADRENO GPU"
<freedreno@...ts.freedesktop.org>, David Airlie <airlied@...ux.ie>,
"open list:DRM DRIVER FOR MSM ADRENO GPU"
<linux-arm-msm@...r.kernel.org>,
open list <linux-kernel@...r.kernel.org>,
Daniel Vetter <daniel@...ll.ch>, Sean Paul <sean@...rly.run>
Subject: Re: [Freedreno] [PATCH 06/14] drm/msm: Protect ring->submits with
it's own lock
On Sun, Oct 04, 2020 at 12:21:38PM -0700, Rob Clark wrote:
> From: Rob Clark <robdclark@...omium.org>
>
> One less place to rely on dev->struct_mutex.
>
Reviewed-by: Jordan Crouse <jcrouse@...eaurora.org>
> Signed-off-by: Rob Clark <robdclark@...omium.org>
> ---
> drivers/gpu/drm/msm/msm_gem_submit.c | 2 ++
> drivers/gpu/drm/msm/msm_gpu.c | 37 ++++++++++++++++++++++------
> drivers/gpu/drm/msm/msm_ringbuffer.c | 1 +
> drivers/gpu/drm/msm/msm_ringbuffer.h | 6 +++++
> 4 files changed, 39 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c
> index aa5c60a7132d..e1d1f005b3d4 100644
> --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> @@ -63,7 +63,9 @@ static struct msm_gem_submit *submit_create(struct drm_device *dev,
> void msm_gem_submit_free(struct msm_gem_submit *submit)
> {
> dma_fence_put(submit->fence);
> + spin_lock(&submit->ring->submit_lock);
> list_del(&submit->node);
> + spin_unlock(&submit->ring->submit_lock);
> put_pid(submit->pid);
> msm_submitqueue_put(submit->queue);
>
> diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
> index ca8c95b32c8b..8d1e254f964a 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.c
> +++ b/drivers/gpu/drm/msm/msm_gpu.c
> @@ -270,6 +270,7 @@ static void update_fences(struct msm_gpu *gpu, struct msm_ringbuffer *ring,
> {
> struct msm_gem_submit *submit;
>
> + spin_lock(&ring->submit_lock);
> list_for_each_entry(submit, &ring->submits, node) {
> if (submit->seqno > fence)
> break;
> @@ -277,6 +278,7 @@ static void update_fences(struct msm_gpu *gpu, struct msm_ringbuffer *ring,
> msm_update_fence(submit->ring->fctx,
> submit->fence->seqno);
> }
> + spin_unlock(&ring->submit_lock);
> }
>
> #ifdef CONFIG_DEV_COREDUMP
> @@ -430,11 +432,14 @@ find_submit(struct msm_ringbuffer *ring, uint32_t fence)
> {
> struct msm_gem_submit *submit;
>
> - WARN_ON(!mutex_is_locked(&ring->gpu->dev->struct_mutex));
> -
> - list_for_each_entry(submit, &ring->submits, node)
> - if (submit->seqno == fence)
> + spin_lock(&ring->submit_lock);
> + list_for_each_entry(submit, &ring->submits, node) {
> + if (submit->seqno == fence) {
> + spin_unlock(&ring->submit_lock);
> return submit;
> + }
> + }
> + spin_unlock(&ring->submit_lock);
>
> return NULL;
> }
> @@ -523,8 +528,10 @@ static void recover_worker(struct work_struct *work)
> for (i = 0; i < gpu->nr_rings; i++) {
> struct msm_ringbuffer *ring = gpu->rb[i];
>
> + spin_lock(&ring->submit_lock);
> list_for_each_entry(submit, &ring->submits, node)
> gpu->funcs->submit(gpu, submit);
> + spin_unlock(&ring->submit_lock);
> }
> }
>
> @@ -711,7 +718,6 @@ static void retire_submit(struct msm_gpu *gpu, struct msm_ringbuffer *ring,
> static void retire_submits(struct msm_gpu *gpu)
> {
> struct drm_device *dev = gpu->dev;
> - struct msm_gem_submit *submit, *tmp;
> int i;
>
> WARN_ON(!mutex_is_locked(&dev->struct_mutex));
> @@ -720,9 +726,24 @@ static void retire_submits(struct msm_gpu *gpu)
> for (i = 0; i < gpu->nr_rings; i++) {
> struct msm_ringbuffer *ring = gpu->rb[i];
>
> - list_for_each_entry_safe(submit, tmp, &ring->submits, node) {
> - if (dma_fence_is_signaled(submit->fence))
> + while (true) {
> + struct msm_gem_submit *submit = NULL;
> +
> + spin_lock(&ring->submit_lock);
> + submit = list_first_entry_or_null(&ring->submits,
> + struct msm_gem_submit, node);
> + spin_unlock(&ring->submit_lock);
> +
> + /*
> + * If no submit, we are done. If submit->fence hasn't
> + * been signalled, then later submits are not signalled
> + * either, so we are also done.
> + */
> + if (submit && dma_fence_is_signaled(submit->fence)) {
> retire_submit(gpu, ring, submit);
> + } else {
> + break;
> + }
> }
> }
> }
> @@ -765,7 +786,9 @@ void msm_gpu_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit)
>
> submit->seqno = ++ring->seqno;
>
> + spin_lock(&ring->submit_lock);
> list_add_tail(&submit->node, &ring->submits);
> + spin_unlock(&ring->submit_lock);
>
> msm_rd_dump_submit(priv->rd, submit, NULL);
>
> diff --git a/drivers/gpu/drm/msm/msm_ringbuffer.c b/drivers/gpu/drm/msm/msm_ringbuffer.c
> index 1b6958e908dc..4d2a2a4abef8 100644
> --- a/drivers/gpu/drm/msm/msm_ringbuffer.c
> +++ b/drivers/gpu/drm/msm/msm_ringbuffer.c
> @@ -46,6 +46,7 @@ struct msm_ringbuffer *msm_ringbuffer_new(struct msm_gpu *gpu, int id,
> ring->memptrs_iova = memptrs_iova;
>
> INIT_LIST_HEAD(&ring->submits);
> + spin_lock_init(&ring->submit_lock);
> spin_lock_init(&ring->preempt_lock);
>
> snprintf(name, sizeof(name), "gpu-ring-%d", ring->id);
> diff --git a/drivers/gpu/drm/msm/msm_ringbuffer.h b/drivers/gpu/drm/msm/msm_ringbuffer.h
> index 4956d1bc5d0e..fe55d4a1aa16 100644
> --- a/drivers/gpu/drm/msm/msm_ringbuffer.h
> +++ b/drivers/gpu/drm/msm/msm_ringbuffer.h
> @@ -39,7 +39,13 @@ struct msm_ringbuffer {
> int id;
> struct drm_gem_object *bo;
> uint32_t *start, *end, *cur, *next;
> +
> + /*
> + * List of in-flight submits on this ring. Protected by submit_lock.
> + */
> struct list_head submits;
> + spinlock_t submit_lock;
> +
> uint64_t iova;
> uint32_t seqno;
> uint32_t hangcheck_fence;
> --
> 2.26.2
>
> _______________________________________________
> Freedreno mailing list
> Freedreno@...ts.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
Powered by blists - more mailing lists