[PATCH v3] drm/amdkfd: Track GPU memory utilization per process
Felix Kuehling
felix.kuehling at amd.com
Mon Apr 27 19:07:25 UTC 2020
Thanks Mukul.
One last thing I noticed. It seems the update the the vram_usage is
protected by the process->mutex. That means the variable will always be
coherent while that lock is held. However, the sysfs-show function
doesn't hold that lock. So if the compiler decides to break the update
into multiple instructions, or sysfs-show does the read with multiple
instructions, you may see inconsistent results. You can ensure that
doesn't happen by only updating the size atomically with WRITE_ONCE and
only reading it atomically with READ_ONCE in the sysfs-show function.
For reference, this is a good article on the subject on lwn.net: [Who's
afraid of a big bad optimizing compiler?](https://lwn.net/Articles/793253/)
Regards,
Felix
Am 2020-04-26 um 11:45 p.m. schrieb Mukul Joshi:
> Track GPU VRAM usage on a per process basis and report it through
> sysfs.
>
> v2:
> - Handle AMDGPU BO-specific details in
> amdgpu_amdkfd_gpuvm_free_memory_of_gpu().
> - Return size of VRAM BO being freed from
> amdgpu_amdkfd_gpuvm_free_memory_of_gpu().
> - Do not consider imported memory for VRAM
> usage calculations.
>
> v3:
> - Move handling of imported BO size from
> kfd_ioctl_free_memory_of_gpu() to
> amdgpu_amdkfd_gpuvm_free_memory_of_gpu().
>
> Signed-off-by: Mukul Joshi <mukul.joshi at amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h | 3 +-
> .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 16 +++++-
> drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 13 ++++-
> drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 7 +++
> drivers/gpu/drm/amd/amdkfd/kfd_process.c | 57 ++++++++++++++++---
> 5 files changed, 84 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> index d065c50582eb..a501026e829c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> @@ -65,6 +65,7 @@ struct kgd_mem {
> struct amdgpu_sync sync;
>
> bool aql_queue;
> + bool is_imported;
> };
>
> /* KFD Memory Eviction */
> @@ -219,7 +220,7 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
> void *vm, struct kgd_mem **mem,
> uint64_t *offset, uint32_t flags);
> int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
> - struct kgd_dev *kgd, struct kgd_mem *mem);
> + struct kgd_dev *kgd, struct kgd_mem *mem, uint64_t *size);
> int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
> struct kgd_dev *kgd, struct kgd_mem *mem, void *vm);
> int amdgpu_amdkfd_gpuvm_unmap_memory_from_gpu(
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index 0768b7eb7683..1247938b1ec1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -1277,7 +1277,7 @@ int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
> }
>
> int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
> - struct kgd_dev *kgd, struct kgd_mem *mem)
> + struct kgd_dev *kgd, struct kgd_mem *mem, uint64_t *size)
> {
> struct amdkfd_process_info *process_info = mem->process_info;
> unsigned long bo_size = mem->bo->tbo.mem.size;
> @@ -1286,9 +1286,11 @@ int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
> struct ttm_validate_buffer *bo_list_entry;
> unsigned int mapped_to_gpu_memory;
> int ret;
> + bool is_imported = 0;
>
> mutex_lock(&mem->lock);
> mapped_to_gpu_memory = mem->mapped_to_gpu_memory;
> + is_imported = mem->is_imported;
> mutex_unlock(&mem->lock);
> /* lock is not needed after this, since mem is unused and will
> * be freed anyway
> @@ -1340,6 +1342,17 @@ int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
> kfree(mem->bo->tbo.sg);
> }
>
> + /* Update the size of the BO being freed if it was allocated from
> + * VRAM and is not imported.
> + */
> + if (size) {
> + if ((mem->bo->preferred_domains == AMDGPU_GEM_DOMAIN_VRAM) &&
> + (!is_imported))
> + *size = bo_size;
> + else
> + *size = 0;
> + }
> +
> /* Free the BO*/
> amdgpu_bo_unref(&mem->bo);
> mutex_destroy(&mem->lock);
> @@ -1694,6 +1707,7 @@ int amdgpu_amdkfd_gpuvm_import_dmabuf(struct kgd_dev *kgd,
> (*mem)->process_info = avm->process_info;
> add_kgd_mem_to_kfd_bo_list(*mem, avm->process_info, false);
> amdgpu_sync_create(&(*mem)->sync);
> + (*mem)->is_imported = true;
>
> return 0;
> }
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> index f8fa03a12add..ede84f76397f 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> @@ -1322,6 +1322,10 @@ static int kfd_ioctl_alloc_memory_of_gpu(struct file *filep,
> goto err_free;
> }
>
> + /* Update the VRAM usage count */
> + if (flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM)
> + pdd->vram_usage += args->size;
> +
> mutex_unlock(&p->mutex);
>
> args->handle = MAKE_HANDLE(args->gpu_id, idr_handle);
> @@ -1337,7 +1341,7 @@ static int kfd_ioctl_alloc_memory_of_gpu(struct file *filep,
> return 0;
>
> err_free:
> - amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd, (struct kgd_mem *)mem);
> + amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd, (struct kgd_mem *)mem, NULL);
> err_unlock:
> mutex_unlock(&p->mutex);
> return err;
> @@ -1351,6 +1355,7 @@ static int kfd_ioctl_free_memory_of_gpu(struct file *filep,
> void *mem;
> struct kfd_dev *dev;
> int ret;
> + uint64_t size = 0;
>
> dev = kfd_device_by_id(GET_GPU_ID(args->handle));
> if (!dev)
> @@ -1373,7 +1378,7 @@ static int kfd_ioctl_free_memory_of_gpu(struct file *filep,
> }
>
> ret = amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd,
> - (struct kgd_mem *)mem);
> + (struct kgd_mem *)mem, &size);
>
> /* If freeing the buffer failed, leave the handle in place for
> * clean-up during process tear-down.
> @@ -1382,6 +1387,8 @@ static int kfd_ioctl_free_memory_of_gpu(struct file *filep,
> kfd_process_device_remove_obj_handle(
> pdd, GET_IDR_HANDLE(args->handle));
>
> + pdd->vram_usage -= size;
> +
> err_unlock:
> mutex_unlock(&p->mutex);
> return ret;
> @@ -1726,7 +1733,7 @@ static int kfd_ioctl_import_dmabuf(struct file *filep,
> return 0;
>
> err_free:
> - amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd, (struct kgd_mem *)mem);
> + amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd, (struct kgd_mem *)mem, NULL);
> err_unlock:
> mutex_unlock(&p->mutex);
> return r;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index 43b888b311c7..e7918fc3cee5 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -616,6 +616,8 @@ enum kfd_pdd_bound {
> PDD_BOUND_SUSPENDED,
> };
>
> +#define MAX_VRAM_FILENAME_LEN 11
> +
> /* Data that is per-process-per device. */
> struct kfd_process_device {
> /*
> @@ -658,6 +660,11 @@ struct kfd_process_device {
>
> /* Is this process/pasid bound to this device? (amd_iommu_bind_pasid) */
> enum kfd_pdd_bound bound;
> +
> + /* VRAM usage */
> + uint64_t vram_usage;
> + struct attribute attr_vram;
> + char vram_filename[MAX_VRAM_FILENAME_LEN];
> };
>
> #define qpd_to_pdd(x) container_of(x, struct kfd_process_device, qpd)
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> index fe0cd49d4ea7..600759949802 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> @@ -79,18 +79,22 @@ static struct kfd_procfs_tree procfs;
> static ssize_t kfd_procfs_show(struct kobject *kobj, struct attribute *attr,
> char *buffer)
> {
> - int val = 0;
> -
> if (strcmp(attr->name, "pasid") == 0) {
> struct kfd_process *p = container_of(attr, struct kfd_process,
> attr_pasid);
> - val = p->pasid;
> +
> + return snprintf(buffer, PAGE_SIZE, "%d\n", p->pasid);
> + } else if (strncmp(attr->name, "vram_", 5) == 0) {
> + struct kfd_process_device *pdd = container_of(attr, struct kfd_process_device,
> + attr_vram);
> + if (pdd)
> + return snprintf(buffer, PAGE_SIZE, "%llu\n", pdd->vram_usage);
> } else {
> pr_err("Invalid attribute");
> return -EINVAL;
> }
>
> - return snprintf(buffer, PAGE_SIZE, "%d\n", val);
> + return 0;
> }
>
> static void kfd_procfs_kobj_release(struct kobject *kobj)
> @@ -206,6 +210,34 @@ int kfd_procfs_add_queue(struct queue *q)
> return 0;
> }
>
> +int kfd_procfs_add_vram_usage(struct kfd_process *p)
> +{
> + int ret = 0;
> + struct kfd_process_device *pdd;
> +
> + if (!p)
> + return -EINVAL;
> +
> + if (!p->kobj)
> + return -EFAULT;
> +
> + /* Create proc/<pid>/vram_<gpuid> file for each GPU */
> + list_for_each_entry(pdd, &p->per_device_data, per_device_list) {
> + snprintf(pdd->vram_filename, MAX_VRAM_FILENAME_LEN, "vram_%u",
> + pdd->dev->id);
> + pdd->attr_vram.name = pdd->vram_filename;
> + pdd->attr_vram.mode = KFD_SYSFS_FILE_MODE;
> + sysfs_attr_init(&pdd->attr_vram);
> + ret = sysfs_create_file(p->kobj, &pdd->attr_vram);
> + if (ret)
> + pr_warn("Creating vram usage for gpu id %d failed",
> + (int)pdd->dev->id);
> + }
> +
> + return ret;
> +}
> +
> +
> void kfd_procfs_del_queue(struct queue *q)
> {
> if (!q)
> @@ -248,7 +280,7 @@ static void kfd_process_free_gpuvm(struct kgd_mem *mem,
> struct kfd_dev *dev = pdd->dev;
>
> amdgpu_amdkfd_gpuvm_unmap_memory_from_gpu(dev->kgd, mem, pdd->vm);
> - amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd, mem);
> + amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd, mem, NULL);
> }
>
> /* kfd_process_alloc_gpuvm - Allocate GPU VM for the KFD process
> @@ -312,7 +344,7 @@ static int kfd_process_alloc_gpuvm(struct kfd_process_device *pdd,
> return err;
>
> err_map_mem:
> - amdgpu_amdkfd_gpuvm_free_memory_of_gpu(kdev->kgd, mem);
> + amdgpu_amdkfd_gpuvm_free_memory_of_gpu(kdev->kgd, mem, NULL);
> err_alloc_mem:
> *kptr = NULL;
> return err;
> @@ -411,6 +443,11 @@ struct kfd_process *kfd_create_process(struct file *filep)
> process->kobj);
> if (!process->kobj_queues)
> pr_warn("Creating KFD proc/queues folder failed");
> +
> + ret = kfd_procfs_add_vram_usage(process);
> + if (ret)
> + pr_warn("Creating vram usage file for pid %d failed",
> + (int)process->lead_thread->pid);
> }
> out:
> if (!IS_ERR(process))
> @@ -488,7 +525,7 @@ static void kfd_process_device_free_bos(struct kfd_process_device *pdd)
> peer_pdd->dev->kgd, mem, peer_pdd->vm);
> }
>
> - amdgpu_amdkfd_gpuvm_free_memory_of_gpu(pdd->dev->kgd, mem);
> + amdgpu_amdkfd_gpuvm_free_memory_of_gpu(pdd->dev->kgd, mem, NULL);
> kfd_process_device_remove_obj_handle(pdd, id);
> }
> }
> @@ -551,6 +588,7 @@ static void kfd_process_wq_release(struct work_struct *work)
> {
> struct kfd_process *p = container_of(work, struct kfd_process,
> release_work);
> + struct kfd_process_device *pdd;
>
> /* Remove the procfs files */
> if (p->kobj) {
> @@ -558,6 +596,10 @@ static void kfd_process_wq_release(struct work_struct *work)
> kobject_del(p->kobj_queues);
> kobject_put(p->kobj_queues);
> p->kobj_queues = NULL;
> +
> + list_for_each_entry(pdd, &p->per_device_data, per_device_list)
> + sysfs_remove_file(p->kobj, &pdd->attr_vram);
> +
> kobject_del(p->kobj);
> kobject_put(p->kobj);
> p->kobj = NULL;
> @@ -862,6 +904,7 @@ struct kfd_process_device *kfd_create_process_device_data(struct kfd_dev *dev,
> pdd->bound = PDD_UNBOUND;
> pdd->already_dequeued = false;
> pdd->runtime_inuse = false;
> + pdd->vram_usage = 0;
> list_add(&pdd->per_device_list, &p->per_device_data);
>
> /* Init idr used for memory handle translation */
More information about the amd-gfx
mailing list