[PATCH 1/2] drm/amd/scheduler: fix page protection of cb

[PATCH 1/2] drm/amd/scheduler: fix page protection of cb

[Chunming Zhou]

We must remove the fence callback.

Change-Id: I5d58a3a43b82fefd6c211c4128b0c9187c191e7f
Signed-off-by: Chunming Zhou <david1.zhou@amd.com>
---
 drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
index af5b2c50abac..310904042dfc 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
@@ -231,6 +231,13 @@ void amd_sched_entity_fini(struct amd_gpu_scheduler *sched,
 		 */
 		kcl_kthread_park(sched->thread);
 		kcl_kthread_unpark(sched->thread);
+		if (entity->dependency) {
+			dma_fence_remove_callback(entity->dependency,
+						  &entity->cb);
+			dma_fence_put(entity->dependency);
+			entity->dependency = NULL;
+		}
+
 		while ((job = to_amd_sched_job(spsc_queue_pop(&entity->job_queue)))) {
 			struct amd_sched_fence *s_fence = job->s_fence;
 			amd_sched_fence_scheduled(s_fence);
-- 
2.14.1

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

[PATCH 2/2] drm/amd/scheduler: remove fence callback when context is killed

[Chunming Zhou]

Otherwise there could be page protection.

Change-Id: I1f6c81002fb2ba21c17cdc14fdde86579b28374e
Signed-off-by: Chunming Zhou <david1.zhou@amd.com>
---
 drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
index 310904042dfc..e38d55961a9f 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
@@ -243,6 +243,12 @@ void amd_sched_entity_fini(struct amd_gpu_scheduler *sched,
 			amd_sched_fence_scheduled(s_fence);
 			kcl_dma_fence_set_error(&s_fence->finished, -ESRCH);
 			amd_sched_fence_finished(s_fence);
+			if (s_fence->parent) {
+				dma_fence_remove_callback(s_fence->parent,
+							  &s_fence->cb);
+				dma_fence_put(s_fence->parent);
+				s_fence->parent = NULL;
+			}
 			dma_fence_put(&s_fence->finished);
 			sched->ops->free_job(job);
 		}
-- 
2.14.1

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

Re: [PATCH 1/2] drm/amd/scheduler: fix page protection of cb

[Christian König]

Am 07.11.2017 um 03:48 schrieb Chunming Zhou:
> We must remove the fence callback.
>
> Change-Id: I5d58a3a43b82fefd6c211c4128b0c9187c191e7f
> Signed-off-by: Chunming Zhou <david1.zhou@amd.com>
> ---
>   drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 7 +++++++
>   1 file changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
> index af5b2c50abac..310904042dfc 100644
> --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
> +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
> @@ -231,6 +231,13 @@ void amd_sched_entity_fini(struct amd_gpu_scheduler *sched,
>   		 */
>   		kcl_kthread_park(sched->thread);
>   		kcl_kthread_unpark(sched->thread);

In general please base all patches on amd-staging-drm-next and not some 
hybrid branch.

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

> +		if (entity->dependency) {
> +			dma_fence_remove_callback(entity->dependency,
> +						  &entity->cb);
> +			dma_fence_put(entity->dependency);
> +			entity->dependency = NULL;
> +		}
> +
>   		while ((job = to_amd_sched_job(spsc_queue_pop(&entity->job_queue)))) {
>   			struct amd_sched_fence *s_fence = job->s_fence;
>   			amd_sched_fence_scheduled(s_fence);


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

Re: [PATCH 2/2] drm/amd/scheduler: remove fence callback when context is killed

[Christian König]

Am 07.11.2017 um 03:48 schrieb Chunming Zhou:
> Otherwise there could be page protection.
>
> Change-Id: I1f6c81002fb2ba21c17cdc14fdde86579b28374e
> Signed-off-by: Chunming Zhou <david1.zhou@amd.com>
> ---
>   drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
> index 310904042dfc..e38d55961a9f 100644
> --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
> +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
> @@ -243,6 +243,12 @@ void amd_sched_entity_fini(struct amd_gpu_scheduler *sched,
>   			amd_sched_fence_scheduled(s_fence);
>   			kcl_dma_fence_set_error(&s_fence->finished, -ESRCH);
>   			amd_sched_fence_finished(s_fence);
> +			if (s_fence->parent) {
> +				dma_fence_remove_callback(s_fence->parent,
> +							  &s_fence->cb);
> +				dma_fence_put(s_fence->parent);
> +				s_fence->parent = NULL;
> +			}

Mhm, that looks unnecessary to me. The fences should have never been 
scheduled to the hardware and so shouldn't have a parent fence set.

I would rather add a WARN_ON(s_fence->parent); here.

Regards,
Christian.

>   			dma_fence_put(&s_fence->finished);
>   			sched->ops->free_job(job);
>   		}


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