I can take care of this.


On 08/10/2018 09:27 AM, Christian König wrote:
Crap, yeah indeed that needs to be protected by some lock.

Going to prepare a patch for that,

Am 09.08.2018 um 21:49 schrieb Andrey Grodzovsky:

Reviewed-by: Andrey Grodzovsky <andrey.grodzov...@amd.com>

But I still  have questions about entity->last_user (didn't notice this before) -

Looks to me there is a race condition with it's current usage, let's say process A was preempted after doing drm_sched_entity_flush->cmpxchg(...)

now process B working on same entity (forked) is inside drm_sched_entity_push_job, he writes his PID to entity->last_user and also

executes drm_sched_rq_add_entity. Now process A runs again and execute drm_sched_rq_remove_entity inadvertently causing process B removal

from it's scheduler rq.

Looks to me like instead we should lock together entity->last_user accesses and adds/removals of entity to the rq.


On 08/06/2018 10:18 AM, Nayan Deshmukh wrote:
I forgot about this since we started discussing possible scenarios of processes and threads.

In any case, this check is redundant. Acked-by: Nayan Deshmukh <nayan26deshm...@gmail.com <mailto:nayan26deshm...@gmail.com>>


On Mon, Aug 6, 2018 at 7:43 PM Christian König <ckoenig.leichtzumer...@gmail.com <mailto:ckoenig.leichtzumer...@gmail.com>> wrote:

    Ping. Any objections to that?


    Am 03.08.2018 um 13:08 schrieb Christian König:
    > That is superflous now.
    > Signed-off-by: Christian König <christian.koe...@amd.com
    > ---
    >   drivers/gpu/drm/scheduler/gpu_scheduler.c | 5 -----
    >   1 file changed, 5 deletions(-)
    > diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c
    > index 85908c7f913e..65078dd3c82c 100644
    > --- a/drivers/gpu/drm/scheduler/gpu_scheduler.c
    > +++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c
    > @@ -590,11 +590,6 @@ void drm_sched_entity_push_job(struct
    drm_sched_job *sched_job,
    >       if (first) {
    >               /* Add the entity to the run queue */
    >               spin_lock(&entity->rq_lock);
    > -             if (!entity->rq) {
    > -                     DRM_ERROR("Trying to push to a killed
    > -  spin_unlock(&entity->rq_lock);
    > -                     return;
    > -             }
    >               drm_sched_rq_add_entity(entity->rq, entity);
    >               spin_unlock(&entity->rq_lock);
    >  drm_sched_wakeup(entity->rq->sched);

dri-devel mailing list

Reply via email to