[PATCH 0/4] drm/scheduler: add dynamic balancing

[PATCH 0/4] drm/scheduler: add dynamic balancing

[Nayan Deshmukh]

This is the my first attempt to include dynamic balancing as part
of the scheduler. I have tried to handle the easy cases first to
get the basic implementation running.

Please share your thoughts!!

*** BLURB HERE ***

Nayan Deshmukh (4):
  drm/scheduler: add a list of run queues to the entity
  drm/scheduler: add counter for total jobs in scheduler
  drm/scheduler: add new function to get least loaded sched
  drm/scheduler: move idle entities to scheduler with less load

 drivers/gpu/drm/scheduler/gpu_scheduler.c | 62 ++++++++++++++++++++++++++++---
 include/drm/gpu_scheduler.h               |  9 ++++-
 2 files changed, 65 insertions(+), 6 deletions(-)

-- 
2.14.3

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

[PATCH 1/4] drm/scheduler: add a list of run queues to the entity

[Nayan Deshmukh]

These are the potential run queues on which the jobs from this
entity can be scheduled. We will use this to do load balancing.

Signed-off-by: Nayan Deshmukh <nayan26deshmukh@gmail.com>
---
 drivers/gpu/drm/scheduler/gpu_scheduler.c | 8 ++++++++
 include/drm/gpu_scheduler.h               | 7 ++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c b/drivers/gpu/drm/scheduler/gpu_scheduler.c
