[PATCH 1/1] drm/amdkfd: Handle drain retry fault race with XNACK mode change

[PATCH 1/1] drm/amdkfd: Handle drain retry fault race with XNACK mode change

[Philip Yang]

Application could change XNACK enabled to disabled while KFD is draining
stall retry fault, the check for whether to drain retry faults must be
before the check for whether xnack_enabled, to avoid report incorrect vm
fault after application changes XNACK mode.

Signed-off-by: Philip Yang <Philip.Yang@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index d011d620dfe7..c76d29a22e9d 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -2700,11 +2700,6 @@ svm_range_restore_pages(struct amdgpu_device *adev, unsigned int pasid,
 		pr_debug("kfd process not founded pasid 0x%x\n", pasid);
 		return 0;
 	}
-	if (!p->xnack_enabled) {
-		pr_debug("XNACK not enabled for pasid 0x%x\n", pasid);
-		r = -EFAULT;
-		goto out;
-	}
 	svms = &p->svms;
 
 	pr_debug("restoring svms 0x%p fault address 0x%llx\n", svms, addr);
@@ -2715,6 +2710,12 @@ svm_range_restore_pages(struct amdgpu_device *adev, unsigned int pasid,
 		goto out;
 	}
 
+	if (!p->xnack_enabled) {
+		pr_debug("XNACK not enabled for pasid 0x%x\n", pasid);
+		r = -EFAULT;
+		goto out;
+	}
+
 	/* p->lead_thread is available as kfd_process_wq_release flush the work
 	 * before releasing task ref.
 	 */
-- 
2.35.1

Re: [PATCH 1/1] drm/amdkfd: Handle drain retry fault race with XNACK mode change

[Felix Kuehling]

Am 2022-04-07 um 21:21 schrieb Philip Yang:
> Application could change XNACK enabled to disabled while KFD is draining
> stall retry fault, the check for whether to drain retry faults must be

typo: stall -> stale

I think there is also a word missing, linking the two sentences. Maybe 
"... stale retry faults, therefore the check ..."  With that fixed, the 
patch is

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


> before the check for whether xnack_enabled, to avoid report incorrect vm
> fault after application changes XNACK mode.
>
> Signed-off-by: Philip Yang <Philip.Yang@amd.com>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 11 ++++++-----
>   1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> index d011d620dfe7..c76d29a22e9d 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> @@ -2700,11 +2700,6 @@ svm_range_restore_pages(struct amdgpu_device *adev, unsigned int pasid,
>   		pr_debug("kfd process not founded pasid 0x%x\n", pasid);
>   		return 0;
>   	}
> -	if (!p->xnack_enabled) {
> -		pr_debug("XNACK not enabled for pasid 0x%x\n", pasid);
> -		r = -EFAULT;
> -		goto out;
> -	}
>   	svms = &p->svms;
>   
>   	pr_debug("restoring svms 0x%p fault address 0x%llx\n", svms, addr);
> @@ -2715,6 +2710,12 @@ svm_range_restore_pages(struct amdgpu_device *adev, unsigned int pasid,
>   		goto out;
>   	}
>   
> +	if (!p->xnack_enabled) {
> +		pr_debug("XNACK not enabled for pasid 0x%x\n", pasid);
> +		r = -EFAULT;
> +		goto out;
> +	}
> +
>   	/* p->lead_thread is available as kfd_process_wq_release flush the work
>   	 * before releasing task ref.
>   	 */