On 09/25/2015 06:15 PM, Inki Dae wrote: > 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@xxxxxxxxxxx> >>>>>>>> --- >>>>>>>> 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 for merge, but you are missing the first patch still, http://www.spinics.net/lists/dri-devel/msg86891.html Thanks. _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
