Re: [PATCH v2 1/4] drm/sched: Fix entities with 0 rqs.

On Wed, Jan 30, 2019 at 5:43 AM Christian König ckoenig.leichtzumerken@gmail.com wrote:

Am 30.01.19 um 02:53 schrieb Bas Nieuwenhuizen:

...

Some blocks in amdgpu can have 0 rqs.

Job creation already fails with -ENOENT when entity->rq is NULL, so jobs cannot be pushed. Without a rq there is no scheduler to pop jobs, and rq selection already does the right thing with a list of length 0.

So the operations we need to fix are:

• Creation, do not set rq to rq_list[0] if the list can have length 0.
• Do not flush any jobs when there is no rq.
• On entity destruction handle the rq = NULL case.
• on set_priority, do not try to change the rq if it is NULL.

Signed-off-by: Bas Nieuwenhuizen bas@basnieuwenhuizen.nl

One minor comment on patch #2, apart from that the series is Reviewed-by: Christian König christian.koenig@amd.com.

I'm going to make the change on #2 and pick them up for inclusion in amd-staging-drm-next.

Hi Christian,

I haven't seen these land yet. Just want to make sure they don't fall through the cracks.

Alex

Thanks for the help, Christian.

...

---

drivers/gpu/drm/scheduler/sched_entity.c | 39 ++++++++++++++++-------- 1 file changed, 26 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c index 4463d3826ecb..8e31b6628d09 100644 --- a/drivers/gpu/drm/scheduler/sched_entity.c +++ b/drivers/gpu/drm/scheduler/sched_entity.c @@ -52,12 +52,12 @@ int drm_sched_entity_init(struct drm_sched_entity *entity, { int i;

• if (!(entity && rq_list && num_rq_list > 0 && rq_list[0]))
  

• if (!(entity && rq_list && (num_rq_list == 0 || rq_list[0])))
          return -EINVAL;
  
  memset(entity, 0, sizeof(struct drm_sched_entity));
  INIT_LIST_HEAD(&entity->list);
  

• entity->rq = rq_list[0];
  

• entity->rq = NULL;
  entity->guilty = guilty;
  entity->num_rq_list = num_rq_list;
  entity->rq_list = kcalloc(num_rq_list, sizeof(struct drm_sched_rq *),
  

@@ -67,6 +67,10 @@ int drm_sched_entity_init(struct drm_sched_entity *entity,

  for (i = 0; i < num_rq_list; ++i)
          entity->rq_list[i] = rq_list[i];

• if (num_rq_list)
  

•         entity->rq = rq_list[0];
  

• entity->last_scheduled = NULL;
  
  spin_lock_init(&entity->rq_lock);
  

@@ -165,6 +169,9 @@ long drm_sched_entity_flush(struct drm_sched_entity *entity, long timeout) struct task_struct *last_user; long ret = timeout;

• if (!entity->rq)
  

•         return 0;
  

• sched = entity->rq->sched;
  /**
   * The client will not queue more IBs during this fini, consume existing
  

@@ -264,20 +271,24 @@ static void drm_sched_entity_kill_jobs(struct drm_sched_entity *entity) */ void drm_sched_entity_fini(struct drm_sched_entity *entity) {

• struct drm_gpu_scheduler *sched;
  

• struct drm_gpu_scheduler *sched = NULL;
  
  

• sched = entity->rq->sched;
  

• drm_sched_rq_remove_entity(entity->rq, entity);
  

• if (entity->rq) {
  

•         sched = entity->rq->sched;
  

•         drm_sched_rq_remove_entity(entity->rq, entity);
  

• }
  
  /* Consumption of existing IBs wasn't completed. Forcefully
   * remove them here.
   */
  if (spsc_queue_peek(&entity->job_queue)) {
  

•         /* Park the kernel for a moment to make sure it isn't processing
  

•          * our enity.
  

•          */
  

•         kthread_park(sched->thread);
  

•         kthread_unpark(sched->thread);
  

•         if (sched) {
  

•                 /* Park the kernel for a moment to make sure it isn't processing
  

•                  * our enity.
  

•                  */
  

•                 kthread_park(sched->thread);
  

•                 kthread_unpark(sched->thread);
  

•         }
          if (entity->dependency) {
                  dma_fence_remove_callback(entity->dependency,
                                            &entity->cb);
  

@@ -362,9 +373,11 @@ void drm_sched_entity_set_priority(struct drm_sched_entity *entity, for (i = 0; i < entity->num_rq_list; ++i) drm_sched_entity_set_rq_priority(&entity->rq_list[i], priority);

• drm_sched_rq_remove_entity(entity->rq, entity);
  

• drm_sched_entity_set_rq_priority(&entity->rq, priority);
  

• drm_sched_rq_add_entity(entity->rq, entity);
  

• if (entity->rq) {
  

•         drm_sched_rq_remove_entity(entity->rq, entity);
  

•         drm_sched_entity_set_rq_priority(&entity->rq, priority);
  

•         drm_sched_rq_add_entity(entity->rq, entity);
  

• }
  
  spin_unlock(&entity->rq_lock);
  

  }

---

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