On 2015년 09월 24일 10:01, Joonyoung Shim wrote:
Hi Inki,
On 08/17/2015 06:03 PM, Inki Dae wrote:
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@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.
I can't find them from your git. Could you merge them?
Oops, sorry. Merged.
Thanks, Inki Dae
Thanks.