* [PATCH v2] drm/scheduler: remove timeout work_struct from drm_sched_job
@ 2018-09-21 18:57 Nayan Deshmukh
2018-09-22 7:23 ` Christian König
0 siblings, 1 reply; 9+ messages in thread
From: Nayan Deshmukh @ 2018-09-21 18:57 UTC (permalink / raw)
To: dri-devel; +Cc: Nayan Deshmukh, christian.koenig
having a delayed work item per job is redundant as we only need one
per scheduler to track the time out the currently executing job.
v2: the first element of the ring mirror list is the currently
executing job so we don't need a additional variable for it
Signed-off-by: Nayan Deshmukh <nayan26deshmukh@gmail.com>
Suggested-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/scheduler/sched_main.c | 28 +++++++++++++++-------------
include/drm/gpu_scheduler.h | 6 +++---
2 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c
index 9ca741f3a0bc..88f6cff136f2 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -197,19 +197,16 @@ static void drm_sched_job_finish(struct work_struct *work)
* manages to find this job as the next job in the list, the fence
* signaled check below will prevent the timeout to be restarted.
*/
- cancel_delayed_work_sync(&s_job->work_tdr);
+ cancel_delayed_work_sync(&sched->work_tdr);
spin_lock(&sched->job_list_lock);
/* queue TDR for next job */
+ list_del(&s_job->node);
if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
- !list_is_last(&s_job->node, &sched->ring_mirror_list)) {
- struct drm_sched_job *next = list_next_entry(s_job, node);
-
- if (!dma_fence_is_signaled(&next->s_fence->finished))
- schedule_delayed_work(&next->work_tdr, sched->timeout);
+ !list_empty(&sched->ring_mirror_list)) {
+ schedule_delayed_work(&sched->work_tdr, sched->timeout);
}
/* remove job from ring_mirror_list */
- list_del(&s_job->node);
spin_unlock(&sched->job_list_lock);
dma_fence_put(&s_job->s_fence->finished);
@@ -236,16 +233,21 @@ static void drm_sched_job_begin(struct drm_sched_job *s_job)
if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
list_first_entry_or_null(&sched->ring_mirror_list,
struct drm_sched_job, node) == s_job)
- schedule_delayed_work(&s_job->work_tdr, sched->timeout);
+ schedule_delayed_work(&sched->work_tdr, sched->timeout);
spin_unlock(&sched->job_list_lock);
}
static void drm_sched_job_timedout(struct work_struct *work)
{
- struct drm_sched_job *job = container_of(work, struct drm_sched_job,
- work_tdr.work);
+ struct drm_gpu_scheduler *sched;
+ struct drm_sched_job *job;
+
+ sched = container_of(work, struct drm_gpu_scheduler, work_tdr.work);
+ job = list_first_entry_or_null(&sched->ring_mirror_list,
+ struct drm_sched_job, node);
- job->sched->ops->timedout_job(job);
+ if (job)
+ job->sched->ops->timedout_job(job);
}
/**
@@ -315,7 +317,7 @@ void drm_sched_job_recovery(struct drm_gpu_scheduler *sched)
s_job = list_first_entry_or_null(&sched->ring_mirror_list,
struct drm_sched_job, node);
if (s_job && sched->timeout != MAX_SCHEDULE_TIMEOUT)
- schedule_delayed_work(&s_job->work_tdr, sched->timeout);
+ schedule_delayed_work(&sched->work_tdr, sched->timeout);
list_for_each_entry_safe(s_job, tmp, &sched->ring_mirror_list, node) {
struct drm_sched_fence *s_fence = s_job->s_fence;
@@ -384,7 +386,6 @@ int drm_sched_job_init(struct drm_sched_job *job,
INIT_WORK(&job->finish_work, drm_sched_job_finish);
INIT_LIST_HEAD(&job->node);
- INIT_DELAYED_WORK(&job->work_tdr, drm_sched_job_timedout);
return 0;
}
@@ -575,6 +576,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);
+ INIT_DELAYED_WORK(&sched->work_tdr, drm_sched_job_timedout);
atomic_set(&sched->num_jobs, 0);
atomic64_set(&sched->job_id_count, 0);
diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
index daec50f887b3..d87b268f1781 100644
--- a/include/drm/gpu_scheduler.h
+++ b/include/drm/gpu_scheduler.h
@@ -175,8 +175,6 @@ struct drm_sched_fence *to_drm_sched_fence(struct dma_fence *f);
* finished to remove the job from the
* @drm_gpu_scheduler.ring_mirror_list.
* @node: used to append this struct to the @drm_gpu_scheduler.ring_mirror_list.
- * @work_tdr: schedules a delayed call to @drm_sched_job_timedout after the timeout
- * interval is over.
* @id: a unique id assigned to each job scheduled on the scheduler.
* @karma: increment on every hang caused by this job. If this exceeds the hang
* limit of the scheduler then the job is marked guilty and will not
@@ -195,7 +193,6 @@ struct drm_sched_job {
struct dma_fence_cb finish_cb;
struct work_struct finish_work;
struct list_head node;
- struct delayed_work work_tdr;
uint64_t id;
atomic_t karma;
enum drm_sched_priority s_priority;
@@ -259,6 +256,8 @@ struct drm_sched_backend_ops {
* finished.
* @hw_rq_count: the number of jobs currently in the hardware queue.
* @job_id_count: used to assign unique id to the each job.
+ * @work_tdr: schedules a delayed call to @drm_sched_job_timedout after the
+ * timeout interval is over.
* @thread: the kthread on which the scheduler which run.
* @ring_mirror_list: the list of jobs which are currently in the job queue.
* @job_list_lock: lock to protect the ring_mirror_list.
@@ -278,6 +277,7 @@ struct drm_gpu_scheduler {
wait_queue_head_t job_scheduled;
atomic_t hw_rq_count;
atomic64_t job_id_count;
+ struct delayed_work work_tdr;
struct task_struct *thread;
struct list_head ring_mirror_list;
spinlock_t job_list_lock;
--
2.14.3
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2] drm/scheduler: remove timeout work_struct from drm_sched_job
2018-09-21 18:57 [PATCH v2] drm/scheduler: remove timeout work_struct from drm_sched_job Nayan Deshmukh
@ 2018-09-22 7:23 ` Christian König
2018-09-25 17:09 ` Nayan Deshmukh
0 siblings, 1 reply; 9+ messages in thread
From: Christian König @ 2018-09-22 7:23 UTC (permalink / raw)
To: Nayan Deshmukh, dri-devel; +Cc: christian.koenig
Am 21.09.2018 um 20:57 schrieb Nayan Deshmukh:
> having a delayed work item per job is redundant as we only need one
> per scheduler to track the time out the currently executing job.
>
> v2: the first element of the ring mirror list is the currently
> executing job so we don't need a additional variable for it
>
> Signed-off-by: Nayan Deshmukh <nayan26deshmukh@gmail.com>
> Suggested-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/scheduler/sched_main.c | 28 +++++++++++++++-------------
> include/drm/gpu_scheduler.h | 6 +++---
> 2 files changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c
> index 9ca741f3a0bc..88f6cff136f2 100644
> --- a/drivers/gpu/drm/scheduler/sched_main.c
> +++ b/drivers/gpu/drm/scheduler/sched_main.c
> @@ -197,19 +197,16 @@ static void drm_sched_job_finish(struct work_struct *work)
> * manages to find this job as the next job in the list, the fence
> * signaled check below will prevent the timeout to be restarted.
> */
> - cancel_delayed_work_sync(&s_job->work_tdr);
> + cancel_delayed_work_sync(&sched->work_tdr);
>
> spin_lock(&sched->job_list_lock);
> /* queue TDR for next job */
> + list_del(&s_job->node);
> if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
> - !list_is_last(&s_job->node, &sched->ring_mirror_list)) {
> - struct drm_sched_job *next = list_next_entry(s_job, node);
> -
> - if (!dma_fence_is_signaled(&next->s_fence->finished))
> - schedule_delayed_work(&next->work_tdr, sched->timeout);
> + !list_empty(&sched->ring_mirror_list)) {
> + schedule_delayed_work(&sched->work_tdr, sched->timeout);
> }
You can remove the {} here as well.
> /* remove job from ring_mirror_list */
That comment should move with the list_del().
Apart from that it looks good to me,
Christian.
> - list_del(&s_job->node);
> spin_unlock(&sched->job_list_lock);
>
> dma_fence_put(&s_job->s_fence->finished);
> @@ -236,16 +233,21 @@ static void drm_sched_job_begin(struct drm_sched_job *s_job)
> if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
> list_first_entry_or_null(&sched->ring_mirror_list,
> struct drm_sched_job, node) == s_job)
> - schedule_delayed_work(&s_job->work_tdr, sched->timeout);
> + schedule_delayed_work(&sched->work_tdr, sched->timeout);
> spin_unlock(&sched->job_list_lock);
> }
>
> static void drm_sched_job_timedout(struct work_struct *work)
> {
> - struct drm_sched_job *job = container_of(work, struct drm_sched_job,
> - work_tdr.work);
> + struct drm_gpu_scheduler *sched;
> + struct drm_sched_job *job;
> +
> + sched = container_of(work, struct drm_gpu_scheduler, work_tdr.work);
> + job = list_first_entry_or_null(&sched->ring_mirror_list,
> + struct drm_sched_job, node);
>
> - job->sched->ops->timedout_job(job);
> + if (job)
> + job->sched->ops->timedout_job(job);
> }
>
> /**
> @@ -315,7 +317,7 @@ void drm_sched_job_recovery(struct drm_gpu_scheduler *sched)
> s_job = list_first_entry_or_null(&sched->ring_mirror_list,
> struct drm_sched_job, node);
> if (s_job && sched->timeout != MAX_SCHEDULE_TIMEOUT)
> - schedule_delayed_work(&s_job->work_tdr, sched->timeout);
> + schedule_delayed_work(&sched->work_tdr, sched->timeout);
>
> list_for_each_entry_safe(s_job, tmp, &sched->ring_mirror_list, node) {
> struct drm_sched_fence *s_fence = s_job->s_fence;
> @@ -384,7 +386,6 @@ int drm_sched_job_init(struct drm_sched_job *job,
>
> INIT_WORK(&job->finish_work, drm_sched_job_finish);
> INIT_LIST_HEAD(&job->node);
> - INIT_DELAYED_WORK(&job->work_tdr, drm_sched_job_timedout);
>
> return 0;
> }
> @@ -575,6 +576,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);
> + INIT_DELAYED_WORK(&sched->work_tdr, drm_sched_job_timedout);
> atomic_set(&sched->num_jobs, 0);
> atomic64_set(&sched->job_id_count, 0);
>
> diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
> index daec50f887b3..d87b268f1781 100644
> --- a/include/drm/gpu_scheduler.h
> +++ b/include/drm/gpu_scheduler.h
> @@ -175,8 +175,6 @@ struct drm_sched_fence *to_drm_sched_fence(struct dma_fence *f);
> * finished to remove the job from the
> * @drm_gpu_scheduler.ring_mirror_list.
> * @node: used to append this struct to the @drm_gpu_scheduler.ring_mirror_list.
> - * @work_tdr: schedules a delayed call to @drm_sched_job_timedout after the timeout
> - * interval is over.
> * @id: a unique id assigned to each job scheduled on the scheduler.
> * @karma: increment on every hang caused by this job. If this exceeds the hang
> * limit of the scheduler then the job is marked guilty and will not
> @@ -195,7 +193,6 @@ struct drm_sched_job {
> struct dma_fence_cb finish_cb;
> struct work_struct finish_work;
> struct list_head node;
> - struct delayed_work work_tdr;
> uint64_t id;
> atomic_t karma;
> enum drm_sched_priority s_priority;
> @@ -259,6 +256,8 @@ struct drm_sched_backend_ops {
> * finished.
> * @hw_rq_count: the number of jobs currently in the hardware queue.
> * @job_id_count: used to assign unique id to the each job.
> + * @work_tdr: schedules a delayed call to @drm_sched_job_timedout after the
> + * timeout interval is over.
> * @thread: the kthread on which the scheduler which run.
> * @ring_mirror_list: the list of jobs which are currently in the job queue.
> * @job_list_lock: lock to protect the ring_mirror_list.
> @@ -278,6 +277,7 @@ struct drm_gpu_scheduler {
> wait_queue_head_t job_scheduled;
> atomic_t hw_rq_count;
> atomic64_t job_id_count;
> + struct delayed_work work_tdr;
> struct task_struct *thread;
> struct list_head ring_mirror_list;
> spinlock_t job_list_lock;
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2] drm/scheduler: remove timeout work_struct from drm_sched_job
2018-09-22 7:23 ` Christian König
@ 2018-09-25 17:09 ` Nayan Deshmukh
2018-09-25 17:34 ` Christian König
2018-09-26 7:39 ` Lucas Stach
0 siblings, 2 replies; 9+ messages in thread
From: Nayan Deshmukh @ 2018-09-25 17:09 UTC (permalink / raw)
To: dri-devel; +Cc: Nayan Deshmukh, christian.koenig
having a delayed work item per job is redundant as we only need one
per scheduler to track the time out the currently executing job.
v2: the first element of the ring mirror list is the currently
executing job so we don't need a additional variable for it
Signed-off-by: Nayan Deshmukh <nayan26deshmukh@gmail.com>
Suggested-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/scheduler/sched_main.c | 31 ++++++++++++++++---------------
include/drm/gpu_scheduler.h | 6 +++---
2 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c
index 9ca741f3a0bc..4e8505d51795 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -197,19 +197,15 @@ static void drm_sched_job_finish(struct work_struct *work)
* manages to find this job as the next job in the list, the fence
* signaled check below will prevent the timeout to be restarted.
*/
- cancel_delayed_work_sync(&s_job->work_tdr);
+ cancel_delayed_work_sync(&sched->work_tdr);
spin_lock(&sched->job_list_lock);
- /* queue TDR for next job */
- if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
- !list_is_last(&s_job->node, &sched->ring_mirror_list)) {
- struct drm_sched_job *next = list_next_entry(s_job, node);
-
- if (!dma_fence_is_signaled(&next->s_fence->finished))
- schedule_delayed_work(&next->work_tdr, sched->timeout);
- }
/* remove job from ring_mirror_list */
list_del(&s_job->node);
+ /* queue TDR for next job */
+ if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
+ !list_empty(&sched->ring_mirror_list))
+ schedule_delayed_work(&sched->work_tdr, sched->timeout);
spin_unlock(&sched->job_list_lock);
dma_fence_put(&s_job->s_fence->finished);
@@ -236,16 +232,21 @@ static void drm_sched_job_begin(struct drm_sched_job *s_job)
if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
list_first_entry_or_null(&sched->ring_mirror_list,
struct drm_sched_job, node) == s_job)
- schedule_delayed_work(&s_job->work_tdr, sched->timeout);
+ schedule_delayed_work(&sched->work_tdr, sched->timeout);
spin_unlock(&sched->job_list_lock);
}
static void drm_sched_job_timedout(struct work_struct *work)
{
- struct drm_sched_job *job = container_of(work, struct drm_sched_job,
- work_tdr.work);
+ struct drm_gpu_scheduler *sched;
+ struct drm_sched_job *job;
+
+ sched = container_of(work, struct drm_gpu_scheduler, work_tdr.work);
+ job = list_first_entry_or_null(&sched->ring_mirror_list,
+ struct drm_sched_job, node);
- job->sched->ops->timedout_job(job);
+ if (job)
+ job->sched->ops->timedout_job(job);
}
/**
@@ -315,7 +316,7 @@ void drm_sched_job_recovery(struct drm_gpu_scheduler *sched)
s_job = list_first_entry_or_null(&sched->ring_mirror_list,
struct drm_sched_job, node);
if (s_job && sched->timeout != MAX_SCHEDULE_TIMEOUT)
- schedule_delayed_work(&s_job->work_tdr, sched->timeout);
+ schedule_delayed_work(&sched->work_tdr, sched->timeout);
list_for_each_entry_safe(s_job, tmp, &sched->ring_mirror_list, node) {
struct drm_sched_fence *s_fence = s_job->s_fence;
@@ -384,7 +385,6 @@ int drm_sched_job_init(struct drm_sched_job *job,
INIT_WORK(&job->finish_work, drm_sched_job_finish);
INIT_LIST_HEAD(&job->node);
- INIT_DELAYED_WORK(&job->work_tdr, drm_sched_job_timedout);
return 0;
}
@@ -575,6 +575,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);
+ INIT_DELAYED_WORK(&sched->work_tdr, drm_sched_job_timedout);
atomic_set(&sched->num_jobs, 0);
atomic64_set(&sched->job_id_count, 0);
diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
index daec50f887b3..d87b268f1781 100644
--- a/include/drm/gpu_scheduler.h
+++ b/include/drm/gpu_scheduler.h
@@ -175,8 +175,6 @@ struct drm_sched_fence *to_drm_sched_fence(struct dma_fence *f);
* finished to remove the job from the
* @drm_gpu_scheduler.ring_mirror_list.
* @node: used to append this struct to the @drm_gpu_scheduler.ring_mirror_list.
- * @work_tdr: schedules a delayed call to @drm_sched_job_timedout after the timeout
- * interval is over.
* @id: a unique id assigned to each job scheduled on the scheduler.
* @karma: increment on every hang caused by this job. If this exceeds the hang
* limit of the scheduler then the job is marked guilty and will not
@@ -195,7 +193,6 @@ struct drm_sched_job {
struct dma_fence_cb finish_cb;
struct work_struct finish_work;
struct list_head node;
- struct delayed_work work_tdr;
uint64_t id;
atomic_t karma;
enum drm_sched_priority s_priority;
@@ -259,6 +256,8 @@ struct drm_sched_backend_ops {
* finished.
* @hw_rq_count: the number of jobs currently in the hardware queue.
* @job_id_count: used to assign unique id to the each job.
+ * @work_tdr: schedules a delayed call to @drm_sched_job_timedout after the
+ * timeout interval is over.
* @thread: the kthread on which the scheduler which run.
* @ring_mirror_list: the list of jobs which are currently in the job queue.
* @job_list_lock: lock to protect the ring_mirror_list.
@@ -278,6 +277,7 @@ struct drm_gpu_scheduler {
wait_queue_head_t job_scheduled;
atomic_t hw_rq_count;
atomic64_t job_id_count;
+ struct delayed_work work_tdr;
struct task_struct *thread;
struct list_head ring_mirror_list;
spinlock_t job_list_lock;
--
2.14.3
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2] drm/scheduler: remove timeout work_struct from drm_sched_job
2018-09-25 17:09 ` Nayan Deshmukh
@ 2018-09-25 17:34 ` Christian König
2018-09-26 7:39 ` Lucas Stach
1 sibling, 0 replies; 9+ messages in thread
From: Christian König @ 2018-09-25 17:34 UTC (permalink / raw)
To: Nayan Deshmukh, dri-devel; +Cc: christian.koenig
Am 25.09.2018 um 19:09 schrieb Nayan Deshmukh:
> having a delayed work item per job is redundant as we only need one
> per scheduler to track the time out the currently executing job.
>
> v2: the first element of the ring mirror list is the currently
> executing job so we don't need a additional variable for it
>
> Signed-off-by: Nayan Deshmukh <nayan26deshmukh@gmail.com>
> Suggested-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
Going to push that into our branch on the next best occasion.
Christian.
> ---
> drivers/gpu/drm/scheduler/sched_main.c | 31 ++++++++++++++++---------------
> include/drm/gpu_scheduler.h | 6 +++---
> 2 files changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c
> index 9ca741f3a0bc..4e8505d51795 100644
> --- a/drivers/gpu/drm/scheduler/sched_main.c
> +++ b/drivers/gpu/drm/scheduler/sched_main.c
> @@ -197,19 +197,15 @@ static void drm_sched_job_finish(struct work_struct *work)
> * manages to find this job as the next job in the list, the fence
> * signaled check below will prevent the timeout to be restarted.
> */
> - cancel_delayed_work_sync(&s_job->work_tdr);
> + cancel_delayed_work_sync(&sched->work_tdr);
>
> spin_lock(&sched->job_list_lock);
> - /* queue TDR for next job */
> - if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
> - !list_is_last(&s_job->node, &sched->ring_mirror_list)) {
> - struct drm_sched_job *next = list_next_entry(s_job, node);
> -
> - if (!dma_fence_is_signaled(&next->s_fence->finished))
> - schedule_delayed_work(&next->work_tdr, sched->timeout);
> - }
> /* remove job from ring_mirror_list */
> list_del(&s_job->node);
> + /* queue TDR for next job */
> + if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
> + !list_empty(&sched->ring_mirror_list))
> + schedule_delayed_work(&sched->work_tdr, sched->timeout);
> spin_unlock(&sched->job_list_lock);
>
> dma_fence_put(&s_job->s_fence->finished);
> @@ -236,16 +232,21 @@ static void drm_sched_job_begin(struct drm_sched_job *s_job)
> if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
> list_first_entry_or_null(&sched->ring_mirror_list,
> struct drm_sched_job, node) == s_job)
> - schedule_delayed_work(&s_job->work_tdr, sched->timeout);
> + schedule_delayed_work(&sched->work_tdr, sched->timeout);
> spin_unlock(&sched->job_list_lock);
> }
>
> static void drm_sched_job_timedout(struct work_struct *work)
> {
> - struct drm_sched_job *job = container_of(work, struct drm_sched_job,
> - work_tdr.work);
> + struct drm_gpu_scheduler *sched;
> + struct drm_sched_job *job;
> +
> + sched = container_of(work, struct drm_gpu_scheduler, work_tdr.work);
> + job = list_first_entry_or_null(&sched->ring_mirror_list,
> + struct drm_sched_job, node);
>
> - job->sched->ops->timedout_job(job);
> + if (job)
> + job->sched->ops->timedout_job(job);
> }
>
> /**
> @@ -315,7 +316,7 @@ void drm_sched_job_recovery(struct drm_gpu_scheduler *sched)
> s_job = list_first_entry_or_null(&sched->ring_mirror_list,
> struct drm_sched_job, node);
> if (s_job && sched->timeout != MAX_SCHEDULE_TIMEOUT)
> - schedule_delayed_work(&s_job->work_tdr, sched->timeout);
> + schedule_delayed_work(&sched->work_tdr, sched->timeout);
>
> list_for_each_entry_safe(s_job, tmp, &sched->ring_mirror_list, node) {
> struct drm_sched_fence *s_fence = s_job->s_fence;
> @@ -384,7 +385,6 @@ int drm_sched_job_init(struct drm_sched_job *job,
>
> INIT_WORK(&job->finish_work, drm_sched_job_finish);
> INIT_LIST_HEAD(&job->node);
> - INIT_DELAYED_WORK(&job->work_tdr, drm_sched_job_timedout);
>
> return 0;
> }
> @@ -575,6 +575,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);
> + INIT_DELAYED_WORK(&sched->work_tdr, drm_sched_job_timedout);
> atomic_set(&sched->num_jobs, 0);
> atomic64_set(&sched->job_id_count, 0);
>
> diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
> index daec50f887b3..d87b268f1781 100644
> --- a/include/drm/gpu_scheduler.h
> +++ b/include/drm/gpu_scheduler.h
> @@ -175,8 +175,6 @@ struct drm_sched_fence *to_drm_sched_fence(struct dma_fence *f);
> * finished to remove the job from the
> * @drm_gpu_scheduler.ring_mirror_list.
> * @node: used to append this struct to the @drm_gpu_scheduler.ring_mirror_list.
> - * @work_tdr: schedules a delayed call to @drm_sched_job_timedout after the timeout
> - * interval is over.
> * @id: a unique id assigned to each job scheduled on the scheduler.
> * @karma: increment on every hang caused by this job. If this exceeds the hang
> * limit of the scheduler then the job is marked guilty and will not
> @@ -195,7 +193,6 @@ struct drm_sched_job {
> struct dma_fence_cb finish_cb;
> struct work_struct finish_work;
> struct list_head node;
> - struct delayed_work work_tdr;
> uint64_t id;
> atomic_t karma;
> enum drm_sched_priority s_priority;
> @@ -259,6 +256,8 @@ struct drm_sched_backend_ops {
> * finished.
> * @hw_rq_count: the number of jobs currently in the hardware queue.
> * @job_id_count: used to assign unique id to the each job.
> + * @work_tdr: schedules a delayed call to @drm_sched_job_timedout after the
> + * timeout interval is over.
> * @thread: the kthread on which the scheduler which run.
> * @ring_mirror_list: the list of jobs which are currently in the job queue.
> * @job_list_lock: lock to protect the ring_mirror_list.
> @@ -278,6 +277,7 @@ struct drm_gpu_scheduler {
> wait_queue_head_t job_scheduled;
> atomic_t hw_rq_count;
> atomic64_t job_id_count;
> + struct delayed_work work_tdr;
> struct task_struct *thread;
> struct list_head ring_mirror_list;
> spinlock_t job_list_lock;
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] drm/scheduler: remove timeout work_struct from drm_sched_job
2018-09-25 17:09 ` Nayan Deshmukh
2018-09-25 17:34 ` Christian König
@ 2018-09-26 7:39 ` Lucas Stach
2018-09-26 8:13 ` Christian König
1 sibling, 1 reply; 9+ messages in thread
From: Lucas Stach @ 2018-09-26 7:39 UTC (permalink / raw)
To: Nayan Deshmukh, dri-devel; +Cc: christian.koenig
Hi Nayan,
Am Mittwoch, den 26.09.2018, 02:09 +0900 schrieb Nayan Deshmukh:
> having a delayed work item per job is redundant as we only need one
> per scheduler to track the time out the currently executing job.
>
> v2: the first element of the ring mirror list is the currently
> executing job so we don't need a additional variable for it
>
> Signed-off-by: Nayan Deshmukh <nayan26deshmukh@gmail.com>
> Suggested-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/scheduler/sched_main.c | 31 ++++++++++++++++---------------
> include/drm/gpu_scheduler.h | 6 +++---
> 2 files changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c
> index 9ca741f3a0bc..4e8505d51795 100644
> --- a/drivers/gpu/drm/scheduler/sched_main.c
> +++ b/drivers/gpu/drm/scheduler/sched_main.c
> @@ -197,19 +197,15 @@ static void drm_sched_job_finish(struct work_struct *work)
> * manages to find this job as the next job in the list, the fence
> * signaled check below will prevent the timeout to be restarted.
> */
> - cancel_delayed_work_sync(&s_job->work_tdr);
> + cancel_delayed_work_sync(&sched->work_tdr);
>
> spin_lock(&sched->job_list_lock);
> - /* queue TDR for next job */
> - if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
> - !list_is_last(&s_job->node, &sched->ring_mirror_list)) {
> - struct drm_sched_job *next = list_next_entry(s_job, node);
> -
> - if (!dma_fence_is_signaled(&next->s_fence->finished))
> - schedule_delayed_work(&next->work_tdr, sched->timeout);
> - }
> /* remove job from ring_mirror_list */
> list_del(&s_job->node);
> + /* queue TDR for next job */
> + if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
> + !list_empty(&sched->ring_mirror_list))
> + schedule_delayed_work(&sched->work_tdr, sched->timeout);
> spin_unlock(&sched->job_list_lock);
>
> dma_fence_put(&s_job->s_fence->finished);
> @@ -236,16 +232,21 @@ static void drm_sched_job_begin(struct drm_sched_job *s_job)
> if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
> list_first_entry_or_null(&sched->ring_mirror_list,
> struct drm_sched_job, node) == s_job)
> - schedule_delayed_work(&s_job->work_tdr, sched->timeout);
> + schedule_delayed_work(&sched->work_tdr, sched->timeout);
> spin_unlock(&sched->job_list_lock);
> }
>
> static void drm_sched_job_timedout(struct work_struct *work)
> {
> - struct drm_sched_job *job = container_of(work, struct drm_sched_job,
> - work_tdr.work);
> + struct drm_gpu_scheduler *sched;
> + struct drm_sched_job *job;
> +
> + sched = container_of(work, struct drm_gpu_scheduler, work_tdr.work);
> + job = list_first_entry_or_null(&sched->ring_mirror_list,
> + struct drm_sched_job, node);
>
> - job->sched->ops->timedout_job(job);
> + if (job)
> + job->sched->ops->timedout_job(job);
I don't think this is fully robust. Jobs are only removed from the
ring_mirror_list once the job_finish worker has run. If execution of
this worker is delayed for any reason (though it's really unlikely for
a delay as long as the job timeout to happen) you are blaming the wrong
job here.
So I think what you need to to is find the first job in the ring mirror
list with an unsignaled finish fence to robustly find the stuck job.
Regards,
Lucas
> }
>
> /**
> @@ -315,7 +316,7 @@ void drm_sched_job_recovery(struct drm_gpu_scheduler *sched)
> s_job = list_first_entry_or_null(&sched->ring_mirror_list,
> struct drm_sched_job, node);
> if (s_job && sched->timeout != MAX_SCHEDULE_TIMEOUT)
> - schedule_delayed_work(&s_job->work_tdr, sched->timeout);
> + schedule_delayed_work(&sched->work_tdr, sched->timeout);
>
> list_for_each_entry_safe(s_job, tmp, &sched->ring_mirror_list, node) {
> struct drm_sched_fence *s_fence = s_job->s_fence;
> @@ -384,7 +385,6 @@ int drm_sched_job_init(struct drm_sched_job *job,
>
> INIT_WORK(&job->finish_work, drm_sched_job_finish);
> INIT_LIST_HEAD(&job->node);
> - INIT_DELAYED_WORK(&job->work_tdr, drm_sched_job_timedout);
>
> return 0;
> }
> @@ -575,6 +575,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);
> + INIT_DELAYED_WORK(&sched->work_tdr, drm_sched_job_timedout);
> atomic_set(&sched->num_jobs, 0);
> atomic64_set(&sched->job_id_count, 0);
>
> diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
> index daec50f887b3..d87b268f1781 100644
> --- a/include/drm/gpu_scheduler.h
> +++ b/include/drm/gpu_scheduler.h
> @@ -175,8 +175,6 @@ struct drm_sched_fence *to_drm_sched_fence(struct dma_fence *f);
> * finished to remove the job from the
> * @drm_gpu_scheduler.ring_mirror_list.
> * @node: used to append this struct to the @drm_gpu_scheduler.ring_mirror_list.
> - * @work_tdr: schedules a delayed call to @drm_sched_job_timedout after the timeout
> - * interval is over.
> * @id: a unique id assigned to each job scheduled on the scheduler.
> * @karma: increment on every hang caused by this job. If this exceeds the hang
> * limit of the scheduler then the job is marked guilty and will not
> @@ -195,7 +193,6 @@ struct drm_sched_job {
> struct dma_fence_cb finish_cb;
> struct work_struct finish_work;
> struct list_head node;
> - struct delayed_work work_tdr;
> uint64_t id;
> atomic_t karma;
> enum drm_sched_priority s_priority;
> @@ -259,6 +256,8 @@ struct drm_sched_backend_ops {
> * finished.
> * @hw_rq_count: the number of jobs currently in the hardware queue.
> * @job_id_count: used to assign unique id to the each job.
> + * @work_tdr: schedules a delayed call to @drm_sched_job_timedout after the
> + * timeout interval is over.
> * @thread: the kthread on which the scheduler which run.
> * @ring_mirror_list: the list of jobs which are currently in the job queue.
> * @job_list_lock: lock to protect the ring_mirror_list.
> @@ -278,6 +277,7 @@ struct drm_gpu_scheduler {
> wait_queue_head_t job_scheduled;
> atomic_t hw_rq_count;
> atomic64_t job_id_count;
> + struct delayed_work work_tdr;
> struct task_struct *thread;
> struct list_head ring_mirror_list;
> spinlock_t job_list_lock;
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] drm/scheduler: remove timeout work_struct from drm_sched_job
2018-09-26 7:39 ` Lucas Stach
@ 2018-09-26 8:13 ` Christian König
2018-09-26 15:55 ` Nayan Deshmukh
0 siblings, 1 reply; 9+ messages in thread
From: Christian König @ 2018-09-26 8:13 UTC (permalink / raw)
To: Lucas Stach, Nayan Deshmukh, dri-devel; +Cc: christian.koenig
Am 26.09.2018 um 09:39 schrieb Lucas Stach:
> Hi Nayan,
>
> Am Mittwoch, den 26.09.2018, 02:09 +0900 schrieb Nayan Deshmukh:
>> having a delayed work item per job is redundant as we only need one
>> per scheduler to track the time out the currently executing job.
>>
>> v2: the first element of the ring mirror list is the currently
>> executing job so we don't need a additional variable for it
>>
>> Signed-off-by: Nayan Deshmukh <nayan26deshmukh@gmail.com>
>> Suggested-by: Christian König <christian.koenig@amd.com>
>> ---
>> drivers/gpu/drm/scheduler/sched_main.c | 31 ++++++++++++++++---------------
>> include/drm/gpu_scheduler.h | 6 +++---
>> 2 files changed, 19 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c
>> index 9ca741f3a0bc..4e8505d51795 100644
>> --- a/drivers/gpu/drm/scheduler/sched_main.c
>> +++ b/drivers/gpu/drm/scheduler/sched_main.c
>> @@ -197,19 +197,15 @@ static void drm_sched_job_finish(struct work_struct *work)
>> * manages to find this job as the next job in the list, the fence
>> * signaled check below will prevent the timeout to be restarted.
>> */
>> - cancel_delayed_work_sync(&s_job->work_tdr);
>> + cancel_delayed_work_sync(&sched->work_tdr);
>>
>> spin_lock(&sched->job_list_lock);
>> - /* queue TDR for next job */
>> - if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
>> - !list_is_last(&s_job->node, &sched->ring_mirror_list)) {
>> - struct drm_sched_job *next = list_next_entry(s_job, node);
>> -
>> - if (!dma_fence_is_signaled(&next->s_fence->finished))
>> - schedule_delayed_work(&next->work_tdr, sched->timeout);
>> - }
>> /* remove job from ring_mirror_list */
>> list_del(&s_job->node);
>> + /* queue TDR for next job */
>> + if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
>> + !list_empty(&sched->ring_mirror_list))
>> + schedule_delayed_work(&sched->work_tdr, sched->timeout);
>> spin_unlock(&sched->job_list_lock);
>>
>> dma_fence_put(&s_job->s_fence->finished);
>> @@ -236,16 +232,21 @@ static void drm_sched_job_begin(struct drm_sched_job *s_job)
>> if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
>> list_first_entry_or_null(&sched->ring_mirror_list,
>> struct drm_sched_job, node) == s_job)
>> - schedule_delayed_work(&s_job->work_tdr, sched->timeout);
>> + schedule_delayed_work(&sched->work_tdr, sched->timeout);
>> spin_unlock(&sched->job_list_lock);
>> }
>>
>> static void drm_sched_job_timedout(struct work_struct *work)
>> {
>> - struct drm_sched_job *job = container_of(work, struct drm_sched_job,
>> - work_tdr.work);
>> + struct drm_gpu_scheduler *sched;
>> + struct drm_sched_job *job;
>> +
>> + sched = container_of(work, struct drm_gpu_scheduler, work_tdr.work);
>> + job = list_first_entry_or_null(&sched->ring_mirror_list,
>> + struct drm_sched_job, node);
>>
>> - job->sched->ops->timedout_job(job);
>> + if (job)
>> + job->sched->ops->timedout_job(job);
> I don't think this is fully robust. Jobs are only removed from the
> ring_mirror_list once the job_finish worker has run. If execution of
> this worker is delayed for any reason (though it's really unlikely for
> a delay as long as the job timeout to happen) you are blaming the wrong
> job here.
>
> So I think what you need to to is find the first job in the ring mirror
> list with an unsignaled finish fence to robustly find the stuck job.
Yeah, that is a known problem I've pointed out as well.
The issue is we have bug reports that this happened before the patch,
but I'm not 100% sure how.
My suggestion is to move a good part of the logic from
drm_sched_hw_job_reset() and drm_sched_job_recovery() into
drm_sched_job_timedout().
E.g. we first call dma_fence_remove_callback() for each job and actually
check the return value if the fence was already signaled.
If we find a signaled fence we abort and add the callback back to the
ones where we removed it.
Nayan do you want to take care of this or should I take a look?
Regards,
Christian.
>
> Regards,
> Lucas
>
>> }
>>
>> /**
>> @@ -315,7 +316,7 @@ void drm_sched_job_recovery(struct drm_gpu_scheduler *sched)
>> s_job = list_first_entry_or_null(&sched->ring_mirror_list,
>> struct drm_sched_job, node);
>> if (s_job && sched->timeout != MAX_SCHEDULE_TIMEOUT)
>> - schedule_delayed_work(&s_job->work_tdr, sched->timeout);
>> + schedule_delayed_work(&sched->work_tdr, sched->timeout);
>>
>> list_for_each_entry_safe(s_job, tmp, &sched->ring_mirror_list, node) {
>> struct drm_sched_fence *s_fence = s_job->s_fence;
>> @@ -384,7 +385,6 @@ int drm_sched_job_init(struct drm_sched_job *job,
>>
>> INIT_WORK(&job->finish_work, drm_sched_job_finish);
>> INIT_LIST_HEAD(&job->node);
>> - INIT_DELAYED_WORK(&job->work_tdr, drm_sched_job_timedout);
>>
>> return 0;
>> }
>> @@ -575,6 +575,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);
>> + INIT_DELAYED_WORK(&sched->work_tdr, drm_sched_job_timedout);
>> atomic_set(&sched->num_jobs, 0);
>> atomic64_set(&sched->job_id_count, 0);
>>
>> diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
>> index daec50f887b3..d87b268f1781 100644
>> --- a/include/drm/gpu_scheduler.h
>> +++ b/include/drm/gpu_scheduler.h
>> @@ -175,8 +175,6 @@ struct drm_sched_fence *to_drm_sched_fence(struct dma_fence *f);
>> * finished to remove the job from the
>> * @drm_gpu_scheduler.ring_mirror_list.
>> * @node: used to append this struct to the @drm_gpu_scheduler.ring_mirror_list.
>> - * @work_tdr: schedules a delayed call to @drm_sched_job_timedout after the timeout
>> - * interval is over.
>> * @id: a unique id assigned to each job scheduled on the scheduler.
>> * @karma: increment on every hang caused by this job. If this exceeds the hang
>> * limit of the scheduler then the job is marked guilty and will not
>> @@ -195,7 +193,6 @@ struct drm_sched_job {
>> struct dma_fence_cb finish_cb;
>> struct work_struct finish_work;
>> struct list_head node;
>> - struct delayed_work work_tdr;
>> uint64_t id;
>> atomic_t karma;
>> enum drm_sched_priority s_priority;
>> @@ -259,6 +256,8 @@ struct drm_sched_backend_ops {
>> * finished.
>> * @hw_rq_count: the number of jobs currently in the hardware queue.
>> * @job_id_count: used to assign unique id to the each job.
>> + * @work_tdr: schedules a delayed call to @drm_sched_job_timedout after the
>> + * timeout interval is over.
>> * @thread: the kthread on which the scheduler which run.
>> * @ring_mirror_list: the list of jobs which are currently in the job queue.
>> * @job_list_lock: lock to protect the ring_mirror_list.
>> @@ -278,6 +277,7 @@ struct drm_gpu_scheduler {
>> wait_queue_head_t job_scheduled;
>> atomic_t hw_rq_count;
>> atomic64_t job_id_count;
>> + struct delayed_work work_tdr;
>> struct task_struct *thread;
>> struct list_head ring_mirror_list;
>> spinlock_t job_list_lock;
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] drm/scheduler: remove timeout work_struct from drm_sched_job
2018-09-26 8:13 ` Christian König
@ 2018-09-26 15:55 ` Nayan Deshmukh
2018-10-04 16:32 ` Nayan Deshmukh
0 siblings, 1 reply; 9+ messages in thread
From: Nayan Deshmukh @ 2018-09-26 15:55 UTC (permalink / raw)
To: Christian König; +Cc: Maling list - DRI developers
[-- Attachment #1.1: Type: text/plain, Size: 8674 bytes --]
Hi Christian,
On Wed, Sep 26, 2018, 10:13 AM Christian König <
ckoenig.leichtzumerken@gmail.com> wrote:
> Am 26.09.2018 um 09:39 schrieb Lucas Stach:
> > Hi Nayan,
> >
> > Am Mittwoch, den 26.09.2018, 02:09 +0900 schrieb Nayan Deshmukh:
> >> having a delayed work item per job is redundant as we only need one
> >> per scheduler to track the time out the currently executing job.
> >>
> >> v2: the first element of the ring mirror list is the currently
> >> executing job so we don't need a additional variable for it
> >>
> >> Signed-off-by: Nayan Deshmukh <nayan26deshmukh@gmail.com>
> >> Suggested-by: Christian König <christian.koenig@amd.com>
> >> ---
> >> drivers/gpu/drm/scheduler/sched_main.c | 31
> ++++++++++++++++---------------
> >> include/drm/gpu_scheduler.h | 6 +++---
> >> 2 files changed, 19 insertions(+), 18 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/scheduler/sched_main.c
> b/drivers/gpu/drm/scheduler/sched_main.c
> >> index 9ca741f3a0bc..4e8505d51795 100644
> >> --- a/drivers/gpu/drm/scheduler/sched_main.c
> >> +++ b/drivers/gpu/drm/scheduler/sched_main.c
> >> @@ -197,19 +197,15 @@ static void drm_sched_job_finish(struct
> work_struct *work)
> >> * manages to find this job as the next job in the list, the fence
> >> * signaled check below will prevent the timeout to be restarted.
> >> */
> >> - cancel_delayed_work_sync(&s_job->work_tdr);
> >> + cancel_delayed_work_sync(&sched->work_tdr);
> >>
> >> spin_lock(&sched->job_list_lock);
> >> - /* queue TDR for next job */
> >> - if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
> >> - !list_is_last(&s_job->node, &sched->ring_mirror_list)) {
> >> - struct drm_sched_job *next = list_next_entry(s_job, node);
> >> -
> >> - if (!dma_fence_is_signaled(&next->s_fence->finished))
> >> - schedule_delayed_work(&next->work_tdr,
> sched->timeout);
> >> - }
> >> /* remove job from ring_mirror_list */
> >> list_del(&s_job->node);
> >> + /* queue TDR for next job */
> >> + if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
> >> + !list_empty(&sched->ring_mirror_list))
> >> + schedule_delayed_work(&sched->work_tdr, sched->timeout);
> >> spin_unlock(&sched->job_list_lock);
> >>
> >> dma_fence_put(&s_job->s_fence->finished);
> >> @@ -236,16 +232,21 @@ static void drm_sched_job_begin(struct
> drm_sched_job *s_job)
> >> if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
> >> list_first_entry_or_null(&sched->ring_mirror_list,
> >> struct drm_sched_job, node) == s_job)
> >> - schedule_delayed_work(&s_job->work_tdr, sched->timeout);
> >> + schedule_delayed_work(&sched->work_tdr, sched->timeout);
> >> spin_unlock(&sched->job_list_lock);
> >> }
> >>
> >> static void drm_sched_job_timedout(struct work_struct *work)
> >> {
> >> - struct drm_sched_job *job = container_of(work, struct
> drm_sched_job,
> >> - work_tdr.work);
> >> + struct drm_gpu_scheduler *sched;
> >> + struct drm_sched_job *job;
> >> +
> >> + sched = container_of(work, struct drm_gpu_scheduler, work_tdr.work
> );
> >> + job = list_first_entry_or_null(&sched->ring_mirror_list,
> >> + struct drm_sched_job, node);
> >>
> >> - job->sched->ops->timedout_job(job);
> >> + if (job)
> >> + job->sched->ops->timedout_job(job);
> > I don't think this is fully robust. Jobs are only removed from the
> > ring_mirror_list once the job_finish worker has run. If execution of
> > this worker is delayed for any reason (though it's really unlikely for
> > a delay as long as the job timeout to happen) you are blaming the wrong
> > job here.
> >
> > So I think what you need to to is find the first job in the ring mirror
> > list with an unsignaled finish fence to robustly find the stuck job.
>
> Yeah, that is a known problem I've pointed out as well.
>
> The issue is we have bug reports that this happened before the patch,
> but I'm not 100% sure how.
>
> My suggestion is to move a good part of the logic from
> drm_sched_hw_job_reset() and drm_sched_job_recovery() into
> drm_sched_job_timedout().
>
> E.g. we first call dma_fence_remove_callback() for each job and actually
> check the return value if the fence was already signaled.
>
> If we find a signaled fence we abort and add the callback back to the
> ones where we removed it.
>
> Nayan do you want to take care of this or should I take a look?
>
I can take care of it.
Regards,
Nayan
>
> Regards,
> Christian.
>
> >
> > Regards,
> > Lucas
> >
> >> }
> >>
> >> /**
> >> @@ -315,7 +316,7 @@ void drm_sched_job_recovery(struct
> drm_gpu_scheduler *sched)
> >> s_job = list_first_entry_or_null(&sched->ring_mirror_list,
> >> struct drm_sched_job, node);
> >> if (s_job && sched->timeout != MAX_SCHEDULE_TIMEOUT)
> >> - schedule_delayed_work(&s_job->work_tdr, sched->timeout);
> >> + schedule_delayed_work(&sched->work_tdr, sched->timeout);
> >>
> >> list_for_each_entry_safe(s_job, tmp, &sched->ring_mirror_list,
> node) {
> >> struct drm_sched_fence *s_fence = s_job->s_fence;
> >> @@ -384,7 +385,6 @@ int drm_sched_job_init(struct drm_sched_job *job,
> >>
> >> INIT_WORK(&job->finish_work, drm_sched_job_finish);
> >> INIT_LIST_HEAD(&job->node);
> >> - INIT_DELAYED_WORK(&job->work_tdr, drm_sched_job_timedout);
> >>
> >> return 0;
> >> }
> >> @@ -575,6 +575,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);
> >> + INIT_DELAYED_WORK(&sched->work_tdr, drm_sched_job_timedout);
> >> atomic_set(&sched->num_jobs, 0);
> >> atomic64_set(&sched->job_id_count, 0);
> >>
> >> diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
> >> index daec50f887b3..d87b268f1781 100644
> >> --- a/include/drm/gpu_scheduler.h
> >> +++ b/include/drm/gpu_scheduler.h
> >> @@ -175,8 +175,6 @@ struct drm_sched_fence *to_drm_sched_fence(struct
> dma_fence *f);
> >> * finished to remove the job from the
> >> * @drm_gpu_scheduler.ring_mirror_list.
> >> * @node: used to append this struct to the
> @drm_gpu_scheduler.ring_mirror_list.
> >> - * @work_tdr: schedules a delayed call to @drm_sched_job_timedout
> after the timeout
> >> - * interval is over.
> >> * @id: a unique id assigned to each job scheduled on the scheduler.
> >> * @karma: increment on every hang caused by this job. If this
> exceeds the hang
> >> * limit of the scheduler then the job is marked guilty and
> will not
> >> @@ -195,7 +193,6 @@ struct drm_sched_job {
> >> struct dma_fence_cb finish_cb;
> >> struct work_struct finish_work;
> >> struct list_head node;
> >> - struct delayed_work work_tdr;
> >> uint64_t id;
> >> atomic_t karma;
> >> enum drm_sched_priority s_priority;
> >> @@ -259,6 +256,8 @@ struct drm_sched_backend_ops {
> >> * finished.
> >> * @hw_rq_count: the number of jobs currently in the hardware queue.
> >> * @job_id_count: used to assign unique id to the each job.
> >> + * @work_tdr: schedules a delayed call to @drm_sched_job_timedout
> after the
> >> + * timeout interval is over.
> >> * @thread: the kthread on which the scheduler which run.
> >> * @ring_mirror_list: the list of jobs which are currently in the job
> queue.
> >> * @job_list_lock: lock to protect the ring_mirror_list.
> >> @@ -278,6 +277,7 @@ struct drm_gpu_scheduler {
> >> wait_queue_head_t job_scheduled;
> >> atomic_t hw_rq_count;
> >> atomic64_t job_id_count;
> >> + struct delayed_work work_tdr;
> >> struct task_struct *thread;
> >> struct list_head ring_mirror_list;
> >> spinlock_t job_list_lock;
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
[-- Attachment #1.2: Type: text/html, Size: 11881 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] drm/scheduler: remove timeout work_struct from drm_sched_job
2018-09-26 15:55 ` Nayan Deshmukh
@ 2018-10-04 16:32 ` Nayan Deshmukh
2018-10-04 17:27 ` Christian König
0 siblings, 1 reply; 9+ messages in thread
From: Nayan Deshmukh @ 2018-10-04 16:32 UTC (permalink / raw)
To: Christian König; +Cc: Maling list - DRI developers
Hi Christian,
On Thu, Sep 27, 2018 at 12:55 AM Nayan Deshmukh
<nayan26deshmukh@gmail.com> wrote:
>
> Hi Christian,
>
>
> On Wed, Sep 26, 2018, 10:13 AM Christian König <ckoenig.leichtzumerken@gmail.com> wrote:
>>
>> Am 26.09.2018 um 09:39 schrieb Lucas Stach:
>> > Hi Nayan,
>> >
>> > Am Mittwoch, den 26.09.2018, 02:09 +0900 schrieb Nayan Deshmukh:
>> >> having a delayed work item per job is redundant as we only need one
>> >> per scheduler to track the time out the currently executing job.
>> >>
>> >> v2: the first element of the ring mirror list is the currently
>> >> executing job so we don't need a additional variable for it
>> >>
>> >> Signed-off-by: Nayan Deshmukh <nayan26deshmukh@gmail.com>
>> >> Suggested-by: Christian König <christian.koenig@amd.com>
>> >> ---
>> >> drivers/gpu/drm/scheduler/sched_main.c | 31 ++++++++++++++++---------------
>> >> include/drm/gpu_scheduler.h | 6 +++---
>> >> 2 files changed, 19 insertions(+), 18 deletions(-)
>> >>
>> >> diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c
>> >> index 9ca741f3a0bc..4e8505d51795 100644
>> >> --- a/drivers/gpu/drm/scheduler/sched_main.c
>> >> +++ b/drivers/gpu/drm/scheduler/sched_main.c
>> >> @@ -197,19 +197,15 @@ static void drm_sched_job_finish(struct work_struct *work)
>> >> * manages to find this job as the next job in the list, the fence
>> >> * signaled check below will prevent the timeout to be restarted.
>> >> */
>> >> - cancel_delayed_work_sync(&s_job->work_tdr);
>> >> + cancel_delayed_work_sync(&sched->work_tdr);
>> >>
>> >> spin_lock(&sched->job_list_lock);
>> >> - /* queue TDR for next job */
>> >> - if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
>> >> - !list_is_last(&s_job->node, &sched->ring_mirror_list)) {
>> >> - struct drm_sched_job *next = list_next_entry(s_job, node);
>> >> -
>> >> - if (!dma_fence_is_signaled(&next->s_fence->finished))
>> >> - schedule_delayed_work(&next->work_tdr, sched->timeout);
>> >> - }
>> >> /* remove job from ring_mirror_list */
>> >> list_del(&s_job->node);
>> >> + /* queue TDR for next job */
>> >> + if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
>> >> + !list_empty(&sched->ring_mirror_list))
>> >> + schedule_delayed_work(&sched->work_tdr, sched->timeout);
>> >> spin_unlock(&sched->job_list_lock);
>> >>
>> >> dma_fence_put(&s_job->s_fence->finished);
>> >> @@ -236,16 +232,21 @@ static void drm_sched_job_begin(struct drm_sched_job *s_job)
>> >> if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
>> >> list_first_entry_or_null(&sched->ring_mirror_list,
>> >> struct drm_sched_job, node) == s_job)
>> >> - schedule_delayed_work(&s_job->work_tdr, sched->timeout);
>> >> + schedule_delayed_work(&sched->work_tdr, sched->timeout);
>> >> spin_unlock(&sched->job_list_lock);
>> >> }
>> >>
>> >> static void drm_sched_job_timedout(struct work_struct *work)
>> >> {
>> >> - struct drm_sched_job *job = container_of(work, struct drm_sched_job,
>> >> - work_tdr.work);
>> >> + struct drm_gpu_scheduler *sched;
>> >> + struct drm_sched_job *job;
>> >> +
>> >> + sched = container_of(work, struct drm_gpu_scheduler, work_tdr.work);
>> >> + job = list_first_entry_or_null(&sched->ring_mirror_list,
>> >> + struct drm_sched_job, node);
>> >>
>> >> - job->sched->ops->timedout_job(job);
>> >> + if (job)
>> >> + job->sched->ops->timedout_job(job);
>> > I don't think this is fully robust. Jobs are only removed from the
>> > ring_mirror_list once the job_finish worker has run. If execution of
>> > this worker is delayed for any reason (though it's really unlikely for
>> > a delay as long as the job timeout to happen) you are blaming the wrong
>> > job here.
>> >
>> > So I think what you need to to is find the first job in the ring mirror
>> > list with an unsignaled finish fence to robustly find the stuck job.
>>
>> Yeah, that is a known problem I've pointed out as well.
>>
>> The issue is we have bug reports that this happened before the patch,
>> but I'm not 100% sure how.
>>
>> My suggestion is to move a good part of the logic from
>> drm_sched_hw_job_reset() and drm_sched_job_recovery() into
>> drm_sched_job_timedout().
>>
>> E.g. we first call dma_fence_remove_callback() for each job and actually
>> check the return value if the fence was already signaled.
>>
We can move this part to drm_sched_job_timedout().
>> If we find a signaled fence we abort and add the callback back to the
>> ones where we removed it.
>>
I was not able to find the abort part. In drm_sched_hw_job_reset() we
don't take a action if the parent fence was already signaled.
We cannot shift a lot from drm_sched_job_recovery() to
drm_sched_job_timedout(). The only part that seems shiftable is the
one where we cancel the guilty job.
Regards,
Nayan
>> Nayan do you want to take care of this or should I take a look?
>
> I can take care of it.
>
> Regards,
> Nayan
>>
>>
>> Regards,
>> Christian.
>>
>> >
>> > Regards,
>> > Lucas
>> >
>> >> }
>> >>
>> >> /**
>> >> @@ -315,7 +316,7 @@ void drm_sched_job_recovery(struct drm_gpu_scheduler *sched)
>> >> s_job = list_first_entry_or_null(&sched->ring_mirror_list,
>> >> struct drm_sched_job, node);
>> >> if (s_job && sched->timeout != MAX_SCHEDULE_TIMEOUT)
>> >> - schedule_delayed_work(&s_job->work_tdr, sched->timeout);
>> >> + schedule_delayed_work(&sched->work_tdr, sched->timeout);
>> >>
>> >> list_for_each_entry_safe(s_job, tmp, &sched->ring_mirror_list, node) {
>> >> struct drm_sched_fence *s_fence = s_job->s_fence;
>> >> @@ -384,7 +385,6 @@ int drm_sched_job_init(struct drm_sched_job *job,
>> >>
>> >> INIT_WORK(&job->finish_work, drm_sched_job_finish);
>> >> INIT_LIST_HEAD(&job->node);
>> >> - INIT_DELAYED_WORK(&job->work_tdr, drm_sched_job_timedout);
>> >>
>> >> return 0;
>> >> }
>> >> @@ -575,6 +575,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);
>> >> + INIT_DELAYED_WORK(&sched->work_tdr, drm_sched_job_timedout);
>> >> atomic_set(&sched->num_jobs, 0);
>> >> atomic64_set(&sched->job_id_count, 0);
>> >>
>> >> diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
>> >> index daec50f887b3..d87b268f1781 100644
>> >> --- a/include/drm/gpu_scheduler.h
>> >> +++ b/include/drm/gpu_scheduler.h
>> >> @@ -175,8 +175,6 @@ struct drm_sched_fence *to_drm_sched_fence(struct dma_fence *f);
>> >> * finished to remove the job from the
>> >> * @drm_gpu_scheduler.ring_mirror_list.
>> >> * @node: used to append this struct to the @drm_gpu_scheduler.ring_mirror_list.
>> >> - * @work_tdr: schedules a delayed call to @drm_sched_job_timedout after the timeout
>> >> - * interval is over.
>> >> * @id: a unique id assigned to each job scheduled on the scheduler.
>> >> * @karma: increment on every hang caused by this job. If this exceeds the hang
>> >> * limit of the scheduler then the job is marked guilty and will not
>> >> @@ -195,7 +193,6 @@ struct drm_sched_job {
>> >> struct dma_fence_cb finish_cb;
>> >> struct work_struct finish_work;
>> >> struct list_head node;
>> >> - struct delayed_work work_tdr;
>> >> uint64_t id;
>> >> atomic_t karma;
>> >> enum drm_sched_priority s_priority;
>> >> @@ -259,6 +256,8 @@ struct drm_sched_backend_ops {
>> >> * finished.
>> >> * @hw_rq_count: the number of jobs currently in the hardware queue.
>> >> * @job_id_count: used to assign unique id to the each job.
>> >> + * @work_tdr: schedules a delayed call to @drm_sched_job_timedout after the
>> >> + * timeout interval is over.
>> >> * @thread: the kthread on which the scheduler which run.
>> >> * @ring_mirror_list: the list of jobs which are currently in the job queue.
>> >> * @job_list_lock: lock to protect the ring_mirror_list.
>> >> @@ -278,6 +277,7 @@ struct drm_gpu_scheduler {
>> >> wait_queue_head_t job_scheduled;
>> >> atomic_t hw_rq_count;
>> >> atomic64_t job_id_count;
>> >> + struct delayed_work work_tdr;
>> >> struct task_struct *thread;
>> >> struct list_head ring_mirror_list;
>> >> spinlock_t job_list_lock;
>> > _______________________________________________
>> > dri-devel mailing list
>> > dri-devel@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] drm/scheduler: remove timeout work_struct from drm_sched_job
2018-10-04 16:32 ` Nayan Deshmukh
@ 2018-10-04 17:27 ` Christian König
0 siblings, 0 replies; 9+ messages in thread
From: Christian König @ 2018-10-04 17:27 UTC (permalink / raw)
To: Nayan Deshmukh, Christian König; +Cc: Maling list - DRI developers
Am 04.10.2018 um 18:32 schrieb Nayan Deshmukh:
> Hi Christian,
>
> On Thu, Sep 27, 2018 at 12:55 AM Nayan Deshmukh
> <nayan26deshmukh@gmail.com> wrote:
>> Hi Christian,
>>
>>
>> On Wed, Sep 26, 2018, 10:13 AM Christian König <ckoenig.leichtzumerken@gmail.com> wrote:
>>> Am 26.09.2018 um 09:39 schrieb Lucas Stach:
>>>> Hi Nayan,
>>>>
>>>> Am Mittwoch, den 26.09.2018, 02:09 +0900 schrieb Nayan Deshmukh:
>>>>> having a delayed work item per job is redundant as we only need one
>>>>> per scheduler to track the time out the currently executing job.
>>>>>
>>>>> v2: the first element of the ring mirror list is the currently
>>>>> executing job so we don't need a additional variable for it
>>>>>
>>>>> Signed-off-by: Nayan Deshmukh <nayan26deshmukh@gmail.com>
>>>>> Suggested-by: Christian König <christian.koenig@amd.com>
>>>>> ---
>>>>> drivers/gpu/drm/scheduler/sched_main.c | 31 ++++++++++++++++---------------
>>>>> include/drm/gpu_scheduler.h | 6 +++---
>>>>> 2 files changed, 19 insertions(+), 18 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c
>>>>> index 9ca741f3a0bc..4e8505d51795 100644
>>>>> --- a/drivers/gpu/drm/scheduler/sched_main.c
>>>>> +++ b/drivers/gpu/drm/scheduler/sched_main.c
>>>>> @@ -197,19 +197,15 @@ static void drm_sched_job_finish(struct work_struct *work)
>>>>> * manages to find this job as the next job in the list, the fence
>>>>> * signaled check below will prevent the timeout to be restarted.
>>>>> */
>>>>> - cancel_delayed_work_sync(&s_job->work_tdr);
>>>>> + cancel_delayed_work_sync(&sched->work_tdr);
>>>>>
>>>>> spin_lock(&sched->job_list_lock);
>>>>> - /* queue TDR for next job */
>>>>> - if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
>>>>> - !list_is_last(&s_job->node, &sched->ring_mirror_list)) {
>>>>> - struct drm_sched_job *next = list_next_entry(s_job, node);
>>>>> -
>>>>> - if (!dma_fence_is_signaled(&next->s_fence->finished))
>>>>> - schedule_delayed_work(&next->work_tdr, sched->timeout);
>>>>> - }
>>>>> /* remove job from ring_mirror_list */
>>>>> list_del(&s_job->node);
>>>>> + /* queue TDR for next job */
>>>>> + if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
>>>>> + !list_empty(&sched->ring_mirror_list))
>>>>> + schedule_delayed_work(&sched->work_tdr, sched->timeout);
>>>>> spin_unlock(&sched->job_list_lock);
>>>>>
>>>>> dma_fence_put(&s_job->s_fence->finished);
>>>>> @@ -236,16 +232,21 @@ static void drm_sched_job_begin(struct drm_sched_job *s_job)
>>>>> if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
>>>>> list_first_entry_or_null(&sched->ring_mirror_list,
>>>>> struct drm_sched_job, node) == s_job)
>>>>> - schedule_delayed_work(&s_job->work_tdr, sched->timeout);
>>>>> + schedule_delayed_work(&sched->work_tdr, sched->timeout);
>>>>> spin_unlock(&sched->job_list_lock);
>>>>> }
>>>>>
>>>>> static void drm_sched_job_timedout(struct work_struct *work)
>>>>> {
>>>>> - struct drm_sched_job *job = container_of(work, struct drm_sched_job,
>>>>> - work_tdr.work);
>>>>> + struct drm_gpu_scheduler *sched;
>>>>> + struct drm_sched_job *job;
>>>>> +
>>>>> + sched = container_of(work, struct drm_gpu_scheduler, work_tdr.work);
>>>>> + job = list_first_entry_or_null(&sched->ring_mirror_list,
>>>>> + struct drm_sched_job, node);
>>>>>
>>>>> - job->sched->ops->timedout_job(job);
>>>>> + if (job)
>>>>> + job->sched->ops->timedout_job(job);
>>>> I don't think this is fully robust. Jobs are only removed from the
>>>> ring_mirror_list once the job_finish worker has run. If execution of
>>>> this worker is delayed for any reason (though it's really unlikely for
>>>> a delay as long as the job timeout to happen) you are blaming the wrong
>>>> job here.
>>>>
>>>> So I think what you need to to is find the first job in the ring mirror
>>>> list with an unsignaled finish fence to robustly find the stuck job.
>>> Yeah, that is a known problem I've pointed out as well.
>>>
>>> The issue is we have bug reports that this happened before the patch,
>>> but I'm not 100% sure how.
>>>
>>> My suggestion is to move a good part of the logic from
>>> drm_sched_hw_job_reset() and drm_sched_job_recovery() into
>>> drm_sched_job_timedout().
>>>
>>> E.g. we first call dma_fence_remove_callback() for each job and actually
>>> check the return value if the fence was already signaled.
>>>
> We can move this part to drm_sched_job_timedout().
>
>>> If we find a signaled fence we abort and add the callback back to the
>>> ones where we removed it.
>>>
> I was not able to find the abort part. In drm_sched_hw_job_reset() we
> don't take a action if the parent fence was already signaled.
That is a bug a swell.
> We cannot shift a lot from drm_sched_job_recovery() to
> drm_sched_job_timedout(). The only part that seems shiftable is the
> one where we cancel the guilty job.
Actually I wanted to completely rework that part since it is rather
unreliable as well.
Considering how buggy all of that is and how important it is to get it
right I think I will rather do it myself.
Regards,
Christian.
>
> Regards,
> Nayan
>>> Nayan do you want to take care of this or should I take a look?
>> I can take care of it.
>>
>> Regards,
>> Nayan
>>>
>>> Regards,
>>> Christian.
>>>
>>>> Regards,
>>>> Lucas
>>>>
>>>>> }
>>>>>
>>>>> /**
>>>>> @@ -315,7 +316,7 @@ void drm_sched_job_recovery(struct drm_gpu_scheduler *sched)
>>>>> s_job = list_first_entry_or_null(&sched->ring_mirror_list,
>>>>> struct drm_sched_job, node);
>>>>> if (s_job && sched->timeout != MAX_SCHEDULE_TIMEOUT)
>>>>> - schedule_delayed_work(&s_job->work_tdr, sched->timeout);
>>>>> + schedule_delayed_work(&sched->work_tdr, sched->timeout);
>>>>>
>>>>> list_for_each_entry_safe(s_job, tmp, &sched->ring_mirror_list, node) {
>>>>> struct drm_sched_fence *s_fence = s_job->s_fence;
>>>>> @@ -384,7 +385,6 @@ int drm_sched_job_init(struct drm_sched_job *job,
>>>>>
>>>>> INIT_WORK(&job->finish_work, drm_sched_job_finish);
>>>>> INIT_LIST_HEAD(&job->node);
>>>>> - INIT_DELAYED_WORK(&job->work_tdr, drm_sched_job_timedout);
>>>>>
>>>>> return 0;
>>>>> }
>>>>> @@ -575,6 +575,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);
>>>>> + INIT_DELAYED_WORK(&sched->work_tdr, drm_sched_job_timedout);
>>>>> atomic_set(&sched->num_jobs, 0);
>>>>> atomic64_set(&sched->job_id_count, 0);
>>>>>
>>>>> diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
>>>>> index daec50f887b3..d87b268f1781 100644
>>>>> --- a/include/drm/gpu_scheduler.h
>>>>> +++ b/include/drm/gpu_scheduler.h
>>>>> @@ -175,8 +175,6 @@ struct drm_sched_fence *to_drm_sched_fence(struct dma_fence *f);
>>>>> * finished to remove the job from the
>>>>> * @drm_gpu_scheduler.ring_mirror_list.
>>>>> * @node: used to append this struct to the @drm_gpu_scheduler.ring_mirror_list.
>>>>> - * @work_tdr: schedules a delayed call to @drm_sched_job_timedout after the timeout
>>>>> - * interval is over.
>>>>> * @id: a unique id assigned to each job scheduled on the scheduler.
>>>>> * @karma: increment on every hang caused by this job. If this exceeds the hang
>>>>> * limit of the scheduler then the job is marked guilty and will not
>>>>> @@ -195,7 +193,6 @@ struct drm_sched_job {
>>>>> struct dma_fence_cb finish_cb;
>>>>> struct work_struct finish_work;
>>>>> struct list_head node;
>>>>> - struct delayed_work work_tdr;
>>>>> uint64_t id;
>>>>> atomic_t karma;
>>>>> enum drm_sched_priority s_priority;
>>>>> @@ -259,6 +256,8 @@ struct drm_sched_backend_ops {
>>>>> * finished.
>>>>> * @hw_rq_count: the number of jobs currently in the hardware queue.
>>>>> * @job_id_count: used to assign unique id to the each job.
>>>>> + * @work_tdr: schedules a delayed call to @drm_sched_job_timedout after the
>>>>> + * timeout interval is over.
>>>>> * @thread: the kthread on which the scheduler which run.
>>>>> * @ring_mirror_list: the list of jobs which are currently in the job queue.
>>>>> * @job_list_lock: lock to protect the ring_mirror_list.
>>>>> @@ -278,6 +277,7 @@ struct drm_gpu_scheduler {
>>>>> wait_queue_head_t job_scheduled;
>>>>> atomic_t hw_rq_count;
>>>>> atomic64_t job_id_count;
>>>>> + struct delayed_work work_tdr;
>>>>> struct task_struct *thread;
>>>>> struct list_head ring_mirror_list;
>>>>> spinlock_t job_list_lock;
>>>> _______________________________________________
>>>> dri-devel mailing list
>>>> dri-devel@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-10-04 17:27 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-09-21 18:57 [PATCH v2] drm/scheduler: remove timeout work_struct from drm_sched_job Nayan Deshmukh
2018-09-22 7:23 ` Christian König
2018-09-25 17:09 ` Nayan Deshmukh
2018-09-25 17:34 ` Christian König
2018-09-26 7:39 ` Lucas Stach
2018-09-26 8:13 ` Christian König
2018-09-26 15:55 ` Nayan Deshmukh
2018-10-04 16:32 ` Nayan Deshmukh
2018-10-04 17:27 ` Christian König
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox