Re: [PATCH] drm/amdkfd: Calculate CPU VCRAT size dynamically

[Felix Kuehling <felix.kuehling@amd.com>]

Am 2020-09-18 um 1:31 p.m. schrieb Kent Russell:
> Instead of guessing at a sufficient size for the CPU VCRAT, base the
> size on the number of online NUMA nodes.
>
> Signed-off-by: Kent Russell <kent.russell@amd.com>
> Change-Id: I5fb6e60f1410ad696ab78d780d0b40d01a4f829b

Please remove the ChangeId.


> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_crat.c | 26 ++++++++++++++++----------
>  1 file changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> index 3fac06b281ce..99182b8e9152 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> @@ -809,11 +809,10 @@ int kfd_create_crat_image_acpi(void **crat_image, size_t *size)
>  
>  /* Memory required to create Virtual CRAT.
>   * Since there is no easy way to predict the amount of memory required, the
> - * following amount are allocated for CPU and GPU Virtual CRAT. This is
> + * following amount is allocated for GPU Virtual CRAT. This is
>   * expected to cover all known conditions. But to be safe additional check
>   * is put in the code to ensure we don't overwrite.
>   */
> -#define VCRAT_SIZE_FOR_CPU	(2 * PAGE_SIZE)
>  #define VCRAT_SIZE_FOR_GPU	(4 * PAGE_SIZE)
>  
>  /* kfd_fill_cu_for_cpu - Fill in Compute info for the given CPU NUMA node
> @@ -964,7 +963,7 @@ static int kfd_create_vcrat_image_cpu(void *pcrat_image, size_t *size)
>  #endif
>  	int ret = 0;
>  
> -	if (!pcrat_image || avail_size < VCRAT_SIZE_FOR_CPU)
> +	if (!pcrat_image)
>  		return -EINVAL;
>  
>  	/* Fill in CRAT Header.
> @@ -1364,24 +1363,31 @@ int kfd_create_crat_image_virtual(void **crat_image, size_t *size,
>  				  uint32_t proximity_domain)
>  {
>  	void *pcrat_image = NULL;
> -	int ret = 0;
> +	int ret = 0, num_nodes;
> +	size_t dyn_size;
>  
>  	if (!crat_image)
>  		return -EINVAL;
>  
>  	*crat_image = NULL;
>  
> -	/* Allocate one VCRAT_SIZE_FOR_CPU for CPU virtual CRAT image and
> -	 * VCRAT_SIZE_FOR_GPU for GPU virtual CRAT image. This should cover
> -	 * all the current conditions. A check is put not to overwrite beyond
> -	 * allocated size
> +	/* Allocate the CPU Virtual CRAT size based on the number of online
> +	 * nodes. Allocate VCRAT_SIZE_FOR_GPU for GPU virtual CRAT image.
> +	 * This should cover all the current conditions. A check is put not
> +	 * to overwrite beyond allocated size for GPUs
>  	 */
>  	switch (flags) {
>  	case COMPUTE_UNIT_CPU:
> -		pcrat_image = kmalloc(VCRAT_SIZE_FOR_CPU, GFP_KERNEL);
> +		num_nodes = num_online_nodes();
> +		dyn_size = sizeof(struct crat_header) +
> +			num_nodes * (sizeof(struct crat_subtype_computeunit) +
> +			sizeof(struct crat_subtype_memory) +
> +			(num_nodes - 1) * sizeof(struct crat_subtype_iolink));
> +		pcrat_image = kmalloc(dyn_size, GFP_KERNEL);

We should use kvmalloc here. Then we also need kvfree to free the
memory. For consistency, use kvmalloc for GPU CRAT allocation, too, and
replace the kmemdup in kfd_create_crat_image_acpi with kvmalloc+memcpy.

Let's make that kmalloc->kvmalloc change a second commit.

This patch is
Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>

Regards,
  Felix


>  		if (!pcrat_image)
>  			return -ENOMEM;
> -		*size = VCRAT_SIZE_FOR_CPU;
> +		*size = dyn_size;
> +		pr_debug("CRAT size is %d", dyn_size);
>  		ret = kfd_create_vcrat_image_cpu(pcrat_image, size);
>  		break;
>  	case COMPUTE_UNIT_GPU:
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx