Reviewed-by: Christian König <christian.koenig at amd.com>
Am 17.05.2018 um 04:32 schrieb 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;
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
