From cf655d615931b905bbb40f7b5387eb0efd1742e0 Mon Sep 17 00:00:00 2001 From: Jordan Crouse Date: Mon, 17 Aug 2020 15:01:36 -0700 Subject: drm/msm: Add a context pointer to the submitqueue Each submitqueue is attached to a context. Add a pointer to the context to the submitqueue at create time and refcount it so that it stays around through the life of the queue. Co-developed-by: Rob Clark Signed-off-by: Jordan Crouse Signed-off-by: Rob Clark Reviewed-by: Bjorn Andersson --- drivers/gpu/drm/msm/msm_gem.h | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/gpu/drm/msm/msm_gem.h') diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h index 972490b14ba5..9c573c4269cb 100644 --- a/drivers/gpu/drm/msm/msm_gem.h +++ b/drivers/gpu/drm/msm/msm_gem.h @@ -142,6 +142,7 @@ struct msm_gem_submit { bool valid; /* true if no cmdstream patching needed */ bool in_rb; /* "sudo" mode, copy cmds into RB */ struct msm_ringbuffer *ring; + struct msm_file_private *ctx; unsigned int nr_cmds; unsigned int nr_bos; u32 ident; /* A "identifier" for the submit for logging */ -- cgit v1.2.3 From 25faf2f2e06565d5cd0b97f77364fbe38f14ef71 Mon Sep 17 00:00:00 2001 From: Rob Clark Date: Mon, 17 Aug 2020 15:01:45 -0700 Subject: drm/msm: Show process names in gem_describe In $debugfs/gem we already show any vma(s) associated with an object. Also show process names if the vma's address space is a per-process address space. Signed-off-by: Rob Clark Reviewed-by: Jordan Crouse Reviewed-by: Bjorn Andersson --- drivers/gpu/drm/msm/msm_drv.c | 2 +- drivers/gpu/drm/msm/msm_gem.c | 25 +++++++++++++++++++++---- drivers/gpu/drm/msm/msm_gem.h | 5 +++++ drivers/gpu/drm/msm/msm_gem_vma.c | 1 + drivers/gpu/drm/msm/msm_gpu.c | 8 +++++--- drivers/gpu/drm/msm/msm_gpu.h | 2 +- 6 files changed, 34 insertions(+), 9 deletions(-) (limited to 'drivers/gpu/drm/msm/msm_gem.h') diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index e460c78e4d79..abf5799d9a22 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -589,7 +589,7 @@ static int context_init(struct drm_device *dev, struct drm_file *file) kref_init(&ctx->ref); msm_submitqueue_init(dev, ctx); - ctx->aspace = msm_gpu_create_private_address_space(priv->gpu); + ctx->aspace = msm_gpu_create_private_address_space(priv->gpu, current); file->driver_priv = ctx; return 0; diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c index 3cb7aeb93fd3..76a6c5271e57 100644 --- a/drivers/gpu/drm/msm/msm_gem.c +++ b/drivers/gpu/drm/msm/msm_gem.c @@ -842,11 +842,28 @@ void msm_gem_describe(struct drm_gem_object *obj, struct seq_file *m) seq_puts(m, " vmas:"); - list_for_each_entry(vma, &msm_obj->vmas, list) - seq_printf(m, " [%s: %08llx,%s,inuse=%d]", - vma->aspace != NULL ? vma->aspace->name : NULL, - vma->iova, vma->mapped ? "mapped" : "unmapped", + list_for_each_entry(vma, &msm_obj->vmas, list) { + const char *name, *comm; + if (vma->aspace) { + struct msm_gem_address_space *aspace = vma->aspace; + struct task_struct *task = + get_pid_task(aspace->pid, PIDTYPE_PID); + if (task) { + comm = kstrdup(task->comm, GFP_KERNEL); + } else { + comm = NULL; + } + name = aspace->name; + } else { + name = comm = NULL; + } + seq_printf(m, " [%s%s%s: aspace=%p, %08llx,%s,inuse=%d]", + name, comm ? ":" : "", comm ? comm : "", + vma->aspace, vma->iova, + vma->mapped ? "mapped" : "unmapped", vma->inuse); + kfree(comm); + } seq_puts(m, "\n"); } diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h index 9c573c4269cb..7b1c7a5f8eef 100644 --- a/drivers/gpu/drm/msm/msm_gem.h +++ b/drivers/gpu/drm/msm/msm_gem.h @@ -24,6 +24,11 @@ struct msm_gem_address_space { spinlock_t lock; /* Protects drm_mm node allocation/removal */ struct msm_mmu *mmu; struct kref kref; + + /* For address spaces associated with a specific process, this + * will be non-NULL: + */ + struct pid *pid; }; struct msm_gem_vma { diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c index 29cc1305cf37..80a8a266d68f 100644 --- a/drivers/gpu/drm/msm/msm_gem_vma.c +++ b/drivers/gpu/drm/msm/msm_gem_vma.c @@ -17,6 +17,7 @@ msm_gem_address_space_destroy(struct kref *kref) drm_mm_takedown(&aspace->mm); if (aspace->mmu) aspace->mmu->funcs->destroy(aspace->mmu); + put_pid(aspace->pid); kfree(aspace); } diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c index 9455c0b713e4..29c8d73c9a03 100644 --- a/drivers/gpu/drm/msm/msm_gpu.c +++ b/drivers/gpu/drm/msm/msm_gpu.c @@ -829,10 +829,9 @@ static int get_clocks(struct platform_device *pdev, struct msm_gpu *gpu) /* Return a new address space for a msm_drm_private instance */ struct msm_gem_address_space * -msm_gpu_create_private_address_space(struct msm_gpu *gpu) +msm_gpu_create_private_address_space(struct msm_gpu *gpu, struct task_struct *task) { struct msm_gem_address_space *aspace = NULL; - if (!gpu) return NULL; @@ -840,8 +839,11 @@ msm_gpu_create_private_address_space(struct msm_gpu *gpu) * If the target doesn't support private address spaces then return * the global one */ - if (gpu->funcs->create_private_address_space) + if (gpu->funcs->create_private_address_space) { aspace = gpu->funcs->create_private_address_space(gpu); + if (!IS_ERR(aspace)) + aspace->pid = get_pid(task_pid(task)); + } if (IS_ERR_OR_NULL(aspace)) aspace = msm_gem_address_space_get(gpu->aspace); diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h index 04a2f7539712..5ee358b480e6 100644 --- a/drivers/gpu/drm/msm/msm_gpu.h +++ b/drivers/gpu/drm/msm/msm_gpu.h @@ -301,7 +301,7 @@ int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev, const char *name, struct msm_gpu_config *config); struct msm_gem_address_space * -msm_gpu_create_private_address_space(struct msm_gpu *gpu); +msm_gpu_create_private_address_space(struct msm_gpu *gpu, struct task_struct *task); void msm_gpu_cleanup(struct msm_gpu *gpu); -- cgit v1.2.3 From 9d8baa2bf224f6f010644b2a60a3ca73d829663e Mon Sep 17 00:00:00 2001 From: Akhil P Oommen Date: Tue, 22 Sep 2020 20:25:26 +0530 Subject: drm/msm: Fix premature purging of BO In the case where we have a back-to-back submission that shares the same BO, this BO will be prematurely moved to inactive_list while retiring the first submit. But it will be still part of the second submit which is being processed by the GPU. Now, if the shrinker happens to be triggered at this point, it will result in a premature purging of this BO. To fix this, we need to refcount BO while doing submit and retire. Then, it should be moved to inactive list when this refcount becomes 0. Signed-off-by: Akhil P Oommen Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_drv.h | 5 ++--- drivers/gpu/drm/msm/msm_gem.c | 32 ++++++++++++++++---------------- drivers/gpu/drm/msm/msm_gem.h | 4 +++- drivers/gpu/drm/msm/msm_gpu.c | 11 +++++++---- 4 files changed, 28 insertions(+), 24 deletions(-) (limited to 'drivers/gpu/drm/msm/msm_gem.h') diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h index 319327462b29..28e3c8d628cb 100644 --- a/drivers/gpu/drm/msm/msm_drv.h +++ b/drivers/gpu/drm/msm/msm_drv.h @@ -309,9 +309,8 @@ void msm_gem_put_vaddr(struct drm_gem_object *obj); int msm_gem_madvise(struct drm_gem_object *obj, unsigned madv); int msm_gem_sync_object(struct drm_gem_object *obj, struct msm_fence_context *fctx, bool exclusive); -void msm_gem_move_to_active(struct drm_gem_object *obj, - struct msm_gpu *gpu, bool exclusive, struct dma_fence *fence); -void msm_gem_move_to_inactive(struct drm_gem_object *obj); +void msm_gem_active_get(struct drm_gem_object *obj, struct msm_gpu *gpu); +void msm_gem_active_put(struct drm_gem_object *obj); int msm_gem_cpu_prep(struct drm_gem_object *obj, uint32_t op, ktime_t *timeout); int msm_gem_cpu_fini(struct drm_gem_object *obj); void msm_gem_free_object(struct drm_gem_object *obj); diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c index 76a6c5271e57..14e14caf90f9 100644 --- a/drivers/gpu/drm/msm/msm_gem.c +++ b/drivers/gpu/drm/msm/msm_gem.c @@ -743,31 +743,31 @@ int msm_gem_sync_object(struct drm_gem_object *obj, return 0; } -void msm_gem_move_to_active(struct drm_gem_object *obj, - struct msm_gpu *gpu, bool exclusive, struct dma_fence *fence) +void msm_gem_active_get(struct drm_gem_object *obj, struct msm_gpu *gpu) { struct msm_gem_object *msm_obj = to_msm_bo(obj); + WARN_ON(!mutex_is_locked(&obj->dev->struct_mutex)); WARN_ON(msm_obj->madv != MSM_MADV_WILLNEED); - msm_obj->gpu = gpu; - if (exclusive) - dma_resv_add_excl_fence(obj->resv, fence); - else - dma_resv_add_shared_fence(obj->resv, fence); - list_del_init(&msm_obj->mm_list); - list_add_tail(&msm_obj->mm_list, &gpu->active_list); + + if (!atomic_fetch_inc(&msm_obj->active_count)) { + msm_obj->gpu = gpu; + list_del_init(&msm_obj->mm_list); + list_add_tail(&msm_obj->mm_list, &gpu->active_list); + } } -void msm_gem_move_to_inactive(struct drm_gem_object *obj) +void msm_gem_active_put(struct drm_gem_object *obj) { - struct drm_device *dev = obj->dev; - struct msm_drm_private *priv = dev->dev_private; struct msm_gem_object *msm_obj = to_msm_bo(obj); + struct msm_drm_private *priv = obj->dev->dev_private; - WARN_ON(!mutex_is_locked(&dev->struct_mutex)); + WARN_ON(!mutex_is_locked(&obj->dev->struct_mutex)); - msm_obj->gpu = NULL; - list_del_init(&msm_obj->mm_list); - list_add_tail(&msm_obj->mm_list, &priv->inactive_list); + if (!atomic_dec_return(&msm_obj->active_count)) { + msm_obj->gpu = NULL; + list_del_init(&msm_obj->mm_list); + list_add_tail(&msm_obj->mm_list, &priv->inactive_list); + } } int msm_gem_cpu_prep(struct drm_gem_object *obj, uint32_t op, ktime_t *timeout) diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h index 7b1c7a5f8eef..a1bf741b9b89 100644 --- a/drivers/gpu/drm/msm/msm_gem.h +++ b/drivers/gpu/drm/msm/msm_gem.h @@ -88,12 +88,14 @@ struct msm_gem_object { struct mutex lock; /* Protects resources associated with bo */ char name[32]; /* Identifier to print for the debugfs files */ + + atomic_t active_count; }; #define to_msm_bo(x) container_of(x, struct msm_gem_object, base) static inline bool is_active(struct msm_gem_object *msm_obj) { - return msm_obj->gpu != NULL; + return atomic_read(&msm_obj->active_count); } static inline bool is_purgeable(struct msm_gem_object *msm_obj) diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c index 29c8d73c9a03..55d16489d0f3 100644 --- a/drivers/gpu/drm/msm/msm_gpu.c +++ b/drivers/gpu/drm/msm/msm_gpu.c @@ -698,8 +698,8 @@ static void retire_submit(struct msm_gpu *gpu, struct msm_ringbuffer *ring, for (i = 0; i < submit->nr_bos; i++) { struct msm_gem_object *msm_obj = submit->bos[i].obj; - /* move to inactive: */ - msm_gem_move_to_inactive(&msm_obj->base); + + msm_gem_active_put(&msm_obj->base); msm_gem_unpin_iova(&msm_obj->base, submit->aspace); drm_gem_object_put_locked(&msm_obj->base); } @@ -774,6 +774,7 @@ void msm_gpu_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit) for (i = 0; i < submit->nr_bos; i++) { struct msm_gem_object *msm_obj = submit->bos[i].obj; + struct drm_gem_object *drm_obj = &msm_obj->base; uint64_t iova; /* can't happen yet.. but when we add 2d support we'll have @@ -786,9 +787,11 @@ void msm_gpu_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit) msm_gem_get_and_pin_iova(&msm_obj->base, submit->aspace, &iova); if (submit->bos[i].flags & MSM_SUBMIT_BO_WRITE) - msm_gem_move_to_active(&msm_obj->base, gpu, true, submit->fence); + dma_resv_add_excl_fence(drm_obj->resv, submit->fence); else if (submit->bos[i].flags & MSM_SUBMIT_BO_READ) - msm_gem_move_to_active(&msm_obj->base, gpu, false, submit->fence); + dma_resv_add_shared_fence(drm_obj->resv, submit->fence); + + msm_gem_active_get(drm_obj, gpu); } gpu->funcs->submit(gpu, submit); -- cgit v1.2.3