Re: [PATCH RFC 2/5] cgroup: Add mechanism to register vendor specific DRM devices
Hi Harish, Am 26.11.18 um 21:59 schrieb Kasiviswanathan, Harish: > Thanks Tejun,Eric and Christian for your replies. > > We want GPUs resource management to work seamlessly with containers and > container orchestration. With the Intel / bpf based approach this is not > possible. I think one lesson learned is that we should describe this goal in the patch covert letter when sending it out. That could have avoid something like have of the initial confusion. > From your response we gather the following. GPU resources need to be > abstracted. We will send a new proposal in same vein. Our current thinking is > to start with a single abstracted resource and build a framework that can be > expanded to include additional resources. We plan to start with “GPU cores”. > We believe all GPUs have some concept of cores or compute unit. Sounds good, just one comment on creating a framework: Before doing something like this think for a moment if it doesn't make sense to rather extend the existing cgroup framework. That approach usually makes more sense because you rarely need something fundamentally new. Regards, Christian. > > Your feedback is highly appreciated. > > Best Regards, > Harish > > > > From: amd-gfx on behalf of Tejun Heo > > Sent: Tuesday, November 20, 2018 5:30 PM > To: Ho, Kenny > Cc: cgro...@vger.kernel.org; intel-...@lists.freedesktop.org; > y2ke...@gmail.com; amd-gfx@lists.freedesktop.org; > dri-de...@lists.freedesktop.org > Subject: Re: [PATCH RFC 2/5] cgroup: Add mechanism to register vendor > specific DRM devices > > > Hello, > > On Tue, Nov 20, 2018 at 10:21:14PM +, Ho, Kenny wrote: >> By this reply, are you suggesting that vendor specific resources >> will never be acceptable to be managed under cgroup? Let say a user > I wouldn't say never but whatever which gets included as a cgroup > controller should have clearly defined resource abstractions and the > control schemes around them including support for delegation. AFAICS, > gpu side still seems to have a long way to go (and it's not clear > whether that's somewhere it will or needs to end up). > >> want to have similar functionality as what cgroup is offering but to >> manage vendor specific resources, what would you suggest as a >> solution? When you say keeping vendor specific resource regulation >> inside drm or specific drivers, do you mean we should replicate the >> cgroup infrastructure there or do you mean either drm or specific >> driver should query existing hierarchy (such as device or perhaps >> cpu) for the process organization information? >> >> To put the questions in more concrete terms, let say a user wants to >> expose certain part of a gpu to a particular cgroup similar to the >> way selective cpu cores are exposed to a cgroup via cpuset, how >> should we go about enabling such functionality? > Do what the intel driver or bpf is doing? It's not difficult to hook > into cgroup for identification purposes. > > Thanks. > ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH RFC 2/5] cgroup: Add mechanism to register vendor specific DRM devices
Thanks Tejun,Eric and Christian for your replies. We want GPUs resource management to work seamlessly with containers and container orchestration. With the Intel / bpf based approach this is not possible. From your response we gather the following. GPU resources need to be abstracted. We will send a new proposal in same vein. Our current thinking is to start with a single abstracted resource and build a framework that can be expanded to include additional resources. We plan to start with “GPU cores”. We believe all GPUs have some concept of cores or compute unit. Your feedback is highly appreciated. Best Regards, Harish From: amd-gfx on behalf of Tejun Heo Sent: Tuesday, November 20, 2018 5:30 PM To: Ho, Kenny Cc: cgro...@vger.kernel.org; intel-...@lists.freedesktop.org; y2ke...@gmail.com; amd-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org Subject: Re: [PATCH RFC 2/5] cgroup: Add mechanism to register vendor specific DRM devices Hello, On Tue, Nov 20, 2018 at 10:21:14PM +, Ho, Kenny wrote: > By this reply, are you suggesting that vendor specific resources > will never be acceptable to be managed under cgroup? Let say a user I wouldn't say never but whatever which gets included as a cgroup controller should have clearly defined resource abstractions and the control schemes around them including support for delegation. AFAICS, gpu side still seems to have a long way to go (and it's not clear whether that's somewhere it will or needs to end up). > want to have similar functionality as what cgroup is offering but to > manage vendor specific resources, what would you suggest as a > solution? When you say keeping vendor specific resource regulation > inside drm or specific drivers, do you mean we should replicate the > cgroup infrastructure there or do you mean either drm or specific > driver should query existing hierarchy (such as device or perhaps > cpu) for the process organization information? > > To put the questions in more concrete terms, let say a user wants to > expose certain part of a gpu to a particular cgroup similar to the > way selective cpu cores are exposed to a cgroup via cpuset, how > should we go about enabling such functionality? Do what the intel driver or bpf is doing? It's not difficult to hook into cgroup for identification purposes. Thanks. -- tejun ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx amd-gfx Info Page - freedesktop.org lists.freedesktop.org To see the collection of prior postings to the list, visit the amd-gfx Archives.. Using amd-gfx: To post a message to all the list members, send email to amd-gfx@lists.freedesktop.org. You can subscribe to the list, or change your existing subscription, in the sections below. ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH RFC 2/5] cgroup: Add mechanism to register vendor specific DRM devices
(resending because previous email switched to HTML mode and was filtered out) Hi Tejun, On Tue, Nov 20, 2018 at 5:30 PM Tejun Heo wrote: > On Tue, Nov 20, 2018 at 10:21:14PM +, Ho, Kenny wrote: > > By this reply, are you suggesting that vendor specific resources > > will never be acceptable to be managed under cgroup? Let say a user > > I wouldn't say never but whatever which gets included as a cgroup > controller should have clearly defined resource abstractions and the > control schemes around them including support for delegation. AFAICS, > gpu side still seems to have a long way to go (and it's not clear > whether that's somewhere it will or needs to end up). Right, I totally understand that it's not obvious from this RFC because the 'resource' counting demonstrated in this RFC is trivial in nature, mostly to illustrate the 'vendor' concept. The structure of this patch actually give us the ability to support both abstracted resources you mentioned and vendor specific resources. It is probably not obvious as the RFC only includes two resources and they are both vendor specific. To be clear, I am not saying there aren't abstracted resources in drm, there are (we are still working on those). What I am saying is that not all resources are abstracted and for the purpose of this RFC I was hoping to get some feedback on the vendor specific parts early just so that we don't go down the wrong path. That said, I think I am getting a better sense of what you are saying. Please correct me if I misinterpreted: your concern is that abstracting by vendor is too high level and it's too much of a free-for-all. Instead, resources should be abstracted at the controller level even if it's only available to a specific vendor (or even a specific product from a specific vendor). Is that a fair read? A couple of additional side questions: * Is statistic/accounting-only use cases like those enabled by cpuacct controller no longer sufficient? If it is still sufficient, can you elaborate more on what you mean by having control schemes and supporting delegation? * When you wrote delegation, do you mean delegation in the sense described in https://www.kernel.org/doc/Documentation/cgroup-v2.txt ? > > To put the questions in more concrete terms, let say a user wants to > > expose certain part of a gpu to a particular cgroup similar to the > > way selective cpu cores are exposed to a cgroup via cpuset, how > > should we go about enabling such functionality? > > Do what the intel driver or bpf is doing? It's not difficult to hook > into cgroup for identification purposes. Does intel driver or bpf present an interface file in cgroupfs for users to configure the core selection like cpuset? I must admit I am not too familiar with the bpf case as I was referencing mostly the way rdma was implemented when putting this RFC together. Perhaps I wasn't communicating clearly so let me see if I can illustrate this discussion with a hypothetical but concrete example using our competitor's product. Nvidia has something called Tensor Cores in some of their GPUs and the purpose of those cores is to accelerate matrix operations for machine learning applications. This is something unique to Nvidia and to my knowledge no one else has something like it. These cores are different from regular shader processors and there are multiple of them in a GPU. Under the structure of this RFC, if Nvidia wants to make Tensor Cores manageable via cgroup (with the "Allocation" distribution model let say), they will probably have an interface file called "drm.nvidia.tensor_core", in which only nvidia's GPUs will be listed. If a GPU has TC, it will have a positive count, otherwise 0. If I understand you correctly Tejun, is that they should not do that. What they should do is have an abstracted resource, possibly named "drm.matrix_accelerator" where all drm devices available on a system will be listed. All GPUs except some Nvidia's will have a count of 0. Or perhaps that is not sufficiently abstracted so instead there should be just "drm.cores" instead and that file list both device, core types and count. For one vendor they may have shader proc, texture map unit, tensor core, ray tracing cores as types. Others may have ALUs, EUs and subslices. Is that an accurate representation of what you are recommending? Regards, Kenny ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH RFC 2/5] cgroup: Add mechanism to register vendor specific DRM devices
Hi Tejun, On Tue, Nov 20, 2018 at 5:30 PM Tejun Heo wrote: > On Tue, Nov 20, 2018 at 10:21:14PM +, Ho, Kenny wrote: > > By this reply, are you suggesting that vendor specific resources > > will never be acceptable to be managed under cgroup? Let say a user > > I wouldn't say never but whatever which gets included as a cgroup > controller should have clearly defined resource abstractions and the > control schemes around them including support for delegation. AFAICS, > gpu side still seems to have a long way to go (and it's not clear > whether that's somewhere it will or needs to end up). Right, I totally understand that it's not obvious from this RFC because the 'resource' counting demonstrated in this RFC is trivial in nature, mostly to illustrate the 'vendor' concept. The structure of this patch actually give us the ability to support both abstracted resources you mentioned and vendor specific resources. But it is probably not very clear as the RFC only includes two resources and they are both vendor specific. To be clear, I am not saying there aren't abstracted resources in drm, there are (we are still working on those). What I am saying is that not all resources are abstracted and for the purpose of this RFC I was hoping to get some feedback on the vendor specific parts early just so that we don't go down the wrong path. That said, I think I am getting a better sense of what you are saying. Please correct me if I misinterpreted: your concern is that abstracting by vendor is too high level and it's too much of a free-for-all. Instead, resources should be abstracted at the controller level even if it's only available to a specific vendor (or even a specific product from a specific vendor). Is that a fair read? A couple of additional side questions: * Is statistic/accounting-only use cases like those enabled by cpuacct controller no longer sufficient? If it is still sufficient, can you elaborate more on what you mean by having control schemes and supporting delegation? * When you wrote delegation, do you mean delegation in the sense described in https://www.kernel.org/doc/Documentation/cgroup-v2.txt ? > > To put the questions in more concrete terms, let say a user wants to > > expose certain part of a gpu to a particular cgroup similar to the > > way selective cpu cores are exposed to a cgroup via cpuset, how > > should we go about enabling such functionality? > > Do what the intel driver or bpf is doing? It's not difficult to hook > into cgroup for identification purposes. Does intel driver or bpf present an interface file in cgroupfs for users to configure the core selection like cpuset? I must admit I am not too familiar with the bpf case as I was referencing mostly the way rdma was implemented when putting this RFC together. Perhaps I wasn't communicating clearly so let me see if I can illustrate this discussion with a hypothetical but concrete example using our competitor's product. Nvidia has something called Tensor Cores in some of their GPUs and the purpose of those cores is to accelerate matrix operations for machine learning applications. This is something unique to Nvidia and to my knowledge no one else has something like it. These cores are different from regular shader processors and there are multiple of them in a GPU. Under the structure of this RFC, if Nvidia wants to make Tensor Cores manageable via cgroup (with the "Allocation" distribution model let say), they will probably have an interface file called "drm.nvidia.tensor_core", in which only nvidia's GPUs will be listed. If a GPU has TC, it will have a positive count, otherwise 0. If I understand you correctly Tejun, is that they should not do that. What they should do is have an abstracted resource, possibly named "drm.matrix_accelerator" where all drm devices available on a system will be listed. All GPUs except some Nvidia's will have a count of 0. Or perhaps that is not sufficiently abstracted so instead there should be just "drm.cores" instead and that file list both device, core types and count. For one vendor they may have shader proc, texture map unit, tensor core, ray tracing cores as types. Others may have ALUs, EUs and subslices. Is that an accurate representation of what you are recommending? Regards, Kenny ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH RFC 2/5] cgroup: Add mechanism to register vendor specific DRM devices
Am 20.11.18 um 19:58 schrieb Kenny Ho:
Since many parts of the DRM subsystem has vendor-specific
implementations, we introduce mechanisms for vendor to register their
specific resources and control files to the DRM cgroup subsystem. A
vendor will register itself with the DRM cgroup subsystem first before
registering individual DRM devices to the cgroup subsystem.
In addition to the cgroup_subsys_state that is common to all DRM
devices, a device-specific state is introduced and it is allocated
according to the vendor of the device.
Mhm, it's most likely just a naming issue but I think we should drop the
term "vendor" here and rather use "driver" instead.
Background is that both Intel as well as AMD have multiple drivers for
different hardware generations and we certainly don't want to handle all
drivers from one vendor the same way.
Christian.
Change-Id: I908ee6975ea0585e4c30eafde4599f87094d8c65
Signed-off-by: Kenny Ho
---
include/drm/drm_cgroup.h | 39
include/drm/drmcgrp_vendors.h | 7 +++
include/linux/cgroup_drm.h| 26 +++
kernel/cgroup/drm.c | 84 +++
4 files changed, 156 insertions(+)
create mode 100644 include/drm/drm_cgroup.h
create mode 100644 include/drm/drmcgrp_vendors.h
diff --git a/include/drm/drm_cgroup.h b/include/drm/drm_cgroup.h
new file mode 100644
index ..26cbea7059a6
--- /dev/null
+++ b/include/drm/drm_cgroup.h
@@ -0,0 +1,39 @@
+/* SPDX-License-Identifier: MIT
+ * Copyright 2018 Advanced Micro Devices, Inc.
+ */
+#ifndef __DRM_CGROUP_H__
+#define __DRM_CGROUP_H__
+
+#define DRMCGRP_VENDOR(_x) _x ## _drmcgrp_vendor_id,
+enum drmcgrp_vendor_id {
+#include
+ DRMCGRP_VENDOR_COUNT,
+};
+#undef DRMCGRP_VENDOR
+
+#define DRMCGRP_VENDOR(_x) extern struct drmcgrp_vendor _x ## _drmcgrp_vendor;
+#include
+#undef DRMCGRP_VENDOR
+
+
+
+#ifdef CONFIG_CGROUP_DRM
+
+extern struct drmcgrp_vendor *drmcgrp_vendors[];
+
+int drmcgrp_register_vendor(struct drmcgrp_vendor *vendor, enum
drmcgrp_vendor_id id);
+int drmcgrp_register_device(struct drm_device *device, enum drmcgrp_vendor_id
id);
+
+#else
+static int drmcgrp_register_vendor(struct drmcgrp_vendor *vendor, enum
drmcgrp_vendor_id id)
+{
+ return 0;
+}
+
+static int drmcgrp_register_device(struct drm_device *device, enum
drmcgrp_vendor_id id)
+{
+ return 0;
+}
+
+#endif /* CONFIG_CGROUP_DRM */
+#endif /* __DRM_CGROUP_H__ */
diff --git a/include/drm/drmcgrp_vendors.h b/include/drm/drmcgrp_vendors.h
new file mode 100644
index ..b04d8649851b
--- /dev/null
+++ b/include/drm/drmcgrp_vendors.h
@@ -0,0 +1,7 @@
+/* SPDX-License-Identifier: MIT
+ * Copyright 2018 Advanced Micro Devices, Inc.
+ */
+#if IS_ENABLED(CONFIG_CGROUP_DRM)
+
+
+#endif
diff --git a/include/linux/cgroup_drm.h b/include/linux/cgroup_drm.h
index 79ab38b0f46d..a776662d9593 100644
--- a/include/linux/cgroup_drm.h
+++ b/include/linux/cgroup_drm.h
@@ -6,10 +6,36 @@
#ifdef CONFIG_CGROUP_DRM
+#include
#include
+#include
+#include
+
+/* limit defined per the way drm_minor_alloc operates */
+#define MAX_DRM_DEV (64 * DRM_MINOR_RENDER)
+
+struct drmcgrp_device {
+ enum drmcgrp_vendor_id vid;
+ struct drm_device *dev;
+ struct mutexmutex;
+};
+
+/* vendor-common resource counting goes here */
+/* this struct should be included in the vendor specific resource */
+struct drmcgrp_device_resource {
+ struct drmcgrp_device *ddev;
+};
+
+struct drmcgrp_vendor {
+ struct cftype *(*get_cftypes)(void);
+ struct drmcgrp_device_resource *(*alloc_dev_resource)(void);
+ void (*free_dev_resource)(struct drmcgrp_device_resource *dev_resource);
+};
+
struct drmcgrp {
struct cgroup_subsys_state css;
+ struct drmcgrp_device_resource *dev_resources[MAX_DRM_DEV];
};
static inline struct drmcgrp *css_drmcgrp(struct cgroup_subsys_state *css)
diff --git a/kernel/cgroup/drm.c b/kernel/cgroup/drm.c
index d9e194b9aead..f9630cc389bc 100644
--- a/kernel/cgroup/drm.c
+++ b/kernel/cgroup/drm.c
@@ -1,8 +1,30 @@
// SPDX-License-Identifier: MIT
// Copyright 2018 Advanced Micro Devices, Inc.
+#include
#include
#include
+#include
+#include
+#include
#include
+#include
+#include
+
+/* generate an array of drm cgroup vendor pointers */
+#define DRMCGRP_VENDOR(_x)[_x ## _drmcgrp_vendor_id] = NULL,
+struct drmcgrp_vendor *drmcgrp_vendors[] = {
+#include
+};
+#undef DRMCGRP_VENDOR
+EXPORT_SYMBOL(drmcgrp_vendors);
+
+static DEFINE_MUTEX(drmcgrp_mutex);
+
+/* indexed by drm_minor for access speed */
+static struct drmcgrp_device *known_drmcgrp_devs[MAX_DRM_DEV];
+
+static int max_minor;
+
static u64 drmcgrp_test_read(struct cgroup_subsys_state *css,
struct cftype *cft)
@@ -13,6 +35,12 @@ static u64 drmcgrp_test_read(struct cgroup_subsys_state *css,
static void drmcgrp_css_free(struct cgroup_subsys_state *css)
Re: [PATCH RFC 2/5] cgroup: Add mechanism to register vendor specific DRM devices
Hello, On Tue, Nov 20, 2018 at 10:21:14PM +, Ho, Kenny wrote: > By this reply, are you suggesting that vendor specific resources > will never be acceptable to be managed under cgroup? Let say a user I wouldn't say never but whatever which gets included as a cgroup controller should have clearly defined resource abstractions and the control schemes around them including support for delegation. AFAICS, gpu side still seems to have a long way to go (and it's not clear whether that's somewhere it will or needs to end up). > want to have similar functionality as what cgroup is offering but to > manage vendor specific resources, what would you suggest as a > solution? When you say keeping vendor specific resource regulation > inside drm or specific drivers, do you mean we should replicate the > cgroup infrastructure there or do you mean either drm or specific > driver should query existing hierarchy (such as device or perhaps > cpu) for the process organization information? > > To put the questions in more concrete terms, let say a user wants to > expose certain part of a gpu to a particular cgroup similar to the > way selective cpu cores are exposed to a cgroup via cpuset, how > should we go about enabling such functionality? Do what the intel driver or bpf is doing? It's not difficult to hook into cgroup for identification purposes. Thanks. -- tejun ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH RFC 2/5] cgroup: Add mechanism to register vendor specific DRM devices
Hi Tejun, Thanks for the reply. A few clarifying questions: On Tue, Nov 20, 2018 at 3:21 PM Tejun Heo wrote: > So, I'm still pretty negative about adding drm controller at this > point. There isn't enough of common resource model defined yet and > until that gets sorted out I think it's in the best interest of > everyone involved to keep it inside drm or specific driver proper. By this reply, are you suggesting that vendor specific resources will never be acceptable to be managed under cgroup? Let say a user want to have similar functionality as what cgroup is offering but to manage vendor specific resources, what would you suggest as a solution? When you say keeping vendor specific resource regulation inside drm or specific drivers, do you mean we should replicate the cgroup infrastructure there or do you mean either drm or specific driver should query existing hierarchy (such as device or perhaps cpu) for the process organization information? To put the questions in more concrete terms, let say a user wants to expose certain part of a gpu to a particular cgroup similar to the way selective cpu cores are exposed to a cgroup via cpuset, how should we go about enabling such functionality? Regards, Kenny ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH RFC 2/5] cgroup: Add mechanism to register vendor specific DRM devices
Hello, On Tue, Nov 20, 2018 at 01:58:11PM -0500, Kenny Ho wrote: > Since many parts of the DRM subsystem has vendor-specific > implementations, we introduce mechanisms for vendor to register their > specific resources and control files to the DRM cgroup subsystem. A > vendor will register itself with the DRM cgroup subsystem first before > registering individual DRM devices to the cgroup subsystem. > > In addition to the cgroup_subsys_state that is common to all DRM > devices, a device-specific state is introduced and it is allocated > according to the vendor of the device. So, I'm still pretty negative about adding drm controller at this point. There isn't enough of common resource model defined yet and until that gets sorted out I think it's in the best interest of everyone involved to keep it inside drm or specific driver proper. Thanks. -- tejun ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
