Re: [PATCH v2 3/3] drm/amdkfd: Use atomic64_t type for pdd->tlb_seq

["Christian König" <christian.koenig@amd.com>]

Am 28.03.22 um 15:07 schrieb Philip Yang:
> To support multi-thread update page table.
>
> Signed-off-by: Philip Yang <Philip.Yang@amd.com>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_priv.h    | 2 +-
>   drivers/gpu/drm/amd/amdkfd/kfd_process.c | 6 +++---
>   2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index 945982a5d688..e1b7e6afa920 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -705,7 +705,7 @@ struct kfd_process_device {
>   	/* VM context for GPUVM allocations */
>   	struct file *drm_file;
>   	void *drm_priv;
> -	uint64_t tlb_seq;
> +	atomic64_t tlb_seq;
>   
>   	/* GPUVM allocations storage */
>   	struct idr alloc_idr;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> index ac8123c1ee8f..43ed8ec1f975 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> @@ -1560,7 +1560,7 @@ int kfd_process_device_init_vm(struct kfd_process_device *pdd,
>   		return ret;
>   	}
>   	pdd->drm_priv = drm_file->private_data;
> -	pdd->tlb_seq = 0;
> +	atomic64_set(&pdd->tlb_seq, 0);
>   
>   	ret = kfd_process_device_reserve_ib_mem(pdd);
>   	if (ret)
> @@ -1954,10 +1954,10 @@ void kfd_flush_tlb(struct kfd_process_device *pdd, enum TLB_FLUSH_TYPE type)
>   	uint64_t tlb_seq = amdgpu_vm_tlb_seq(vm);
>   	struct kfd_dev *dev = pdd->dev;
>   
> -	if (pdd->tlb_seq == tlb_seq)
> +	if (atomic64_read(&pdd->tlb_seq) == tlb_seq)
>   		return;
>   
> -	pdd->tlb_seq = tlb_seq;
> +	atomic64_set(&pdd->tlb_seq, tlb_seq);

That should probably use atomic64_xchg() instead of read+set or 
otherwise using an atomic doesn't make much sense.

Christian.

>   	if (dev->dqm->sched_policy == KFD_SCHED_POLICY_NO_HWS) {
>   		/* Nothing to flush until a VMID is assigned, which
>   		 * only happens when the first queue is created.