Re: [RFC PATCH 0/4] Add support for DRM cgroup memory accounting.
Hey, On 2023-05-10 20:46, Tejun Heo wrote: > Hello, > > On Wed, May 10, 2023 at 04:59:01PM +0200, Maarten Lankhorst wrote: >> The misc controller is not granular enough. A single computer may have any >> number of >> graphics cards, some of them with multiple regions of vram inside a single >> card. > Extending the misc controller to support dynamic keys shouldn't be that > difficult. > > ... >> In the next version, I will move all the code for handling the resource >> limit to >> TTM's eviction layer, because otherwise it cannot handle the resource limit >> correctly. >> >> The effect of moving the code to TTM, is that it will make the code even >> more generic >> for drivers that have vram and use TTM. When using TTM, you only have to >> describe your >> VRAM, update some fields in the TTM manager and (un)register your device >> with the >> cgroup handler on (un)load. It's quite trivial to add vram accounting to >> amdgpu and >> nouveau. [2] >> >> If you want to add a knob for scheduling weight for a process, it makes >> sense to >> also add resource usage as a knob, otherwise the effect of that knob is very >> limited. So even for Tvrtko's original proposed usecase, it would make sense. > It does make sense but unlike Tvrtko's scheduling weights what's being > proposed doesn't seem to encapsulate GPU memory resource in a generic enough > manner at least to my untrained eyes. ie. w/ drm.weight, I don't need any > specific knoweldge of how a specific GPU operates to say "this guy should > get 2x processing power over that guy". This more or less holds for other > major resources including CPU, memory and IO. What you're proposing seems a > lot more tied to hardware details and users would have to know a lot more > about how memory is configured on that particular GPU. There's not much need of knowing the specifics of a card, but there might be a need of knowing the workload to determine what allocation limits to set. I've left region to be implementation specific, but it would make sense to standardise it. TTM, the layer used by drivers that support VRAM, have the following regions: * sysmem - All system memory allocated; includes evicted VRAM. * mapped - All physical system memory that is mapped to the GPU, when unbound moves to sysmem. When evicting VRAM to sysmem, it's temporarily mapped here. * vramN - All VRAM regions of the device. * driver specific regions - probably doesn't make sense to put in cgroup at all, this includes stolen from the PoC. That leaves the question, what regions would make sense for a cgroup? Since vramN can be moved to mapped and sysmem (VRAM eviction, suspend/resume, driver_madvise), it becomes a subject of debate if we should include the other regions, since things become complicated fast. For the first iteration, I focus on a single category, vramN. Even when not knowing anything about a GPU, it will be easy to partition its memory like that. If you can assign a weight for the scheduler, then you can also partition it's vram by parsing /drm.capacity for total amount, and then splitting it across cgroups. > Now, if this is inherent to how all, or at least most, GPUs operate, sure, > but otherwise let's start small in terms of interface and not take up space > which should be for something universal. If this turns out to be the way, > expanding to take up the generic interface space isn't difficult. > > I don't know GPU space so please educate me where I'm wrong. Most GPU's have dedicated vram that works roughly in the same way, some integrated chips like i915 or arm use shared memory from the host system only. I would say amd, nvidia and intel's chips with dedicated memory work roughly in the same way for vram. I hope this explains it a little bit more, ~Maarten
Re: [RFC PATCH 0/4] Add support for DRM cgroup memory accounting.
Hello, On Wed, May 10, 2023 at 04:59:01PM +0200, Maarten Lankhorst wrote: > The misc controller is not granular enough. A single computer may have any > number of > graphics cards, some of them with multiple regions of vram inside a single > card. Extending the misc controller to support dynamic keys shouldn't be that difficult. ... > In the next version, I will move all the code for handling the resource limit > to > TTM's eviction layer, because otherwise it cannot handle the resource limit > correctly. > > The effect of moving the code to TTM, is that it will make the code even more > generic > for drivers that have vram and use TTM. When using TTM, you only have to > describe your > VRAM, update some fields in the TTM manager and (un)register your device with > the > cgroup handler on (un)load. It's quite trivial to add vram accounting to > amdgpu and > nouveau. [2] > > If you want to add a knob for scheduling weight for a process, it makes sense > to > also add resource usage as a knob, otherwise the effect of that knob is very > limited. So even for Tvrtko's original proposed usecase, it would make sense. It does make sense but unlike Tvrtko's scheduling weights what's being proposed doesn't seem to encapsulate GPU memory resource in a generic enough manner at least to my untrained eyes. ie. w/ drm.weight, I don't need any specific knoweldge of how a specific GPU operates to say "this guy should get 2x processing power over that guy". This more or less holds for other major resources including CPU, memory and IO. What you're proposing seems a lot more tied to hardware details and users would have to know a lot more about how memory is configured on that particular GPU. Now, if this is inherent to how all, or at least most, GPUs operate, sure, but otherwise let's start small in terms of interface and not take up space which should be for something universal. If this turns out to be the way, expanding to take up the generic interface space isn't difficult. I don't know GPU space so please educate me where I'm wrong. Thanks. -- tejun
Re: [RFC PATCH 0/4] Add support for DRM cgroup memory accounting.
Hey, On 2023-05-05 21:50, Tejun Heo wrote: Hello, On Wed, May 03, 2023 at 10:34:56AM +0200, Maarten Lankhorst wrote: RFC as I'm looking for comments. For long running compute, it can be beneficial to partition the GPU memory between cgroups, so each cgroup can use its maximum amount of memory without interfering with other scheduled jobs. Done properly, this can alleviate the need for eviction, which might result in a job being terminated if the GPU doesn't support mid-thread preemption or recoverable page faults. This is done by adding a bunch of knobs to cgroup: drm.capacity: Shows maximum capacity of each resource region. drm.max: Display or limit max amount of memory. drm.current: Current amount of memory in use. TTM has not been made cgroup aware yet, so instead of evicting from the current cgroup to stay within the cgroup limits, it simply returns the error -ENOSPC to userspace. I've used Tvrtko's cgroup controller series as a base, but it implemented scheduling weight, not memory accounting, so I only ended up keeping the base patch. Xe is not upstream yet, so the driver specific patch will only apply on https://gitlab.freedesktop.org/drm/xe/kernel Some high-level feedbacks. * There have been multiple attempts at this but the track record is kinda poor. People don't seem to agree what should constitute DRM memory and how they should be accounted / controlled. Thanks for the feedback. I think for a lot of drivers, what is VRAM might have different meaning, but the intention is it being accounted in the same way. Most drivers use TTM, which has a standard way of allocating memory, and a standard way of evicting VRAM. This makes it very useful for the usecase which I'm looking at, long running compute. When you have long running jobs, you don't want them to be interrupted because a completely unrelated process needs some VRAM, and one of the compute jobs buffers are being evicted. Some hardware does not support mid-thread preemption or page fault recovery, this means that when memory is evicted, the compute job is terminated. The full problem statement is in drm-compute.rst in the memory accounting patch. * I like Tvrtko's scheduling patchset because it exposes a generic interface which makes sense regardless of hardware details and then each driver can implement the configured control in whatever way they can. However, even for that, there doesn't seem much buy-in from other drivers. Yeah, that is correct. But it tries to solve a different part of the problem. * This proposal seems narrowly scoped trying to solve a specific problem which may not translate to different hardware configurations. Please let me know if I got that wrong, but if that's the case, I think a better and easier approach might be just being a part of the misc controller. That doesn't require much extra code and should be able to provide everything necessary for statically limiting specific resources. The misc controller is not granular enough. A single computer may have any number of graphics cards, some of them with multiple regions of vram inside a single card. For compute and shared hosting you might want to limit the usage of a single memory region on a single card, and then limit the same limits for the rest too, to prevent triggering eviction. The current version doesn't handle eviction correctly, because I was still working on it and I wanted to post a RFC. As a result, the case where resource limit is hit will evict the device's entire memory or get stuck in a loop. With some changes, the next version will not have this bug. This results in a few changes to the core code. [1] In the next version, I will move all the code for handling the resource limit to TTM's eviction layer, because otherwise it cannot handle the resource limit correctly. The effect of moving the code to TTM, is that it will make the code even more generic for drivers that have vram and use TTM. When using TTM, you only have to describe your VRAM, update some fields in the TTM manager and (un)register your device with the cgroup handler on (un)load. It's quite trivial to add vram accounting to amdgpu and nouveau. [2] If you want to add a knob for scheduling weight for a process, it makes sense to also add resource usage as a knob, otherwise the effect of that knob is very limited. So even for Tvrtko's original proposed usecase, it would make sense. Cheers, ~Maarten [1] Compared to this version: static inline int drmcg_try_charge(struct drmcgroup_state **drmcs, + struct drmcgroup_state **limitcs, struct drmcgroup_device *cgdev, u32 index, u64 size) This now returns which cgroup's limit is hit on -EAGAIN. +bool drmcs_grouped(struct drmcgroup_state *limitcs, + struct drmcgroup_state *testcs); Tells if testcs is the same as limitcs, or a subgroup
Re: [RFC PATCH 0/4] Add support for DRM cgroup memory accounting.
Hello, On Wed, May 03, 2023 at 10:34:56AM +0200, Maarten Lankhorst wrote: > RFC as I'm looking for comments. > > For long running compute, it can be beneficial to partition the GPU memory > between cgroups, so each cgroup can use its maximum amount of memory without > interfering with other scheduled jobs. Done properly, this can alleviate the > need for eviction, which might result in a job being terminated if the GPU > doesn't support mid-thread preemption or recoverable page faults. > > This is done by adding a bunch of knobs to cgroup: > drm.capacity: Shows maximum capacity of each resource region. > drm.max: Display or limit max amount of memory. > drm.current: Current amount of memory in use. > > TTM has not been made cgroup aware yet, so instead of evicting from > the current cgroup to stay within the cgroup limits, it simply returns > the error -ENOSPC to userspace. > > I've used Tvrtko's cgroup controller series as a base, but it implemented > scheduling weight, not memory accounting, so I only ended up keeping the > base patch. > > Xe is not upstream yet, so the driver specific patch will only apply on > https://gitlab.freedesktop.org/drm/xe/kernel Some high-level feedbacks. * There have been multiple attempts at this but the track record is kinda poor. People don't seem to agree what should constitute DRM memory and how they should be accounted / controlled. * I like Tvrtko's scheduling patchset because it exposes a generic interface which makes sense regardless of hardware details and then each driver can implement the configured control in whatever way they can. However, even for that, there doesn't seem much buy-in from other drivers. * This proposal seems narrowly scoped trying to solve a specific problem which may not translate to different hardware configurations. Please let me know if I got that wrong, but if that's the case, I think a better and easier approach might be just being a part of the misc controller. That doesn't require much extra code and should be able to provide everything necessary for statically limiting specific resources. Thanks. -- tejun
