[PATCH 09/14] drm/exynos: remove call to drm_gem_free_mmap_offset()

Inki Dae inki.dae at samsung.com
Mon Aug 17 02:03:57 PDT 2015

On 2015년 08월 17일 17:17, Joonyoung Shim wrote:
> On 08/17/2015 04:52 PM, Inki Dae wrote:
>> On 2015년 08월 17일 14:29, Joonyoung Shim wrote:
>>> On 08/16/2015 02:07 PM, Inki Dae wrote:
>>>> On 2015년 07월 28일 17:53, Joonyoung Shim wrote:
>>>>> The drm_gem_object_release() function already performs this cleanup,
>>>>> so there is no reason to do it explicitly.
>>>>> Signed-off-by: Joonyoung Shim <jy0922.shim at samsung.com>
>>>>> ---
>>>>>  drivers/gpu/drm/exynos/exynos_drm_gem.c | 3 ---
>>>>>  1 file changed, 3 deletions(-)
>>>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>>>>> index c76aa8a..ab7d029 100644
>>>>> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
>>>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>>>>> @@ -100,8 +100,6 @@ out:
>>>>>  	exynos_drm_fini_buf(obj->dev, buf);
>>>>>  	exynos_gem_obj->buffer = NULL;
>>>>> -	drm_gem_free_mmap_offset(obj);
>>>>> -
>>>>>  	/* release file pointer to gem object. */
>>>>>  	drm_gem_object_release(obj);
>>>>> @@ -600,7 +598,6 @@ int exynos_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
>>>>>  err_close_vm:
>>>>>  	drm_gem_vm_close(vma);
>>>>> -	drm_gem_free_mmap_offset(obj);
>>>> Without previous patch, drm_gem_free_mmap_offset is required. I guess
>>>> the reason you removed above line is that you thought
>>>> drm_gem_object_release function would be called by drm_gem_vm_close
>>>> function which drops a reference of the gem object.
>>>> However, drm_gem_vm_close should be a pair with drm_gem_vm_open
>>>> function. These will be called whenever a process opens or closes the
>>>> VMA. So the reference count of the gem object had already been taken by
>>>> open operation when a new reference to the VMA had been created.
>>> This changes is not related with drm_gem_vm_close and prior patch. Why
>>> should free mmap offset when mmap operation is failed? The mmap offset
>>> can be used repeatedly.
>> Isn't vm space of vm manager still used even if any user-space process
>> doesn't use the region? And if mmap is failed, then the user-space
>> process will be terminated. Do you think it can be re-tried? However,
>> mmap system call never return -EAGAIN. Is it reasonable to you? I cannot
>> understand how the mmap offset can be re-used. So can you show me some
>> example?
> Currently, mmap offset of exynos-drm gem is made by                 
> DRM_IOCTL_MODE_MAP_DUMB ioctl and mmap() ioctl just uses the mmap     
> offset. User will use same mmap offset about same gem. It's why mmap
> offset made by DRM_IOCTL_MODE_MAP_DUMB ioctl is unnecessary, it's just
> enough to make mmap offset from when gem is create. You can get a
> reference from drm_gem_cma_helper.c file.

Hmm... It's not that the mmap offset can be re-used or not. It's that
the mmap offset should be released or not when mmap failed. As your
original comment, the call of drm_gem_free_mmap_offset() is unnecessary
if mmap offset is created when gem creation because the mmap offset is
removed by drm_gem_object_release() when gem is destroyed - gem should
also be released when mmap failed.

Ok, let's create mmap offset at gem creation and remove it gem
releasing. Will merge these two patches.

Inki Dae


More information about the dri-devel mailing list