[RFC PATCH v2 4/5] drm, cgroup: Add total GEM buffer allocation limit

[Christian König]

Am 16.05.19 um 16:03 schrieb Kenny Ho:
> On Thu, May 16, 2019 at 3:25 AM Christian König
> <ckoenig.leichtzumerken at gmail.com> wrote:
>> Am 16.05.19 um 09:16 schrieb Koenig, Christian:
>>> Am 16.05.19 um 04:29 schrieb Kenny Ho:
>>>> On Wed, May 15, 2019 at 5:26 PM Welty, Brian <brian.welty at intel.com> wrote:
>>>>> On 5/9/2019 2:04 PM, Kenny Ho wrote:
>>>>>> Each file is multi-lined with one entry/line per drm device.
>>>>> Multi-line is correct for multiple devices, but I believe you need
>>>>> to use a KEY to denote device for both your set and get routines.
>>>>> I didn't see your set functions reading a key, or the get functions
>>>>> printing the key in output.
>>>>> cgroups-v2 conventions mention using KEY of major:minor, but I think
>>>>> you can use drm_minor as key?
>>>> Given this controller is specific to the drm kernel subsystem which
>>>> uses minor to identify drm device,
>>> Wait a second, using the DRM minor is a good idea in the first place.
>> Well that should have read "is not a good idea"..
>>
>> I have a test system with a Vega10 and a Vega20. Which device gets which
>> minor is not stable, but rather defined by the scan order of the PCIe bus.
>>
>> Normally the scan order is always the same, but adding or removing
>> devices or delaying things just a little bit during init is enough to
>> change this.
>>
>> We need something like the Linux sysfs location or similar to have a
>> stable implementation.
> I get that, which is why I don't use minor to identify cards in user
> space apps I wrote:
> https://github.com/RadeonOpenCompute/k8s-device-plugin/blob/c2659c9d1d0713cad36fb5256681125121e6e32f/internal/pkg/amdgpu/amdgpu.go#L85

Yeah, that is certainly a possibility.

> But within the kernel, I think my use of minor is consistent with the
> rest of the drm subsystem.  I hope I don't need to reform the way the
> drm subsystem use minor in order to introduce a cgroup controller.

Well I would try to avoid using the minor and at least look for 
alternatives. E.g. what does udev uses to identify the devices for 
example? And IIRC we have something like a "device-name" in the kernel 
as well (what's printed in the logs).

The minimum we need to do is get away from the minor=linenum approach, 
cause as Daniel pointed out the minor allocation is quite a mess and not 
necessary contiguous.

Regards,
Christian.

>
> Regards,
> Kenny
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx