From: Thomas Zimmermann <tzimmermann@suse.de>
To: "Dmitry Osipenko" <dmitry.osipenko@collabora.com>,
"David Airlie" <airlied@gmail.com>,
"Gerd Hoffmann" <kraxel@redhat.com>,
"Gurchetan Singh" <gurchetansingh@chromium.org>,
"Chia-I Wu" <olvaffe@gmail.com>,
"Daniel Vetter" <daniel@ffwll.ch>,
"Daniel Almeida" <daniel.almeida@collabora.com>,
"Gustavo Padovan" <gustavo.padovan@collabora.com>,
"Daniel Stone" <daniel@fooishbar.org>,
"Tomeu Vizoso" <tomeu.vizoso@collabora.com>,
"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
"Maxime Ripard" <mripard@kernel.org>,
"Rob Clark" <robdclark@gmail.com>,
"Sumit Semwal" <sumit.semwal@linaro.org>,
"Christian König" <christian.koenig@amd.com>,
"Qiang Yu" <yuq825@gmail.com>,
"Steven Price" <steven.price@arm.com>,
"Alyssa Rosenzweig" <alyssa.rosenzweig@collabora.com>,
"Rob Herring" <robh@kernel.org>, "Sean Paul" <sean@poorly.run>,
"Dmitry Baryshkov" <dmitry.baryshkov@linaro.org>,
"Abhinav Kumar" <quic_abhinavk@quicinc.com>
Cc: kernel@collabora.com, linux-kernel@vger.kernel.org,
dri-devel@lists.freedesktop.org,
virtualization@lists.linux-foundation.org
Subject: Re: [PATCH v10 07/11] drm/shmem-helper: Switch to reservation lock
Date: Fri, 17 Feb 2023 14:29:40 +0100 [thread overview]
Message-ID: <c61fdee7-d474-6c52-912b-412eb6ad4a19@suse.de> (raw)
In-Reply-To: <20230108210445.3948344-8-dmitry.osipenko@collabora.com>
[-- Attachment #1.1: Type: text/plain, Size: 22939 bytes --]
Hi
Am 08.01.23 um 22:04 schrieb Dmitry Osipenko:
> Replace all drm-shmem locks with a GEM reservation lock. This makes locks
> consistent with dma-buf locking convention where importers are responsible
> for holding reservation lock for all operations performed over dma-bufs,
> preventing deadlock between dma-buf importers and exporters.
>
> Suggested-by: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
I don't dare to r-b this, but take my
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
if you want to land this patch.
Best regards
Thomas
> ---
> drivers/gpu/drm/drm_gem_shmem_helper.c | 185 +++++++-----------
> drivers/gpu/drm/lima/lima_gem.c | 8 +-
> drivers/gpu/drm/panfrost/panfrost_drv.c | 7 +-
> .../gpu/drm/panfrost/panfrost_gem_shrinker.c | 6 +-
> drivers/gpu/drm/panfrost/panfrost_mmu.c | 19 +-
> include/drm/drm_gem_shmem_helper.h | 14 +-
> 6 files changed, 94 insertions(+), 145 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index 1392cbd3cc02..a1f2f2158c50 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -88,8 +88,6 @@ __drm_gem_shmem_create(struct drm_device *dev, size_t size, bool private)
> if (ret)
> goto err_release;
>
> - mutex_init(&shmem->pages_lock);
> - mutex_init(&shmem->vmap_lock);
> INIT_LIST_HEAD(&shmem->madv_list);
>
> if (!private) {
> @@ -141,11 +139,13 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object *shmem)
> {
> struct drm_gem_object *obj = &shmem->base;
>
> - drm_WARN_ON(obj->dev, shmem->vmap_use_count);
> -
> if (obj->import_attach) {
> drm_prime_gem_destroy(obj, shmem->sgt);
> } else {
> + dma_resv_lock(shmem->base.resv, NULL);
> +
> + drm_WARN_ON(obj->dev, shmem->vmap_use_count);
> +
> if (shmem->sgt) {
> dma_unmap_sgtable(obj->dev->dev, shmem->sgt,
> DMA_BIDIRECTIONAL, 0);
> @@ -154,18 +154,18 @@ void drm_gem_shmem_free(struct drm_gem_shmem_object *shmem)
> }
> if (shmem->pages)
> drm_gem_shmem_put_pages(shmem);
> - }
>
> - drm_WARN_ON(obj->dev, shmem->pages_use_count);
> + drm_WARN_ON(obj->dev, shmem->pages_use_count);
> +
> + dma_resv_unlock(shmem->base.resv);
> + }
>
> drm_gem_object_release(obj);
> - mutex_destroy(&shmem->pages_lock);
> - mutex_destroy(&shmem->vmap_lock);
> kfree(shmem);
> }
> EXPORT_SYMBOL_GPL(drm_gem_shmem_free);
>
> -static int drm_gem_shmem_get_pages_locked(struct drm_gem_shmem_object *shmem)
> +static int drm_gem_shmem_get_pages(struct drm_gem_shmem_object *shmem)
> {
> struct drm_gem_object *obj = &shmem->base;
> struct page **pages;
> @@ -197,35 +197,16 @@ static int drm_gem_shmem_get_pages_locked(struct drm_gem_shmem_object *shmem)
> }
>
> /*
> - * drm_gem_shmem_get_pages - Allocate backing pages for a shmem GEM object
> + * drm_gem_shmem_put_pages - Decrease use count on the backing pages for a shmem GEM object
> * @shmem: shmem GEM object
> *
> - * This function makes sure that backing pages exists for the shmem GEM object
> - * and increases the use count.
> - *
> - * Returns:
> - * 0 on success or a negative error code on failure.
> + * This function decreases the use count and puts the backing pages when use drops to zero.
> */
> -int drm_gem_shmem_get_pages(struct drm_gem_shmem_object *shmem)
> +void drm_gem_shmem_put_pages(struct drm_gem_shmem_object *shmem)
> {
> struct drm_gem_object *obj = &shmem->base;
> - int ret;
>
> - drm_WARN_ON(obj->dev, obj->import_attach);
> -
> - ret = mutex_lock_interruptible(&shmem->pages_lock);
> - if (ret)
> - return ret;
> - ret = drm_gem_shmem_get_pages_locked(shmem);
> - mutex_unlock(&shmem->pages_lock);
> -
> - return ret;
> -}
> -EXPORT_SYMBOL(drm_gem_shmem_get_pages);
> -
> -static void drm_gem_shmem_put_pages_locked(struct drm_gem_shmem_object *shmem)
> -{
> - struct drm_gem_object *obj = &shmem->base;
> + dma_resv_assert_held(shmem->base.resv);
>
> if (drm_WARN_ON_ONCE(obj->dev, !shmem->pages_use_count))
> return;
> @@ -243,19 +224,6 @@ static void drm_gem_shmem_put_pages_locked(struct drm_gem_shmem_object *shmem)
> shmem->pages_mark_accessed_on_put);
> shmem->pages = NULL;
> }
> -
> -/*
> - * drm_gem_shmem_put_pages - Decrease use count on the backing pages for a shmem GEM object
> - * @shmem: shmem GEM object
> - *
> - * This function decreases the use count and puts the backing pages when use drops to zero.
> - */
> -void drm_gem_shmem_put_pages(struct drm_gem_shmem_object *shmem)
> -{
> - mutex_lock(&shmem->pages_lock);
> - drm_gem_shmem_put_pages_locked(shmem);
> - mutex_unlock(&shmem->pages_lock);
> -}
> EXPORT_SYMBOL(drm_gem_shmem_put_pages);
>
> /**
> @@ -272,6 +240,8 @@ int drm_gem_shmem_pin(struct drm_gem_shmem_object *shmem)
> {
> struct drm_gem_object *obj = &shmem->base;
>
> + dma_resv_assert_held(shmem->base.resv);
> +
> drm_WARN_ON(obj->dev, obj->import_attach);
>
> return drm_gem_shmem_get_pages(shmem);
> @@ -289,14 +259,31 @@ void drm_gem_shmem_unpin(struct drm_gem_shmem_object *shmem)
> {
> struct drm_gem_object *obj = &shmem->base;
>
> + dma_resv_assert_held(shmem->base.resv);
> +
> drm_WARN_ON(obj->dev, obj->import_attach);
>
> drm_gem_shmem_put_pages(shmem);
> }
> EXPORT_SYMBOL(drm_gem_shmem_unpin);
>
> -static int drm_gem_shmem_vmap_locked(struct drm_gem_shmem_object *shmem,
> - struct iosys_map *map)
> +/*
> + * drm_gem_shmem_vmap - Create a virtual mapping for a shmem GEM object
> + * @shmem: shmem GEM object
> + * @map: Returns the kernel virtual address of the SHMEM GEM object's backing
> + * store.
> + *
> + * This function makes sure that a contiguous kernel virtual address mapping
> + * exists for the buffer backing the shmem GEM object. It hides the differences
> + * between dma-buf imported and natively allocated objects.
> + *
> + * Acquired mappings should be cleaned up by calling drm_gem_shmem_vunmap().
> + *
> + * Returns:
> + * 0 on success or a negative error code on failure.
> + */
> +int drm_gem_shmem_vmap(struct drm_gem_shmem_object *shmem,
> + struct iosys_map *map)
> {
> struct drm_gem_object *obj = &shmem->base;
> int ret = 0;
> @@ -312,6 +299,8 @@ static int drm_gem_shmem_vmap_locked(struct drm_gem_shmem_object *shmem,
> } else {
> pgprot_t prot = PAGE_KERNEL;
>
> + dma_resv_assert_held(shmem->base.resv);
> +
> if (shmem->vmap_use_count++ > 0) {
> iosys_map_set_vaddr(map, shmem->vaddr);
> return 0;
> @@ -346,45 +335,30 @@ static int drm_gem_shmem_vmap_locked(struct drm_gem_shmem_object *shmem,
>
> return ret;
> }
> +EXPORT_SYMBOL(drm_gem_shmem_vmap);
>
> /*
> - * drm_gem_shmem_vmap - Create a virtual mapping for a shmem GEM object
> + * drm_gem_shmem_vunmap - Unmap a virtual mapping for a shmem GEM object
> * @shmem: shmem GEM object
> - * @map: Returns the kernel virtual address of the SHMEM GEM object's backing
> - * store.
> - *
> - * This function makes sure that a contiguous kernel virtual address mapping
> - * exists for the buffer backing the shmem GEM object. It hides the differences
> - * between dma-buf imported and natively allocated objects.
> + * @map: Kernel virtual address where the SHMEM GEM object was mapped
> *
> - * Acquired mappings should be cleaned up by calling drm_gem_shmem_vunmap().
> + * This function cleans up a kernel virtual address mapping acquired by
> + * drm_gem_shmem_vmap(). The mapping is only removed when the use count drops to
> + * zero.
> *
> - * Returns:
> - * 0 on success or a negative error code on failure.
> + * This function hides the differences between dma-buf imported and natively
> + * allocated objects.
> */
> -int drm_gem_shmem_vmap(struct drm_gem_shmem_object *shmem,
> - struct iosys_map *map)
> -{
> - int ret;
> -
> - ret = mutex_lock_interruptible(&shmem->vmap_lock);
> - if (ret)
> - return ret;
> - ret = drm_gem_shmem_vmap_locked(shmem, map);
> - mutex_unlock(&shmem->vmap_lock);
> -
> - return ret;
> -}
> -EXPORT_SYMBOL(drm_gem_shmem_vmap);
> -
> -static void drm_gem_shmem_vunmap_locked(struct drm_gem_shmem_object *shmem,
> - struct iosys_map *map)
> +void drm_gem_shmem_vunmap(struct drm_gem_shmem_object *shmem,
> + struct iosys_map *map)
> {
> struct drm_gem_object *obj = &shmem->base;
>
> if (obj->import_attach) {
> dma_buf_vunmap(obj->import_attach->dmabuf, map);
> } else {
> + dma_resv_assert_held(shmem->base.resv);
> +
> if (drm_WARN_ON_ONCE(obj->dev, !shmem->vmap_use_count))
> return;
>
> @@ -397,26 +371,6 @@ static void drm_gem_shmem_vunmap_locked(struct drm_gem_shmem_object *shmem,
>
> shmem->vaddr = NULL;
> }
> -
> -/*
> - * drm_gem_shmem_vunmap - Unmap a virtual mapping for a shmem GEM object
> - * @shmem: shmem GEM object
> - * @map: Kernel virtual address where the SHMEM GEM object was mapped
> - *
> - * This function cleans up a kernel virtual address mapping acquired by
> - * drm_gem_shmem_vmap(). The mapping is only removed when the use count drops to
> - * zero.
> - *
> - * This function hides the differences between dma-buf imported and natively
> - * allocated objects.
> - */
> -void drm_gem_shmem_vunmap(struct drm_gem_shmem_object *shmem,
> - struct iosys_map *map)
> -{
> - mutex_lock(&shmem->vmap_lock);
> - drm_gem_shmem_vunmap_locked(shmem, map);
> - mutex_unlock(&shmem->vmap_lock);
> -}
> EXPORT_SYMBOL(drm_gem_shmem_vunmap);
>
> static struct drm_gem_shmem_object *
> @@ -449,24 +403,24 @@ drm_gem_shmem_create_with_handle(struct drm_file *file_priv,
> */
> int drm_gem_shmem_madvise(struct drm_gem_shmem_object *shmem, int madv)
> {
> - mutex_lock(&shmem->pages_lock);
> + dma_resv_assert_held(shmem->base.resv);
>
> if (shmem->madv >= 0)
> shmem->madv = madv;
>
> madv = shmem->madv;
>
> - mutex_unlock(&shmem->pages_lock);
> -
> return (madv >= 0);
> }
> EXPORT_SYMBOL(drm_gem_shmem_madvise);
>
> -void drm_gem_shmem_purge_locked(struct drm_gem_shmem_object *shmem)
> +void drm_gem_shmem_purge(struct drm_gem_shmem_object *shmem)
> {
> struct drm_gem_object *obj = &shmem->base;
> struct drm_device *dev = obj->dev;
>
> + dma_resv_assert_held(shmem->base.resv);
> +
> drm_WARN_ON(obj->dev, !drm_gem_shmem_is_purgeable(shmem));
>
> dma_unmap_sgtable(dev->dev, shmem->sgt, DMA_BIDIRECTIONAL, 0);
> @@ -474,7 +428,7 @@ void drm_gem_shmem_purge_locked(struct drm_gem_shmem_object *shmem)
> kfree(shmem->sgt);
> shmem->sgt = NULL;
>
> - drm_gem_shmem_put_pages_locked(shmem);
> + drm_gem_shmem_put_pages(shmem);
>
> shmem->madv = -1;
>
> @@ -490,17 +444,6 @@ void drm_gem_shmem_purge_locked(struct drm_gem_shmem_object *shmem)
>
> invalidate_mapping_pages(file_inode(obj->filp)->i_mapping, 0, (loff_t)-1);
> }
> -EXPORT_SYMBOL(drm_gem_shmem_purge_locked);
> -
> -bool drm_gem_shmem_purge(struct drm_gem_shmem_object *shmem)
> -{
> - if (!mutex_trylock(&shmem->pages_lock))
> - return false;
> - drm_gem_shmem_purge_locked(shmem);
> - mutex_unlock(&shmem->pages_lock);
> -
> - return true;
> -}
> EXPORT_SYMBOL(drm_gem_shmem_purge);
>
> /**
> @@ -556,7 +499,7 @@ static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
> /* We don't use vmf->pgoff since that has the fake offset */
> page_offset = (vmf->address - vma->vm_start) >> PAGE_SHIFT;
>
> - mutex_lock(&shmem->pages_lock);
> + dma_resv_lock(shmem->base.resv, NULL);
>
> if (page_offset >= num_pages ||
> drm_WARN_ON_ONCE(obj->dev, !shmem->pages) ||
> @@ -568,7 +511,7 @@ static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
> ret = vmf_insert_pfn(vma, vmf->address, page_to_pfn(page));
> }
>
> - mutex_unlock(&shmem->pages_lock);
> + dma_resv_unlock(shmem->base.resv);
>
> return ret;
> }
> @@ -580,7 +523,7 @@ static void drm_gem_shmem_vm_open(struct vm_area_struct *vma)
>
> drm_WARN_ON(obj->dev, obj->import_attach);
>
> - mutex_lock(&shmem->pages_lock);
> + dma_resv_lock(shmem->base.resv, NULL);
>
> /*
> * We should have already pinned the pages when the buffer was first
> @@ -590,7 +533,7 @@ static void drm_gem_shmem_vm_open(struct vm_area_struct *vma)
> if (!drm_WARN_ON_ONCE(obj->dev, !shmem->pages_use_count))
> shmem->pages_use_count++;
>
> - mutex_unlock(&shmem->pages_lock);
> + dma_resv_unlock(shmem->base.resv);
>
> drm_gem_vm_open(vma);
> }
> @@ -600,7 +543,10 @@ static void drm_gem_shmem_vm_close(struct vm_area_struct *vma)
> struct drm_gem_object *obj = vma->vm_private_data;
> struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
>
> + dma_resv_lock(shmem->base.resv, NULL);
> drm_gem_shmem_put_pages(shmem);
> + dma_resv_unlock(shmem->base.resv);
> +
> drm_gem_vm_close(vma);
> }
>
> @@ -635,7 +581,10 @@ int drm_gem_shmem_mmap(struct drm_gem_shmem_object *shmem, struct vm_area_struct
> return dma_buf_mmap(obj->dma_buf, vma, 0);
> }
>
> + dma_resv_lock(shmem->base.resv, NULL);
> ret = drm_gem_shmem_get_pages(shmem);
> + dma_resv_unlock(shmem->base.resv);
> +
> if (ret)
> return ret;
>
> @@ -721,9 +670,11 @@ struct sg_table *drm_gem_shmem_get_pages_sgt(struct drm_gem_shmem_object *shmem)
>
> drm_WARN_ON(obj->dev, obj->import_attach);
>
> + dma_resv_lock(shmem->base.resv, NULL);
> +
> ret = drm_gem_shmem_get_pages(shmem);
> if (ret)
> - return ERR_PTR(ret);
> + goto err_unlock;
>
> sgt = drm_gem_shmem_get_sg_table(shmem);
> if (IS_ERR(sgt)) {
> @@ -737,6 +688,8 @@ struct sg_table *drm_gem_shmem_get_pages_sgt(struct drm_gem_shmem_object *shmem)
>
> shmem->sgt = sgt;
>
> + dma_resv_unlock(shmem->base.resv);
> +
> return sgt;
>
> err_free_sgt:
> @@ -744,6 +697,8 @@ struct sg_table *drm_gem_shmem_get_pages_sgt(struct drm_gem_shmem_object *shmem)
> kfree(sgt);
> err_put_pages:
> drm_gem_shmem_put_pages(shmem);
> +err_unlock:
> + dma_resv_unlock(shmem->base.resv);
> return ERR_PTR(ret);
> }
> EXPORT_SYMBOL_GPL(drm_gem_shmem_get_pages_sgt);
> diff --git a/drivers/gpu/drm/lima/lima_gem.c b/drivers/gpu/drm/lima/lima_gem.c
> index 0f1ca0b0db49..5008f0c2428f 100644
> --- a/drivers/gpu/drm/lima/lima_gem.c
> +++ b/drivers/gpu/drm/lima/lima_gem.c
> @@ -34,7 +34,7 @@ int lima_heap_alloc(struct lima_bo *bo, struct lima_vm *vm)
>
> new_size = min(new_size, bo->base.base.size);
>
> - mutex_lock(&bo->base.pages_lock);
> + dma_resv_lock(bo->base.base.resv, NULL);
>
> if (bo->base.pages) {
> pages = bo->base.pages;
> @@ -42,7 +42,7 @@ int lima_heap_alloc(struct lima_bo *bo, struct lima_vm *vm)
> pages = kvmalloc_array(bo->base.base.size >> PAGE_SHIFT,
> sizeof(*pages), GFP_KERNEL | __GFP_ZERO);
> if (!pages) {
> - mutex_unlock(&bo->base.pages_lock);
> + dma_resv_unlock(bo->base.base.resv);
> return -ENOMEM;
> }
>
> @@ -56,13 +56,13 @@ int lima_heap_alloc(struct lima_bo *bo, struct lima_vm *vm)
> struct page *page = shmem_read_mapping_page(mapping, i);
>
> if (IS_ERR(page)) {
> - mutex_unlock(&bo->base.pages_lock);
> + dma_resv_unlock(bo->base.base.resv);
> return PTR_ERR(page);
> }
> pages[i] = page;
> }
>
> - mutex_unlock(&bo->base.pages_lock);
> + dma_resv_unlock(bo->base.base.resv);
>
> ret = sg_alloc_table_from_pages(&sgt, pages, i, 0,
> new_size, GFP_KERNEL);
> diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
> index abb0dadd8f63..9f3f2283b67a 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
> @@ -414,6 +414,10 @@ static int panfrost_ioctl_madvise(struct drm_device *dev, void *data,
>
> bo = to_panfrost_bo(gem_obj);
>
> + ret = dma_resv_lock_interruptible(bo->base.base.resv, NULL);
> + if (ret)
> + goto out_put_object;
> +
> mutex_lock(&pfdev->shrinker_lock);
> mutex_lock(&bo->mappings.lock);
> if (args->madv == PANFROST_MADV_DONTNEED) {
> @@ -451,7 +455,8 @@ static int panfrost_ioctl_madvise(struct drm_device *dev, void *data,
> out_unlock_mappings:
> mutex_unlock(&bo->mappings.lock);
> mutex_unlock(&pfdev->shrinker_lock);
> -
> + dma_resv_unlock(bo->base.base.resv);
> +out_put_object:
> drm_gem_object_put(gem_obj);
> return ret;
> }
> diff --git a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c
> index bf0170782f25..6a71a2555f85 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c
> @@ -48,14 +48,14 @@ static bool panfrost_gem_purge(struct drm_gem_object *obj)
> if (!mutex_trylock(&bo->mappings.lock))
> return false;
>
> - if (!mutex_trylock(&shmem->pages_lock))
> + if (!dma_resv_trylock(shmem->base.resv))
> goto unlock_mappings;
>
> panfrost_gem_teardown_mappings_locked(bo);
> - drm_gem_shmem_purge_locked(&bo->base);
> + drm_gem_shmem_purge(&bo->base);
> ret = true;
>
> - mutex_unlock(&shmem->pages_lock);
> + dma_resv_unlock(shmem->base.resv);
>
> unlock_mappings:
> mutex_unlock(&bo->mappings.lock);
> diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> index 666a5e53fe19..0679df57f394 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> @@ -443,6 +443,7 @@ static int panfrost_mmu_map_fault_addr(struct panfrost_device *pfdev, int as,
> struct panfrost_gem_mapping *bomapping;
> struct panfrost_gem_object *bo;
> struct address_space *mapping;
> + struct drm_gem_object *obj;
> pgoff_t page_offset;
> struct sg_table *sgt;
> struct page **pages;
> @@ -465,15 +466,16 @@ static int panfrost_mmu_map_fault_addr(struct panfrost_device *pfdev, int as,
> page_offset = addr >> PAGE_SHIFT;
> page_offset -= bomapping->mmnode.start;
>
> - mutex_lock(&bo->base.pages_lock);
> + obj = &bo->base.base;
> +
> + dma_resv_lock(obj->resv, NULL);
>
> if (!bo->base.pages) {
> bo->sgts = kvmalloc_array(bo->base.base.size / SZ_2M,
> sizeof(struct sg_table), GFP_KERNEL | __GFP_ZERO);
> if (!bo->sgts) {
> - mutex_unlock(&bo->base.pages_lock);
> ret = -ENOMEM;
> - goto err_bo;
> + goto err_unlock;
> }
>
> pages = kvmalloc_array(bo->base.base.size >> PAGE_SHIFT,
> @@ -481,9 +483,8 @@ static int panfrost_mmu_map_fault_addr(struct panfrost_device *pfdev, int as,
> if (!pages) {
> kvfree(bo->sgts);
> bo->sgts = NULL;
> - mutex_unlock(&bo->base.pages_lock);
> ret = -ENOMEM;
> - goto err_bo;
> + goto err_unlock;
> }
> bo->base.pages = pages;
> bo->base.pages_use_count = 1;
> @@ -491,7 +492,6 @@ static int panfrost_mmu_map_fault_addr(struct panfrost_device *pfdev, int as,
> pages = bo->base.pages;
> if (pages[page_offset]) {
> /* Pages are already mapped, bail out. */
> - mutex_unlock(&bo->base.pages_lock);
> goto out;
> }
> }
> @@ -502,14 +502,11 @@ static int panfrost_mmu_map_fault_addr(struct panfrost_device *pfdev, int as,
> for (i = page_offset; i < page_offset + NUM_FAULT_PAGES; i++) {
> pages[i] = shmem_read_mapping_page(mapping, i);
> if (IS_ERR(pages[i])) {
> - mutex_unlock(&bo->base.pages_lock);
> ret = PTR_ERR(pages[i]);
> goto err_pages;
> }
> }
>
> - mutex_unlock(&bo->base.pages_lock);
> -
> sgt = &bo->sgts[page_offset / (SZ_2M / PAGE_SIZE)];
> ret = sg_alloc_table_from_pages(sgt, pages + page_offset,
> NUM_FAULT_PAGES, 0, SZ_2M, GFP_KERNEL);
> @@ -528,6 +525,8 @@ static int panfrost_mmu_map_fault_addr(struct panfrost_device *pfdev, int as,
> dev_dbg(pfdev->dev, "mapped page fault @ AS%d %llx", as, addr);
>
> out:
> + dma_resv_unlock(obj->resv);
> +
> panfrost_gem_mapping_put(bomapping);
>
> return 0;
> @@ -536,6 +535,8 @@ static int panfrost_mmu_map_fault_addr(struct panfrost_device *pfdev, int as,
> sg_free_table(sgt);
> err_pages:
> drm_gem_shmem_put_pages(&bo->base);
> +err_unlock:
> + dma_resv_unlock(obj->resv);
> err_bo:
> panfrost_gem_mapping_put(bomapping);
> return ret;
> diff --git a/include/drm/drm_gem_shmem_helper.h b/include/drm/drm_gem_shmem_helper.h
> index 5994fed5e327..20ddcd799df9 100644
> --- a/include/drm/drm_gem_shmem_helper.h
> +++ b/include/drm/drm_gem_shmem_helper.h
> @@ -26,11 +26,6 @@ struct drm_gem_shmem_object {
> */
> struct drm_gem_object base;
>
> - /**
> - * @pages_lock: Protects the page table and use count
> - */
> - struct mutex pages_lock;
> -
> /**
> * @pages: Page table
> */
> @@ -65,11 +60,6 @@ struct drm_gem_shmem_object {
> */
> struct sg_table *sgt;
>
> - /**
> - * @vmap_lock: Protects the vmap address and use count
> - */
> - struct mutex vmap_lock;
> -
> /**
> * @vaddr: Kernel virtual address of the backing memory
> */
> @@ -109,7 +99,6 @@ struct drm_gem_shmem_object {
> struct drm_gem_shmem_object *drm_gem_shmem_create(struct drm_device *dev, size_t size);
> void drm_gem_shmem_free(struct drm_gem_shmem_object *shmem);
>
> -int drm_gem_shmem_get_pages(struct drm_gem_shmem_object *shmem);
> void drm_gem_shmem_put_pages(struct drm_gem_shmem_object *shmem);
> int drm_gem_shmem_pin(struct drm_gem_shmem_object *shmem);
> void drm_gem_shmem_unpin(struct drm_gem_shmem_object *shmem);
> @@ -128,8 +117,7 @@ static inline bool drm_gem_shmem_is_purgeable(struct drm_gem_shmem_object *shmem
> !shmem->base.dma_buf && !shmem->base.import_attach;
> }
>
> -void drm_gem_shmem_purge_locked(struct drm_gem_shmem_object *shmem);
> -bool drm_gem_shmem_purge(struct drm_gem_shmem_object *shmem);
> +void drm_gem_shmem_purge(struct drm_gem_shmem_object *shmem);
>
> struct sg_table *drm_gem_shmem_get_sg_table(struct drm_gem_shmem_object *shmem);
> struct sg_table *drm_gem_shmem_get_pages_sgt(struct drm_gem_shmem_object *shmem);
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
next prev parent reply other threads:[~2023-02-17 13:29 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-08 21:04 [PATCH v10 00/11] Add generic memory shrinker to VirtIO-GPU and Panfrost DRM drivers Dmitry Osipenko
2023-01-08 21:04 ` [PATCH v10 01/11] drm/msm/gem: Prevent blocking within shrinker loop Dmitry Osipenko
2023-02-17 12:02 ` Thomas Zimmermann
2023-02-27 4:27 ` Dmitry Osipenko
2023-01-08 21:04 ` [PATCH v10 02/11] drm/panfrost: Don't sync rpm suspension after mmu flushing Dmitry Osipenko
2023-01-08 21:04 ` [PATCH v10 03/11] drm/gem: Add evict() callback to drm_gem_object_funcs Dmitry Osipenko
2023-02-17 12:23 ` Thomas Zimmermann
2023-01-08 21:04 ` [PATCH v10 04/11] drm/shmem: Put booleans in the end of struct drm_gem_shmem_object Dmitry Osipenko
2023-02-17 12:25 ` Thomas Zimmermann
2023-01-08 21:04 ` [PATCH v10 05/11] drm/shmem: Switch to use drm_* debug helpers Dmitry Osipenko
2023-01-26 12:15 ` Gerd Hoffmann
2023-02-17 12:28 ` Thomas Zimmermann
2023-01-08 21:04 ` [PATCH v10 06/11] drm/shmem-helper: Don't use vmap_use_count for dma-bufs Dmitry Osipenko
2023-01-26 12:17 ` Gerd Hoffmann
2023-01-26 12:24 ` Dmitry Osipenko
2023-01-27 8:06 ` Gerd Hoffmann
2023-02-17 12:41 ` Thomas Zimmermann
2023-01-08 21:04 ` [PATCH v10 07/11] drm/shmem-helper: Switch to reservation lock Dmitry Osipenko
2023-02-17 12:52 ` Thomas Zimmermann
2023-02-17 13:33 ` Dmitry Osipenko
2023-02-17 13:29 ` Thomas Zimmermann [this message]
2023-01-08 21:04 ` [PATCH v10 08/11] drm/shmem-helper: Add memory shrinker Dmitry Osipenko
2023-02-17 13:19 ` Thomas Zimmermann
2023-02-27 4:34 ` Dmitry Osipenko
2023-01-08 21:04 ` [PATCH v10 09/11] drm/gem: Add drm_gem_pin_unlocked() Dmitry Osipenko
2023-02-17 13:42 ` Thomas Zimmermann
2023-01-08 21:04 ` [PATCH v10 10/11] drm/virtio: Support memory shrinking Dmitry Osipenko
2023-01-27 8:04 ` Gerd Hoffmann
2023-01-08 21:04 ` [PATCH v10 11/11] drm/panfrost: Switch to generic memory shrinker Dmitry Osipenko
2023-01-25 22:55 ` [PATCH v10 00/11] Add generic memory shrinker to VirtIO-GPU and Panfrost DRM drivers Dmitry Osipenko
2023-01-27 8:13 ` Gerd Hoffmann
2023-01-30 12:02 ` Dmitry Osipenko
2023-02-16 12:15 ` Daniel Vetter
2023-02-16 13:08 ` AngeloGioacchino Del Regno
2023-02-16 20:43 ` Dmitry Osipenko
2023-02-16 22:07 ` Daniel Vetter
2023-02-17 13:28 ` Thomas Zimmermann
2023-02-17 13:41 ` Dmitry Osipenko
2023-02-27 4:19 ` Dmitry Osipenko
2023-02-27 10:37 ` Jani Nikula
2023-02-27 11:00 ` Dmitry Osipenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c61fdee7-d474-6c52-912b-412eb6ad4a19@suse.de \
--to=tzimmermann@suse.de \
--cc=airlied@gmail.com \
--cc=alyssa.rosenzweig@collabora.com \
--cc=christian.koenig@amd.com \
--cc=daniel.almeida@collabora.com \
--cc=daniel@ffwll.ch \
--cc=daniel@fooishbar.org \
--cc=dmitry.baryshkov@linaro.org \
--cc=dmitry.osipenko@collabora.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=gurchetansingh@chromium.org \
--cc=gustavo.padovan@collabora.com \
--cc=kernel@collabora.com \
--cc=kraxel@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=maarten.lankhorst@linux.intel.com \
--cc=mripard@kernel.org \
--cc=olvaffe@gmail.com \
--cc=quic_abhinavk@quicinc.com \
--cc=robdclark@gmail.com \
--cc=robh@kernel.org \
--cc=sean@poorly.run \
--cc=steven.price@arm.com \
--cc=sumit.semwal@linaro.org \
--cc=tomeu.vizoso@collabora.com \
--cc=virtualization@lists.linux-foundation.org \
--cc=yuq825@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).