On Wed, Nov 13, 2019 at 02:56:12PM +0100, Gerd Hoffmann wrote:
> When mapping ttm objects via drm_gem_ttm_mmap() helper
> drm_gem_mmap_obj() will take an object reference. That gets
> never released due to ttm having its own reference counting.
>
> Fix that by dropping the gem object reference once the ttm mmap
> completed (and ttm refcount got bumped).
>
> For that to work properly the drm_gem_object_get() call in
> drm_gem_ttm_mmap() must be moved so it happens before calling
> obj->funcs->mmap(), otherwise the gem refcount would go down
> to zero.
>
> Fixes: 231927d939f0 ("drm/ttm: add drm_gem_ttm_mmap()")
Since the offending patch is in drm-next and we're in the merge window
freeze past -rc6 please remember to apply this to drm-misc-next-fixes.
Otherwise it'll miss the merge window.
> Signed-off-by: Gerd Hoffmann <kraxel@xxxxxxxxxx>
I was wondering whether we'd need a cc: stable, in case someone is really
fast and gets the vm_close in before we finish the mmap. But I think we
should be serialized by mmap_sem here enough ...
Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
> ---
> drivers/gpu/drm/drm_gem.c | 24 ++++++++++++++----------
> drivers/gpu/drm/drm_gem_ttm_helper.c | 13 ++++++++++++-
> 2 files changed, 26 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 2f2b889096b0..000fa4a1899f 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -1105,21 +1105,33 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
> if (obj_size < vma->vm_end - vma->vm_start)
> return -EINVAL;
>
> + /* Take a ref for this mapping of the object, so that the fault
> + * handler can dereference the mmap offset's pointer to the object.
> + * This reference is cleaned up by the corresponding vm_close
> + * (which should happen whether the vma was created by this call, or
> + * by a vm_open due to mremap or partial unmap or whatever).
> + */
> + drm_gem_object_get(obj);
> +
> if (obj->funcs && obj->funcs->mmap) {
> /* Remove the fake offset */
> vma->vm_pgoff -= drm_vma_node_start(&obj->vma_node);
>
> ret = obj->funcs->mmap(obj, vma);
> - if (ret)
> + if (ret) {
> + drm_gem_object_put_unlocked(obj);
> return ret;
> + }
> WARN_ON(!(vma->vm_flags & VM_DONTEXPAND));
> } else {
> if (obj->funcs && obj->funcs->vm_ops)
> vma->vm_ops = obj->funcs->vm_ops;
> else if (dev->driver->gem_vm_ops)
> vma->vm_ops = dev->driver->gem_vm_ops;
> - else
> + else {
> + drm_gem_object_put_unlocked(obj);
> return -EINVAL;
> + }
>
> vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP;
> vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
> @@ -1128,14 +1140,6 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
>
> vma->vm_private_data = obj;
>
> - /* Take a ref for this mapping of the object, so that the fault
> - * handler can dereference the mmap offset's pointer to the object.
> - * This reference is cleaned up by the corresponding vm_close
> - * (which should happen whether the vma was created by this call, or
> - * by a vm_open due to mremap or partial unmap or whatever).
> - */
> - drm_gem_object_get(obj);
> -
> return 0;
> }
> EXPORT_SYMBOL(drm_gem_mmap_obj);
> diff --git a/drivers/gpu/drm/drm_gem_ttm_helper.c b/drivers/gpu/drm/drm_gem_ttm_helper.c
> index 7412bfc5c05a..605a8a3da7f9 100644
> --- a/drivers/gpu/drm/drm_gem_ttm_helper.c
> +++ b/drivers/gpu/drm/drm_gem_ttm_helper.c
> @@ -64,8 +64,19 @@ int drm_gem_ttm_mmap(struct drm_gem_object *gem,
> struct vm_area_struct *vma)
> {
> struct ttm_buffer_object *bo = drm_gem_ttm_of_gem(gem);
> + int ret;
>
> - return ttm_bo_mmap_obj(vma, bo);
> + ret = ttm_bo_mmap_obj(vma, bo);
> + if (ret < 0)
> + return ret;
> +
> + /*
> + * ttm has its own object refcounting, so drop gem reference
> + * to avoid double accounting counting.
> + */
> + drm_gem_object_put_unlocked(gem);
> +
> + return 0;
> }
> EXPORT_SYMBOL(drm_gem_ttm_mmap);
>
> --
> 2.18.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
