drm/msm: Separate locking of buffer resources from struct_mutex

Buffer object specific resources like pages, domains, sg list
need not be protected with struct_mutex. They can be protected
with a buffer object level lock. This simplifies locking and
makes it easier to avoid potential recursive locking scenarios
for SVM involving mmap_sem and struct_mutex. This also removes
unnecessary serialization when creating buffer objects, and also
between buffer object creation and GPU command submission.

Signed-off-by: Sushmita Susheelendra <[e-mail]>
[robclark: squash in handling new locking for shrinker]
Signed-off-by: Rob Clark <[e-mail]>

drivers/gpu/drm/msm/adreno/a5xx_gpu.c

···
 	struct drm_gem_object *bo;
 	void *ptr;
 
-	bo = msm_gem_new(drm, fw->size - 4, MSM_BO_UNCACHED);
+	bo = msm_gem_new_locked(drm, fw->size - 4, MSM_BO_UNCACHED);
 	if (IS_ERR(bo))
 		return bo;
 
-	ptr = msm_gem_get_vaddr_locked(bo);
+	ptr = msm_gem_get_vaddr(bo);
 	if (!ptr) {
 		drm_gem_object_unreference(bo);
 		return ERR_PTR(-ENOMEM);
 	}
 
 	if (iova) {
-		int ret = msm_gem_get_iova_locked(bo, gpu->aspace, iova);
+		int ret = msm_gem_get_iova(bo, gpu->aspace, iova);
 
 		if (ret) {
 			drm_gem_object_unreference(bo);
···
 
 	memcpy(ptr, &fw->data[4], fw->size - 4);
 
-	msm_gem_put_vaddr_locked(bo);
+	msm_gem_put_vaddr(bo);
 	return bo;
 }
 

drivers/gpu/drm/msm/adreno/a5xx_power.c

···
 	 */
 	bosize = (cmds_size + (cmds_size / TYPE4_MAX_PAYLOAD) + 1) << 2;
 
-	a5xx_gpu->gpmu_bo = msm_gem_new(drm, bosize, MSM_BO_UNCACHED);
+	a5xx_gpu->gpmu_bo = msm_gem_new_locked(drm, bosize, MSM_BO_UNCACHED);
 	if (IS_ERR(a5xx_gpu->gpmu_bo))
 		goto err;
 
-	if (msm_gem_get_iova_locked(a5xx_gpu->gpmu_bo, gpu->aspace,
+	if (msm_gem_get_iova(a5xx_gpu->gpmu_bo, gpu->aspace,
 			&a5xx_gpu->gpmu_iova))
 		goto err;
 
-	ptr = msm_gem_get_vaddr_locked(a5xx_gpu->gpmu_bo);
+	ptr = msm_gem_get_vaddr(a5xx_gpu->gpmu_bo);
 	if (!ptr)
 		goto err;
 
···
 		cmds_size -= _size;
 	}
 
-	msm_gem_put_vaddr_locked(a5xx_gpu->gpmu_bo);
+	msm_gem_put_vaddr(a5xx_gpu->gpmu_bo);
 	a5xx_gpu->gpmu_dwords = dwords;
 
 	goto out;

drivers/gpu/drm/msm/adreno/adreno_gpu.c

···
 
 	DBG("%s", gpu->name);
 
-	ret = msm_gem_get_iova_locked(gpu->rb->bo, gpu->aspace, &gpu->rb_iova);
+	ret = msm_gem_get_iova(gpu->rb->bo, gpu->aspace, &gpu->rb_iova);
 	if (ret) {
 		gpu->rb_iova = 0;
 		dev_err(gpu->dev->dev, "could not map ringbuffer: %d\n", ret);
···
 			return ret;
 	}
 
-	mutex_lock(&drm->struct_mutex);
 	adreno_gpu->memptrs_bo = msm_gem_new(drm, sizeof(*adreno_gpu->memptrs),
 			MSM_BO_UNCACHED);
-	mutex_unlock(&drm->struct_mutex);
 	if (IS_ERR(adreno_gpu->memptrs_bo)) {
 		ret = PTR_ERR(adreno_gpu->memptrs_bo);
 		adreno_gpu->memptrs_bo = NULL;

drivers/gpu/drm/msm/dsi/dsi_host.c

···
 	uint64_t iova;
 
 	if (cfg_hnd->major == MSM_DSI_VER_MAJOR_6G) {
-		mutex_lock(&dev->struct_mutex);
 		msm_host->tx_gem_obj = msm_gem_new(dev, size, MSM_BO_UNCACHED);
 		if (IS_ERR(msm_host->tx_gem_obj)) {
 			ret = PTR_ERR(msm_host->tx_gem_obj);
 			pr_err("%s: failed to allocate gem, %d\n",
 				__func__, ret);
 			msm_host->tx_gem_obj = NULL;
-			mutex_unlock(&dev->struct_mutex);
 			return ret;
 		}
 
-		ret = msm_gem_get_iova_locked(msm_host->tx_gem_obj,
+		ret = msm_gem_get_iova(msm_host->tx_gem_obj,
 				priv->kms->aspace, &iova);
 		mutex_unlock(&dev->struct_mutex);
 		if (ret) {

drivers/gpu/drm/msm/mdp/mdp4/mdp4_crtc.c

···
 		if (next_bo) {
 			/* take a obj ref + iova ref when we start scanning out: */
 			drm_gem_object_reference(next_bo);
-			msm_gem_get_iova_locked(next_bo, kms->aspace, &iova);
+			msm_gem_get_iova(next_bo, kms->aspace, &iova);
 
 			/* enable cursor: */
 			mdp4_write(mdp4_kms, REG_MDP4_DMA_CURSOR_SIZE(dma),

drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c

···
 		goto fail;
 	}
 
-	mutex_lock(&dev->struct_mutex);
 	mdp4_kms->blank_cursor_bo = msm_gem_new(dev, SZ_16K, MSM_BO_WC);
-	mutex_unlock(&dev->struct_mutex);
 	if (IS_ERR(mdp4_kms->blank_cursor_bo)) {
 		ret = PTR_ERR(mdp4_kms->blank_cursor_bo);
 		dev_err(dev->dev, "could not allocate blank-cursor bo: %d\n", ret);

drivers/gpu/drm/msm/msm_drv.c

···
 		priv->vram.size = size;
 
 		drm_mm_init(&priv->vram.mm, 0, (size >> PAGE_SHIFT) - 1);
+		spin_lock_init(&priv->vram.lock);
 
 		attrs |= DMA_ATTR_NO_KERNEL_MAPPING;
 		attrs |= DMA_ATTR_WRITE_COMBINE;

drivers/gpu/drm/msm/msm_drv.h

···
 		 * and position mm_node->start is in # of pages:
 		 */
 		struct drm_mm mm;
+		spinlock_t lock; /* Protects drm_mm node allocation/removal */
 	} vram;
 
 	struct notifier_block vmap_notifier;
···
 int msm_gem_mmap(struct file *filp, struct vm_area_struct *vma);
 int msm_gem_fault(struct vm_fault *vmf);
 uint64_t msm_gem_mmap_offset(struct drm_gem_object *obj);
-int msm_gem_get_iova_locked(struct drm_gem_object *obj,
-		struct msm_gem_address_space *aspace, uint64_t *iova);
 int msm_gem_get_iova(struct drm_gem_object *obj,
 		struct msm_gem_address_space *aspace, uint64_t *iova);
 uint64_t msm_gem_iova(struct drm_gem_object *obj,
···
 		struct dma_buf_attachment *attach, struct sg_table *sg);
 int msm_gem_prime_pin(struct drm_gem_object *obj);
 void msm_gem_prime_unpin(struct drm_gem_object *obj);
-void *msm_gem_get_vaddr_locked(struct drm_gem_object *obj);
 void *msm_gem_get_vaddr(struct drm_gem_object *obj);
-void msm_gem_put_vaddr_locked(struct drm_gem_object *obj);
 void msm_gem_put_vaddr(struct drm_gem_object *obj);
 int msm_gem_madvise(struct drm_gem_object *obj, unsigned madv);
-void msm_gem_purge(struct drm_gem_object *obj);
-void msm_gem_vunmap(struct drm_gem_object *obj);
 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,
···
 int msm_gem_new_handle(struct drm_device *dev, struct drm_file *file,
 		uint32_t size, uint32_t flags, uint32_t *handle);
 struct drm_gem_object *msm_gem_new(struct drm_device *dev,
+		uint32_t size, uint32_t flags);
+struct drm_gem_object *msm_gem_new_locked(struct drm_device *dev,
 		uint32_t size, uint32_t flags);
 struct drm_gem_object *msm_gem_import(struct drm_device *dev,
 		struct dma_buf *dmabuf, struct sg_table *sgt);

drivers/gpu/drm/msm/msm_fbdev.c

···
 	/* allocate backing bo */
 	size = mode_cmd.pitches[0] * mode_cmd.height;
 	DBG("allocating %d bytes for fb %d", size, dev->primary->index);
-	mutex_lock(&dev->struct_mutex);
 	fbdev->bo = msm_gem_new(dev, size, MSM_BO_SCANOUT |
 			MSM_BO_WC | MSM_BO_STOLEN);
-	mutex_unlock(&dev->struct_mutex);
 	if (IS_ERR(fbdev->bo)) {
 		ret = PTR_ERR(fbdev->bo);
 		fbdev->bo = NULL;
···
 	 * in panic (ie. lock-safe, etc) we could avoid pinning the
 	 * buffer now:
 	 */
-	ret = msm_gem_get_iova_locked(fbdev->bo, priv->kms->aspace, &paddr);
+	ret = msm_gem_get_iova(fbdev->bo, priv->kms->aspace, &paddr);
 	if (ret) {
 		dev_err(dev->dev, "failed to get buffer obj iova: %d\n", ret);
 		goto fail_unlock;
···
 
 	dev->mode_config.fb_base = paddr;
 
-	fbi->screen_base = msm_gem_get_vaddr_locked(fbdev->bo);
+	fbi->screen_base = msm_gem_get_vaddr(fbdev->bo);
 	if (IS_ERR(fbi->screen_base)) {
 		ret = PTR_ERR(fbi->screen_base);
 		goto fail_unlock;

drivers/gpu/drm/msm/msm_gem.c

···
 #include "msm_gpu.h"
 #include "msm_mmu.h"
 
+static void msm_gem_vunmap_locked(struct drm_gem_object *obj);
+
+
 static dma_addr_t physaddr(struct drm_gem_object *obj)
 {
 	struct msm_gem_object *msm_obj = to_msm_bo(obj);
···
 }
 
 /* allocate pages from VRAM carveout, used when no IOMMU: */
-static struct page **get_pages_vram(struct drm_gem_object *obj,
-		int npages)
+static struct page **get_pages_vram(struct drm_gem_object *obj, int npages)
 {
 	struct msm_gem_object *msm_obj = to_msm_bo(obj);
 	struct msm_drm_private *priv = obj->dev->dev_private;
···
 	if (!p)
 		return ERR_PTR(-ENOMEM);
 
+	spin_lock(&priv->vram.lock);
 	ret = drm_mm_insert_node(&priv->vram.mm, msm_obj->vram_node, npages);
+	spin_unlock(&priv->vram.lock);
 	if (ret) {
 		kvfree(p);
 		return ERR_PTR(ret);
···
 	return p;
 }
 
-/* called with dev->struct_mutex held */
 static struct page **get_pages(struct drm_gem_object *obj)
 {
 	struct msm_gem_object *msm_obj = to_msm_bo(obj);
···
 	return msm_obj->pages;
 }
 
+static void put_pages_vram(struct drm_gem_object *obj)
+{
+	struct msm_gem_object *msm_obj = to_msm_bo(obj);
+	struct msm_drm_private *priv = obj->dev->dev_private;
+
+	spin_lock(&priv->vram.lock);
+	drm_mm_remove_node(msm_obj->vram_node);
+	spin_unlock(&priv->vram.lock);
+
+	kvfree(msm_obj->pages);
+}
+
 static void put_pages(struct drm_gem_object *obj)
 {
 	struct msm_gem_object *msm_obj = to_msm_bo(obj);
···
 
 		if (use_pages(obj))
 			drm_gem_put_pages(obj, msm_obj->pages, true, false);
-		else {
-			drm_mm_remove_node(msm_obj->vram_node);
-			kvfree(msm_obj->pages);
-		}
+		else
+			put_pages_vram(obj);
 
 		msm_obj->pages = NULL;
 	}
···
 
 struct page **msm_gem_get_pages(struct drm_gem_object *obj)
 {
-	struct drm_device *dev = obj->dev;
+	struct msm_gem_object *msm_obj = to_msm_bo(obj);
 	struct page **p;
-	mutex_lock(&dev->struct_mutex);
+
+	mutex_lock(&msm_obj->lock);
+
+	if (WARN_ON(msm_obj->madv != MSM_MADV_WILLNEED)) {
+		mutex_unlock(&msm_obj->lock);
+		return ERR_PTR(-EBUSY);
+	}
+
 	p = get_pages(obj);
-	mutex_unlock(&dev->struct_mutex);
+	mutex_unlock(&msm_obj->lock);
 	return p;
 }
 
···
 {
 	struct vm_area_struct *vma = vmf->vma;
 	struct drm_gem_object *obj = vma->vm_private_data;
-	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 page **pages;
 	unsigned long pfn;
 	pgoff_t pgoff;
 	int ret;
 
-	/* This should only happen if userspace tries to pass a mmap'd
-	 * but unfaulted gem bo vaddr into submit ioctl, triggering
-	 * a page fault while struct_mutex is already held.  This is
-	 * not a valid use-case so just bail.
+	/*
+	 * vm_ops.open/drm_gem_mmap_obj and close get and put
+	 * a reference on obj. So, we dont need to hold one here.
 	 */
-	if (priv->struct_mutex_task == current)
-		return VM_FAULT_SIGBUS;
-
-	/* Make sure we don't parallel update on a fault, nor move or remove
-	 * something from beneath our feet
-	 */
-	ret = mutex_lock_interruptible(&dev->struct_mutex);
+	ret = mutex_lock_interruptible(&msm_obj->lock);
 	if (ret)
 		goto out;
+
+	if (WARN_ON(msm_obj->madv != MSM_MADV_WILLNEED)) {
+		mutex_unlock(&msm_obj->lock);
+		return VM_FAULT_SIGBUS;
+	}
 
 	/* make sure we have pages attached now */
 	pages = get_pages(obj);
···
 	ret = vm_insert_mixed(vma, vmf->address, __pfn_to_pfn_t(pfn, PFN_DEV));
 
 out_unlock:
-	mutex_unlock(&dev->struct_mutex);
+	mutex_unlock(&msm_obj->lock);
 out:
 	switch (ret) {
 	case -EAGAIN:
···
 static uint64_t mmap_offset(struct drm_gem_object *obj)
 {
 	struct drm_device *dev = obj->dev;
+	struct msm_gem_object *msm_obj = to_msm_bo(obj);
 	int ret;
 
-	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
+	WARN_ON(!mutex_is_locked(&msm_obj->lock));
 
 	/* Make it mmapable */
 	ret = drm_gem_create_mmap_offset(obj);
···
 uint64_t msm_gem_mmap_offset(struct drm_gem_object *obj)
 {
 	uint64_t offset;
-	mutex_lock(&obj->dev->struct_mutex);
+	struct msm_gem_object *msm_obj = to_msm_bo(obj);
+
+	mutex_lock(&msm_obj->lock);
 	offset = mmap_offset(obj);
-	mutex_unlock(&obj->dev->struct_mutex);
+	mutex_unlock(&msm_obj->lock);
 	return offset;
 }
 
···
 {
 	struct msm_gem_object *msm_obj = to_msm_bo(obj);
 	struct msm_gem_vma *vma;
+
+	WARN_ON(!mutex_is_locked(&msm_obj->lock));
 
 	vma = kzalloc(sizeof(*vma), GFP_KERNEL);
 	if (!vma)
···
 	struct msm_gem_object *msm_obj = to_msm_bo(obj);
 	struct msm_gem_vma *vma;
 
-	WARN_ON(!mutex_is_locked(&obj->dev->struct_mutex));
+	WARN_ON(!mutex_is_locked(&msm_obj->lock));
 
 	list_for_each_entry(vma, &msm_obj->vmas, list) {
 		if (vma->aspace == aspace)
···
 	kfree(vma);
 }
 
+/* Called with msm_obj->lock locked */
 static void
 put_iova(struct drm_gem_object *obj)
 {
 	struct msm_gem_object *msm_obj = to_msm_bo(obj);
 	struct msm_gem_vma *vma, *tmp;
 
-	WARN_ON(!mutex_is_locked(&obj->dev->struct_mutex));
+	WARN_ON(!mutex_is_locked(&msm_obj->lock));
 
 	list_for_each_entry_safe(vma, tmp, &msm_obj->vmas, list) {
 		msm_gem_unmap_vma(vma->aspace, vma, msm_obj->sgt);
···
 	}
 }
 
-/* should be called under struct_mutex.. although it can be called
- * from atomic context without struct_mutex to acquire an extra
- * iova ref if you know one is already held.
- *
- * That means when I do eventually need to add support for unpinning
- * the refcnt counter needs to be atomic_t.
- */
-int msm_gem_get_iova_locked(struct drm_gem_object *obj,
+/* get iova, taking a reference.  Should have a matching put */
+int msm_gem_get_iova(struct drm_gem_object *obj,
 		struct msm_gem_address_space *aspace, uint64_t *iova)
 {
 	struct msm_gem_object *msm_obj = to_msm_bo(obj);
 	struct msm_gem_vma *vma;
 	int ret = 0;
 
-	WARN_ON(!mutex_is_locked(&obj->dev->struct_mutex));
+	mutex_lock(&msm_obj->lock);
+
+	if (WARN_ON(msm_obj->madv != MSM_MADV_WILLNEED)) {
+		mutex_unlock(&msm_obj->lock);
+		return -EBUSY;
+	}
 
 	vma = lookup_vma(obj, aspace);
 
···
 	}
 
 	*iova = vma->iova;
+
+	mutex_unlock(&msm_obj->lock);
 	return 0;
 
 fail:
 	del_vma(vma);
 
-	return ret;
-}
-
-/* get iova, taking a reference.  Should have a matching put */
-int msm_gem_get_iova(struct drm_gem_object *obj,
-		struct msm_gem_address_space *aspace, uint64_t *iova)
-{
-	int ret;
-
-	mutex_lock(&obj->dev->struct_mutex);
-	ret = msm_gem_get_iova_locked(obj, aspace, iova);
-	mutex_unlock(&obj->dev->struct_mutex);
-
+	mutex_unlock(&msm_obj->lock);
 	return ret;
 }
 
···
 uint64_t msm_gem_iova(struct drm_gem_object *obj,
 		struct msm_gem_address_space *aspace)
 {
+	struct msm_gem_object *msm_obj = to_msm_bo(obj);
 	struct msm_gem_vma *vma;
 
-	mutex_lock(&obj->dev->struct_mutex);
+	mutex_lock(&msm_obj->lock);
 	vma = lookup_vma(obj, aspace);
-	mutex_unlock(&obj->dev->struct_mutex);
+	mutex_unlock(&msm_obj->lock);
 	WARN_ON(!vma);
 
 	return vma ? vma->iova : 0;
···
 	return ret;
 }
 
-void *msm_gem_get_vaddr_locked(struct drm_gem_object *obj)
-{
-	struct msm_gem_object *msm_obj = to_msm_bo(obj);
-	WARN_ON(!mutex_is_locked(&obj->dev->struct_mutex));
-	if (!msm_obj->vaddr) {
-		struct page **pages = get_pages(obj);
-		if (IS_ERR(pages))
-			return ERR_CAST(pages);
-		msm_obj->vaddr = vmap(pages, obj->size >> PAGE_SHIFT,
-				VM_MAP, pgprot_writecombine(PAGE_KERNEL));
-		if (msm_obj->vaddr == NULL)
-			return ERR_PTR(-ENOMEM);
-	}
-	msm_obj->vmap_count++;
-	return msm_obj->vaddr;
-}
-
 void *msm_gem_get_vaddr(struct drm_gem_object *obj)
 {
-	void *ret;
-	mutex_lock(&obj->dev->struct_mutex);
-	ret = msm_gem_get_vaddr_locked(obj);
-	mutex_unlock(&obj->dev->struct_mutex);
-	return ret;
-}
-
-void msm_gem_put_vaddr_locked(struct drm_gem_object *obj)
-{
 	struct msm_gem_object *msm_obj = to_msm_bo(obj);
-	WARN_ON(!mutex_is_locked(&obj->dev->struct_mutex));
-	WARN_ON(msm_obj->vmap_count < 1);
+	int ret = 0;
+
+	mutex_lock(&msm_obj->lock);
+
+	if (WARN_ON(msm_obj->madv != MSM_MADV_WILLNEED)) {
+		mutex_unlock(&msm_obj->lock);
+		return ERR_PTR(-EBUSY);
+	}
+
+	/* increment vmap_count *before* vmap() call, so shrinker can
+	 * check vmap_count (is_vunmapable()) outside of msm_obj->lock.
+	 * This guarantees that we won't try to msm_gem_vunmap() this
+	 * same object from within the vmap() call (while we already
+	 * hold msm_obj->lock)
+	 */
+	msm_obj->vmap_count++;
+
+	if (!msm_obj->vaddr) {
+		struct page **pages = get_pages(obj);
+		if (IS_ERR(pages)) {
+			ret = PTR_ERR(pages);
+			goto fail;
+		}
+		msm_obj->vaddr = vmap(pages, obj->size >> PAGE_SHIFT,
+				VM_MAP, pgprot_writecombine(PAGE_KERNEL));
+		if (msm_obj->vaddr == NULL) {
+			ret = -ENOMEM;
+			goto fail;
+		}
+	}
+
+	mutex_unlock(&msm_obj->lock);
+	return msm_obj->vaddr;
+
+fail:
 	msm_obj->vmap_count--;
+	mutex_unlock(&msm_obj->lock);
+	return ERR_PTR(ret);
 }
 
 void msm_gem_put_vaddr(struct drm_gem_object *obj)
 {
-	mutex_lock(&obj->dev->struct_mutex);
-	msm_gem_put_vaddr_locked(obj);
-	mutex_unlock(&obj->dev->struct_mutex);
+	struct msm_gem_object *msm_obj = to_msm_bo(obj);
+
+	mutex_lock(&msm_obj->lock);
+	WARN_ON(msm_obj->vmap_count < 1);
+	msm_obj->vmap_count--;
+	mutex_unlock(&msm_obj->lock);
 }
 
 /* Update madvise status, returns true if not purged, else
···
 {
 	struct msm_gem_object *msm_obj = to_msm_bo(obj);
 
+	mutex_lock(&msm_obj->lock);
+
 	WARN_ON(!mutex_is_locked(&obj->dev->struct_mutex));
 
 	if (msm_obj->madv != __MSM_MADV_PURGED)
 		msm_obj->madv = madv;
 
-	return (msm_obj->madv != __MSM_MADV_PURGED);
+	madv = msm_obj->madv;
+
+	mutex_unlock(&msm_obj->lock);
+
+	return (madv != __MSM_MADV_PURGED);
 }
 
-void msm_gem_purge(struct drm_gem_object *obj)
+void msm_gem_purge(struct drm_gem_object *obj, enum msm_gem_lock subclass)
 {
 	struct drm_device *dev = obj->dev;
 	struct msm_gem_object *msm_obj = to_msm_bo(obj);
···
 	WARN_ON(!is_purgeable(msm_obj));
 	WARN_ON(obj->import_attach);
 
+	mutex_lock_nested(&msm_obj->lock, subclass);
+
 	put_iova(obj);
 
-	msm_gem_vunmap(obj);
+	msm_gem_vunmap_locked(obj);
 
 	put_pages(obj);
 
···
 
 	invalidate_mapping_pages(file_inode(obj->filp)->i_mapping,
 			0, (loff_t)-1);
+
+	mutex_unlock(&msm_obj->lock);
 }
 
-void msm_gem_vunmap(struct drm_gem_object *obj)
+static void msm_gem_vunmap_locked(struct drm_gem_object *obj)
 {
 	struct msm_gem_object *msm_obj = to_msm_bo(obj);
+
+	WARN_ON(!mutex_is_locked(&msm_obj->lock));
 
 	if (!msm_obj->vaddr || WARN_ON(!is_vunmapable(msm_obj)))
 		return;
 
 	vunmap(msm_obj->vaddr);
 	msm_obj->vaddr = NULL;
+}
+
+void msm_gem_vunmap(struct drm_gem_object *obj, enum msm_gem_lock subclass)
+{
+	struct msm_gem_object *msm_obj = to_msm_bo(obj);
+
+	mutex_lock_nested(&msm_obj->lock, subclass);
+	msm_gem_vunmap_locked(obj);
+	mutex_unlock(&msm_obj->lock);
 }
 
 /* must be called before _move_to_active().. */
···
 	uint64_t off = drm_vma_node_start(&obj->vma_node);
 	const char *madv;
 
-	WARN_ON(!mutex_is_locked(&obj->dev->struct_mutex));
+	mutex_lock(&msm_obj->lock);
 
 	switch (msm_obj->madv) {
 	case __MSM_MADV_PURGED:
···
 	if (fence)
 		describe_fence(fence, "Exclusive", m);
 	rcu_read_unlock();
+
+	mutex_unlock(&msm_obj->lock);
 }
 
 void msm_gem_describe_objects(struct list_head *list, struct seq_file *m)
···
 
 	list_del(&msm_obj->mm_list);
 
+	mutex_lock(&msm_obj->lock);
+
 	put_iova(obj);
 
 	if (obj->import_attach) {
···
 
 		drm_prime_gem_destroy(obj, msm_obj->sgt);
 	} else {
-		msm_gem_vunmap(obj);
+		msm_gem_vunmap_locked(obj);
 		put_pages(obj);
 	}
 
···
 
 	drm_gem_object_release(obj);
 
+	mutex_unlock(&msm_obj->lock);
 	kfree(msm_obj);
 }
 
···
 	struct drm_gem_object *obj;
 	int ret;
 
-	ret = mutex_lock_interruptible(&dev->struct_mutex);
-	if (ret)
-		return ret;
-
 	obj = msm_gem_new(dev, size, flags);
-
-	mutex_unlock(&dev->struct_mutex);
 
 	if (IS_ERR(obj))
 		return PTR_ERR(obj);
···
 static int msm_gem_new_impl(struct drm_device *dev,
 		uint32_t size, uint32_t flags,
 		struct reservation_object *resv,
-		struct drm_gem_object **obj)
+		struct drm_gem_object **obj,
+		bool struct_mutex_locked)
 {
 	struct msm_drm_private *priv = dev->dev_private;
 	struct msm_gem_object *msm_obj;
-
-	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
 
 	switch (flags & MSM_BO_CACHE_MASK) {
 	case MSM_BO_UNCACHED:
···
 	if (!msm_obj)
 		return -ENOMEM;
 
+	mutex_init(&msm_obj->lock);
+
 	msm_obj->flags = flags;
 	msm_obj->madv = MSM_MADV_WILLNEED;
 
···
 	INIT_LIST_HEAD(&msm_obj->submit_entry);
 	INIT_LIST_HEAD(&msm_obj->vmas);
 
-	list_add_tail(&msm_obj->mm_list, &priv->inactive_list);
+	if (struct_mutex_locked) {
+		WARN_ON(!mutex_is_locked(&dev->struct_mutex));
+		list_add_tail(&msm_obj->mm_list, &priv->inactive_list);
+	} else {
+		mutex_lock(&dev->struct_mutex);
+		list_add_tail(&msm_obj->mm_list, &priv->inactive_list);
+		mutex_unlock(&dev->struct_mutex);
+	}
 
 	*obj = &msm_obj->base;
 
 	return 0;
 }
 
-struct drm_gem_object *msm_gem_new(struct drm_device *dev,
-		uint32_t size, uint32_t flags)
+static struct drm_gem_object *_msm_gem_new(struct drm_device *dev,
+		uint32_t size, uint32_t flags, bool struct_mutex_locked)
 {
 	struct msm_drm_private *priv = dev->dev_private;
 	struct drm_gem_object *obj = NULL;
 	bool use_vram = false;
 	int ret;
-
-	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
 
 	size = PAGE_ALIGN(size);
 
···
 	if (size == 0)
 		return ERR_PTR(-EINVAL);
 
-	ret = msm_gem_new_impl(dev, size, flags, NULL, &obj);
+	ret = msm_gem_new_impl(dev, size, flags, NULL, &obj, struct_mutex_locked);
 	if (ret)
 		goto fail;
 
···
 	return obj;
 
 fail:
-	drm_gem_object_unreference(obj);
+	drm_gem_object_unreference_unlocked(obj);
 	return ERR_PTR(ret);
+}
+
+struct drm_gem_object *msm_gem_new_locked(struct drm_device *dev,
+		uint32_t size, uint32_t flags)
+{
+	return _msm_gem_new(dev, size, flags, true);
+}
+
+struct drm_gem_object *msm_gem_new(struct drm_device *dev,
+		uint32_t size, uint32_t flags)
+{
+	return _msm_gem_new(dev, size, flags, false);
 }
 
 struct drm_gem_object *msm_gem_import(struct drm_device *dev,
···
 
 	size = PAGE_ALIGN(dmabuf->size);
 
-	/* Take mutex so we can modify the inactive list in msm_gem_new_impl */
-	mutex_lock(&dev->struct_mutex);
-	ret = msm_gem_new_impl(dev, size, MSM_BO_WC, dmabuf->resv, &obj);
-	mutex_unlock(&dev->struct_mutex);
-
+	ret = msm_gem_new_impl(dev, size, MSM_BO_WC, dmabuf->resv, &obj, false);
 	if (ret)
 		goto fail;
 
···
 	npages = size / PAGE_SIZE;
 
 	msm_obj = to_msm_bo(obj);
+	mutex_lock(&msm_obj->lock);
 	msm_obj->sgt = sgt;
 	msm_obj->pages = kvmalloc_array(npages, sizeof(struct page *), GFP_KERNEL);
 	if (!msm_obj->pages) {
+		mutex_unlock(&msm_obj->lock);
 		ret = -ENOMEM;
 		goto fail;
 	}
 
 	ret = drm_prime_sg_to_page_addr_arrays(sgt, msm_obj->pages, NULL, npages);
-	if (ret)
+	if (ret) {
+		mutex_unlock(&msm_obj->lock);
 		goto fail;
+	}
 
+	mutex_unlock(&msm_obj->lock);
 	return obj;
 
 fail:

drivers/gpu/drm/msm/msm_gem.h

···
 	 * and position mm_node->start is in # of pages:
 	 */
 	struct drm_mm mm;
+	spinlock_t lock; /* Protects drm_mm node allocation/removal */
 	struct msm_mmu *mmu;
 	struct kref kref;
 };
···
 	 * an IOMMU.  Also used for stolen/splashscreen buffer.
 	 */
 	struct drm_mm_node *vram_node;
+	struct mutex lock; /* Protects resources associated with bo */
 };
 #define to_msm_bo(x) container_of(x, struct msm_gem_object, base)
 
···
 
 static inline bool is_purgeable(struct msm_gem_object *msm_obj)
 {
+	WARN_ON(!mutex_is_locked(&msm_obj->base.dev->struct_mutex));
 	return (msm_obj->madv == MSM_MADV_DONTNEED) && msm_obj->sgt &&
 			!msm_obj->base.dma_buf && !msm_obj->base.import_attach;
 }
···
 {
 	return (msm_obj->vmap_count == 0) && msm_obj->vaddr;
 }
+
+/* The shrinker can be triggered while we hold objA->lock, and need
+ * to grab objB->lock to purge it.  Lockdep just sees these as a single
+ * class of lock, so we use subclasses to teach it the difference.
+ *
+ * OBJ_LOCK_NORMAL is implicit (ie. normal mutex_lock() call), and
+ * OBJ_LOCK_SHRINKER is used by shrinker.
+ *
+ * It is *essential* that we never go down paths that could trigger the
+ * shrinker for a purgable object.  This is ensured by checking that
+ * msm_obj->madv == MSM_MADV_WILLNEED.
+ */
+enum msm_gem_lock {
+	OBJ_LOCK_NORMAL,
+	OBJ_LOCK_SHRINKER,
+};
+
+void msm_gem_purge(struct drm_gem_object *obj, enum msm_gem_lock subclass);
+void msm_gem_vunmap(struct drm_gem_object *obj, enum msm_gem_lock subclass);
 
 /* Created per submit-ioctl, to track bo's and cmdstream bufs, etc,
  * associated with the cmdstream submission for synchronization (and

drivers/gpu/drm/msm/msm_gem_shrinker.c

···
 
 static bool msm_gem_shrinker_lock(struct drm_device *dev, bool *unlock)
 {
+	/* NOTE: we are *closer* to being able to get rid of
+	 * mutex_trylock_recursive().. the msm_gem code itself does
+	 * not need struct_mutex, although codepaths that can trigger
+	 * shrinker are still called in code-paths that hold the
+	 * struct_mutex.
+	 *
+	 * Also, msm_obj->madv is protected by struct_mutex.
+	 *
+	 * The next step is probably split out a seperate lock for
+	 * protecting inactive_list, so that shrinker does not need
+	 * struct_mutex.
+	 */
 	switch (mutex_trylock_recursive(&dev->struct_mutex)) {
 	case MUTEX_TRYLOCK_FAILED:
 		return false;
···
 		if (freed >= sc->nr_to_scan)
 			break;
 		if (is_purgeable(msm_obj)) {
-			msm_gem_purge(&msm_obj->base);
+			msm_gem_purge(&msm_obj->base, OBJ_LOCK_SHRINKER);
 			freed += msm_obj->base.size >> PAGE_SHIFT;
 		}
 	}
···
 
 	list_for_each_entry(msm_obj, &priv->inactive_list, mm_list) {
 		if (is_vunmapable(msm_obj)) {
-			msm_gem_vunmap(&msm_obj->base);
+			msm_gem_vunmap(&msm_obj->base, OBJ_LOCK_SHRINKER);
 			/* since we don't know any better, lets bail after a few
 			 * and if necessary the shrinker will be invoked again.
 			 * Seems better than unmapping *everything*

drivers/gpu/drm/msm/msm_gem_submit.c

···
 		uint64_t iova;
 
 		/* if locking succeeded, pin bo: */
-		ret = msm_gem_get_iova_locked(&msm_obj->base,
+		ret = msm_gem_get_iova(&msm_obj->base,
 				submit->gpu->aspace, &iova);
 
 		if (ret)
···
 	/* For now, just map the entire thing.  Eventually we probably
 	 * to do it page-by-page, w/ kmap() if not vmap()d..
 	 */
-	ptr = msm_gem_get_vaddr_locked(&obj->base);
+	ptr = msm_gem_get_vaddr(&obj->base);
 
 	if (IS_ERR(ptr)) {
 		ret = PTR_ERR(ptr);
···
 	}
 
 out:
-	msm_gem_put_vaddr_locked(&obj->base);
+	msm_gem_put_vaddr(&obj->base);
 
 	return ret;
 }

drivers/gpu/drm/msm/msm_gem_vma.c

···
 		aspace->mmu->funcs->unmap(aspace->mmu, vma->iova, sgt, size);
 	}
 
+	spin_lock(&aspace->lock);
 	drm_mm_remove_node(&vma->node);
+	spin_unlock(&aspace->lock);
 
 	vma->iova = 0;
 
···
 {
 	int ret;
 
-	if (WARN_ON(drm_mm_node_allocated(&vma->node)))
+	spin_lock(&aspace->lock);
+	if (WARN_ON(drm_mm_node_allocated(&vma->node))) {
+		spin_unlock(&aspace->lock);
 		return 0;
+	}
 
 	ret = drm_mm_insert_node(&aspace->mm, &vma->node, npages);
+	spin_unlock(&aspace->lock);
+
 	if (ret)
 		return ret;
 
···
 	if (!aspace)
 		return ERR_PTR(-ENOMEM);
 
+	spin_lock_init(&aspace->lock);
 	aspace->name = name;
 	aspace->mmu = msm_iommu_new(dev, domain);
 

drivers/gpu/drm/msm/msm_gpu.c

···
 
 		/* submit takes a reference to the bo and iova until retired: */
 		drm_gem_object_reference(&msm_obj->base);
-		msm_gem_get_iova_locked(&msm_obj->base,
+		msm_gem_get_iova(&msm_obj->base,
 				submit->gpu->aspace, &iova);
 
 		if (submit->bos[i].flags & MSM_SUBMIT_BO_WRITE)
···
 	}
 
 	/* Create ringbuffer: */
-	mutex_lock(&drm->struct_mutex);
 	gpu->rb = msm_ringbuffer_new(gpu, config->ringsz);
-	mutex_unlock(&drm->struct_mutex);
 	if (IS_ERR(gpu->rb)) {
 		ret = PTR_ERR(gpu->rb);
 		gpu->rb = NULL;

drivers/gpu/drm/msm/msm_rd.c

···
 	struct msm_gem_object *obj = submit->bos[idx].obj;
 	const char *buf;
 
-	buf = msm_gem_get_vaddr_locked(&obj->base);
+	buf = msm_gem_get_vaddr(&obj->base);
 	if (IS_ERR(buf))
 		return;
 
···
 			(uint32_t[3]){ iova, size, iova >> 32 }, 12);
 	rd_write_section(rd, RD_BUFFER_CONTENTS, buf, size);
 
-	msm_gem_put_vaddr_locked(&obj->base);
+	msm_gem_put_vaddr(&obj->base);
 }
 
 /* called under struct_mutex */

drivers/gpu/drm/msm/msm_ringbuffer.c

···
 		goto fail;
 	}
 
-	ring->start = msm_gem_get_vaddr_locked(ring->bo);
+	ring->start = msm_gem_get_vaddr(ring->bo);
 	if (IS_ERR(ring->start)) {
 		ret = PTR_ERR(ring->start);
 		goto fail;