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(-)
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;
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,
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) {
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) {
I will take care of that in v2.
On Tue, Jul 31, 2018 at 4:57 PM Christian König < ckoenig.leichtzumerken@gmail.com> 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@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) {
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);
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);
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); }
dri-devel@lists.freedesktop.org
-
Christian König -
Nayan Deshmukh