On Tue, Jul 31, 2018 at 5:02 PM Christian König <
ckoenig.leichtzumerken@gmail.com> wrote:

> Am 31.07.2018 um 12:37 schrieb Nayan Deshmukh:
> > This is the first attempt to move entities between schedulers to
> > have dynamic load balancing. We just move entities with no jobs for
> > now as moving the ones with jobs will lead to other compilcations
> > like ensuring that the other scheduler does not remove a job from
> > the current entity while we are moving.
> >
> > Signed-off-by: Nayan Deshmukh <nayan26deshmukh@gmail.com>
> > ---
> >   drivers/gpu/drm/scheduler/gpu_scheduler.c | 25
> ++++++++++++++++++++-----
> >   1 file changed, 20 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c
> b/drivers/gpu/drm/scheduler/gpu_scheduler.c
> > index c67f65ad8f15..f665a84d48ef 100644
> > --- a/drivers/gpu/drm/scheduler/gpu_scheduler.c
> > +++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c
> > @@ -540,6 +540,8 @@ drm_sched_entity_pop_job(struct drm_sched_entity
> *entity)
> >       if (!sched_job)
> >               return NULL;
> >
> > +     sched_job->sched = sched;
> > +     sched_job->s_fence->sched = sched;
> >       while ((entity->dependency = sched->ops->dependency(sched_job,
> entity)))
> >               if (drm_sched_entity_add_dependency_cb(entity))
> >                       return NULL;
> > @@ -570,16 +572,29 @@ drm_sched_entity_pop_job(struct drm_sched_entity
> *entity)
> >   void drm_sched_entity_push_job(struct drm_sched_job *sched_job,
> >                              struct drm_sched_entity *entity)
> >   {
> > -     struct drm_gpu_scheduler *sched = sched_job->sched;
> > -     bool first = false;
> > +     struct drm_gpu_scheduler *sched = entity->rq->sched;
>
> Is the local "sched" variable actually still used?
>
> Might be a good idea to drop that since we potentially changing the
> scheduler/rq.
>
Yes dropping it is a good idea to avoid confusion. I had kept it for
debugging purpose but forgot to remove it in the end.


>
> > +     struct drm_sched_rq *rq = entity->rq;
> > +     bool first = false, reschedule, idle;
> >
> > -     trace_drm_sched_job(sched_job, entity);
> > +     idle = entity->last_scheduled == NULL ||
> > +             dma_fence_is_signaled(entity->last_scheduled);
> > +     first = spsc_queue_count(&entity->job_queue) == 0;
> > +     reschedule = idle && first && (entity->num_rq_list > 1);
> > +
> > +     if (reschedule) {
> > +             rq = drm_sched_entity_get_free_sched(entity);
> > +             spin_lock(&entity->rq_lock);
> > +             drm_sched_rq_remove_entity(entity->rq, entity);
> > +             entity->rq = rq;
> > +             spin_unlock(&entity->rq_lock);
> > +     }
> >
> > +     trace_drm_sched_job(sched_job, entity);
> >       atomic_inc(&entity->rq->sched->num_jobs);
> >       first = spsc_queue_push(&entity->job_queue,
> &sched_job->queue_node);
> >
> >       /* first job wakes up scheduler */
> > -     if (first) {
> > +     if (first || reschedule) {
>
> You can drop that extra check since we can only rescheduler when there
> wasn't any jobs in the entity.

Will fix it in v2.

>
> Christian.
>
> >               /* Add the entity to the run queue */
> >               spin_lock(&entity->rq_lock);
> >               if (!entity->rq) {
> > @@ -589,7 +604,7 @@ void drm_sched_entity_push_job(struct drm_sched_job
> *sched_job,
> >               }
> >               drm_sched_rq_add_entity(entity->rq, entity);
> >               spin_unlock(&entity->rq_lock);
> > -             drm_sched_wakeup(sched);
> > +             drm_sched_wakeup(entity->rq->sched);
> >       }
> >   }
> >   EXPORT_SYMBOL(drm_sched_entity_push_job);
>
>