[PATCH v2] drm/scheduler: Remove obsolete spinlock.

[zhoucm1]

Yes, every thing is clear, Acked-by: Chunming Zhou <david1.zhou at amd.com>


On 2018年05月16日 23:33, Andrey Grodzovsky wrote:
> This spinlock is superfluous, any call to drm_sched_entity_push_job
> should already be under a lock together with matching drm_sched_job_init
> to match the order of insertion into queue with job's fence seqence
> number.
>
> v2:
> Improve patch description.
> Add functions documentation describing the locking considerations
>
> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky at amd.com>
> ---
>   drivers/gpu/drm/scheduler/gpu_scheduler.c | 15 ++++++++++-----
>   include/drm/gpu_scheduler.h               |  1 -
>   2 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c b/drivers/gpu/drm/scheduler/gpu_scheduler.c
> index 1f1dd70..80dd66c 100644
> --- a/drivers/gpu/drm/scheduler/gpu_scheduler.c
> +++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c
> @@ -140,7 +140,6 @@ int drm_sched_entity_init(struct drm_gpu_scheduler *sched,
>   	entity->last_scheduled = NULL;
>   
>   	spin_lock_init(&entity->rq_lock);
> -	spin_lock_init(&entity->queue_lock);
>   	spsc_queue_init(&entity->job_queue);
>   
>   	atomic_set(&entity->fence_seq, 0);
> @@ -414,6 +413,10 @@ drm_sched_entity_pop_job(struct drm_sched_entity *entity)
>    *
>    * @sched_job		The pointer to job required to submit
>    *
> + * Note: To guarantee that the order of insertion to queue matches
> + * the job's fence sequence number this function should be
> + * called with drm_sched_job_init under common lock.
> + *
>    * Returns 0 for success, negative error code otherwise.
>    */
>   void drm_sched_entity_push_job(struct drm_sched_job *sched_job,
> @@ -424,11 +427,8 @@ void drm_sched_entity_push_job(struct drm_sched_job *sched_job,
>   
>   	trace_drm_sched_job(sched_job, entity);
>   
> -	spin_lock(&entity->queue_lock);
>   	first = spsc_queue_push(&entity->job_queue, &sched_job->queue_node);
>   
> -	spin_unlock(&entity->queue_lock);
> -
>   	/* first job wakes up scheduler */
>   	if (first) {
>   		/* Add the entity to the run queue */
> @@ -594,7 +594,12 @@ void drm_sched_job_recovery(struct drm_gpu_scheduler *sched)
>   }
>   EXPORT_SYMBOL(drm_sched_job_recovery);
>   
> -/* init a sched_job with basic field */
> +/**
> + * Init a sched_job with basic field
> + *
> + * Note: Refer to drm_sched_entity_push_job documentation
> + * for locking considerations.
> + */
>   int drm_sched_job_init(struct drm_sched_job *job,
>   		       struct drm_gpu_scheduler *sched,
>   		       struct drm_sched_entity *entity,
> diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
> index 350a62c..683eb65 100644
> --- a/include/drm/gpu_scheduler.h
> +++ b/include/drm/gpu_scheduler.h
> @@ -56,7 +56,6 @@ struct drm_sched_entity {
>   	spinlock_t			rq_lock;
>   	struct drm_gpu_scheduler	*sched;
>   
> -	spinlock_t			queue_lock;
>   	struct spsc_queue		job_queue;
>   
>   	atomic_t			fence_seq;