index 3f2fc5e8242a..a3eacc35cf98 100644
--- a/drivers/gpu/drm/scheduler/gpu_scheduler.c
+++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c
@@ -179,6 +179,8 @@ int drm_sched_entity_init(struct drm_sched_entity *entity,
 			  unsigned int num_rq_list,
 			  atomic_t *guilty)
 {
+	int i;
+
 	if (!(entity && rq_list && num_rq_list > 0 && rq_list[0]))
 		return -EINVAL;
 
@@ -186,6 +188,11 @@ int drm_sched_entity_init(struct drm_sched_entity *entity,
 	INIT_LIST_HEAD(&entity->list);
 	entity->rq = rq_list[0];
 	entity->guilty = guilty;
+	entity->num_rq_list = num_rq_list;
+	entity->rq_list = kcalloc(num_rq_list, sizeof(struct drm_sched_rq *),
+				GFP_KERNEL);
+	for (i = 0; i < num_rq_list; ++i)
+		entity->rq_list[i] = rq_list[i];
 	entity->last_scheduled = NULL;
 
 	spin_lock_init(&entity->rq_lock);
@@ -363,6 +370,7 @@ void drm_sched_entity_fini(struct drm_sched_entity *entity)
 
 	dma_fence_put(entity->last_scheduled);
 	entity->last_scheduled = NULL;
+	kfree(entity->rq_list);
 }
 EXPORT_SYMBOL(drm_sched_entity_fini);
 
diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
index 091b9afcd184..a60896222a3e 100644
--- a/include/drm/gpu_scheduler.h
+++ b/include/drm/gpu_scheduler.h
@@ -50,7 +50,10 @@ enum drm_sched_priority {
  *
  * @list: used to append this struct to the list of entities in the
  *        runqueue.
- * @rq: runqueue to which this entity belongs.
+ * @rq: runqueue on which this entity is currently scheduled.
+ * @rq_list: a list of run queues on which jobs from this entity can
+ *           be scheduled
+ * @num_rq_list: number of run queues in the rq_list
  * @rq_lock: lock to modify the runqueue to which this entity belongs.
  * @job_queue: the list of jobs of this entity.
  * @fence_seq: a linearly increasing seqno incremented with each
@@ -74,6 +77,8 @@ enum drm_sched_priority {
 struct drm_sched_entity {
 	struct list_head		list;
 	struct drm_sched_rq		*rq;
+	struct drm_sched_rq		**rq_list;
+	unsigned int                    num_rq_list;
 	spinlock_t			rq_lock;
 
 	struct spsc_queue		job_queue;
-- 
2.14.3

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

[PATCH 2/4] drm/scheduler: add counter for total jobs in scheduler

[Nayan Deshmukh]

Signed-off-by: Nayan Deshmukh <nayan26deshmukh@gmail.com>
---
 drivers/gpu/drm/scheduler/gpu_scheduler.c | 3 +++
 include/drm/gpu_scheduler.h               | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c b/drivers/gpu/drm/scheduler/gpu_scheduler.c
index a3eacc35cf98..375f6f7f6a93 100644
--- a/drivers/gpu/drm/scheduler/gpu_scheduler.c
+++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c
@@ -549,6 +549,7 @@ void drm_sched_entity_push_job(struct drm_sched_job *sched_job,
 
 	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 */
@@ -836,6 +837,7 @@ static void drm_sched_process_job(struct dma_fence *f, struct dma_fence_cb *cb)
 
 	dma_fence_get(&s_fence->finished);
 	atomic_dec(&sched->hw_rq_count);
+	atomic_dec(&sched->num_jobs);
 	drm_sched_fence_finished(s_fence);
 
 	trace_drm_sched_process_job(s_fence);
@@ -953,6 +955,7 @@ int drm_sched_init(struct drm_gpu_scheduler *sched,
 	INIT_LIST_HEAD(&sched->ring_mirror_list);
 	spin_lock_init(&sched->job_list_lock);
 	atomic_set(&sched->hw_rq_count, 0);
+	atomic_set(&sched->num_jobs, 0);
 	atomic64_set(&sched->job_id_count, 0);
 
 	/* Each scheduler will run on a seperate kernel thread */
diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
index a60896222a3e..89881ce974a5 100644
--- a/include/drm/gpu_scheduler.h
+++ b/include/drm/gpu_scheduler.h
@@ -260,6 +260,7 @@ struct drm_sched_backend_ops {
  * @job_list_lock: lock to protect the ring_mirror_list.
  * @hang_limit: once the hangs by a job crosses this limit then it is marked
  *              guilty and it will be considered for scheduling further.
+ * @num_jobs: the number of jobs in queue in the scheduler
  *
  * One scheduler is implemented for each hardware ring.
  */
@@ -277,6 +278,7 @@ struct drm_gpu_scheduler {
 	struct list_head		ring_mirror_list;
 	spinlock_t			job_list_lock;
 	int				hang_limit;
+	atomic_t                        num_jobs;
 };
 
 int drm_sched_init(struct drm_gpu_scheduler *sched,
-- 
2.14.3

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

[PATCH 3/4] drm/scheduler: add new function to get least loaded sched

[Nayan Deshmukh]

The function selects the run queue from the rq_list with the
least load. The load is decided by the number of jobs in a
scheduler.

Signed-off-by: Nayan Deshmukh <nayan26deshmukh@gmail.com>
---
 drivers/gpu/drm/scheduler/gpu_scheduler.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c b/drivers/gpu/drm/scheduler/gpu_scheduler.c
index 375f6f7f6a93..c67f65ad8f15 100644
--- a/drivers/gpu/drm/scheduler/gpu_scheduler.c
+++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c
@@ -255,6 +255,32 @@ static bool drm_sched_entity_is_ready(struct drm_sched_entity *entity)
 	return true;
 }
 
+/**
+ * drm_sched_entity_get_free_sched - Get the rq from rq_list with least load
+ *
+ * @entity: scheduler entity
+ *
+ * Return the pointer to the rq with least load.
+ */
+static struct drm_sched_rq *
+drm_sched_entity_get_free_sched(struct drm_sched_entity *entity)
+{
+	struct drm_sched_rq *rq = NULL;
+	unsigned int min_jobs = UINT_MAX;
+	int i;
+
+	for (i = 0; i < entity->num_rq_list; ++i) {
+		if (atomic_read(&entity->rq_list[i]->sched->num_jobs) <
+				min_jobs) {
+			min_jobs = atomic_read(
+					&entity->rq_list[i]->sched->num_jobs);
+			rq = entity->rq_list[i];
+		}
+	}
+
+	return rq;
+}
+
 static void drm_sched_entity_kill_jobs_cb(struct dma_fence *f,
 				    struct dma_fence_cb *cb)
 {
-- 
2.14.3

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

Re: [PATCH 3/4] drm/scheduler: add new function to get least loaded sched

[Christian König]

Am 31.07.2018 um 12:37 schrieb Nayan Deshmukh:
> The function selects the run queue from the rq_list with the
> least load. The load is decided by the number of jobs in a
> scheduler.
>
> Signed-off-by: Nayan Deshmukh <nayan26deshmukh@gmail.com>
> ---
>   drivers/gpu/drm/scheduler/gpu_scheduler.c | 26 ++++++++++++++++++++++++++
>   1 file changed, 26 insertions(+)
>
> diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c b/drivers/gpu/drm/scheduler/gpu_scheduler.c
> index 375f6f7f6a93..c67f65ad8f15 100644
> --- a/drivers/gpu/drm/scheduler/gpu_scheduler.c
> +++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c
> @@ -255,6 +255,32 @@ static bool drm_sched_entity_is_ready(struct drm_sched_entity *entity)
>   	return true;
>   }
>   
> +/**
> + * drm_sched_entity_get_free_sched - Get the rq from rq_list with least load
> + *
> + * @entity: scheduler entity
> + *
> + * Return the pointer to the rq with least load.
> + */
> +static struct drm_sched_rq *
> +drm_sched_entity_get_free_sched(struct drm_sched_entity *entity)
> +{
> +	struct drm_sched_rq *rq = NULL;
> +	unsigned int min_jobs = UINT_MAX;
> +	int i;
> +
> +	for (i = 0; i < entity->num_rq_list; ++i) {
> +		if (atomic_read(&entity->rq_list[i]->sched->num_jobs) <
> +				min_jobs) {
> +			min_jobs = atomic_read(
> +					&entity->rq_list[i]->sched->num_jobs);

When you use atomic_read twice you might get different results because 
the atomic has changed in the meantime.

In other words you need to store the result locally:

unsigned int num_jobs = atomic_read(....);

if (num_jobs < min_jobs) {
     min_jobs = num_jobs;
     .....

Christian.

> +			rq = entity->rq_list[i];
> +		}
> +	}
> +
> +	return rq;
> +}
> +
>   static void drm_sched_entity_kill_jobs_cb(struct dma_fence *f,
>   				    struct dma_fence_cb *cb)
>   {

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/4] drm/scheduler: add new function to get least loaded sched

[Nayan Deshmukh]

[-- Attachment #1.1: Type: text/plain, Size: 2311 bytes --]

I will take care of that in v2.

On Tue, Jul 31, 2018 at 4:57 PM Christian König <
ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> Am 31.07.2018 um 12:37 schrieb Nayan Deshmukh:
> > The function selects the run queue from the rq_list with the
> > least load. The load is decided by the number of jobs in a
> > scheduler.
> >
> > Signed-off-by: Nayan Deshmukh <nayan26deshmukh-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > ---
> >   drivers/gpu/drm/scheduler/gpu_scheduler.c | 26
> ++++++++++++++++++++++++++
> >   1 file changed, 26 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c
> b/drivers/gpu/drm/scheduler/gpu_scheduler.c
> > index 375f6f7f6a93..c67f65ad8f15 100644
> > --- a/drivers/gpu/drm/scheduler/gpu_scheduler.c
> > +++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c
> > @@ -255,6 +255,32 @@ static bool drm_sched_entity_is_ready(struct
> drm_sched_entity *entity)
> >       return true;
> >   }
> >
> > +/**
> > + * drm_sched_entity_get_free_sched - Get the rq from rq_list with least
> load
> > + *
> > + * @entity: scheduler entity
> > + *
> > + * Return the pointer to the rq with least load.
> > + */
> > +static struct drm_sched_rq *
> > +drm_sched_entity_get_free_sched(struct drm_sched_entity *entity)
> > +{
> > +     struct drm_sched_rq *rq = NULL;
> > +     unsigned int min_jobs = UINT_MAX;
> > +     int i;
> > +
> > +     for (i = 0; i < entity->num_rq_list; ++i) {
> > +             if (atomic_read(&entity->rq_list[i]->sched->num_jobs) <
> > +                             min_jobs) {
> > +                     min_jobs = atomic_read(
> > +
>  &entity->rq_list[i]->sched->num_jobs);
>
> When you use atomic_read twice you might get different results because
> the atomic has changed in the meantime.
>
> In other words you need to store the result locally:
>
> unsigned int num_jobs = atomic_read(....);
>
> if (num_jobs < min_jobs) {
>      min_jobs = num_jobs;
>      .....
>
> Christian.
>
> > +                     rq = entity->rq_list[i];
> > +             }
> > +     }
> > +
> > +     return rq;
> > +}
> > +
> >   static void drm_sched_entity_kill_jobs_cb(struct dma_fence *f,
> >                                   struct dma_fence_cb *cb)
> >   {
>
>

[-- Attachment #1.2: Type: text/html, Size: 3123 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

[PATCH 4/4] drm/scheduler: move idle entities to scheduler with less load

[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;
+	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) {
 		/* 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);
-- 
2.14.3

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

Re: [PATCH 4/4] drm/scheduler: move idle entities to scheduler with less load

[Christian König]

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.


> +	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.

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);

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/4] drm/scheduler: move idle entities to scheduler with less load

[Nayan Deshmukh]

[-- Attachment #1.1: Type: text/plain, Size: 3782 bytes --]

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);
>
>

[-- Attachment #1.2: Type: text/html, Size: 5115 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/4] drm/scheduler: add a list of run queues to the entity

[Nayan Deshmukh]

These are the potential run queues on which the jobs from this
entity can be scheduled. We will use this to do load balancing.

Signed-off-by: Nayan Deshmukh <nayan26deshmukh@gmail.com>
---
 drivers/gpu/drm/scheduler/gpu_scheduler.c | 8 ++++++++
 include/drm/gpu_scheduler.h               | 7 ++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c b/drivers/gpu/drm/scheduler/gpu_scheduler.c
index 3f2fc5e8242a..a3eacc35cf98 100644
--- a/drivers/gpu/drm/scheduler/gpu_scheduler.c
+++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c
@@ -179,6 +179,8 @@ int drm_sched_entity_init(struct drm_sched_entity *entity,
 			  unsigned int num_rq_list,
 			  atomic_t *guilty)
 {
+	int i;
+
 	if (!(entity && rq_list && num_rq_list > 0 && rq_list[0]))
 		return -EINVAL;
 
@@ -186,6 +188,11 @@ int drm_sched_entity_init(struct drm_sched_entity *entity,
 	INIT_LIST_HEAD(&entity->list);
 	entity->rq = rq_list[0];
 	entity->guilty = guilty;
+	entity->num_rq_list = num_rq_list;
+	entity->rq_list = kcalloc(num_rq_list, sizeof(struct drm_sched_rq *),
+				GFP_KERNEL);
+	for (i = 0; i < num_rq_list; ++i)
+		entity->rq_list[i] = rq_list[i];
 	entity->last_scheduled = NULL;
 
 	spin_lock_init(&entity->rq_lock);
@@ -363,6 +370,7 @@ void drm_sched_entity_fini(struct drm_sched_entity *entity)
 
 	dma_fence_put(entity->last_scheduled);
 	entity->last_scheduled = NULL;
+	kfree(entity->rq_list);
 }
 EXPORT_SYMBOL(drm_sched_entity_fini);
 
diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
index 091b9afcd184..a60896222a3e 100644
--- a/include/drm/gpu_scheduler.h
+++ b/include/drm/gpu_scheduler.h
@@ -50,7 +50,10 @@ enum drm_sched_priority {
  *
  * @list: used to append this struct to the list of entities in the
  *        runqueue.
- * @rq: runqueue to which this entity belongs.
+ * @rq: runqueue on which this entity is currently scheduled.
+ * @rq_list: a list of run queues on which jobs from this entity can
+ *           be scheduled
+ * @num_rq_list: number of run queues in the rq_list
  * @rq_lock: lock to modify the runqueue to which this entity belongs.
  * @job_queue: the list of jobs of this entity.
  * @fence_seq: a linearly increasing seqno incremented with each
@@ -74,6 +77,8 @@ enum drm_sched_priority {
 struct drm_sched_entity {
 	struct list_head		list;
 	struct drm_sched_rq		*rq;
+	struct drm_sched_rq		**rq_list;
+	unsigned int                    num_rq_list;
 	spinlock_t			rq_lock;
 
 	struct spsc_queue		job_queue;
-- 
2.14.3

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

Re: [PATCH 1/4] drm/scheduler: add a list of run queues to the entity

[Christian König]

Am 01.08.2018 um 10:19 schrieb Nayan Deshmukh:
> These are the potential run queues on which the jobs from this
> entity can be scheduled. We will use this to do load balancing.
>
> Signed-off-by: Nayan Deshmukh <nayan26deshmukh@gmail.com>

Reviewed-by: Christian König <christian.koenig@amd.com> for the whole 
series.

I also just pushed them into our internal branch for upstreaming.

Thanks for all the work,
Christian.

> ---
>   drivers/gpu/drm/scheduler/gpu_scheduler.c | 8 ++++++++
>   include/drm/gpu_scheduler.h               | 7 ++++++-
>   2 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c b/drivers/gpu/drm/scheduler/gpu_scheduler.c
> index 3f2fc5e8242a..a3eacc35cf98 100644
> --- a/drivers/gpu/drm/scheduler/gpu_scheduler.c
> +++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c
> @@ -179,6 +179,8 @@ int drm_sched_entity_init(struct drm_sched_entity *entity,
>   			  unsigned int num_rq_list,
>   			  atomic_t *guilty)
>   {
> +	int i;
> +
>   	if (!(entity && rq_list && num_rq_list > 0 && rq_list[0]))
>   		return -EINVAL;
>   
> @@ -186,6 +188,11 @@ int drm_sched_entity_init(struct drm_sched_entity *entity,
>   	INIT_LIST_HEAD(&entity->list);
>   	entity->rq = rq_list[0];
>   	entity->guilty = guilty;
> +	entity->num_rq_list = num_rq_list;
> +	entity->rq_list = kcalloc(num_rq_list, sizeof(struct drm_sched_rq *),
> +				GFP_KERNEL);
> +	for (i = 0; i < num_rq_list; ++i)
> +		entity->rq_list[i] = rq_list[i];
>   	entity->last_scheduled = NULL;
>   
>   	spin_lock_init(&entity->rq_lock);
> @@ -363,6 +370,7 @@ void drm_sched_entity_fini(struct drm_sched_entity *entity)
>   
>   	dma_fence_put(entity->last_scheduled);
>   	entity->last_scheduled = NULL;
> +	kfree(entity->rq_list);
>   }
>   EXPORT_SYMBOL(drm_sched_entity_fini);
>   
> diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
> index 091b9afcd184..a60896222a3e 100644
> --- a/include/drm/gpu_scheduler.h
> +++ b/include/drm/gpu_scheduler.h
> @@ -50,7 +50,10 @@ enum drm_sched_priority {
>    *
>    * @list: used to append this struct to the list of entities in the
>    *        runqueue.
> - * @rq: runqueue to which this entity belongs.
> + * @rq: runqueue on which this entity is currently scheduled.
> + * @rq_list: a list of run queues on which jobs from this entity can
> + *           be scheduled
> + * @num_rq_list: number of run queues in the rq_list
>    * @rq_lock: lock to modify the runqueue to which this entity belongs.
>    * @job_queue: the list of jobs of this entity.
>    * @fence_seq: a linearly increasing seqno incremented with each
> @@ -74,6 +77,8 @@ enum drm_sched_priority {
>   struct drm_sched_entity {
>   	struct list_head		list;
>   	struct drm_sched_rq		*rq;
> +	struct drm_sched_rq		**rq_list;
> +	unsigned int                    num_rq_list;
>   	spinlock_t			rq_lock;
>   
>   	struct spsc_queue		job_queue;

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