[PATCH] drm/amdgpu: Checkpoint and Restore VRAM BOs without VA

[Felix Kuehling]

On 2023-07-24 11:57, Ramesh Errabolu wrote:
> Extend checkpoint logic to allow inclusion of VRAM BOs that
> do not have a VA attached
>
> Signed-off-by: Ramesh Errabolu <Ramesh.Errabolu at amd.com>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> index 40ac093b5035..5cc00ff4b635 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> @@ -1845,7 +1845,8 @@ static uint32_t get_process_num_bos(struct kfd_process *p)
>   		idr_for_each_entry(&pdd->alloc_idr, mem, id) {
>   			struct kgd_mem *kgd_mem = (struct kgd_mem *)mem;
>   
> -			if ((uint64_t)kgd_mem->va > pdd->gpuvm_base)
> +			if (((uint64_t)kgd_mem->va > pdd->gpuvm_base) ||
> +			    (kgd_mem->va == 0))

I'm trying to remember what this condition is there to protect against, 
because it almost looks like we could drop the entire condition. I think 
it's there to avoid checkpointing the TMA/TBA BOs allocated by KFD itself.

That said, you have some unnecessary parentheses in this expression. And 
just using !x to check for 0 is usually preferred in the kernel. This 
should work and is more readable IMO:

+			if ((uint64_t)kgd_mem->va > pdd->gpuvm_base || !kgd_mem->va)


>   				num_of_bos++;
>   		}
>   	}
> @@ -1917,7 +1918,8 @@ static int criu_checkpoint_bos(struct kfd_process *p,
>   			kgd_mem = (struct kgd_mem *)mem;
>   			dumper_bo = kgd_mem->bo;
>   
> -			if ((uint64_t)kgd_mem->va <= pdd->gpuvm_base)
> +			if (((uint64_t)kgd_mem->va <= pdd->gpuvm_base) &&
> +				!(kgd_mem->va == 0))

Similar to above:

+			if (kgd_mem->va && (uint64_t)kgd_mem->va <= pdd->gpuvm_base)

Regards,
   Felix


>   				continue;
>   
>   			bo_bucket = &bo_buckets[bo_index];