[PATCH] drm/msm/gem: Add some cleanup.h
From: Rob Clark
Date: Mon Oct 26 2026 - 17:38:57 EST
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.
Reported-by: Bart Van Assche <bvanassche@xxxxxxx>
Signed-off-by: Rob Clark <robin.clark@xxxxxxxxxxxxxxxx>
---
drivers/gpu/drm/msm/msm_drv.c | 38 ++++++++------------------
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 +-
5 files changed, 29 insertions(+), 62 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 5e8f3ce26b32..42381db0c468 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,31 +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 out;
+ return -ENOMEM;
}
msm_obj->metadata = new_metadata;
msm_obj->metadata_size = metadata_size;
memcpy(msm_obj->metadata, buf, metadata_size);
- msm_gem_unlock(obj);
-
-out:
- kfree(buf);
-
- return ret;
+ return 0;
}
static int msm_ioctl_gem_info_get_metadata(struct drm_gem_object *obj,
@@ -607,21 +600,14 @@ 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)) {
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