drm: Convert prime dma-buf <-> handle to rhashtable

Submitted by Chris Wilson on Sept. 22, 2016, 2:43 p.m.

Details

Message ID 20160922144328.28432-1-chris@chris-wilson.co.uk
State New
Headers show
Series "drm: Convert prime dma-buf <-> handle to rhashtable" ( rev: 1 ) in DRI devel

Not browsing as part of any series.

Commit Message

Chris Wilson Sept. 22, 2016, 2:43 p.m.
Currently we use a linear walk to lookup a handle and return a dma-buf,
and vice versa. A long overdue TODO task is to convert that to a
hashtable. Since the initial implementation of dma-buf/prime, we now
have resizeable hashtables we can use (and now a future task is to RCU
enable the lookup!).

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94631
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/drm_prime.c | 94 +++++++++++++++++++++++++++++++++++----------
 include/drm/drmP.h          |  5 ++-
 2 files changed, 77 insertions(+), 22 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 780589b420a4..ad077def660d 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -28,6 +28,7 @@ 
 
 #include <linux/export.h>
 #include <linux/dma-buf.h>
+#include <linux/rhashtable.h>
 #include <drm/drmP.h>
 #include <drm/drm_gem.h>
 
@@ -61,9 +62,11 @@ 
  */
 
 struct drm_prime_member {
-	struct list_head entry;
 	struct dma_buf *dma_buf;
 	uint32_t handle;
+
+	struct rhash_head dma_buf_rht;
+	struct rhash_head handle_rht;
 };
 
 struct drm_prime_attachment {
@@ -71,10 +74,31 @@  struct drm_prime_attachment {
 	enum dma_data_direction dir;
 };
 
+static const struct rhashtable_params dma_buf_params = {
+	.head_offset = offsetof(struct drm_prime_member, dma_buf_rht),
+	.key_len = sizeof(struct dma_buf *),
+	.key_offset = offsetof(struct drm_prime_member, dma_buf),
+	.hashfn = jhash,
+	.nulls_base = 1u << RHT_BASE_SHIFT,
+	.automatic_shrinking = true,
+	.nelem_hint = 2,
+};
+
+static const struct rhashtable_params handle_params = {
+	.head_offset = offsetof(struct drm_prime_member, handle_rht),
+	.key_len = sizeof(uint32_t),
+	.key_offset = offsetof(struct drm_prime_member, handle),
+	.hashfn = jhash,
+	.nulls_base = 1u << RHT_BASE_SHIFT,
+	.automatic_shrinking = true,
+	.nelem_hint = 2,
+};
+
 static int drm_prime_add_buf_handle(struct drm_prime_file_private *prime_fpriv,
 				    struct dma_buf *dma_buf, uint32_t handle)
 {
 	struct drm_prime_member *member;
+	int err;
 
 	member = kmalloc(sizeof(*member), GFP_KERNEL);
 	if (!member)
@@ -83,8 +107,28 @@  static int drm_prime_add_buf_handle(struct drm_prime_file_private *prime_fpriv,
 	get_dma_buf(dma_buf);
 	member->dma_buf = dma_buf;
 	member->handle = handle;
-	list_add(&member->entry, &prime_fpriv->head);
+
+	err = rhashtable_insert_fast(&prime_fpriv->dma_bufs,
+				     &member->dma_buf_rht,
+				     dma_buf_params);
+	if (err)
+		goto err_dma_buf;
+
+	err = rhashtable_insert_fast(&prime_fpriv->handles,
+				     &member->handle_rht,
+				     handle_params);
+	if (err)
+		goto err_dma_rht;
+
 	return 0;
+
+err_dma_rht:
+	rhashtable_remove_fast(&prime_fpriv->dma_bufs,
+			       &member->dma_buf_rht,
+			       dma_buf_params);
+err_dma_buf:
+	dma_buf_put(dma_buf);
+	return err;
 }
 
 static struct dma_buf *drm_prime_lookup_buf_by_handle(struct drm_prime_file_private *prime_fpriv,
@@ -92,10 +136,10 @@  static struct dma_buf *drm_prime_lookup_buf_by_handle(struct drm_prime_file_priv
 {
 	struct drm_prime_member *member;
 
-	list_for_each_entry(member, &prime_fpriv->head, entry) {
-		if (member->handle == handle)
-			return member->dma_buf;
-	}
+	member = rhashtable_lookup_fast(&prime_fpriv->handles,
+					&handle, handle_params);
+	if (member)
+		return member->dma_buf;
 
 	return NULL;
 }
@@ -106,12 +150,13 @@  static int drm_prime_lookup_buf_handle(struct drm_prime_file_private *prime_fpri
 {
 	struct drm_prime_member *member;
 
-	list_for_each_entry(member, &prime_fpriv->head, entry) {
-		if (member->dma_buf == dma_buf) {
-			*handle = member->handle;
-			return 0;
-		}
+	member = rhashtable_lookup_fast(&prime_fpriv->dma_bufs,
+					&dma_buf, dma_buf_params);
+	if (member) {
+		*handle = member->handle;
+		return 0;
 	}
+
 	return -ENOENT;
 }
 
@@ -166,14 +211,21 @@  static void drm_gem_map_detach(struct dma_buf *dma_buf,
 void drm_prime_remove_buf_handle_locked(struct drm_prime_file_private *prime_fpriv,
 					struct dma_buf *dma_buf)
 {
-	struct drm_prime_member *member, *safe;
+	struct drm_prime_member *member;
 
-	list_for_each_entry_safe(member, safe, &prime_fpriv->head, entry) {
-		if (member->dma_buf == dma_buf) {
-			dma_buf_put(dma_buf);
-			list_del(&member->entry);
-			kfree(member);
-		}
+	member = rhashtable_lookup_fast(&prime_fpriv->dma_bufs,
+					&dma_buf, dma_buf_params);
+	if (member) {
+		rhashtable_remove_fast(&prime_fpriv->dma_bufs,
+				       &member->dma_buf_rht,
+				       dma_buf_params);
+
+		rhashtable_remove_fast(&prime_fpriv->handles,
+				       &member->handle_rht,
+				       handle_params);
+
+		dma_buf_put(dma_buf);
+		kfree(member);
 	}
 }
 
@@ -759,12 +811,14 @@  EXPORT_SYMBOL(drm_prime_gem_destroy);
 
 void drm_prime_init_file_private(struct drm_prime_file_private *prime_fpriv)
 {
-	INIT_LIST_HEAD(&prime_fpriv->head);
 	mutex_init(&prime_fpriv->lock);
+	rhashtable_init(&prime_fpriv->dma_bufs, &dma_buf_params);
+	rhashtable_init(&prime_fpriv->handles, &handle_params);
 }
 
 void drm_prime_destroy_file_private(struct drm_prime_file_private *prime_fpriv)
 {
 	/* by now drm_gem_release should've made sure the list is empty */
-	WARN_ON(!list_empty(&prime_fpriv->head));
+	rhashtable_destroy(&prime_fpriv->dma_bufs);
+	rhashtable_destroy(&prime_fpriv->handles);
 }
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index c53dc90942e0..6966fb030d0f 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -51,6 +51,7 @@ 
 #include <linux/platform_device.h>
 #include <linux/poll.h>
 #include <linux/ratelimit.h>
+#include <linux/rhashtable.h>
 #include <linux/sched.h>
 #include <linux/slab.h>
 #include <linux/types.h>
@@ -371,10 +372,10 @@  struct drm_pending_event {
 		      we deliver the event, for tracing only */
 };
 
-/* initial implementaton using a linked list - todo hashtab */
 struct drm_prime_file_private {
-	struct list_head head;
 	struct mutex lock;
+	struct rhashtable dma_bufs;
+	struct rhashtable handles;
 };
 
 /** File private data */

Comments

On Thu, Sep 22, 2016 at 7:43 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Currently we use a linear walk to lookup a handle and return a dma-buf,
> and vice versa. A long overdue TODO task is to convert that to a
> hashtable. Since the initial implementation of dma-buf/prime, we now
> have resizeable hashtables we can use (and now a future task is to RCU
> enable the lookup!).
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94631
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Looks like a nice improvement

Reviewed-by: Sean Paul <seanpaul@chromium.org>

> ---
>  drivers/gpu/drm/drm_prime.c | 94 +++++++++++++++++++++++++++++++++++----------
>  include/drm/drmP.h          |  5 ++-
>  2 files changed, 77 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 780589b420a4..ad077def660d 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -28,6 +28,7 @@
>
>  #include <linux/export.h>
>  #include <linux/dma-buf.h>
> +#include <linux/rhashtable.h>
>  #include <drm/drmP.h>
>  #include <drm/drm_gem.h>
>
> @@ -61,9 +62,11 @@
>   */
>
>  struct drm_prime_member {
> -       struct list_head entry;
>         struct dma_buf *dma_buf;
>         uint32_t handle;
> +
> +       struct rhash_head dma_buf_rht;
> +       struct rhash_head handle_rht;
>  };
>
>  struct drm_prime_attachment {
> @@ -71,10 +74,31 @@ struct drm_prime_attachment {
>         enum dma_data_direction dir;
>  };
>
> +static const struct rhashtable_params dma_buf_params = {
> +       .head_offset = offsetof(struct drm_prime_member, dma_buf_rht),
> +       .key_len = sizeof(struct dma_buf *),
> +       .key_offset = offsetof(struct drm_prime_member, dma_buf),
> +       .hashfn = jhash,
> +       .nulls_base = 1u << RHT_BASE_SHIFT,
> +       .automatic_shrinking = true,
> +       .nelem_hint = 2,
> +};
> +
> +static const struct rhashtable_params handle_params = {
> +       .head_offset = offsetof(struct drm_prime_member, handle_rht),
> +       .key_len = sizeof(uint32_t),
> +       .key_offset = offsetof(struct drm_prime_member, handle),
> +       .hashfn = jhash,
> +       .nulls_base = 1u << RHT_BASE_SHIFT,
> +       .automatic_shrinking = true,
> +       .nelem_hint = 2,
> +};
> +
>  static int drm_prime_add_buf_handle(struct drm_prime_file_private *prime_fpriv,
>                                     struct dma_buf *dma_buf, uint32_t handle)
>  {
>         struct drm_prime_member *member;
> +       int err;
>
>         member = kmalloc(sizeof(*member), GFP_KERNEL);
>         if (!member)
> @@ -83,8 +107,28 @@ static int drm_prime_add_buf_handle(struct drm_prime_file_private *prime_fpriv,
>         get_dma_buf(dma_buf);
>         member->dma_buf = dma_buf;
>         member->handle = handle;
> -       list_add(&member->entry, &prime_fpriv->head);
> +
> +       err = rhashtable_insert_fast(&prime_fpriv->dma_bufs,
> +                                    &member->dma_buf_rht,
> +                                    dma_buf_params);
> +       if (err)
> +               goto err_dma_buf;
> +
> +       err = rhashtable_insert_fast(&prime_fpriv->handles,
> +                                    &member->handle_rht,
> +                                    handle_params);
> +       if (err)
> +               goto err_dma_rht;
> +
>         return 0;
> +
> +err_dma_rht:
> +       rhashtable_remove_fast(&prime_fpriv->dma_bufs,
> +                              &member->dma_buf_rht,
> +                              dma_buf_params);
> +err_dma_buf:
> +       dma_buf_put(dma_buf);
> +       return err;
>  }
>
>  static struct dma_buf *drm_prime_lookup_buf_by_handle(struct drm_prime_file_private *prime_fpriv,
> @@ -92,10 +136,10 @@ static struct dma_buf *drm_prime_lookup_buf_by_handle(struct drm_prime_file_priv
>  {
>         struct drm_prime_member *member;
>
> -       list_for_each_entry(member, &prime_fpriv->head, entry) {
> -               if (member->handle == handle)
> -                       return member->dma_buf;
> -       }
> +       member = rhashtable_lookup_fast(&prime_fpriv->handles,
> +                                       &handle, handle_params);
> +       if (member)
> +               return member->dma_buf;
>
>         return NULL;
>  }
> @@ -106,12 +150,13 @@ static int drm_prime_lookup_buf_handle(struct drm_prime_file_private *prime_fpri
>  {
>         struct drm_prime_member *member;
>
> -       list_for_each_entry(member, &prime_fpriv->head, entry) {
> -               if (member->dma_buf == dma_buf) {
> -                       *handle = member->handle;
> -                       return 0;
> -               }
> +       member = rhashtable_lookup_fast(&prime_fpriv->dma_bufs,
> +                                       &dma_buf, dma_buf_params);
> +       if (member) {
> +               *handle = member->handle;
> +               return 0;
>         }
> +
>         return -ENOENT;
>  }
>
> @@ -166,14 +211,21 @@ static void drm_gem_map_detach(struct dma_buf *dma_buf,
>  void drm_prime_remove_buf_handle_locked(struct drm_prime_file_private *prime_fpriv,
>                                         struct dma_buf *dma_buf)
>  {
> -       struct drm_prime_member *member, *safe;
> +       struct drm_prime_member *member;
>
> -       list_for_each_entry_safe(member, safe, &prime_fpriv->head, entry) {
> -               if (member->dma_buf == dma_buf) {
> -                       dma_buf_put(dma_buf);
> -                       list_del(&member->entry);
> -                       kfree(member);
> -               }
> +       member = rhashtable_lookup_fast(&prime_fpriv->dma_bufs,
> +                                       &dma_buf, dma_buf_params);
> +       if (member) {
> +               rhashtable_remove_fast(&prime_fpriv->dma_bufs,
> +                                      &member->dma_buf_rht,
> +                                      dma_buf_params);
> +
> +               rhashtable_remove_fast(&prime_fpriv->handles,
> +                                      &member->handle_rht,
> +                                      handle_params);
> +
> +               dma_buf_put(dma_buf);
> +               kfree(member);
>         }
>  }
>
> @@ -759,12 +811,14 @@ EXPORT_SYMBOL(drm_prime_gem_destroy);
>
>  void drm_prime_init_file_private(struct drm_prime_file_private *prime_fpriv)
>  {
> -       INIT_LIST_HEAD(&prime_fpriv->head);
>         mutex_init(&prime_fpriv->lock);
> +       rhashtable_init(&prime_fpriv->dma_bufs, &dma_buf_params);
> +       rhashtable_init(&prime_fpriv->handles, &handle_params);
>  }
>
>  void drm_prime_destroy_file_private(struct drm_prime_file_private *prime_fpriv)
>  {
>         /* by now drm_gem_release should've made sure the list is empty */
> -       WARN_ON(!list_empty(&prime_fpriv->head));
> +       rhashtable_destroy(&prime_fpriv->dma_bufs);
> +       rhashtable_destroy(&prime_fpriv->handles);
>  }
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index c53dc90942e0..6966fb030d0f 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -51,6 +51,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/poll.h>
>  #include <linux/ratelimit.h>
> +#include <linux/rhashtable.h>
>  #include <linux/sched.h>
>  #include <linux/slab.h>
>  #include <linux/types.h>
> @@ -371,10 +372,10 @@ struct drm_pending_event {
>                       we deliver the event, for tracing only */
>  };
>
> -/* initial implementaton using a linked list - todo hashtab */
>  struct drm_prime_file_private {
> -       struct list_head head;
>         struct mutex lock;
> +       struct rhashtable dma_bufs;
> +       struct rhashtable handles;
>  };
>
>  /** File private data */
> --
> 2.9.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx