Re: [PATCH] drm/amdgpu: Adjust the VM size based on system memory size

["Christian König" <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

Am 21.08.2018 um 23:45 schrieb Felix Kuehling:
> This patch is meant for amd-staging-drm-next. It's part of my effort to
> reduce differences between kfd-staging and upstream and eventually
> replace all our memory manager hacks with upstream solutions.
>
> This commit should only affect GFXv8 and older. It should not have any
> negative impact, except bigger GPUVM page tables on systems with lots of
> memory. Conversely, this could actually allow making the GPUVM page
> tables smaller on systems with little memory, if we wanted to take
> advantage of that opportunity.
>
> The current minimum is 64GB, which I believe means 32KB page tables and
> page directories. The smallest VM size that makes sense with 2-levels of
> 4KB page tables would be 1GB. If we really want to go that small, I
> should also factor the local VRAM size into the equation.
>
> Regards,
>    Felix
>
>
> On 2018-08-21 05:25 PM, Felix Kuehling wrote:
>> Set the VM size based on system memory size between the ASIC-specific
>> limits given by min_vm_size and max_bits. GFXv9 GPUs will keep their
>> default VM size of 256TB (48 bit). Only older GPUs will adjust VM size
>> depending on system memory size.
>>
>> This makes more VM space available for ROCm applications on GFXv8 GPUs
>> that want to map all available VRAM and system memory in their SVM
>> address space.
>>
>> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 26 ++++++++++++++++++++++----
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  2 +-
>>   2 files changed, 23 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index 662e8a3..48971cc 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -2482,28 +2482,46 @@ static uint32_t amdgpu_vm_get_block_size(uint64_t vm_size)
>>    * amdgpu_vm_adjust_size - adjust vm size, block size and fragment size
>>    *
>>    * @adev: amdgpu_device pointer
>> - * @vm_size: the default vm size if it's set auto
>> + * @min_vm_size: the minimum vm size in GB if it's set auto
>>    * @fragment_size_default: Default PTE fragment size
>>    * @max_level: max VMPT level
>>    * @max_bits: max address space size in bits
>>    *
>>    */
>> -void amdgpu_vm_adjust_size(struct amdgpu_device *adev, uint32_t vm_size,
>> +void amdgpu_vm_adjust_size(struct amdgpu_device *adev, uint32_t min_vm_size,
>>   			   uint32_t fragment_size_default, unsigned max_level,
>>   			   unsigned max_bits)
>>   {
>> +	unsigned int max_size = 1 << (max_bits - 30);
>> +	unsigned int vm_size;
>>   	uint64_t tmp;
>>   
>>   	/* adjust vm size first */
>>   	if (amdgpu_vm_size != -1) {
>> -		unsigned max_size = 1 << (max_bits - 30);
>> -
>>   		vm_size = amdgpu_vm_size;
>>   		if (vm_size > max_size) {
>>   			dev_warn(adev->dev, "VM size (%d) too large, max is %u GB\n",
>>   				 amdgpu_vm_size, max_size);
>>   			vm_size = max_size;
>>   		}
>> +	} else {
>> +		struct sysinfo si;
>> +		unsigned int phys_ram_gb;
>> +
>> +		/* Optimal VM size depends on the amount of physical
>> +		 * RAM available. Underlying requirements and
>> +		 * assumptions:
>> +		 *
>> +		 *  - Need to map system memory and VRAM from all GPUs
>> +		 *     - VRAM from other GPUs not known here
>> +		 *     - Assume VRAM <= system memory
>> +		 *  - On GFX8 and older, VM space can be segmented for
>> +		 *    different MTYPEs
>> +		 *  - Need to allow room for fragmentation, guard pages etc.
>> +		 */
>> +		si_meminfo(&si);
>> +		phys_ram_gb = ((uint64_t)si.totalram * si.mem_unit) >> 30;

Looks good to me, but I would make sure that round that up before 
shifting it.

>> +		vm_size = min(max(phys_ram_gb * 3, min_vm_size), max_size);

Mhm, "phys_ram_gb * 3"? Maybe add a comment with the rational for that.

Christian.

>>   	}
>>   
>>   	adev->vm_manager.max_pfn = (uint64_t)vm_size << 18;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> index 1162c2b..ab1d23e 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> @@ -345,7 +345,7 @@ struct amdgpu_bo_va_mapping *amdgpu_vm_bo_lookup_mapping(struct amdgpu_vm *vm,
>>   void amdgpu_vm_bo_trace_cs(struct amdgpu_vm *vm, struct ww_acquire_ctx *ticket);
>>   void amdgpu_vm_bo_rmv(struct amdgpu_device *adev,
>>   		      struct amdgpu_bo_va *bo_va);
>> -void amdgpu_vm_adjust_size(struct amdgpu_device *adev, uint32_t vm_size,
>> +void amdgpu_vm_adjust_size(struct amdgpu_device *adev, uint32_t min_vm_size,
>>   			   uint32_t fragment_size_default, unsigned max_level,
>>   			   unsigned max_bits);
>>   int amdgpu_vm_ioctl(struct drm_device *dev, void *data, struct drm_file *filp);
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx