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

[Christian König]

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/a7e83884/attachment.html>