From: "Christian König" <ckoenig.leichtzumerken@gmail.com>
To: "Felix Kuehling" <felix.kuehling@amd.com>,
"Christian König" <christian.koenig@amd.com>,
"Daniel Vetter" <daniel@ffwll.ch>,
"Bhardwaj, Rajneesh" <rajneesh.bhardwaj@amd.com>,
"Adrian Reber" <adrian@lisas.de>
Cc: daniel.vetter@ffwll.ch, amd-gfx@lists.freedesktop.org,
David Yat Sin <david.yatsin@amd.com>,
dri-devel@lists.freedesktop.org, alexander.deucher@amd.com,
airlied@redhat.com
Subject: Re: [PATCH] drm/ttm: Don't inherit GEM object VMAs in child process
Date: Wed, 5 Jan 2022 09:08:57 +0100 [thread overview]
Message-ID: <1ab2558b-1af0-3319-dce6-b805320a49d0@gmail.com> (raw)
In-Reply-To: <279c7ffc-99e5-f052-5de1-9b957c455d85@amd.com>
Am 04.01.22 um 19:08 schrieb Felix Kuehling:
> [+Adrian]
>
> Am 2021-12-23 um 2:05 a.m. schrieb Christian König:
>
>> Am 22.12.21 um 21:53 schrieb Daniel Vetter:
>>> On Mon, Dec 20, 2021 at 01:12:51PM -0500, Bhardwaj, Rajneesh wrote:
>>>
>>> [SNIP]
>>> Still sounds funky. I think minimally we should have an ack from CRIU
>>> developers that this is officially the right way to solve this
>>> problem. I
>>> really don't want to have random one-off hacks that don't work across
>>> the
>>> board, for a problem where we (drm subsystem) really shouldn't be the
>>> only
>>> one with this problem. Where "this problem" means that the mmap space is
>>> per file description, and not per underlying inode or real device or
>>> whatever. That part sounds like a CRIU problem, and I expect CRIU folks
>>> want a consistent solution across the board for this. Hence please
>>> grab an
>>> ack from them.
>> Unfortunately it's a KFD design problem. AMD used a single device
>> node, then mmaped different objects from the same offset to different
>> processes and expected it to work the rest of the fs subsystem without
>> churn.
> This may be true for mmaps in the KFD device, but not for mmaps in the
> DRM render nodes.
Correct, yes.
>> So yes, this is indeed because the mmap space is per file descriptor
>> for the use case here.
> No. This is a different problem.
I was already wondering which mmaps through the KFD node we have left
which cause problems here.
> The problem has to do with the way that DRM manages mmap permissions. In
> order to be able to mmap an offset in the render node, there needs to be
> a BO that was created in the same render node. If you fork a process, it
> inherits the VMA.
Yeah, so far it works like designed.
> But KFD doesn't know anything about the inherited BOs
> from the parent process.
Ok, why that? When the KFD is reinitializing it's context why shouldn't
it cleanup those VMAs?
> Therefore those BOs don't get checkpointed and
> restored in the child process. When the CRIU checkpoint is restored, our
> CRIU plugin never creates a BO corresponding to the VMA in the child
> process' render node FD. We've also lost the relationship between the
> parent and child-process' render node FDs. After "fork" the render node
> FD points to the same struct file in parent and child. After restoring
> the CRIU checkpoint, they are separate struct files, created by separate
> "open" system calls. Therefore the mmap call that restores the VMA fails
> in the child process.
>
> At least for KFD, there is no point inheriting BOs from a child process,
> because the GPU has no way of accessing the BOs in the child process.
> The child process has no GPU address space, no user mode queues, no way
> to do anything with the GPU before it completely reinitializes its KFD
> context.
>
> We can workaround this issue in user mode with madvise(...,
> MADV_DONTFORK). In fact we've already done this for some BOs to avoid a
> memory leak in the parent process while a child process exists. But it's
> slightly racy because there is a short time window where VMA exists
> without the VM_DONTCOPY flag. A fork during that time window could still
> create a child process with an inherited VMA.
>
> Therefore a safer solution is to set the vm_flags in the VMA in the
> driver when the VMA is first created.
Thanks for the full explanation, it makes much more sense now.
Regards,
Christian.
>
> Regards,
> Felix
>
>
>> And thanks for pointing this out, this indeed makes the whole change
>> extremely questionable.
>>
>> Regards,
>> Christian.
>>
>>> Cheers, Daniel
>>>
next prev parent reply other threads:[~2022-01-05 8:09 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-08 20:53 [PATCH] drm/ttm: Don't inherit GEM object VMAs in child process Rajneesh Bhardwaj
2021-12-09 7:54 ` Christian König
2021-12-09 15:23 ` Bhardwaj, Rajneesh
2021-12-09 15:27 ` Christian König
2021-12-09 15:29 ` Bhardwaj, Rajneesh
2021-12-09 15:30 ` Christian König
2021-12-09 18:28 ` Felix Kuehling
2021-12-10 6:58 ` Christian König
2021-12-20 9:29 ` Daniel Vetter
2021-12-20 18:12 ` Bhardwaj, Rajneesh
2021-12-22 20:53 ` Daniel Vetter
2021-12-23 1:49 ` Bhardwaj, Rajneesh
2021-12-23 1:51 ` Bhardwaj, Rajneesh
2021-12-23 7:05 ` Christian König
2022-01-04 18:08 ` Felix Kuehling
2022-01-05 8:08 ` Christian König [this message]
2022-01-05 16:16 ` Felix Kuehling
2022-01-05 16:27 ` Felix Kuehling
2022-01-06 9:05 ` Christian König
2022-01-06 16:45 ` Felix Kuehling
2022-01-06 16:48 ` Christian König
2022-01-06 16:51 ` Felix Kuehling
2022-01-07 8:56 ` Christian König
2022-01-07 17:47 ` Felix Kuehling
2022-01-14 16:44 ` Daniel Vetter
2022-01-14 17:26 ` Christian König
2022-01-14 17:40 ` Felix Kuehling
2022-01-17 11:44 ` Christian König
2022-01-17 14:17 ` Felix Kuehling
2022-01-17 14:21 ` Christian König
2022-01-17 14:34 ` Felix Kuehling
2022-01-17 14:50 ` Marek Olšák
2022-01-17 16:23 ` Christian König
2022-01-10 17:30 ` Bhardwaj, Rajneesh
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1ab2558b-1af0-3319-dce6-b805320a49d0@gmail.com \
--to=ckoenig.leichtzumerken@gmail.com \
--cc=adrian@lisas.de \
--cc=airlied@redhat.com \
--cc=alexander.deucher@amd.com \
--cc=amd-gfx@lists.freedesktop.org \
--cc=christian.koenig@amd.com \
--cc=daniel.vetter@ffwll.ch \
--cc=daniel@ffwll.ch \
--cc=david.yatsin@amd.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=felix.kuehling@amd.com \
--cc=rajneesh.bhardwaj@amd.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).