[RFC PATCH v2 4/5] drm, cgroup: Add total GEM buffer allocation limit
tj at kernel.org
Thu May 16 14:10:15 UTC 2019
I haven't gone through the patchset yet but some quick comments.
On Wed, May 15, 2019 at 10:29:21PM -0400, Kenny Ho wrote:
> Given this controller is specific to the drm kernel subsystem which
> uses minor to identify drm device, I don't see a need to complicate
> the interfaces more by having major and a key. As you can see in the
> examples below, the drm device minor corresponds to the line number.
> I am not sure how strict cgroup upstream is about the convention but I
We're pretty strict.
> am hoping there are flexibility here to allow for what I have
> implemented. There are a couple of other things I have done that is
So, please follow the interface conventions. We can definitely add
new ones but that would need functional reasons.
> not described in the convention: 1) inclusion of read-only *.help file
> at the root cgroup, 2) use read-only (which I can potentially make rw)
> *.default file instead of having a default entries (since the default
> can be different for different devices) inside the control files (this
> way, the resetting of cgroup values for all the drm devices, can be
> done by a simple 'cp'.)
Again, please follow the existing conventions. There's a lot more
harm than good in every controller being creative in their own way.
It's trivial to build convenience features in userspace. Please do it
> > Is this really useful for an administrator to control?
> > Isn't the resource we want to control actually the physical backing store?
> That's correct. This is just the first level of control since the
> backing store can be backed by different type of memory. I am in the
> process of adding at least two more resources. Stay tuned. I am
> doing the charge here to enforce the idea of "creator is deemed owner"
> at a place where the code is shared by all (the init function.)
Ideally, controller should only control hard resources which impact
behaviors and performance which are immediately visible to users.
More information about the amd-gfx