[PATCH] drm/etnaviv: fix another potential dma_resv DAG violation

[PATCH] drm/etnaviv: fix another potential dma_resv DAG violation

[Christian König]

Setting the exclusive fence without waiting for the shared
fences violates the DAG and is illegal.

We really need to get away from this ASAP and make the
driver interface more bullet prove.

Signed-off-by: Christian König <christian.koenig@amd.com>
Cc: <stable@vger.kernel.org>
---
 drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
index 7e17bc2b5df1..4db40e88f67f 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
@@ -185,19 +185,15 @@ static int submit_fence_sync(struct etnaviv_gem_submit *submit)
 				return ret;
 		}
 
-		if (submit->flags & ETNA_SUBMIT_NO_IMPLICIT)
-			continue;
-
 		if (bo->flags & ETNA_SUBMIT_BO_WRITE) {
 			ret = dma_resv_get_fences(robj, &bo->excl,
 						  &bo->nr_shared,
 						  &bo->shared);
 			if (ret)
 				return ret;
-		} else {
+		} else if (!(submit->flags & ETNA_SUBMIT_NO_IMPLICIT)) {
 			bo->excl = dma_fence_get(dma_resv_excl_fence(robj));
 		}
-
 	}
 
 	return ret;
-- 
2.25.1

Re: [PATCH] drm/etnaviv: fix another potential dma_resv DAG violation

[Lucas Stach]

Am Mittwoch, dem 22.09.2021 um 13:41 +0200 schrieb Christian König:
> Setting the exclusive fence without waiting for the shared
> fences violates the DAG and is illegal.
> 
> We really need to get away from this ASAP and make the
> driver interface more bullet prove.

Uh, what happened to Daniels patch "drm/etnaviv: Don't break exclusive
fence ordering", which fixed this without totally doing away with the
no-implicit handling? I have to admit that I haven't followed up with
that patch series due to ETOOMUCHOTHERSTUFF, but from a quick glance
this patch looked better than what you propose here.

Regards,
Lucas

> 
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Cc: <stable@vger.kernel.org>
> ---
>  drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> index 7e17bc2b5df1..4db40e88f67f 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> @@ -185,19 +185,15 @@ static int submit_fence_sync(struct etnaviv_gem_submit *submit)
>  				return ret;
>  		}
>  
> -		if (submit->flags & ETNA_SUBMIT_NO_IMPLICIT)
> -			continue;
> -
>  		if (bo->flags & ETNA_SUBMIT_BO_WRITE) {
>  			ret = dma_resv_get_fences(robj, &bo->excl,
>  						  &bo->nr_shared,
>  						  &bo->shared);
>  			if (ret)
>  				return ret;
> -		} else {
> +		} else if (!(submit->flags & ETNA_SUBMIT_NO_IMPLICIT)) {
>  			bo->excl = dma_fence_get(dma_resv_excl_fence(robj));
>  		}
> -
>  	}
>  
>  	return ret;

Re: [PATCH] drm/etnaviv: fix another potential dma_resv DAG violation

[Christian König]

Am 22.09.21 um 14:28 schrieb Lucas Stach:
> Am Mittwoch, dem 22.09.2021 um 13:41 +0200 schrieb Christian König:
>> Setting the exclusive fence without waiting for the shared
>> fences violates the DAG and is illegal.
>>
>> We really need to get away from this ASAP and make the
>> driver interface more bullet prove.
> Uh, what happened to Daniels patch "drm/etnaviv: Don't break exclusive
> fence ordering", which fixed this without totally doing away with the
> no-implicit handling?

I was wondering the same thing. My memory was that Daniel has already 
fixed this, but that still looks totally broken to me.

> I have to admit that I haven't followed up with
> that patch series due to ETOOMUCHOTHERSTUFF, but from a quick glance
> this patch looked better than what you propose here.

This patch here should have the same result as Daniels version, just 
with CC: stable on it, fewer changes lines and not depending on the 
drm_sched_job_await_implicit() change.

Regards,
Christian.

>
> Regards,
> Lucas
>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> Cc: <stable@vger.kernel.org>
>> ---
>>   drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 6 +-----
>>   1 file changed, 1 insertion(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
>> index 7e17bc2b5df1..4db40e88f67f 100644
>> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
>> @@ -185,19 +185,15 @@ static int submit_fence_sync(struct etnaviv_gem_submit *submit)
>>   				return ret;
>>   		}
>>   
>> -		if (submit->flags & ETNA_SUBMIT_NO_IMPLICIT)
>> -			continue;
>> -
>>   		if (bo->flags & ETNA_SUBMIT_BO_WRITE) {
>>   			ret = dma_resv_get_fences(robj, &bo->excl,
>>   						  &bo->nr_shared,
>>   						  &bo->shared);
>>   			if (ret)
>>   				return ret;
>> -		} else {
>> +		} else if (!(submit->flags & ETNA_SUBMIT_NO_IMPLICIT)) {
>>   			bo->excl = dma_fence_get(dma_resv_excl_fence(robj));
>>   		}
>> -
>>   	}
>>   
>>   	return ret;
>