Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes:
> Adding to the tail of the client request list as the only other user is
> in the throttle ioctl that iterates forwards over the list. It only
> needs protection against deletion of a request as it reads it, it simply
> won't see a new request added to the end of the list, or it would be too
> early and rejected. We can further reduce the number of spinlocks
> required when throttling by removing stale requests from the client_list
> as we throttle.
>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Reviewed-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
> drivers/gpu/drm/i915/i915_gem.c | 14 ++++++------
> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 13 ++++++++----
> drivers/gpu/drm/i915/i915_gem_request.c | 34 ++++++------------------------
> drivers/gpu/drm/i915/i915_gem_request.h | 4 +---
> 5 files changed, 23 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 1a28b5279bec..ddae8e442176 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -506,7 +506,7 @@ static int i915_gem_object_info(struct seq_file *m, void *data)
> mutex_lock(&dev->struct_mutex);
> request = list_first_entry_or_null(&file_priv->mm.request_list,
> struct drm_i915_gem_request,
> - client_list);
> + client_link);
> rcu_read_lock();
> task = pid_task(request && request->ctx->pid ?
> request->ctx->pid : file->pid,
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index de1fc98e041d..92ab989bb05f 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3667,16 +3667,14 @@ i915_gem_ring_throttle(struct drm_device *dev, struct drm_file *file)
> return -EIO;
>
> spin_lock(&file_priv->mm.lock);
> - list_for_each_entry(request, &file_priv->mm.request_list, client_list) {
> + list_for_each_entry(request, &file_priv->mm.request_list, client_link) {
> if (time_after_eq(request->emitted_jiffies, recent_enough))
> break;
>
> - /*
> - * Note that the request might not have been submitted yet.
> - * In which case emitted_jiffies will be zero.
> - */
> - if (!request->emitted_jiffies)
> - continue;
> + if (target) {
> + list_del(&target->client_link);
> + target->file_priv = NULL;
> + }
>
> target = request;
> }
> @@ -4735,7 +4733,7 @@ void i915_gem_release(struct drm_device *dev, struct drm_file *file)
> * file_priv.
> */
> spin_lock(&file_priv->mm.lock);
> - list_for_each_entry(request, &file_priv->mm.request_list, client_list)
> + list_for_each_entry(request, &file_priv->mm.request_list, client_link)
> request->file_priv = NULL;
> spin_unlock(&file_priv->mm.lock);
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index e8ffe0c9a20e..2b570d0b2392 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -1420,6 +1420,14 @@ i915_gem_execbuffer_parse(struct intel_engine_cs *engine,
> return vma;
> }
>
> +static void
> +add_to_client(struct drm_i915_gem_request *req,
> + struct drm_file *file)
> +{
> + req->file_priv = file->driver_priv;
> + list_add_tail(&req->client_link, &req->file_priv->mm.request_list);
> +}
> +
> static int
> execbuf_submit(struct i915_execbuffer_params *params,
> struct drm_i915_gem_execbuffer2 *args,
> @@ -1507,6 +1515,7 @@ execbuf_submit(struct i915_execbuffer_params *params,
> return ret;
>
> i915_gem_execbuffer_move_to_active(vmas, params->request);
> + add_to_client(params->request, params->file);
>
> return 0;
> }
> @@ -1886,10 +1895,6 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
> */
> params->request->batch = params->batch;
>
> - ret = i915_gem_request_add_to_client(params->request, file);
> - if (ret)
> - goto err_request;
> -
> /*
> * Save assorted stuff away to pass through to *_submission().
> * NB: This data should be 'persistent' and not local as it will
> diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
> index 3a159cac2172..5bca3e25bf61 100644
> --- a/drivers/gpu/drm/i915/i915_gem_request.c
> +++ b/drivers/gpu/drm/i915/i915_gem_request.c
> @@ -82,42 +82,20 @@ const struct dma_fence_ops i915_fence_ops = {
> .release = i915_fence_release,
> };
>
> -int i915_gem_request_add_to_client(struct drm_i915_gem_request *req,
> - struct drm_file *file)
> -{
> - struct drm_i915_private *dev_private;
> - struct drm_i915_file_private *file_priv;
> -
> - WARN_ON(!req || !file || req->file_priv);
> -
> - if (!req || !file)
> - return -EINVAL;
> -
> - if (req->file_priv)
> - return -EINVAL;
> -
> - dev_private = req->i915;
> - file_priv = file->driver_priv;
> -
> - spin_lock(&file_priv->mm.lock);
> - req->file_priv = file_priv;
> - list_add_tail(&req->client_list, &file_priv->mm.request_list);
> - spin_unlock(&file_priv->mm.lock);
> -
> - return 0;
> -}
> -
> static inline void
> i915_gem_request_remove_from_client(struct drm_i915_gem_request *request)
> {
> - struct drm_i915_file_private *file_priv = request->file_priv;
> + struct drm_i915_file_private *file_priv;
>
> + file_priv = request->file_priv;
> if (!file_priv)
> return;
>
> spin_lock(&file_priv->mm.lock);
> - list_del(&request->client_list);
> - request->file_priv = NULL;
> + if (request->file_priv) {
> + list_del(&request->client_link);
> + request->file_priv = NULL;
> + }
> spin_unlock(&file_priv->mm.lock);
> }
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_request.h b/drivers/gpu/drm/i915/i915_gem_request.h
> index cc24a6c72748..1edc0fa7794c 100644
> --- a/drivers/gpu/drm/i915/i915_gem_request.h
> +++ b/drivers/gpu/drm/i915/i915_gem_request.h
> @@ -191,7 +191,7 @@ struct drm_i915_gem_request {
>
> struct drm_i915_file_private *file_priv;
> /** file_priv list entry for this request */
> - struct list_head client_list;
> + struct list_head client_link;
> };
>
> extern const struct dma_fence_ops i915_fence_ops;
> @@ -204,8 +204,6 @@ static inline bool dma_fence_is_i915(const struct dma_fence *fence)
> struct drm_i915_gem_request * __must_check
> i915_gem_request_alloc(struct intel_engine_cs *engine,
> struct i915_gem_context *ctx);
> -int i915_gem_request_add_to_client(struct drm_i915_gem_request *req,
> - struct drm_file *file);
> void i915_gem_request_retire_upto(struct drm_i915_gem_request *req);
>
> static inline struct drm_i915_gem_request *
> --
> 2.11.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
