Am 18.05.21 um 17:03 schrieb Andrey Grodzovsky:
On 2021-05-18 10:07 a.m., Christian König wrote:
In a separate discussion with Daniel we once more iterated over the
dma_resv requirements and I came to the conclusion that this approach
here won't work reliable.
The problem is as following:
1. device A schedules some rendering with into a buffer and exports
it as DMA-buf.
2. device B imports the DMA-buf and wants to consume the rendering,
for the the fence of device A is replaced with a new operation.
3. device B is hot plugged and the new operation canceled/newer
scheduled.
The problem is now that we can't do this since the operation of
device A is still running and by signaling our fences we run into the
problem of potential memory corruption.
I am not sure this problem you describe above is related to this patch.
Well it is kind of related.
Here we purely expand the criteria for when sched_entity is
considered idle in order to prevent a hang on device remove.
And exactly that is problematic. See the jobs on the entity need to
cleanly wait for their dependencies before they can be completed.
drm_sched_entity_kill_jobs() is also not handling that correctly at the
moment, we only wait for the last scheduled fence but not for the
dependencies of the job.
Were you addressing the patch from yesterday in which you commented
that you found a problem with how we finish fences ? It was
'[PATCH v7 12/16] drm/amdgpu: Fix hang on device removal.'
Also, in the patch series as it is now we only signal HW fences for the
extracted device B, we are not touching any other fences. In fact as you
may remember, I dropped all new logic to forcing fence completion in
this patch series and only call amdgpu_fence_driver_force_completion
for the HW fences of the extracted device as it's done today anyway.
Signaling hardware fences is unproblematic since they are emitted when
the software scheduling is already completed.
Christian.
Andrey
Not sure how to handle that case. One possibility would be to wait
for all dependencies of unscheduled jobs before signaling their
fences as canceled.
Christian.
Am 12.05.21 um 16:26 schrieb Andrey Grodzovsky:
Problem: If scheduler is already stopped by the time sched_entity
is released and entity's job_queue not empty I encountred
a hang in drm_sched_entity_flush. This is because
drm_sched_entity_is_idle
never becomes false.
Fix: In drm_sched_fini detach all sched_entities from the
scheduler's run queues. This will satisfy drm_sched_entity_is_idle.
Also wakeup all those processes stuck in sched_entity flushing
as the scheduler main thread which wakes them up is stopped by now.
v2:
Reverse order of drm_sched_rq_remove_entity and marking
s_entity as stopped to prevent reinserion back to rq due
to race.
v3:
Drop drm_sched_rq_remove_entity, only modify entity->stopped
and check for it in drm_sched_entity_is_idle
Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@xxxxxxx>
Reviewed-by: Christian König <christian.koenig@xxxxxxx>
---
drivers/gpu/drm/scheduler/sched_entity.c | 3 ++-
drivers/gpu/drm/scheduler/sched_main.c | 24
++++++++++++++++++++++++
2 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/scheduler/sched_entity.c
b/drivers/gpu/drm/scheduler/sched_entity.c
index 0249c7450188..2e93e881b65f 100644
--- a/drivers/gpu/drm/scheduler/sched_entity.c
+++ b/drivers/gpu/drm/scheduler/sched_entity.c
@@ -116,7 +116,8 @@ static bool drm_sched_entity_is_idle(struct
drm_sched_entity *entity)
rmb(); /* for list_empty to work without lock */
if (list_empty(&entity->list) ||
- spsc_queue_count(&entity->job_queue) == 0)
+ spsc_queue_count(&entity->job_queue) == 0 ||
+ entity->stopped)
return true;
return false;
diff --git a/drivers/gpu/drm/scheduler/sched_main.c
b/drivers/gpu/drm/scheduler/sched_main.c
index 8d1211e87101..a2a953693b45 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -898,9 +898,33 @@ EXPORT_SYMBOL(drm_sched_init);
*/
void drm_sched_fini(struct drm_gpu_scheduler *sched)
{
+ struct drm_sched_entity *s_entity;
+ int i;
+
if (sched->thread)
kthread_stop(sched->thread);
+ for (i = DRM_SCHED_PRIORITY_COUNT - 1; i >=
DRM_SCHED_PRIORITY_MIN; i--) {
+ struct drm_sched_rq *rq = &sched->sched_rq[i];
+
+ if (!rq)
+ continue;
+
+ spin_lock(&rq->lock);
+ list_for_each_entry(s_entity, &rq->entities, list)
+ /*
+ * Prevents reinsertion and marks job_queue as idle,
+ * it will removed from rq in drm_sched_entity_fini
+ * eventually
+ */
+ s_entity->stopped = true;
+ spin_unlock(&rq->lock);
+
+ }
+
+ /* Wakeup everyone stuck in drm_sched_entity_flush for this
scheduler */
+ wake_up_all(&sched->job_scheduled);
+
/* Confirm no work left behind accessing device structures */
cancel_delayed_work_sync(&sched->work_tdr);
