[PATCH 1/2] drm/amdgpu: Real return value can be over-written when clean up

[PATCH 1/2] drm/amdgpu: Real return value can be over-written when clean up

[Alex Xie]

Change-Id: Ib69f035eeb213a1aec5025e0a9f4515065706118
Signed-off-by: Alex Xie <AlexBin.Xie@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
index 3453052..76be2d2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
@@ -117,6 +117,11 @@ static void amdgpu_benchmark_move(struct amdgpu_device *adev, unsigned size,
 	}
 
 out_cleanup:
+	/* Check error value now. The value can be overwritten when clean up.*/
+	if (r) {
+		DRM_ERROR("Error while benchmarking BO move.\n");
+	}
+
 	if (sobj) {
 		r = amdgpu_bo_reserve(sobj, false);
 		if (likely(r == 0)) {
@@ -133,10 +138,6 @@ static void amdgpu_benchmark_move(struct amdgpu_device *adev, unsigned size,
 		}
 		amdgpu_bo_unref(&dobj);
 	}
-
-	if (r) {
-		DRM_ERROR("Error while benchmarking BO move.\n");
-	}
 }
 
 void amdgpu_benchmark(struct amdgpu_device *adev, int test_number)
-- 
1.9.1

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

[PATCH 2/2] drm/amdgpu: Fix use of interruptible waiting 1. The signal interrupt can affect the expected behaviour. 2. There is no good mechanism to handle the corresponding error. When signal interrupt happens, unpin is not called. As a result, inside AMDGPU, the statistic of pin size will be wrong.

[Alex Xie]

Change-Id: I6889a4d9dd2703bcf5d448d18f6af51c496a93c9
Signed-off-by: Alex Xie <AlexBin.Xie@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
index 76be2d2..75bd76f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
@@ -123,7 +123,7 @@ static void amdgpu_benchmark_move(struct amdgpu_device *adev, unsigned size,
 	}
 
 	if (sobj) {
-		r = amdgpu_bo_reserve(sobj, false);
+		r = amdgpu_bo_reserve(sobj, true);
 		if (likely(r == 0)) {
 			amdgpu_bo_unpin(sobj);
 			amdgpu_bo_unreserve(sobj);
@@ -131,7 +131,7 @@ static void amdgpu_benchmark_move(struct amdgpu_device *adev, unsigned size,
 		amdgpu_bo_unref(&sobj);
 	}
 	if (dobj) {
-		r = amdgpu_bo_reserve(dobj, false);
+		r = amdgpu_bo_reserve(dobj, true);
 		if (likely(r == 0)) {
 			amdgpu_bo_unpin(dobj);
 			amdgpu_bo_unreserve(dobj);
-- 
1.9.1

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

Re: [PATCH 1/2] drm/amdgpu: Real return value can be over-written when clean up

[Christian König]

Am 24.04.2017 um 21:34 schrieb Alex Xie:
> Change-Id: Ib69f035eeb213a1aec5025e0a9f4515065706118
> Signed-off-by: Alex Xie <AlexBin.Xie@amd.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
> index 3453052..76be2d2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
> @@ -117,6 +117,11 @@ static void amdgpu_benchmark_move(struct amdgpu_device *adev, unsigned size,
>   	}
>   
>   out_cleanup:
> +	/* Check error value now. The value can be overwritten when clean up.*/
> +	if (r) {
> +		DRM_ERROR("Error while benchmarking BO move.\n");
> +	}
> +
>   	if (sobj) {
>   		r = amdgpu_bo_reserve(sobj, false);
>   		if (likely(r == 0)) {
> @@ -133,10 +138,6 @@ static void amdgpu_benchmark_move(struct amdgpu_device *adev, unsigned size,
>   		}
>   		amdgpu_bo_unref(&dobj);
>   	}
> -
> -	if (r) {
> -		DRM_ERROR("Error while benchmarking BO move.\n");
> -	}
>   }
>   
>   void amdgpu_benchmark(struct amdgpu_device *adev, int test_number)


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

Re: [PATCH 2/2] drm/amdgpu: Fix use of interruptible waiting 1. The signal interrupt can affect the expected behaviour. 2. There is no good mechanism to handle the corresponding error. When signal interrupt happens, unpin is not called. As a result, inside AMDGPU, the statistic of pin size will be wrong.

[Christian König]

You somehow messed up the commit message. Everything got mangled into 
the subject line while sending the mails.

Apart from that the change looks good to me and with the commit message 
fixed is Reviewed-by: Christian König <christian.koenig@amd.com>

Regards,
Christian.

Am 24.04.2017 um 21:34 schrieb Alex Xie:
> Change-Id: I6889a4d9dd2703bcf5d448d18f6af51c496a93c9
> Signed-off-by: Alex Xie <AlexBin.Xie@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
> index 76be2d2..75bd76f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_benchmark.c
> @@ -123,7 +123,7 @@ static void amdgpu_benchmark_move(struct amdgpu_device *adev, unsigned size,
>   	}
>   
>   	if (sobj) {
> -		r = amdgpu_bo_reserve(sobj, false);
> +		r = amdgpu_bo_reserve(sobj, true);
>   		if (likely(r == 0)) {
>   			amdgpu_bo_unpin(sobj);
>   			amdgpu_bo_unreserve(sobj);
> @@ -131,7 +131,7 @@ static void amdgpu_benchmark_move(struct amdgpu_device *adev, unsigned size,
>   		amdgpu_bo_unref(&sobj);
>   	}
>   	if (dobj) {
> -		r = amdgpu_bo_reserve(dobj, false);
> +		r = amdgpu_bo_reserve(dobj, true);
>   		if (likely(r == 0)) {
>   			amdgpu_bo_unpin(dobj);
>   			amdgpu_bo_unreserve(dobj);


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