[PATCH] drm/ttm: Don't inherit GEM object VMAs in child process
Christian König
christian.koenig at amd.com
Thu Dec 9 15:30:51 UTC 2021
That still won't work.
But I think we could do this change for the amdgpu mmap callback only.
Regards,
Christian.
Am 09.12.21 um 16:29 schrieb Bhardwaj, Rajneesh:
> Sounds good. I will send a v2 with only ttm_bo_mmap_obj change. Thank
> you!
>
> On 12/9/2021 10:27 AM, Christian König wrote:
>> Hi Rajneesh,
>>
>> yes, separating this from the drm_gem_mmap_obj() change is certainly
>> a good idea.
>>
>>> The child cannot access the BOs mapped by the parent anyway with
>>> access restrictions applied
>>
>> exactly that is not correct. That behavior is actively used by some
>> userspace stacks as far as I know.
>>
>> Regards,
>> Christian.
>>
>> Am 09.12.21 um 16:23 schrieb Bhardwaj, Rajneesh:
>>> Thanks Christian. Would it make it less intrusive if I just use the
>>> flag for ttm bo mmap and remove the drm_gem_mmap_obj change from
>>> this patch? For our use case, just the ttm_bo_mmap_obj change should
>>> suffice and we don't want to put any more work arounds in the user
>>> space (thunk, in our case).
>>>
>>> The child cannot access the BOs mapped by the parent anyway with
>>> access restrictions applied so I wonder why even inherit the vma?
>>>
>>> On 12/9/2021 2:54 AM, Christian König wrote:
>>>> Am 08.12.21 um 21:53 schrieb Rajneesh Bhardwaj:
>>>>> When an application having open file access to a node forks, its
>>>>> shared
>>>>> mappings also get reflected in the address space of child process
>>>>> even
>>>>> though it cannot access them with the object permissions applied.
>>>>> With the
>>>>> existing permission checks on the gem objects, it might be
>>>>> reasonable to
>>>>> also create the VMAs with VM_DONTCOPY flag so a user space
>>>>> application
>>>>> doesn't need to explicitly call the madvise(addr, len, MADV_DONTFORK)
>>>>> system call to prevent the pages in the mapped range to appear in the
>>>>> address space of the child process. It also prevents the memory leaks
>>>>> due to additional reference counts on the mapped BOs in the child
>>>>> process that prevented freeing the memory in the parent for which
>>>>> we had
>>>>> worked around earlier in the user space inside the thunk library.
>>>>>
>>>>> Additionally, we faced this issue when using CRIU to checkpoint
>>>>> restore
>>>>> an application that had such inherited mappings in the child which
>>>>> confuse CRIU when it mmaps on restore. Having this flag set for the
>>>>> render node VMAs helps. VMAs mapped via KFD already take care of
>>>>> this so
>>>>> this is needed only for the render nodes.
>>>>
>>>> Unfortunately that is most likely a NAK. We already tried something
>>>> similar.
>>>>
>>>> While it is illegal by the OpenGL specification and doesn't work
>>>> for most userspace stacks, we do have some implementations which
>>>> call fork() with a GL context open and expect it to work.
>>>>
>>>> Regards,
>>>> Christian.
>>>>
>>>>>
>>>>> Cc: Felix Kuehling <Felix.Kuehling at amd.com>
>>>>>
>>>>> Signed-off-by: David Yat Sin <david.yatsin at amd.com>
>>>>> Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhardwaj at amd.com>
>>>>> ---
>>>>> drivers/gpu/drm/drm_gem.c | 3 ++-
>>>>> drivers/gpu/drm/ttm/ttm_bo_vm.c | 2 +-
>>>>> 2 files changed, 3 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
>>>>> index 09c820045859..d9c4149f36dd 100644
>>>>> --- a/drivers/gpu/drm/drm_gem.c
>>>>> +++ b/drivers/gpu/drm/drm_gem.c
>>>>> @@ -1058,7 +1058,8 @@ int drm_gem_mmap_obj(struct drm_gem_object
>>>>> *obj, unsigned long obj_size,
>>>>> goto err_drm_gem_object_put;
>>>>> }
>>>>> - vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND |
>>>>> VM_DONTDUMP;
>>>>> + vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND
>>>>> + | VM_DONTDUMP | VM_DONTCOPY;
>>>>> vma->vm_page_prot =
>>>>> pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
>>>>> vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
>>>>> }
>>>>> diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c
>>>>> b/drivers/gpu/drm/ttm/ttm_bo_vm.c
>>>>> index 33680c94127c..420a4898fdd2 100644
>>>>> --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
>>>>> +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
>>>>> @@ -566,7 +566,7 @@ int ttm_bo_mmap_obj(struct vm_area_struct
>>>>> *vma, struct ttm_buffer_object *bo)
>>>>> vma->vm_private_data = bo;
>>>>> - vma->vm_flags |= VM_PFNMAP;
>>>>> + vma->vm_flags |= VM_PFNMAP | VM_DONTCOPY;
>>>>> vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
>>>>> return 0;
>>>>> }
>>>>
>>
More information about the amd-gfx
mailing list