Re: [PATCH RFC v4 16/16] drm/amdgpu: Integrate with DRM cgroup
On 2019-08-29 2:05 a.m., Kenny Ho wrote:
> The number of logical gpu (lgpu) is defined to be the number of compute
> unit (CU) for a device. The lgpu allocation limit only applies to
> compute workload for the moment (enforced via kfd queue creation.) Any
> cu_mask update is validated against the availability of the compute unit
> as defined by the drmcg the kfd process belongs to.
There is something missing here. There is an API for the application to
specify a CU mask. Right now it looks like the application-specified and
CGroup-specified CU masks would clobber each other. Instead the two
should be merged.
The CGroup-specified mask should specify a subset of CUs available for
application-specified CU masks. When the cgroup CU mask changes, you'd
need to take any application-specified CU masks into account before
updating the hardware.
The KFD topology APIs report the number of available CUs to the
application. CGroups would change that number at runtime and
applications would not expect that. I think the best way to deal with
that would be to have multiple bits in the application-specified CU mask
map to the same CU. How to do that in a fair way is not obvious. I guess
a more coarse-grain division of the GPU into LGPUs would make this
somewhat easier.
How is this problem handled for CPU cores and the interaction with CPU
pthread_setaffinity_np?
Regards,
Felix
>
> Change-Id: I69a57452c549173a1cd623c30dc57195b3b6563e
> Signed-off-by: Kenny Ho
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h| 4 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 21 +++
> drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 6 +
> drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 3 +
> .../amd/amdkfd/kfd_process_queue_manager.c| 140 ++
> 5 files changed, 174 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> index 55cb1b2094fd..369915337213 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> @@ -198,6 +198,10 @@ uint8_t amdgpu_amdkfd_get_xgmi_hops_count(struct kgd_dev
> *dst, struct kgd_dev *s
> valid; \
> })
>
> +int amdgpu_amdkfd_update_cu_mask_for_process(struct task_struct *task,
> + struct amdgpu_device *adev, unsigned long *lgpu_bitmap,
> + unsigned int nbits);
> +
> /* GPUVM API */
> int amdgpu_amdkfd_gpuvm_create_process_vm(struct kgd_dev *kgd, unsigned int
> pasid,
> void **vm, void **process_info,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 163a4fbf0611..8abeffdd2e5b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -1398,9 +1398,29 @@ amdgpu_get_crtc_scanout_position(struct drm_device
> *dev, unsigned int pipe,
> static void amdgpu_drmcg_custom_init(struct drm_device *dev,
> struct drmcg_props *props)
> {
> + struct amdgpu_device *adev = dev->dev_private;
> +
> + props->lgpu_capacity = adev->gfx.cu_info.number;
> +
> props->limit_enforced = true;
> }
>
> +static void amdgpu_drmcg_limit_updated(struct drm_device *dev,
> + struct task_struct *task, struct drmcg_device_resource *ddr,
> + enum drmcg_res_type res_type)
> +{
> + struct amdgpu_device *adev = dev->dev_private;
> +
> + switch (res_type) {
> + case DRMCG_TYPE_LGPU:
> + amdgpu_amdkfd_update_cu_mask_for_process(task, adev,
> +ddr->lgpu_allocated, dev->drmcg_props.lgpu_capacity);
> + break;
> + default:
> + break;
> + }
> +}
> +
> static struct drm_driver kms_driver = {
> .driver_features =
> DRIVER_USE_AGP | DRIVER_ATOMIC |
> @@ -1438,6 +1458,7 @@ static struct drm_driver kms_driver = {
> .gem_prime_mmap = amdgpu_gem_prime_mmap,
>
> .drmcg_custom_init = amdgpu_drmcg_custom_init,
> + .drmcg_limit_updated = amdgpu_drmcg_limit_updated,
>
> .name = DRIVER_NAME,
> .desc = DRIVER_DESC,
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> index 138c70454e2b..fa765b803f97 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> @@ -450,6 +450,12 @@ static int kfd_ioctl_set_cu_mask(struct file *filp,
> struct kfd_process *p,
> return -EFAULT;
> }
>
> + if (!pqm_drmcg_lgpu_validate(p, args->queue_id, properties.cu_mask,
> cu_mask_size)) {
> + pr_debug("CU mask not permitted by DRM Cgroup");
> + kfree(properties.cu_mask);
> + return -EACCES;
> + }
> +
> mutex_lock(&p->mutex);
>
> retval = pqm_set_cu_mask(&p->pqm, args->queue_id, &properties);
> diff --git a/
[PATCH RFC v4 16/16] drm/amdgpu: Integrate with DRM cgroup
The number of logical gpu (lgpu) is defined to be the number of compute
unit (CU) for a device. The lgpu allocation limit only applies to
compute workload for the moment (enforced via kfd queue creation.) Any
cu_mask update is validated against the availability of the compute unit
as defined by the drmcg the kfd process belongs to.
Change-Id: I69a57452c549173a1cd623c30dc57195b3b6563e
Signed-off-by: Kenny Ho
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h| 4 +
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 21 +++
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 6 +
drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 3 +
.../amd/amdkfd/kfd_process_queue_manager.c| 140 ++
5 files changed, 174 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
index 55cb1b2094fd..369915337213 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
@@ -198,6 +198,10 @@ uint8_t amdgpu_amdkfd_get_xgmi_hops_count(struct kgd_dev
*dst, struct kgd_dev *s
valid; \
})
+int amdgpu_amdkfd_update_cu_mask_for_process(struct task_struct *task,
+ struct amdgpu_device *adev, unsigned long *lgpu_bitmap,
+ unsigned int nbits);
+
/* GPUVM API */
int amdgpu_amdkfd_gpuvm_create_process_vm(struct kgd_dev *kgd, unsigned int
pasid,
void **vm, void **process_info,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 163a4fbf0611..8abeffdd2e5b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1398,9 +1398,29 @@ amdgpu_get_crtc_scanout_position(struct drm_device *dev,
unsigned int pipe,
static void amdgpu_drmcg_custom_init(struct drm_device *dev,
struct drmcg_props *props)
{
+ struct amdgpu_device *adev = dev->dev_private;
+
+ props->lgpu_capacity = adev->gfx.cu_info.number;
+
props->limit_enforced = true;
}
+static void amdgpu_drmcg_limit_updated(struct drm_device *dev,
+ struct task_struct *task, struct drmcg_device_resource *ddr,
+ enum drmcg_res_type res_type)
+{
+ struct amdgpu_device *adev = dev->dev_private;
+
+ switch (res_type) {
+ case DRMCG_TYPE_LGPU:
+ amdgpu_amdkfd_update_cu_mask_for_process(task, adev,
+ddr->lgpu_allocated, dev->drmcg_props.lgpu_capacity);
+ break;
+ default:
+ break;
+ }
+}
+
static struct drm_driver kms_driver = {
.driver_features =
DRIVER_USE_AGP | DRIVER_ATOMIC |
@@ -1438,6 +1458,7 @@ static struct drm_driver kms_driver = {
.gem_prime_mmap = amdgpu_gem_prime_mmap,
.drmcg_custom_init = amdgpu_drmcg_custom_init,
+ .drmcg_limit_updated = amdgpu_drmcg_limit_updated,
.name = DRIVER_NAME,
.desc = DRIVER_DESC,
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index 138c70454e2b..fa765b803f97 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -450,6 +450,12 @@ static int kfd_ioctl_set_cu_mask(struct file *filp, struct
kfd_process *p,
return -EFAULT;
}
+ if (!pqm_drmcg_lgpu_validate(p, args->queue_id, properties.cu_mask,
cu_mask_size)) {
+ pr_debug("CU mask not permitted by DRM Cgroup");
+ kfree(properties.cu_mask);
+ return -EACCES;
+ }
+
mutex_lock(&p->mutex);
retval = pqm_set_cu_mask(&p->pqm, args->queue_id, &properties);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index 8b0eee5b3521..1bec7550 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -1038,6 +1038,9 @@ int pqm_get_wave_state(struct process_queue_manager *pqm,
u32 *ctl_stack_used_size,
u32 *save_area_used_size);
+bool pqm_drmcg_lgpu_validate(struct kfd_process *p, int qid, u32 *cu_mask,
+ unsigned int cu_mask_size);
+
int amdkfd_fence_wait_timeout(unsigned int *fence_addr,
unsigned int fence_value,
unsigned int timeout_ms);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
index 7e6c3ee82f5b..a896de290307 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
@@ -23,9 +23,11 @@
#include
#include
+#include
#include "kfd_device_queue_manager.h"
#include "kfd_priv.h"
#include "kfd_kernel_queue.h"
+#include "amdgpu.h"
#include "amdgpu_amdkfd.h"
static inline struct process_queue_no
