Convert (most of) the msm_gem_lock() and friends over to scope based cleanup.
Other than fixing a lock leak in msm_ioctl_gem_info_set_metadata() there should be no functional change. Signed-off-by: Rob Clark <[email protected]> --- Rebased onto: https://patchwork.freedesktop.org/series/176884/ drivers/gpu/drm/msm/Makefile | 2 ++ drivers/gpu/drm/msm/msm_drv.c | 40 +++++++++------------------ drivers/gpu/drm/msm/msm_gem.c | 41 +++++++--------------------- drivers/gpu/drm/msm/msm_gem.h | 3 ++ drivers/gpu/drm/msm/msm_gem_submit.c | 6 ++-- drivers/gpu/drm/msm/msm_gpu.c | 3 +- 6 files changed, 32 insertions(+), 63 deletions(-) diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile index b5d64ee7eb74..042fd8fd3f57 100644 --- a/drivers/gpu/drm/msm/Makefile +++ b/drivers/gpu/drm/msm/Makefile @@ -254,3 +254,5 @@ $(addprefix $(obj)/,$(msm-y)): $(addprefix $(obj)/,$(ADRENO_HEADERS)) $(addprefix $(obj)/,$(msm-display-y)): $(addprefix $(obj)/,$(DISPLAY_HEADERS)) targets += $(ADRENO_HEADERS) $(DISPLAY_HEADERS) $(ADRENO_PERFCNTRS) + +CONTEXT_ANALYSIS_msm.o := y diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 7ab808322be6..4b2798efd9da 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -550,7 +550,6 @@ static int msm_ioctl_gem_info_set_metadata(struct drm_gem_object *obj, { struct msm_gem_object *msm_obj = to_msm_bo(obj); void *new_metadata; - void *buf; int ret; /* Impose a moderate upper bound on metadata size: */ @@ -559,32 +558,25 @@ static int msm_ioctl_gem_info_set_metadata(struct drm_gem_object *obj, } /* Use a temporary buf to keep copy_from_user() outside of gem obj lock: */ - buf = memdup_user(metadata, metadata_size); + void *buf __free(kfree) = memdup_user(metadata, metadata_size); if (IS_ERR(buf)) return PTR_ERR(buf); - ret = msm_gem_lock_interruptible(obj); - if (ret) - goto out; + ACQUIRE(gem_lock_intr, lock)(obj); + if ((ret = ACQUIRE_ERR(gem_lock_intr, &lock))) + return ret; new_metadata = krealloc(msm_obj->metadata, metadata_size, GFP_KERNEL); if (!new_metadata) { - ret = -ENOMEM; - goto unlock; + return -ENOMEM; } msm_obj->metadata = new_metadata; msm_obj->metadata_size = metadata_size; memcpy(msm_obj->metadata, buf, metadata_size); -unlock: - msm_gem_unlock(obj); - -out: - kfree(buf); - - return ret; + return 0; } static int msm_ioctl_gem_info_get_metadata(struct drm_gem_object *obj, @@ -608,26 +600,20 @@ static int msm_ioctl_gem_info_get_metadata(struct drm_gem_object *obj, return 0; } - ret = msm_gem_lock_interruptible(obj); - if (ret) - return ret; - - /* Avoid copy_to_user() under gem obj lock: */ - len = msm_obj->metadata_size; - buf = kmemdup(msm_obj->metadata, len, GFP_KERNEL); - - if (!buf) { - msm_gem_unlock(obj); - return -ENOMEM; + scoped_cond_guard (gem_lock_intr, return -EINTR, obj) { + /* Avoid copy_to_user() under gem obj lock by making a copy: */ + len = msm_obj->metadata_size; + buf = kmemdup(msm_obj->metadata, len, GFP_KERNEL); + if (!buf) + return -ENOMEM; } - msm_gem_unlock(obj); - if (*metadata_size < len) { ret = -ETOOSMALL; } else if (copy_to_user(metadata, buf, len)) { ret = -EFAULT; } else { + ret = 0; *metadata_size = len; } diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c index 72e3cd8bb1e7..9abcc7beb268 100644 --- a/drivers/gpu/drm/msm/msm_gem.c +++ b/drivers/gpu/drm/msm/msm_gem.c @@ -376,8 +376,6 @@ static vm_fault_t msm_gem_fault(struct vm_fault *vmf) struct page **pages; unsigned long pfn; pgoff_t pgoff; - int err; - vm_fault_t ret; if (drm_WARN_ON_ONCE(obj->dev, drm_gem_is_imported(obj))) return VM_FAULT_SIGBUS; @@ -386,22 +384,19 @@ static vm_fault_t msm_gem_fault(struct vm_fault *vmf) * vm_ops.open/drm_gem_mmap_obj and close get and put * a reference on obj. So, we dont need to hold one here. */ - err = msm_gem_lock_interruptible(obj); - if (err) { - ret = VM_FAULT_NOPAGE; - goto out; + ACQUIRE(gem_lock_intr, lock)(obj); + if (ACQUIRE_ERR(gem_lock_intr, &lock)) { + return VM_FAULT_NOPAGE; } if (GEM_WARN_ON(msm_obj->madv != MSM_MADV_WILLNEED)) { - msm_gem_unlock(obj); return VM_FAULT_SIGBUS; } /* make sure we have pages attached now */ pages = get_pages(obj); if (IS_ERR(pages)) { - ret = vmf_error(PTR_ERR(pages)); - goto out_unlock; + return vmf_error(PTR_ERR(pages)); } /* We don't use vmf->pgoff since that has the fake offset: */ @@ -412,12 +407,7 @@ static vm_fault_t msm_gem_fault(struct vm_fault *vmf) VERB("Inserting %p pfn %lx, pa %lx", (void *)vmf->address, pfn, pfn << PAGE_SHIFT); - ret = vmf_insert_pfn(vma, vmf->address, pfn); - -out_unlock: - msm_gem_unlock(obj); -out: - return ret; + return vmf_insert_pfn(vma, vmf->address, pfn); } @@ -803,13 +793,8 @@ void *msm_gem_get_vaddr_locked(struct drm_gem_object *obj) void *msm_gem_get_vaddr(struct drm_gem_object *obj) { - void *ret; - - msm_gem_lock(obj); - ret = msm_gem_get_vaddr_locked(obj); - msm_gem_unlock(obj); - - return ret; + guard(gem_lock)(obj); + return msm_gem_get_vaddr_locked(obj); } /* @@ -836,9 +821,8 @@ void msm_gem_put_vaddr_locked(struct drm_gem_object *obj) void msm_gem_put_vaddr(struct drm_gem_object *obj) { - msm_gem_lock(obj); + guard(gem_lock)(obj); msm_gem_put_vaddr_locked(obj); - msm_gem_unlock(obj); } /* Update madvise status, returns true if not purged, else @@ -849,9 +833,8 @@ int msm_gem_madvise(struct drm_gem_object *obj, unsigned madv) struct drm_device *dev = obj->dev; struct msm_gem_object *msm_obj = to_msm_bo(obj); - msm_gem_lock(obj); - - mutex_lock(&dev->gem_lru_mutex); + guard(gem_lock)(obj); + guard(mutex)(&dev->gem_lru_mutex); if (msm_obj->madv != __MSM_MADV_PURGED) msm_obj->madv = madv; @@ -863,10 +846,6 @@ int msm_gem_madvise(struct drm_gem_object *obj, unsigned madv) */ update_lru_locked(obj); - mutex_unlock(&dev->gem_lru_mutex); - - msm_gem_unlock(obj); - return (madv != __MSM_MADV_PURGED); } diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h index d8ec696d89d7..bd7968232d73 100644 --- a/drivers/gpu/drm/msm/msm_gem.h +++ b/drivers/gpu/drm/msm/msm_gem.h @@ -347,6 +347,9 @@ msm_gem_unlock(struct drm_gem_object *obj) dma_resv_unlock(obj->resv); } +DEFINE_LOCK_GUARD_1(gem_lock, struct drm_gem_object, msm_gem_lock(_T->lock), msm_gem_unlock(_T->lock)); +DEFINE_LOCK_GUARD_1_COND(gem_lock, _intr, msm_gem_lock_interruptible(_T->lock), _RET == 0); + /** * msm_gem_lock_vm_and_obj() - Helper to lock an obj + VM * @exec: the exec context helper which will be initalized diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c index 5862db05297a..ff6cf39587c9 100644 --- a/drivers/gpu/drm/msm/msm_gem_submit.c +++ b/drivers/gpu/drm/msm/msm_gem_submit.c @@ -536,9 +536,9 @@ void msm_submit_retire(struct msm_gem_submit *submit) struct drm_gem_object *obj = submit->bos[i].obj; struct drm_gpuvm_bo *vm_bo = submit->bos[i].vm_bo; - msm_gem_lock(obj); - drm_gpuvm_bo_put(vm_bo); - msm_gem_unlock(obj); + scoped_guard (gem_lock, obj) + drm_gpuvm_bo_put(vm_bo); + drm_gem_object_put(obj); } } diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c index 0c2c35636251..fb07803f5aba 100644 --- a/drivers/gpu/drm/msm/msm_gpu.c +++ b/drivers/gpu/drm/msm/msm_gpu.c @@ -308,10 +308,9 @@ static void crashstate_get_bos(struct msm_gpu_state *state, struct msm_gem_submi struct drm_gem_object *obj = submit->bos[i].obj; bool dump = rd_full || (submit->bos[i].flags & MSM_SUBMIT_BO_DUMP); - msm_gem_lock(obj); + guard(gem_lock)(obj); msm_gpu_crashstate_get_bo(state, obj, submit->bos[i].iova, dump, 0, obj->size); - msm_gem_unlock(obj); } } } -- 2.56.0
