drm/i915: Drop spinlocks around adding to the client request list

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@chris-wilson.co.uk>
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20170302122525.19675-1-chris@chris-wilson.co.uk

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index b84ee7a323d2faac511bab2d14c5a3b25a89c046..15a76096fbb03aa5d63d98a921f3e0c3315b1797 100644 (file)
--- 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 275e9e0799b9f97c2355213c3abf9cc5fc7b89e3..edc59bd45f53331fb0313e2b0a621888d45b96b3 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3666,16 +3666,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;
        }
@@ -4734,7 +4732,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 aa75ea2d0578c7ca522f48980d29b5bab947280c..dd7181ed5eca2fad84695f4912c8de5712a2cb9e 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1407,6 +1407,14 @@ out:
        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,
@@ -1772,10 +1780,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
@@ -1793,6 +1797,8 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
        ret = execbuf_submit(params, args, &eb->vmas);
 err_request:
        __i915_add_request(params->request, ret == 0);
+       add_to_client(params->request, file);
+
        if (out_fence) {
                if (ret == 0) {
                        fd_install(out_fence_fd, out_fence->file);

diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
index b881d4e1bd1e2fd76c73d1f74e2672c0c0f6c3e8..c2cee9674cb0d4cc624d43a9b736c8ce33d44b10 100644 (file)
--- 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 0efee879df23d9aa06818c31e43decb81b7bb31a..5018e55922f0b4c1f6a123056a3f205031267c91 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem_request.h
+++ b/drivers/gpu/drm/i915/i915_gem_request.h
@@ -180,7 +180,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;
@@ -193,8 +193,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 *