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. > > 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 >>>>>>>>>>>> >>>>>>>>>>>> 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
