Re: [PATCH v2] drm/amdkfd: Track GPU memory utilization per process

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 2020-04-22 um 11:13 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.

Thanks. This looks almost ready. See one more nit-pick inline.


>
> Signed-off-by: Mukul Joshi <mukul.joshi@xxxxxxx>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h    |  3 +-
>  .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |  9 ++-
>  drivers/gpu/drm/amd/amdkfd/kfd_chardev.c      | 17 +++++-
>  drivers/gpu/drm/amd/amdkfd/kfd_priv.h         |  7 +++
>  drivers/gpu/drm/amd/amdkfd/kfd_process.c      | 57 ++++++++++++++++---
>  5 files changed, 81 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..fe6615a06cd0 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;
> @@ -1340,6 +1340,12 @@ 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
> +	 */
> +	if (size && (mem->bo->preferred_domains == AMDGPU_GEM_DOMAIN_VRAM))
> +		*size = bo_size;
> +
>  	/* Free the BO*/
>  	amdgpu_bo_unref(&mem->bo);
>  	mutex_destroy(&mem->lock);
> @@ -1694,6 +1700,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..aac2cdb65eb5 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,8 @@ static int kfd_ioctl_free_memory_of_gpu(struct file *filep,
>  	void *mem;
>  	struct kfd_dev *dev;
>  	int ret;
> +	uint64_t size = 0;
> +	bool is_imported = 0;
>  
>  	dev = kfd_device_by_id(GET_GPU_ID(args->handle));
>  	if (!dev)
> @@ -1372,8 +1378,10 @@ static int kfd_ioctl_free_memory_of_gpu(struct file *filep,
>  		goto err_unlock;
>  	}
>  
> +	is_imported = ((struct kgd_mem *)mem)->is_imported;
> +
>  	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 +1390,9 @@ static int kfd_ioctl_free_memory_of_gpu(struct file *filep,
>  		kfd_process_device_remove_obj_handle(
>  			pdd, GET_IDR_HANDLE(args->handle));
>  
> +	if (!is_imported)
> +		pdd->vram_usage -= size;

This check should be in amdgpu_amdkfd_free_memory_of_gpu so that that
function returns 0 freed size for imported BOs. That would keep all the
handling of the is_imported flag in the same source file.

It would also save you an awkward cast above that works around the
kgd-kfd-interface abstraction.

Regards,
  Felix


> +
>  err_unlock:
>  	mutex_unlock(&p->mutex);
>  	return ret;
> @@ -1726,7 +1737,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 */
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux