[PATCH 1/2] drm/scheduler: only kill entity if last user is killed v2

[Nayan Deshmukh]

Hi Christian,

The code looks good to me. But I was just wondering what will happen when
the last user is killed and some other user tries to push to the entity.

Regards,
Nayan Deshmukh

On Mon, Jul 30, 2018 at 4:33 PM Christian König <
ckoenig.leichtzumerken at gmail.com> wrote:

> Note which task is using the entity and only kill it if the last user of
> the entity is killed. This should prevent problems when entities are
> leaked to
> child processes.
>
> v2: add missing kernel doc
>
> Signed-off-by: Christian König <christian.koenig at amd.com>
> ---
>  drivers/gpu/drm/scheduler/gpu_scheduler.c | 6 +++++-
>  include/drm/gpu_scheduler.h               | 2 ++
>  2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c
> b/drivers/gpu/drm/scheduler/gpu_scheduler.c
> index 3f2fc5e8242a..f563e4fbb4b6 100644
> --- a/drivers/gpu/drm/scheduler/gpu_scheduler.c
> +++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c
> @@ -275,6 +275,7 @@ static void drm_sched_entity_kill_jobs_cb(struct
> dma_fence *f,
>  long drm_sched_entity_flush(struct drm_sched_entity *entity, long timeout)
>  {
>         struct drm_gpu_scheduler *sched;
> +       struct task_struct *last_user;
>         long ret = timeout;
>
>         sched = entity->rq->sched;
> @@ -295,7 +296,9 @@ long drm_sched_entity_flush(struct drm_sched_entity
> *entity, long timeout)
>
>
>         /* For killed process disable any more IBs enqueue right now */
> -       if ((current->flags & PF_EXITING) && (current->exit_code ==
> SIGKILL))
> +       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);
>
>         return ret;
> @@ -541,6 +544,7 @@ void drm_sched_entity_push_job(struct drm_sched_job
> *sched_job,
>
>         trace_drm_sched_job(sched_job, entity);
>
> +       WRITE_ONCE(entity->last_user, current->group_leader);
>         first = spsc_queue_push(&entity->job_queue,
> &sched_job->queue_node);
>
>         /* first job wakes up scheduler */
> diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
> index 091b9afcd184..21c648b0b2a1 100644
> --- a/include/drm/gpu_scheduler.h
> +++ b/include/drm/gpu_scheduler.h
> @@ -66,6 +66,7 @@ enum drm_sched_priority {
>   * @guilty: points to ctx's guilty.
>   * @fini_status: contains the exit status in case the process was
> signalled.
>   * @last_scheduled: points to the finished fence of the last scheduled
> job.
> + * @last_user: last group leader pushing a job into the entity.
>   *
>   * Entities will emit jobs in order to their corresponding hardware
>   * ring, and the scheduler will alternate between entities based on
> @@ -85,6 +86,7 @@ struct drm_sched_entity {
>         struct dma_fence_cb             cb;
>         atomic_t                        *guilty;
>         struct dma_fence                *last_scheduled;
> +       struct task_struct              *last_user;
>  };
>
>  /**
> --
> 2.14.1
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20180730/23dd15e7/attachment.html>