[PATCH 2/2] drm/scheduler: stop setting rq to NULL

[Andrey Grodzovsky]

Change is Reviewed-by: Andrey Grodzovsky <andrey.grodzovsky at amd.com>

Andrey


On 07/31/2018 02:50 AM, Christian König wrote:
> Am 30.07.2018 um 22:51 schrieb Andrey Grodzovsky:
>>
>>
>>
>> On 07/30/2018 09:30 AM, Nayan Deshmukh wrote:
>>>
>>>
>>> On Mon, Jul 30, 2018 at 4:33 PM Christian König 
>>> <ckoenig.leichtzumerken at gmail.com 
>>> <mailto:ckoenig.leichtzumerken at gmail.com>> wrote:
>>>
>>>     We removed the redundancy of having an extra scheduler field, so we
>>>     can't set the rq to NULL any more or otherwise won't know which
>>>     scheduler to use for the cleanup.
>>>
>>>     Just remove the entity from the scheduling list instead.
>>>
>>>     Signed-off-by: Christian König <christian.koenig at amd.com
>>>     <mailto:christian.koenig at amd.com>>
>>>
>>> Good catch.
>>>
>>> Acked-by: Nayan Deshmukh <nayan26deshmukh at gmail.com 
>>> <mailto:nayan26deshmukh at gmail.com>>
>>>
>>>     ---
>>>      drivers/gpu/drm/scheduler/gpu_scheduler.c | 35
>>>     +++++++------------------------
>>>      1 file changed, 8 insertions(+), 27 deletions(-)
>>>
>>>     diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c
>>>     b/drivers/gpu/drm/scheduler/gpu_scheduler.c
>>>     index f563e4fbb4b6..1b733229201e 100644
>>>     --- a/drivers/gpu/drm/scheduler/gpu_scheduler.c
>>>     +++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c
>>>     @@ -198,21 +198,6 @@ int drm_sched_entity_init(struct
>>>     drm_sched_entity *entity,
>>>      }
>>>      EXPORT_SYMBOL(drm_sched_entity_init);
>>>
>>>     -/**
>>>     - * drm_sched_entity_is_initialized - Query if entity is initialized
>>>     - *
>>>     - * @sched: Pointer to scheduler instance
>>>     - * @entity: The pointer to a valid scheduler entity
>>>     - *
>>>     - * return true if entity is initialized, false otherwise
>>>     -*/
>>>     -static bool drm_sched_entity_is_initialized(struct
>>>     drm_gpu_scheduler *sched,
>>>     -                                           struct
>>>     drm_sched_entity *entity)
>>>     -{
>>>     -       return entity->rq != NULL &&
>>>     -               entity->rq->sched == sched;
>>>     -}
>>>     -
>>>      /**
>>>       * drm_sched_entity_is_idle - Check if entity is idle
>>>       *
>>>     @@ -224,7 +209,8 @@ static bool drm_sched_entity_is_idle(struct
>>>     drm_sched_entity *entity)
>>>      {
>>>             rmb();
>>>
>>>     -       if (!entity->rq || spsc_queue_peek(&entity->job_queue)
>>>     == NULL)
>>>     +       if (list_empty(&entity->list) ||
>>>     +           spsc_queue_peek(&entity->job_queue) == NULL)
>>>                     return true;
>>>
>>>             return false;
>>>     @@ -279,8 +265,6 @@ long drm_sched_entity_flush(struct
>>>     drm_sched_entity *entity, long timeout)
>>>             long ret = timeout;
>>>
>>>             sched = entity->rq->sched;
>>>     -       if (!drm_sched_entity_is_initialized(sched, entity))
>>>     -               return ret;
>>>
>>
>> Just to be clear, you remove this function because it's redundant to 
>> call it?
>
> Yes, exactly.
>
> Christian.
>
>>
>> Andrey
>>
>>>       /**
>>>              * The client will not queue more IBs during this fini,
>>>     consume existing
>>>              * queued IBs or discard them on SIGKILL
>>>     @@ -299,7 +283,7 @@ long drm_sched_entity_flush(struct
>>>     drm_sched_entity *entity, long timeout)
>>>             last_user = cmpxchg(&entity->last_user,
>>>     current->group_leader, NULL);
>>>             if ((!last_user || last_user == current->group_leader) &&
>>>                 (current->flags & PF_EXITING) && (current->exit_code
>>>     == SIGKILL))
>>>     -               drm_sched_entity_set_rq(entity, NULL);
>>>     +  drm_sched_rq_remove_entity(entity->rq, entity);
>>>
>>>             return ret;
>>>      }
>>>     @@ -320,7 +304,7 @@ void drm_sched_entity_fini(struct
>>>     drm_sched_entity *entity)
>>>             struct drm_gpu_scheduler *sched;
>>>
>>>             sched = entity->rq->sched;
>>>     -       drm_sched_entity_set_rq(entity, NULL);
>>>     +       drm_sched_rq_remove_entity(entity->rq, entity);
>>>
>>>             /* Consumption of existing IBs wasn't completed. Forcefully
>>>              * remove them here.
>>>     @@ -416,15 +400,12 @@ void drm_sched_entity_set_rq(struct
>>>     drm_sched_entity *entity,
>>>             if (entity->rq == rq)
>>>                     return;
>>>
>>>     -       spin_lock(&entity->rq_lock);
>>>     -
>>>     -       if (entity->rq)
>>>     -  drm_sched_rq_remove_entity(entity->rq, entity);
>>>     +       BUG_ON(!rq);
>>>
>>>     +       spin_lock(&entity->rq_lock);
>>>     +       drm_sched_rq_remove_entity(entity->rq, entity);
>>>             entity->rq = rq;
>>>     -       if (rq)
>>>     -               drm_sched_rq_add_entity(rq, entity);
>>>     -
>>>     +       drm_sched_rq_add_entity(rq, entity);
>>>             spin_unlock(&entity->rq_lock);
>>>      }
>>>      EXPORT_SYMBOL(drm_sched_entity_set_rq);
>>>     -- 
>>>     2.14.1
>>>
>>>
>>>
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20180731/84c6aff4/attachment.html>