------------------------------------------------------------------
发件人：Felix Kuehling <felix.kuehl...@amd.com>
发送时间：2024年7月9日(星期二) 06:40
收件人：周春明(日月) <riyue....@alibaba-inc.com>; Tvrtko Ursulin <tursu...@ursulin.net>;
dri-de...@lists.freedesktop.org <dri-de...@lists.freedesktop.org>;
amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>; Dave Airlie
<airl...@redhat.com>; Daniel Vetter <dan...@ffwll.ch>; criu <c...@openvz.org>
抄 送："Errabolu, Ramesh" <ramesh.errab...@amd.com>; "Christian König"
<christian.koe...@amd.com>
主 题：Re: Re：Proposal to add CRIU support to DRM render nodes
On 2024-07-08 2:51, 周春明(日月) wrote:
>
> Hi Felix,
>
> When I learn CRIU you introduced in
> https://github.com/checkpoint-restore/criu/tree/criu-dev/plugins/amdgpu
> <https://github.com/checkpoint-restore/criu/tree/criu-dev/plugins/amdgpu >
> <https://github.com/checkpoint-restore/criu/tree/criu-dev/plugins/amdgpu>
> <https://github.com/checkpoint-restore/criu/tree/criu-dev/plugins/amdgpu> > ,
> there is a sentence
> "ROCm manages memory in the form of buffer objects (BOs). We are also working
> on a new memory management API that will be based on virtual address
> ranges...",
> Out of curious, how about "new memory management based on virtual address
> ranges"? Any introduction for that?
>Hi David,
>This refers to the SVM API that has been in the upstream driver for a while
>now:
>https://elixir.bootlin.com/linux/v6.9.8/source/include/uapi/linux/kfd_ioctl.h#L732
>
><https://elixir.bootlin.com/linux/v6.9.8/source/include/uapi/linux/kfd_ioctl.h#L732
> >
[David] Can all ROCm runtime memory management switch to use svm apis? No need
BOs any more?
Thanks,
-David
Regards,
Felix
>
> Thanks,
> -David
>
> ------------------------------------------------------------------
> 发件人：Felix Kuehling <felix.kuehl...@amd.com>
> 发送时间：2024年5月3日(星期五) 22:44
> 收件人：Tvrtko Ursulin <tursu...@ursulin.net>; dri-de...@lists.freedesktop.org
> <dri-de...@lists.freedesktop.org>; amd-gfx@lists.freedesktop.org
> <amd-gfx@lists.freedesktop.org>; Dave Airlie <airl...@redhat.com>; Daniel
> Vetter <dan...@ffwll.ch>; criu <c...@openvz.org>
> 抄 送："Errabolu, Ramesh" <ramesh.errab...@amd.com>; "Christian König"
> <christian.koe...@amd.com>
> 主 题：Re: Proposal to add CRIU support to DRM render nodes
>
>
>
> On 2024-04-16 10:04, Tvrtko Ursulin wrote:
> >
> > On 01/04/2024 18:58, Felix Kuehling wrote:
> >>
> >> On 2024-04-01 12:56, Tvrtko Ursulin wrote:
> >>>
> >>> On 01/04/2024 17:37, Felix Kuehling wrote:
> >>>> On 2024-04-01 11:09, Tvrtko Ursulin wrote:
> >>>>>
> >>>>> On 28/03/2024 20:42, Felix Kuehling wrote:
> >>>>>>
> >>>>>> On 2024-03-28 12:03, Tvrtko Ursulin wrote:
> >>>>>>>
> >>>>>>> Hi Felix,
> >>>>>>>
> >>>>>>> I had one more thought while browsing around the amdgpu CRIU plugin.
> >>>>>>> It appears it relies on the KFD support being compiled in and
> >>>>>>> /dev/kfd present, correct? AFAICT at least, it relies on that to
> >>>>>>> figure out the amdgpu DRM node.
> >>>>>>>
> >>>>>>> In would be probably good to consider designing things without that
> >>>>>>> dependency. So that checkpointing an application which does not use
> >>>>>>> /dev/kfd is possible. Or if the kernel does not even have the KFD
> >>>>>>> support compiled in.
> >>>>>>
> >>>>>> Yeah, if we want to support graphics apps that don't use KFD, we
> >>>>>> should definitely do that. Currently we get a lot of topology
> >>>>>> information from KFD, not even from the /dev/kfd device but from the
> >>>>>> sysfs nodes exposed by KFD. We'd need to get GPU device info from the
> >>>>>> render nodes instead. And if KFD is available, we may need to
> >>>>>> integrate both sources of information.
> >>>>>>
> >>>>>>
> >>>>>>>
> >>>>>>> It could perhaps mean no more than adding some GPU discovery code
> >>>>>>> into CRIU. Which shuold be flexible enough to account for things like
> >>>>>>> re-assigned minor numbers due driver reload.
> >>>>>>
> >>>>>> Do you mean adding GPU discovery to the core CRIU, or to the plugin. I
> >>>>>> was thinking this is still part of the plugin.
> >>>>>
> >>>>> Yes I agree. I was only thinking about adding some DRM device discovery
> >>>>> code in a more decoupled fashion from the current plugin, for both the
> >>>>> reason discussed above (decoupling a bit from reliance on kfd sysfs),
> >>>>> and then also if/when a new DRM driver might want to implement this the
> >>>>> code could be move to some common plugin area.
> >>>>>
> >>>>> I am not sure how feasible that would be though. The "gpu id" concept
> >>>>> and it's matching in the current kernel code and CRIU plugin - is that
> >>>>> value tied to the physical GPU instance or how it works?
> >>>>
> >>>> The concept of the GPU ID is that it's stable while the system is up,
> >>>> even when devices get added and removed dynamically. It was baked into
> >>>> the API early on, but I don't think we ever fully validated device hot
> >>>> plug. I think the closest we're getting is with our latest MI GPUs and
> >>>> dynamic partition mode change.
> >>>
> >>> Doesn't it read the saved gpu id from the image file while doing restore
> >>> and tries to open the render node to match it? Maybe I am misreading the
> >>> code.. But if it does, does it imply that in practice it could be stable
> >>> across reboots? Or that it is not possible to restore to a different
> >>> instance of maybe the same GPU model installed in a system?
> >>
> >> Ah, the idea is, that when you restore on a different system, you may get
> >> different GPU IDs. Or you may checkpoint an app running on GPU 1 but
> >> restore it on GPU 2 on the same system. That's why we need to translate
> >> GPU IDs in restored applications. User mode still uses the old GPU IDs,
> >> but the kernel mode driver translates them to the actual GPU IDs of the
> >> GPUs that the process was restored on.
> >
> > I see.. I think. Normal flow is ppd->user_gpu_id set during client init,
> > but for restored clients it gets overriden during restore so that any
> > further ioctls can actually not instantly fail.
> >
> > And then in amdgpu_plugin_restore_file, when it is opening the render node,
> > it relies on the kfd topology to have filled in (more or less) the
> > target_gpu_id corresponding to the render node gpu id of the target GPU -
> > the one associated with the new kfd gpu_id?
>
> Yes.
>
> >
> > I am digging into this because I am trying to see if some part of GPU
> > discovery could somehow be decoupled.. to offer you to work on at least
> > that until you start to tackle the main body of the feature. But it looks
> > properly tangled up.
>
> OK. Most of the interesting plugin code should be in
> amdgpu_plugin_topology.c. It currently has some pretty complicated logic to
> find a set of devices that matches the topology in the checkpoint, including
> shader ISA versions, numbers of compute units, memory sizes, firmware
> versions and IO-Links between GPUs. This was originally done to support P2P
> with XGMI links. I'm not sure we ever updated it to properly support PCIe P2P.
>
>
> >
> > Do you have any suggestions with what I could help with? Maybe developing
> > some sort of drm device enumeration library if you see a way that would be
> > useful in decoupling the device discovery from kfd. We would need to define
> > what sort of information you would need to be queryable from it.
>
> Maybe. I think a lot of device information is available with some amdgpu
> info-ioctl. It may not cover all the things we're checking in the KFD
> topology, though.
>
> >
> >>>> This also highlights another aspect on those spatially partitioned GPUs.
> >>>> GPU IDs identify device partitions, not devices. Similarly, each
> >>>> partition has its own render node, and the KFD topology info in sysfs
> >>>> points to the render-minor number corresponding to each GPU ID.
> >>>
> >>> I am not familiar with this. This is not SR-IOV but some other kind of
> >>> partitioning? Would you have any links where I could read more?
> >>
> >> Right, the bare-metal driver can partition a PF spatially without SRIOV.
> >> SRIOV can also use spatial partitioning and expose each partition through
> >> its own VF, but that's not useful for bare metal. Spatial partitioning is
> >> new in MI300. There is some high-level info in this whitepaper:
> >> https://www.amd.com/content/dam/amd/en/documents/instinct-tech-docs/white-papers/amd-cdna-3-white-paper.pdf
> >>
> >> <https://www.amd.com/content/dam/amd/en/documents/instinct-tech-docs/white-papers/amd-cdna-3-white-paper.pdf
> >> >
> >> <https://www.amd.com/content/dam/amd/en/documents/instinct-tech-docs/white-papers/amd-cdna-3-white-paper.pdf>
> >>
> >> <https://www.amd.com/content/dam/amd/en/documents/instinct-tech-docs/white-papers/amd-cdna-3-white-paper.pdf>
> >> >.
> >
> > From the outside (userspace) this looks simply like multiple DRM render
> > nodes or how exactly?
>
> Yes, that's correct. Each partition has its own render node and its own node
> in the KFD topology.
>
> Regards,
> Felix
>
> >
> > Regards,
> >
> > Tvrtko
> >
> >>
> >> Regards,
> >> Felix
> >>
> >>
> >>>
> >>> Regards,
> >>>
> >>> Tvrtko
> >>>
> >>>>>>> Otherwise I am eagerly awaiting to hear more about the design
> >>>>>>> specifics around dma-buf handling. And also seeing how to extend to
> >>>>>>> other DRM related anonymous fds.
> >>>>>>
> >>>>>> I've been pretty far under-water lately. I hope I'll find time to work
> >>>>>> on this more, but it's probably going to be at least a few weeks.
> >>>>>
> >>>>> Got it.
> >>>>>
> >>>>> Regards,
> >>>>>
> >>>>> Tvrtko
> >>>>>
> >>>>>>
> >>>>>> Regards,
> >>>>>> Felix
> >>>>>>
> >>>>>>
> >>>>>>>
> >>>>>>> Regards,
> >>>>>>>
> >>>>>>> Tvrtko
> >>>>>>>
> >>>>>>> On 15/03/2024 18:36, Tvrtko Ursulin wrote:
> >>>>>>>>
> >>>>>>>> On 15/03/2024 02:33, Felix Kuehling wrote:
> >>>>>>>>>
> >>>>>>>>> On 2024-03-12 5:45, Tvrtko Ursulin wrote:
> >>>>>>>>>>
> >>>>>>>>>> On 11/03/2024 14:48, Tvrtko Ursulin wrote:
> >>>>>>>>>>>
> >>>>>>>>>>> Hi Felix,
> >>>>>>>>>>>
> >>>>>>>>>>> On 06/12/2023 21:23, Felix Kuehling wrote:
> >>>>>>>>>>>> Executive Summary: We need to add CRIU support to DRM render
> >>>>>>>>>>>> nodes in order to maintain CRIU support for ROCm application
> >>>>>>>>>>>> once they start relying on render nodes for more GPU memory
> >>>>>>>>>>>> management. In this email I'm providing some background why we
> >>>>>>>>>>>> are doing this, and outlining some of the problems we need to
> >>>>>>>>>>>> solve to checkpoint and restore render node state and shared
> >>>>>>>>>>>> memory (DMABuf) state. I have some thoughts on the API design,
> >>>>>>>>>>>> leaning on what we did for KFD, but would like to get feedback
> >>>>>>>>>>>> from the DRI community regarding that API and to what extent
> >>>>>>>>>>>> there is interest in making that generic.
> >>>>>>>>>>>>
> >>>>>>>>>>>> We are working on using DRM render nodes for virtual address
> >>>>>>>>>>>> mappings in ROCm applications to implement the CUDA11-style VM
> >>>>>>>>>>>> API and improve interoperability between graphics and compute.
> >>>>>>>>>>>> This uses DMABufs for sharing buffer objects between KFD and
> >>>>>>>>>>>> multiple render node devices, as well as between processes. In
> >>>>>>>>>>>> the long run this also provides a path to moving all or most
> >>>>>>>>>>>> memory management from the KFD ioctl API to libdrm.
> >>>>>>>>>>>>
> >>>>>>>>>>>> Once ROCm user mode starts using render nodes for virtual
> >>>>>>>>>>>> address management, that creates a problem for checkpointing and
> >>>>>>>>>>>> restoring ROCm applications with CRIU. Currently there is no
> >>>>>>>>>>>> support for checkpointing and restoring render node state, other
> >>>>>>>>>>>> than CPU virtual address mappings. Support will be needed for
> >>>>>>>>>>>> checkpointing GEM buffer objects and handles, their GPU virtual
> >>>>>>>>>>>> address mappings and memory sharing relationships between
> >>>>>>>>>>>> devices and processes.
> >>>>>>>>>>>>
> >>>>>>>>>>>> Eventually, if full CRIU support for graphics applications is
> >>>>>>>>>>>> desired, more state would need to be captured, including
> >>>>>>>>>>>> scheduler contexts and BO lists. Most of this state is
> >>>>>>>>>>>> driver-specific.
> >>>>>>>>>>>>
> >>>>>>>>>>>> After some internal discussions we decided to take our design
> >>>>>>>>>>>> process public as this potentially touches DRM GEM and DMABuf
> >>>>>>>>>>>> APIs and may have implications for other drivers in the future.
> >>>>>>>>>>>>
> >>>>>>>>>>>> One basic question before going into any API details: Is there a
> >>>>>>>>>>>> desire to have CRIU support for other DRM drivers?
> >>>>>>>>>>>
> >>>>>>>>>>> This sounds like a very interesting feature on the overall,
> >>>>>>>>>>> although I cannot answer on the last question here.
> >>>>>>>>>>
> >>>>>>>>>> I forgot to finish this thought. I cannot answer / don't know of
> >>>>>>>>>> any concrete plans, but I think feature is pretty cool and if
> >>>>>>>>>> amdgpu gets it working I wouldn't be surprised if other drivers
> >>>>>>>>>> would get interested.
> >>>>>>>>>
> >>>>>>>>> Thanks, that's good to hear!
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>> Funnily enough, it has a tiny relation to an i915 feature I
> >>>>>>>>>>> recently implemented on Mesa's request, which is to be able to
> >>>>>>>>>>> "upload" the GPU context from the GPU hang error state and replay
> >>>>>>>>>>> the hanging request. It is kind of (at a stretch) a very special
> >>>>>>>>>>> tiny subset of checkout and restore so I am not mentioning it as
> >>>>>>>>>>> a curiosity.
> >>>>>>>>>>>
> >>>>>>>>>>> And there is also another partical conceptual intersect with the
> >>>>>>>>>>> (at the moment not yet upstream) i915 online debugger. This part
> >>>>>>>>>>> being in the area of discovering and enumerating GPU resources
> >>>>>>>>>>> beloning to the client.
> >>>>>>>>>>>
> >>>>>>>>>>> I don't see an immediate design or code sharing opportunities
> >>>>>>>>>>> though but just mentioning.
> >>>>>>>>>>>
> >>>>>>>>>>> I did spend some time reading your plugin and kernel
> >>>>>>>>>>> implementation out of curiousity and have some comments and
> >>>>>>>>>>> questions.
> >>>>>>>>>>>
> >>>>>>>>>>>> With that out of the way, some considerations for a possible DRM
> >>>>>>>>>>>> CRIU API (either generic of AMDGPU driver specific): The API
> >>>>>>>>>>>> goes through several phases during checkpoint and restore:
> >>>>>>>>>>>>
> >>>>>>>>>>>> Checkpoint:
> >>>>>>>>>>>>
> >>>>>>>>>>>> 1. Process-info (enumerates objects and sizes so user mode can
> >>>>>>>>>>>> allocate
> >>>>>>>>>>>> memory for the checkpoint, stops execution on the GPU)
> >>>>>>>>>>>> 2. Checkpoint (store object metadata for BOs, queues, etc.)
> >>>>>>>>>>>> 3. Unpause (resumes execution after the checkpoint is complete)
> >>>>>>>>>>>>
> >>>>>>>>>>>> Restore:
> >>>>>>>>>>>>
> >>>>>>>>>>>> 1. Restore (restore objects, VMAs are not in the right place at
> >>>>>>>>>>>> this time)
> >>>>>>>>>>>> 2. Resume (final fixups after the VMAs are sorted out, resume
> >>>>>>>>>>>> execution)
> >>>>>>>>>>>
> >>>>>>>>>>> Btw is check-pointing guaranteeing all relevant activity is
> >>>>>>>>>>> idled? For instance dma_resv objects are free of fences which
> >>>>>>>>>>> would need to restored for things to continue executing sensibly?
> >>>>>>>>>>> Or how is that handled?
> >>>>>>>>>
> >>>>>>>>> In our compute use cases, we suspend user mode queues. This can
> >>>>>>>>> include CWSR (compute-wave-save-restore) where the state of
> >>>>>>>>> in-flight waves is stored in memory and can be reloaded and resumed
> >>>>>>>>> from memory later. We don't use any fences other than "eviction
> >>>>>>>>> fences", that are signaled after the queues are suspended. And
> >>>>>>>>> those fences are never handed to user mode. So we don't need to
> >>>>>>>>> worry about any fence state in the checkpoint.
> >>>>>>>>>
> >>>>>>>>> If we extended this to support the kernel mode command submission
> >>>>>>>>> APIs, I would expect that we'd wait for all current submissions to
> >>>>>>>>> complete, and stop new ones from being sent to the HW before taking
> >>>>>>>>> the checkpoint. When we take the checkpoint in the CRIU plugin, the
> >>>>>>>>> CPU threads are already frozen and cannot submit any more work. If
> >>>>>>>>> we wait for all currently pending submissions to drain, I think we
> >>>>>>>>> don't need to save any fence state because all the fences will have
> >>>>>>>>> signaled. (I may be missing some intricacies and I'm afraid it may
> >>>>>>>>> not be that simple in reality, but that's my opening bid. ;)
> >>>>>>>>
> >>>>>>>> It feels feasible to me too, for the normally behaving clients at
> >>>>>>>> least.
> >>>>>>>>
> >>>>>>>> Presumably, given that the whole checkpointing is not instant, it
> >>>>>>>> would be okay to wait a second or two longer for the in-progress
> >>>>>>>> submissions complete. After which kernel would need to prune all
> >>>>>>>> signalled fences from the respective container objects before
> >>>>>>>> checkpointing.
> >>>>>>>>
> >>>>>>>> For the "misbehaving" clients who have perhaps queued up too much
> >>>>>>>> work, either still in the scheduler with unsatisfied dependencies,
> >>>>>>>> or already submitted to the hardware and/or driver backend, is there
> >>>>>>>> a timeout concept in CRIU so it would be possible to say something
> >>>>>>>> like "try to checkpoint but if the kernel says no time period t then
> >>>>>>>> give up"?
> >>>>>>>>
> >>>>>>>>>>>> For some more background about our implementation in KFD, you
> >>>>>>>>>>>> can refer to this whitepaper:
> >>>>>>>>>>>> https://github.com/checkpoint-restore/criu/blob/criu-dev/plugins/amdgpu/README.md
> >>>>>>>>>>>>
> >>>>>>>>>>>> <https://github.com/checkpoint-restore/criu/blob/criu-dev/plugins/amdgpu/README.md
> >>>>>>>>>>>> >
> >>>>>>>>>>>> <https://github.com/checkpoint-restore/criu/blob/criu-dev/plugins/amdgpu/README.md>
> >>>>>>>>>>>>
> >>>>>>>>>>>> <https://github.com/checkpoint-restore/criu/blob/criu-dev/plugins/amdgpu/README.md>
> >>>>>>>>>>>> >
> >>>>>>>>>>>>
> >>>>>>>>>>>> Potential objections to a KFD-style CRIU API in DRM render
> >>>>>>>>>>>> nodes, I'll address each of them in more detail below:
> >>>>>>>>>>>>
> >>>>>>>>>>>> * Opaque information in the checkpoint data that user mode can't
> >>>>>>>>>>>> interpret or do anything with
> >>>>>>>>>>>> * A second API for creating objects (e.g. BOs) that is separate
> >>>>>>>>>>>> from
> >>>>>>>>>>>> the regular BO creation API
> >>>>>>>>>>>> * Kernel mode would need to be involved in restoring BO sharing
> >>>>>>>>>>>> relationships rather than replaying BO creation, export and
> >>>>>>>>>>>> import
> >>>>>>>>>>>> from user mode
> >>>>>>>>>>>>
> >>>>>>>>>>>> # Opaque information in the checkpoint
> >>>>>>>>>>>>
> >>>>>>>>>>>> This comes out of ABI compatibility considerations. Adding any
> >>>>>>>>>>>> new objects or attributes to the driver/HW state that needs to
> >>>>>>>>>>>> be checkpointed could potentially break the ABI of the CRIU
> >>>>>>>>>>>> checkpoint/restore ioctl if the plugin needs to parse that
> >>>>>>>>>>>> information. Therefore, much of the information in our KFD CRIU
> >>>>>>>>>>>> ioctl API is opaque. It is written by kernel mode in the
> >>>>>>>>>>>> checkpoint, it is consumed by kernel mode when restoring the
> >>>>>>>>>>>> checkpoint, but user mode doesn't care about the contents or
> >>>>>>>>>>>> binary layout, so there is no user mode ABI to break. This is
> >>>>>>>>>>>> how we were able to maintain CRIU support when we added the SVM
> >>>>>>>>>>>> API to KFD without changing the CRIU plugin and without breaking
> >>>>>>>>>>>> our ABI.
> >>>>>>>>>>>>
> >>>>>>>>>>>> Opaque information may also lend itself to API abstraction, if
> >>>>>>>>>>>> this becomes a generic DRM API with driver-specific callbacks
> >>>>>>>>>>>> that fill in HW-specific opaque data.
> >>>>>>>>>>>
> >>>>>>>>>>> This feels sound in principle to me. Fundamentally the state is
> >>>>>>>>>>> very hardware specfic, and/or driver version specific, so I don't
> >>>>>>>>>>> see anything could be gained in practice by making it much less
> >>>>>>>>>>> opaque. (Apart from making things more complicated.)
> >>>>>>>>>>>
> >>>>>>>>>>> I was however unsure of the current split of how you dump buffer
> >>>>>>>>>>> objects with some data in the defined bo structure, and some in
> >>>>>>>>>>> completely opaque private data. Is there a benefit to that split,
> >>>>>>>>>>> or maybe in other words, is there a benefit on having part
> >>>>>>>>>>> transparent and part opaque for buffer objects?
> >>>>>>>>>
> >>>>>>>>> Some of the buffer object state is needed by the plugin. E.g. the
> >>>>>>>>> size and mmap offset are needed to match VMAs with BOs. I'd have to
> >>>>>>>>> review the plugin in detail to prove that all the fields are, in
> >>>>>>>>> fact, needed by the plugin, but that was the intent. Anything that
> >>>>>>>>> the plugin doesn't need to know should be in the opaque data
> >>>>>>>>> structures.
> >>>>>>>>
> >>>>>>>> Right, got it.
> >>>>>>>>
> >>>>>>>> Would it make sense to make the opaque data in the same block as the
> >>>>>>>> defined one? I mean instead of separating the two in the binary
> >>>>>>>> image for instance have struct kfd_criu_bo_bucket have a trailing
> >>>>>>>> priv_data blob? Maybe it is too late now if the image format is not
> >>>>>>>> versioned or something.
> >>>>>>>>
> >>>>>>>>>>> To slightly touch upon the question of whether this could become
> >>>>>>>>>>> a generic DRM API. It feels it would be hard to do it from the
> >>>>>>>>>>> start. What sounds more feasible is if/when generic looking
> >>>>>>>>>>> helpers can be spotted while developing the RFC then potentially
> >>>>>>>>>>> structure the code they can easily be promoted to shared/common
> >>>>>>>>>>> at some future moment.
> >>>>>>>>>
> >>>>>>>>> Yes, that's how this usually goes, in my experience. Thanks for
> >>>>>>>>> confirming.
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>>> # Second API for creating objects
> >>>>>>>>>>>>
> >>>>>>>>>>>> Creating BOs and other objects when restoring a checkpoint needs
> >>>>>>>>>>>> more information than the usual BO alloc and similar APIs
> >>>>>>>>>>>> provide. For example, we need to restore BOs with the same GEM
> >>>>>>>>>>>> handles so that user mode can continue using those handles after
> >>>>>>>>>>>> resuming execution. If BOs are shared through DMABufs without
> >>>>>>>>>>>> dynamic attachment, we need to restore pinned BOs as pinned.
> >>>>>>>>>>>> Validation of virtual addresses and handling MMU notifiers must
> >>>>>>>>>>>> be suspended until the virtual address space is restored. For
> >>>>>>>>>>>> user mode queues we need to save and restore a lot of queue
> >>>>>>>>>>>> execution state so that execution can resume cleanly.
> >>>>>>>>>>>
> >>>>>>>>>>> This also sounds justified to me. Restore creating all internal
> >>>>>>>>>>> objects is definitely special and sounds better to add uapi to
> >>>>>>>>>>> create them directly with the correct properties, than to add
> >>>>>>>>>>> uapi to adjust internal properties after creation. And in case
> >>>>>>>>>>> you would always need some new uapi - so at least to adjust after
> >>>>>>>>>>> creation. At which point you may have both in one. Internally
> >>>>>>>>>>> implementation can be split or common, whatever makes sense for a
> >>>>>>>>>>> given object type, but new uapi definitely sounds is required.
> >>>>>>>>>>>> # Restoring buffer sharing relationships
> >>>>>>>>>>>>
> >>>>>>>>>>>> Different GEM handles in different render nodes and processes
> >>>>>>>>>>>> can refer to the same underlying shared memory, either by
> >>>>>>>>>>>> directly pointing to the same GEM object, or by creating an
> >>>>>>>>>>>> import attachment that may get its SG tables invalidated and
> >>>>>>>>>>>> updated dynamically through dynamic attachment callbacks. In the
> >>>>>>>>>>>> latter case it's obvious, who is the exporter and who is the
> >>>>>>>>>>>> importer. In the first case, either one could be the exporter,
> >>>>>>>>>>>> and it's not clear who would need to create the BO and who would
> >>>>>>>>>>>> need to
> >>>>>>>>>>>
> >>>>>>>>>>> To see if I follow the former case correctly.
> >>>>>>>>>>>
> >>>>>>>>>>> This could be two clients A and B, where B has imported a dma-buf
> >>>>>>>>>>> shared BO from A and has since closed the dma-buf fd? Which
> >>>>>>>>>>> results in a single BO with reference count of 2 and
> >>>>>>>>>>> obj->import_attach unset. History of who created the object is
> >>>>>>>>>>> lost.
> >>>>>>>>>
> >>>>>>>>> Yes. In the amdgpu driver this happens when the exporter and import
> >>>>>>>>> device are the same.
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>> In fact it could even be that two imported objects remain
> >>>>>>>>>>> (clients A, B and C) and A, who originally created the BO, has
> >>>>>>>>>>> since fully released it. So any kind of "creator" tracking if
> >>>>>>>>>>> added wouldn't be fully reliable either.
> >>>>>>>>>
> >>>>>>>>> That's a good point.
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>>> import it when restoring the checkpoint. To further complicate
> >>>>>>>>>>>> things, multiple processes in a checkpoint get restored
> >>>>>>>>>>>> concurrently. So there is no guarantee that an exporter has
> >>>>>>>>>>>> restored a shared BO at the time an importer is trying to
> >>>>>>>>>>>> restore its import.
> >>>>>>>>>>>>
> >>>>>>>>>>>> A proposal to deal with these problems would be to treat
> >>>>>>>>>>>> importers and exporters the same. Whoever restores first, ends
> >>>>>>>>>>>> up creating the BO and potentially attaching to it. The other
> >>>>>>>>>>>> process(es) can find BOs that were already restored by another
> >>>>>>>>>>>> process by looking it up with a unique ID that could be based on
> >>>>>>>>>>>> the DMABuf inode number. An alternative would be a two-pass
> >>>>>>>>>>>> approach that needs to restore BOs on two passes:
> >>>>>>>>>>>>
> >>>>>>>>>>>> 1. Restore exported BOs
> >>>>>>>>>>>> 2. Restore imports
> >>>>>>>>>>>>
> >>>>>>>>>>>> With some inter-process synchronization in CRIU itself between
> >>>>>>>>>>>> these two passes. This may require changes in the core CRIU,
> >>>>>>>>>>>> outside our plugin. Both approaches depend on identifying BOs
> >>>>>>>>>>>> with some unique ID that could be based on the DMABuf inode
> >>>>>>>>>>>> number in the checkpoint. However, we would need to identify the
> >>>>>>>>>>>> processes in the same restore session, possibly based on
> >>>>>>>>>>>> parent/child process relationships, to create a scope where
> >>>>>>>>>>>> those IDs are valid during restore.
> >>>>>>>>>>>
> >>>>>>>>>>> If my understanding above is on the right track, then I think
> >>>>>>>>>>> this is the only thing which can be done (for all scenarios).
> >>>>>>>>>
> >>>>>>>>> I presented two alternatives. I think you're in favor of the first
> >>>>>>>>> one, where it doesn't matter who is the importer and exporter. I
> >>>>>>>>> think the two-pass approach requires that you can identify an
> >>>>>>>>> exporter. And as you pointed out, the exporter may already have
> >>>>>>>>> dropped their reference to the BO.
> >>>>>>>>
> >>>>>>>> Yep.
> >>>>>>>>
> >>>>>>>>>>> I also *think* it would be safe. At least at the moment I cannot
> >>>>>>>>>>> think what could go wrong. Semantics are that it doesn't really
> >>>>>>>>>>> matter who created the object.
> >>>>>>>>>
> >>>>>>>>> I would agree. What matters is that the object is recreated on the
> >>>>>>>>> correct device, and that all the direct references and import
> >>>>>>>>> attachments pointing to it are restored.
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>>> Finally, we would also need to checkpoint and restore DMABuf
> >>>>>>>>>>>> file descriptors themselves. These are anonymous file
> >>>>>>>>>>>> descriptors. The CRIU plugin could probably be taught to
> >>>>>>>>>>>> recreate them from the original exported BO based on the inode
> >>>>>>>>>>>> number that could be queried with fstat in the checkpoint. It
> >>>>>>>>>>>> would need help from the render node CRIU API to find the right
> >>>>>>>>>>>> BO from the inode, which may be from a different process in the
> >>>>>>>>>>>> same restore session.
> >>>>>>>>>>>
> >>>>>>>>>>> This part feels like it is breaking the component separation a
> >>>>>>>>>>> bit because even for buffers fully owned by amdgpu, strictly
> >>>>>>>>>>> speaking the dma-buf fd is not. At least my understanding from
> >>>>>>>>>>> the above is that you propose to attempt to import the fd, from
> >>>>>>>>>>> the kernel side, during the checkpoint process? Like:
> >>>>>>>>>>>
> >>>>>>>>>>> Checkpoint:
> >>>>>>>>>>>
> >>>>>>>>>>> CRIU for each anon fd:
> >>>>>>>>>>> amdgpu_plugin(fd)
> >>>>>>>>>>> -> attempt in kernel dma buf import (passes fd to kernel via
> >>>>>>>>>>> ioctl?)
> >>>>>>>>>>> -> is it ours? (no -> error)
> >>>>>>>>>>> -> create a record mapping fd number to amdgpu BO
> >>>>>>>>>>>
> >>>>>>>>>>> Restore:
> >>>>>>>>>>>
> >>>>>>>>>>> for each dma-buf fd record:
> >>>>>>>>>>> create BO if does not exists
> >>>>>>>>>>> export BO to same fd
> >>>>>>>>>>> close BO handle if not in regular BO handle records
> >>>>>>>>>>>
> >>>>>>>>>>> Or since you mention lookup by inode, that would need to be
> >>>>>>>>>>> dmabuf_plugin so it can lookup inodes in the private mount space.
> >>>>>>>>>>> However how would it co-operate with amdgpu_plugin is not clear
> >>>>>>>>>>> to me.
> >>>>>>>>>
> >>>>>>>>> The way I think about the ownership is, whichever driver created
> >>>>>>>>> the underlying BO owns the checkpointing of the dmabuf. You need
> >>>>>>>>> driver-specific information to link the dmabuf with the driver's BO
> >>>>>>>>> and you need the right driver to recreate the BO and the dmabuf fd
> >>>>>>>>> when restoring the checkpoint.
> >>>>>>>>>
> >>>>>>>>> It gets really interesting if you have an amdgpu plugin and an i915
> >>>>>>>>> plugin, and they checkpoint an application that shares BOs between
> >>>>>>>>> the two devices through DMABufs. E.g. if i915 created a BO and
> >>>>>>>>> amdgpu imported it, then during restore, i915 needs to restore the
> >>>>>>>>> dmabuf before the amdgpu import of it can be restored. I think that
> >>>>>>>>> brings us back to a two-phase approach to restoring the memory
> >>>>>>>>> sharing relationships. Uff.
> >>>>>>>>
> >>>>>>>> I think this part of the discussion somewhat depends on the previous
> >>>>>>>> part about idling. If it is feasible to completely idle and prune,
> >>>>>>>> and fail if that is not happening quickly enough, then maybe there
> >>>>>>>> wouldn't be too much hierarchical state to save.
> >>>>>>>>
> >>>>>>>> Otherwise my idea was that there is a top-level drm_plugin.so which
> >>>>>>>> understands amdgpu fds, i915, syncobj, sync_file, and uses some new
> >>>>>>>> uapi to uniquely identify each, associate with the correct driver,
> >>>>>>>> and then internally dispatches to amdgpu|i915|dmabuf|..._plugin.so.
> >>>>>>>> Building the in memory representation of their relationships. As
> >>>>>>>> long as all objects and their relationships have been recorded I
> >>>>>>>> think everything could then be correctly restored.
> >>>>>>>>
> >>>>>>>> It is possible there is flaw in my thinking and that something in
> >>>>>>>> CRIU design would make this impossible? I think it would require the
> >>>>>>>> top-level drm_plugin.so to hold all state in memory until the whole
> >>>>>>>> checkpointing is done, and then verify something is not incomplete,
> >>>>>>>> failing it all if it was. (For instance one plugin discovered an
> >>>>>>>> reference to an object which was not discoverd by any other plugin
> >>>>>>>> or things like that.) May need some further tweaks to CRIU common
> >>>>>>>> code.
> >>>>>>>>
> >>>>>>>> Maybe I need to better understand how exactly you mean to query the
> >>>>>>>> DRM driver about random anonymous fds. I see it as a problem in the
> >>>>>>>> design, possibly even implementation, but maybe I am missing
> >>>>>>>> something which makes it not so. I mean even with my general idea I
> >>>>>>>> don't know how would one determine which driver to query about a
> >>>>>>>> particular anonymous inode.
> >>>>>>>>
> >>>>>>>>>> I later also realised that I was maybe increasing the scope for
> >>>>>>>>>> you here. :) You did state focus is ROCm applications which
> >>>>>>>>>> possibly doesn't care about dma-resv, fences, syncobjs etc?
> >>>>>>>>>
> >>>>>>>>> That's my focus for now. But I don't want to engineer a solution
> >>>>>>>>> that would preclude your use cases in the future.
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> But I think the "how to handle dma-bufs" design question is still
> >>>>>>>>>> relevant and interesting. For example I had this thought that
> >>>>>>>>>> perhaps what would be needed is a CRIU plugin hierarchy.
> >>>>>>>>>>
> >>>>>>>>>> Because fundamentally we would be snapshoting a hierarcy of kernel
> >>>>>>>>>> objects belonging to different drivers (kfd, amdgpu, dma-buf,
> >>>>>>>>>> ...). And if one day someone would to try to handle dma fences and
> >>>>>>>>>> drm syncobjs, the argument for a hierarchial design would be even
> >>>>>>>>>> stronger I think.
> >>>>>>>>>>
> >>>>>>>>>> Something like a drm_plugin.so could call sub-plugins (amdgpu,
> >>>>>>>>>> dma-buf, sync file, ...) and internally build the representation
> >>>>>>>>>> of the whole state and how the relationship between the objects.
> >>>>>>>>>
> >>>>>>>>> Maybe. I guess a structure similar to libdrm makes sense. I'm not
> >>>>>>>>> sure it's strictly a hierarchy. Maybe more like some common code
> >>>>>>>>> shared by multiple GPU driver plugins. I think the common
> >>>>>>>>> checkpoint state is quite limited and restoring it requires the
> >>>>>>>>> GPU-specific drivers anyway.
> >>>>>>>>>
> >>>>>>>>> Also the idea of building a representation of the whole state
> >>>>>>>>> doesn't work well with the CRIU design, because "the whole state"
> >>>>>>>>> can include multiple processes that restore themselves concurrently
> >>>>>>>>> and only synchronize with each other in a few places in the restore
> >>>>>>>>> process. I feel, if we can work out how to checkpoint and restore
> >>>>>>>>> shared objects between processes, we can do the same for shared
> >>>>>>>>> objects between drivers without imposing a strict hierarchy and
> >>>>>>>>> some omniscient entity that needs to know "the whole state".
> >>>>>>>>
> >>>>>>>> Okay, this continues on the same problem space as above. And you
> >>>>>>>> obviously know how CRIU works much better than me.
> >>>>>>>>
> >>>>>>>>>> With that kind of design there probably would be a need to define
> >>>>>>>>>> some common kernel side api and uapi, so all involved objects can
> >>>>>>>>>> be enumerated with some unique ids etc.
> >>>>>>>>>>
> >>>>>>>>>> Now.. the counter argument.. the more state from different drivers
> >>>>>>>>>> would one want to handle the bigger this project would get. Would
> >>>>>>>>>> it even be feasible is the question, to the point that it may be
> >>>>>>>>>> simpler to just run the workload in a VM via SR-IOV and simply
> >>>>>>>>>> hibernate the whole thin guest. :)
> >>>>>>>>>
> >>>>>>>>> Well, CRIU kind of tries to do that, but with containers instead of
> >>>>>>>>> VMs. ;)
> >>>>>>>>
> >>>>>>>> It would definitely be useful for hardware and drivers without
> >>>>>>>> SR-IOV support so lets hope it is doable. :)
> >>>>>>>>
> >>>>>>>> Regards,
> >>>>>>>>
> >>>>>>>> Tvrtko
