[PATCH 0/3] gpu: drm: Fix memory leak in vmwgfx_shader.c

[PATCH 0/3] gpu: drm: Fix memory leak in vmwgfx_shader.c

[Masaru Nomura]

This patch fixes memory leak detected by Kernel memory leak detector, 
and cleans up functions which call drm_ht_remove_item() and 
vmw_compat_shader_free() so that an unused parameter is not passed.

Part of logs from /sys/kernel/debug/kmemleak is as follows:

unreferenced object 0xffffc900086ed000 (size 32768):
  comm "plymouthd", pid 287, jiffies 4294682116 (age 5911.149s)
  hex dump (first 32 bytes):
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<ffffffff816f7b6e>] kmemleak_alloc+0x4e/0xb0
    [<ffffffff811b9382>] __vmalloc_node_range+0x1b2/0x2a0
    [<ffffffff811b950b>] vzalloc+0x4b/0x50
    [<ffffffffa009c7e5>] drm_ht_create+0x65/0xa0 [drm]
    [<ffffffffa011a068>] vmw_compat_shader_man_create+0x78/0xb0 [vmwgfx]
    [<ffffffffa0109ac2>] vmw_driver_open+0x62/0xa0 [vmwgfx]
    [<ffffffffa0093307>] drm_open+0x1b7/0x4c0 [drm]
    [<ffffffffa00936b5>] drm_stub_open+0xa5/0x100 [drm]
    [<ffffffff811f6c29>] chrdev_open+0xb9/0x1a0
    [<ffffffff811ef58f>] do_dentry_open+0x1ff/0x340
    [<ffffffff811ef8a1>] finish_open+0x31/0x40
    [<ffffffff812017e4>] do_last+0xa64/0x1190
    [<ffffffff81201fdd>] path_openat+0xcd/0x670
    [<ffffffff81202ddd>] do_filp_open+0x4d/0xb0
    [<ffffffff811f12fd>] do_sys_open+0x13d/0x230
    [<ffffffff811f140e>] SyS_open+0x1e/0x20
unreferenced object 0xffffc900086e4000 (size 32768):
  comm "Xorg", pid 751, jiffies 4294687683 (age 5917.505s)
  hex dump (first 32 bytes):
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<ffffffff816f7b6e>] kmemleak_alloc+0x4e/0xb0
    [<ffffffff811b9382>] __vmalloc_node_range+0x1b2/0x2a0
    [<ffffffff811b950b>] vzalloc+0x4b/0x50
    [<ffffffffa009c7e5>] drm_ht_create+0x65/0xa0 [drm]
    [<ffffffffa011a068>] vmw_compat_shader_man_create+0x78/0xb0 [vmwgfx]
    [<ffffffffa0109ac2>] vmw_driver_open+0x62/0xa0 [vmwgfx]
    [<ffffffffa0093307>] drm_open+0x1b7/0x4c0 [drm]
    [<ffffffffa00936b5>] drm_stub_open+0xa5/0x100 [drm]
    [<ffffffff811f6c29>] chrdev_open+0xb9/0x1a0
    [<ffffffff811ef58f>] do_dentry_open+0x1ff/0x340
    [<ffffffff811ef8a1>] finish_open+0x31/0x40
    [<ffffffff812017e4>] do_last+0xa64/0x1190
    [<ffffffff81201fdd>] path_openat+0xcd/0x670
    [<ffffffff81202ddd>] do_filp_open+0x4d/0xb0
    [<ffffffff811f12fd>] do_sys_open+0x13d/0x230
    [<ffffffff811f140e>] SyS_open+0x1e/0x20

Masaru Nomura (3):
  gpu: drm: vmwgfx: Fix memory leak by adding drm_ht_remove()
  gpu: drm: Remove unnecessary parameter from drm_ht_remove_item()
  gpu: drm: vmwgfx: Remove unnecessary parameter from
    vmw_compat_shader_free()

 drivers/gpu/drm/drm_auth.c              |  2 +-
 drivers/gpu/drm/drm_hashtab.c           |  2 +-
 drivers/gpu/drm/drm_stub.c              |  2 +-
 drivers/gpu/drm/ttm/ttm_object.c        |  8 +++-----
 drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c |  4 ++--
 drivers/gpu/drm/vmwgfx/vmwgfx_shader.c  | 16 ++++++++--------
 include/drm/drm_hashtab.h               |  2 +-
 7 files changed, 17 insertions(+), 19 deletions(-)

-- 
1.9.3

[PATCH 1/3] gpu: drm: vmwgfx: Fix memory leak by adding drm_ht_remove()

[Masaru Nomura]

drm_ht_remove() should be called in vmw_compat_shader_man_destroy()
This is because memory was allocated for (&man->shaders)->table by
vmw_compat_shader_man_create() -> drm_ht_create()
but this memory is not freed when vmw_compat_shader_mager is destroied.

Signed-off-by: Masaru Nomura <massa.nomura@gmail.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_shader.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c b/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c
index c1559eea..01cc8ea 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c
@@ -806,6 +806,8 @@ void vmw_compat_shader_man_destroy(struct vmw_compat_shader_manager *man)
 	list_for_each_entry_safe(entry, next, &man->list, head)
 		vmw_compat_shader_free(man, entry);
 
+	drm_ht_remove(&man->shaders);
+
 	mutex_unlock(&man->dev_priv->cmdbuf_mutex);
 	kfree(man);
 }
-- 
1.9.3

[PATCH 2/3] gpu: drm: Remove unnecessary parameter from drm_ht_remove_item()

[Masaru Nomura]

removed drm_open_hash from drm_ht_remove_item() as the parameter is
not used within the function.

Signed-off-by: Masaru Nomura <massa.nomura@gmail.com>
---
Please review this patch carefully. The reason the parameter is passed
might be some historical one or clarity of which drm_open_hash
we remove an item from.

 drivers/gpu/drm/drm_auth.c              | 2 +-
 drivers/gpu/drm/drm_hashtab.c           | 2 +-
 drivers/gpu/drm/drm_stub.c              | 2 +-
 drivers/gpu/drm/ttm/ttm_object.c        | 8 +++-----
 drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 4 ++--
 drivers/gpu/drm/vmwgfx/vmwgfx_shader.c  | 4 ++--
 include/drm/drm_hashtab.h               | 2 +-
 7 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index 3cedae1..f7ceea0 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -115,7 +115,7 @@ int drm_remove_magic(struct drm_master *master, drm_magic_t magic)
 		return -EINVAL;
 	}
 	pt = drm_hash_entry(hash, struct drm_magic_entry, hash_item);
-	drm_ht_remove_item(&master->magiclist, hash);
+	drm_ht_remove_item(hash);
 	list_del(&pt->head);
 	mutex_unlock(&dev->struct_mutex);
 
diff --git a/drivers/gpu/drm/drm_hashtab.c b/drivers/gpu/drm/drm_hashtab.c
index c3b80fd..a66447f 100644
--- a/drivers/gpu/drm/drm_hashtab.c
+++ b/drivers/gpu/drm/drm_hashtab.c
@@ -188,7 +188,7 @@ int drm_ht_remove_key(struct drm_open_hash *ht, unsigned long key)
 	return -EINVAL;
 }
 
-int drm_ht_remove_item(struct drm_open_hash *ht, struct drm_hash_item *item)
+int drm_ht_remove_item(struct drm_hash_item *item)
 {
 	hlist_del_init_rcu(&item->head);
 	return 0;
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index 14d1646..6ffed45 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -166,7 +166,7 @@ static void drm_master_destroy(struct kref *kref)
 
 	list_for_each_entry_safe(pt, next, &master->magicfree, head) {
 		list_del(&pt->head);
-		drm_ht_remove_item(&master->magiclist, &pt->hash_item);
+		drm_ht_remove_item(&pt->hash_item);
 		kfree(pt);
 	}
 
diff --git a/drivers/gpu/drm/ttm/ttm_object.c b/drivers/gpu/drm/ttm/ttm_object.c
index d2a0533..42f73a8 100644
--- a/drivers/gpu/drm/ttm/ttm_object.c
+++ b/drivers/gpu/drm/ttm/ttm_object.c
@@ -188,7 +188,7 @@ int ttm_base_object_init(struct ttm_object_file *tfile,
 	return 0;
 out_err1:
 	spin_lock(&tdev->object_lock);
-	(void)drm_ht_remove_item_rcu(&tdev->object_hash, &base->hash);
+	drm_ht_remove_item_rcu(&base->hash);
 	spin_unlock(&tdev->object_lock);
 out_err0:
 	return ret;
@@ -202,7 +202,7 @@ static void ttm_release_base(struct kref *kref)
 	struct ttm_object_device *tdev = base->tfile->tdev;
 
 	spin_lock(&tdev->object_lock);
-	(void)drm_ht_remove_item_rcu(&tdev->object_hash, &base->hash);
+	drm_ht_remove_item_rcu(&base->hash);
 	spin_unlock(&tdev->object_lock);
 
 	/*
@@ -390,11 +390,9 @@ static void ttm_ref_object_release(struct kref *kref)
 	    container_of(kref, struct ttm_ref_object, kref);
 	struct ttm_base_object *base = ref->obj;
 	struct ttm_object_file *tfile = ref->tfile;
-	struct drm_open_hash *ht;
 	struct ttm_mem_global *mem_glob = tfile->tdev->mem_glob;
 
-	ht = &tfile->ref_hash[ref->ref_type];
-	(void)drm_ht_remove_item_rcu(ht, &ref->hash);
+	drm_ht_remove_item_rcu(&ref->hash);
 	list_del(&ref->head);
 	spin_unlock(&tfile->lock);
 
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
index 87df0b3..1780f5e 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
@@ -2185,13 +2185,13 @@ static void vmw_clear_validations(struct vmw_sw_context *sw_context)
 				 base.head) {
 		list_del(&entry->base.head);
 		ttm_bo_unref(&entry->base.bo);
-		(void) drm_ht_remove_item(&sw_context->res_ht, &entry->hash);
+		drm_ht_remove_item(&entry->hash);
 		sw_context->cur_val_buf--;
 	}
 	BUG_ON(sw_context->cur_val_buf != 0);
 
 	list_for_each_entry(val, &sw_context->resource_list, head)
-		(void) drm_ht_remove_item(&sw_context->res_ht, &val->hash);
+		drm_ht_remove_item(&val->hash);
 }
 
 static int vmw_validate_single_buffer(struct vmw_private *dev_priv,
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c b/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c
index 01cc8ea..bbb30c3 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c
@@ -542,7 +542,7 @@ static void vmw_compat_shader_free(struct vmw_compat_shader_manager *man,
 				   struct vmw_compat_shader *entry)
 {
 	list_del(&entry->head);
-	WARN_ON(drm_ht_remove_item(&man->shaders, &entry->hash));
+	WARN_ON(drm_ht_remove_item(&entry->hash));
 	WARN_ON(ttm_ref_object_base_unref(entry->tfile, entry->handle,
 					  TTM_REF_USAGE));
 	kfree(entry);
@@ -652,7 +652,7 @@ int vmw_compat_shader_remove(struct vmw_compat_shader_manager *man,
 		vmw_compat_shader_free(man, entry);
 		break;
 	case VMW_COMPAT_COMMITED:
-		(void) drm_ht_remove_item(&man->shaders, &entry->hash);
+		drm_ht_remove_item(&entry->hash);
 		list_del(&entry->head);
 		entry->state = VMW_COMPAT_DEL;
 		list_add_tail(&entry->head, list);
diff --git a/include/drm/drm_hashtab.h b/include/drm/drm_hashtab.h
index fce2ef3..ae34607 100644
--- a/include/drm/drm_hashtab.h
+++ b/include/drm/drm_hashtab.h
@@ -58,7 +58,7 @@ extern int drm_ht_find_item(struct drm_open_hash *ht, unsigned long key, struct
 
 extern void drm_ht_verbose_list(struct drm_open_hash *ht, unsigned long key);
 extern int drm_ht_remove_key(struct drm_open_hash *ht, unsigned long key);
-extern int drm_ht_remove_item(struct drm_open_hash *ht, struct drm_hash_item *item);
+extern int drm_ht_remove_item(struct drm_hash_item *item);
 extern void drm_ht_remove(struct drm_open_hash *ht);
 
 /*
-- 
1.9.3

[PATCH 3/3] gpu: drm: vmwgfx: Remove unnecessary parameter from vmw_compat_shader_free()

[Masaru Nomura]

vm_compat_shader_manager is only used for drm_ht_remove_item() within the function.
As drm_ht_remove_item() does not need a paremeter drm_open_hash(&man-> shaders),
vm_compat_shader_manager(*man) does not have to be passed to this function.

Signed-off-by: Masaru Nomura <massa.nomura@gmail.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_shader.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c b/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c
index bbb30c3..2fdbf8e 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c
@@ -532,14 +532,12 @@ int vmw_compat_shader_lookup(struct vmw_compat_shader_manager *man,
 /**
  * vmw_compat_shader_free - Free a compat shader.
  *
- * @man: Pointer to the compat shader manager.
  * @entry: Pointer to a struct vmw_compat_shader.
  *
  * Frees a struct vmw_compat_shder entry and drops its reference to the
  * guest backed shader.
  */
-static void vmw_compat_shader_free(struct vmw_compat_shader_manager *man,
-				   struct vmw_compat_shader *entry)
+static void vmw_compat_shader_free(struct vmw_compat_shader *entry)
 {
 	list_del(&entry->head);
 	WARN_ON(drm_ht_remove_item(&entry->hash));
@@ -602,7 +600,7 @@ void vmw_compat_shaders_revert(struct vmw_compat_shader_manager *man,
 	list_for_each_entry_safe(entry, next, list, head) {
 		switch (entry->state) {
 		case VMW_COMPAT_ADD:
-			vmw_compat_shader_free(man, entry);
+			vmw_compat_shader_free(entry);
 			break;
 		case VMW_COMPAT_DEL:
 			ret = drm_ht_insert_item(&man->shaders, &entry->hash);
@@ -649,7 +647,7 @@ int vmw_compat_shader_remove(struct vmw_compat_shader_manager *man,
 
 	switch (entry->state) {
 	case VMW_COMPAT_ADD:
-		vmw_compat_shader_free(man, entry);
+		vmw_compat_shader_free(entry);
 		break;
 	case VMW_COMPAT_COMMITED:
 		drm_ht_remove_item(&entry->hash);
@@ -804,7 +802,7 @@ void vmw_compat_shader_man_destroy(struct vmw_compat_shader_manager *man)
 
 	mutex_lock(&man->dev_priv->cmdbuf_mutex);
 	list_for_each_entry_safe(entry, next, &man->list, head)
-		vmw_compat_shader_free(man, entry);
+		vmw_compat_shader_free(entry);
 
 	drm_ht_remove(&man->shaders);
 
-- 
1.9.3

Re: [PATCH 2/3] gpu: drm: Remove unnecessary parameter from drm_ht_remove_item()

[Daniel Vetter]

On Tue, Jun 24, 2014 at 10:52:13PM +0100, Masaru Nomura wrote:
> removed drm_open_hash from drm_ht_remove_item() as the parameter is
> not used within the function.
> 
> Signed-off-by: Masaru Nomura <massa.nomura@gmail.com>
> ---
> Please review this patch carefully. The reason the parameter is passed
> might be some historical one or clarity of which drm_open_hash
> we remove an item from.

Reasons for this are probably lost. On the patch:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Aside: Imo we could/should just move all the users to directly employ the
linux hashtab instead of partially reinventing the wheel here in drm.
-Daniel

> 
>  drivers/gpu/drm/drm_auth.c              | 2 +-
>  drivers/gpu/drm/drm_hashtab.c           | 2 +-
>  drivers/gpu/drm/drm_stub.c              | 2 +-
>  drivers/gpu/drm/ttm/ttm_object.c        | 8 +++-----
>  drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 4 ++--
>  drivers/gpu/drm/vmwgfx/vmwgfx_shader.c  | 4 ++--
>  include/drm/drm_hashtab.h               | 2 +-
>  7 files changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
> index 3cedae1..f7ceea0 100644
> --- a/drivers/gpu/drm/drm_auth.c
> +++ b/drivers/gpu/drm/drm_auth.c
> @@ -115,7 +115,7 @@ int drm_remove_magic(struct drm_master *master, drm_magic_t magic)
>  		return -EINVAL;
>  	}
>  	pt = drm_hash_entry(hash, struct drm_magic_entry, hash_item);
> -	drm_ht_remove_item(&master->magiclist, hash);
> +	drm_ht_remove_item(hash);
>  	list_del(&pt->head);
>  	mutex_unlock(&dev->struct_mutex);
>  
> diff --git a/drivers/gpu/drm/drm_hashtab.c b/drivers/gpu/drm/drm_hashtab.c
> index c3b80fd..a66447f 100644
> --- a/drivers/gpu/drm/drm_hashtab.c
> +++ b/drivers/gpu/drm/drm_hashtab.c
> @@ -188,7 +188,7 @@ int drm_ht_remove_key(struct drm_open_hash *ht, unsigned long key)
>  	return -EINVAL;
>  }
>  
> -int drm_ht_remove_item(struct drm_open_hash *ht, struct drm_hash_item *item)
> +int drm_ht_remove_item(struct drm_hash_item *item)
>  {
>  	hlist_del_init_rcu(&item->head);
>  	return 0;
> diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
> index 14d1646..6ffed45 100644
> --- a/drivers/gpu/drm/drm_stub.c
> +++ b/drivers/gpu/drm/drm_stub.c
> @@ -166,7 +166,7 @@ static void drm_master_destroy(struct kref *kref)
>  
>  	list_for_each_entry_safe(pt, next, &master->magicfree, head) {
>  		list_del(&pt->head);
> -		drm_ht_remove_item(&master->magiclist, &pt->hash_item);
> +		drm_ht_remove_item(&pt->hash_item);
>  		kfree(pt);
>  	}
>  
> diff --git a/drivers/gpu/drm/ttm/ttm_object.c b/drivers/gpu/drm/ttm/ttm_object.c
> index d2a0533..42f73a8 100644
> --- a/drivers/gpu/drm/ttm/ttm_object.c
> +++ b/drivers/gpu/drm/ttm/ttm_object.c
> @@ -188,7 +188,7 @@ int ttm_base_object_init(struct ttm_object_file *tfile,
>  	return 0;
>  out_err1:
>  	spin_lock(&tdev->object_lock);
> -	(void)drm_ht_remove_item_rcu(&tdev->object_hash, &base->hash);
> +	drm_ht_remove_item_rcu(&base->hash);
>  	spin_unlock(&tdev->object_lock);
>  out_err0:
>  	return ret;
> @@ -202,7 +202,7 @@ static void ttm_release_base(struct kref *kref)
>  	struct ttm_object_device *tdev = base->tfile->tdev;
>  
>  	spin_lock(&tdev->object_lock);
> -	(void)drm_ht_remove_item_rcu(&tdev->object_hash, &base->hash);
> +	drm_ht_remove_item_rcu(&base->hash);
>  	spin_unlock(&tdev->object_lock);
>  
>  	/*
> @@ -390,11 +390,9 @@ static void ttm_ref_object_release(struct kref *kref)
>  	    container_of(kref, struct ttm_ref_object, kref);
>  	struct ttm_base_object *base = ref->obj;
>  	struct ttm_object_file *tfile = ref->tfile;
> -	struct drm_open_hash *ht;
>  	struct ttm_mem_global *mem_glob = tfile->tdev->mem_glob;
>  
> -	ht = &tfile->ref_hash[ref->ref_type];
> -	(void)drm_ht_remove_item_rcu(ht, &ref->hash);
> +	drm_ht_remove_item_rcu(&ref->hash);
>  	list_del(&ref->head);
>  	spin_unlock(&tfile->lock);
>  
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
> index 87df0b3..1780f5e 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
> @@ -2185,13 +2185,13 @@ static void vmw_clear_validations(struct vmw_sw_context *sw_context)
>  				 base.head) {
>  		list_del(&entry->base.head);
>  		ttm_bo_unref(&entry->base.bo);
> -		(void) drm_ht_remove_item(&sw_context->res_ht, &entry->hash);
> +		drm_ht_remove_item(&entry->hash);
>  		sw_context->cur_val_buf--;
>  	}
>  	BUG_ON(sw_context->cur_val_buf != 0);
>  
>  	list_for_each_entry(val, &sw_context->resource_list, head)
> -		(void) drm_ht_remove_item(&sw_context->res_ht, &val->hash);
> +		drm_ht_remove_item(&val->hash);
>  }
>  
>  static int vmw_validate_single_buffer(struct vmw_private *dev_priv,
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c b/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c
> index 01cc8ea..bbb30c3 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_shader.c
> @@ -542,7 +542,7 @@ static void vmw_compat_shader_free(struct vmw_compat_shader_manager *man,
>  				   struct vmw_compat_shader *entry)
>  {
>  	list_del(&entry->head);
> -	WARN_ON(drm_ht_remove_item(&man->shaders, &entry->hash));
> +	WARN_ON(drm_ht_remove_item(&entry->hash));
>  	WARN_ON(ttm_ref_object_base_unref(entry->tfile, entry->handle,
>  					  TTM_REF_USAGE));
>  	kfree(entry);
> @@ -652,7 +652,7 @@ int vmw_compat_shader_remove(struct vmw_compat_shader_manager *man,
>  		vmw_compat_shader_free(man, entry);
>  		break;
>  	case VMW_COMPAT_COMMITED:
> -		(void) drm_ht_remove_item(&man->shaders, &entry->hash);
> +		drm_ht_remove_item(&entry->hash);
>  		list_del(&entry->head);
>  		entry->state = VMW_COMPAT_DEL;
>  		list_add_tail(&entry->head, list);
> diff --git a/include/drm/drm_hashtab.h b/include/drm/drm_hashtab.h
> index fce2ef3..ae34607 100644
> --- a/include/drm/drm_hashtab.h
> +++ b/include/drm/drm_hashtab.h
> @@ -58,7 +58,7 @@ extern int drm_ht_find_item(struct drm_open_hash *ht, unsigned long key, struct
>  
>  extern void drm_ht_verbose_list(struct drm_open_hash *ht, unsigned long key);
>  extern int drm_ht_remove_key(struct drm_open_hash *ht, unsigned long key);
> -extern int drm_ht_remove_item(struct drm_open_hash *ht, struct drm_hash_item *item);
> +extern int drm_ht_remove_item(struct drm_hash_item *item);
>  extern void drm_ht_remove(struct drm_open_hash *ht);
>  
>  /*
> -- 
> 1.9.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 2/3] gpu: drm: Remove unnecessary parameter from drm_ht_remove_item()

[Thomas Hellstrom]

On 07/08/2014 01:24 PM, Daniel Vetter wrote:
> On Tue, Jun 24, 2014 at 10:52:13PM +0100, Masaru Nomura wrote:
>> removed drm_open_hash from drm_ht_remove_item() as the parameter is
>> not used within the function.
>>
>> Signed-off-by: Masaru Nomura <massa.nomura@gmail.com>
>> ---
>> Please review this patch carefully. The reason the parameter is passed
>> might be some historical one or clarity of which drm_open_hash
>> we remove an item from.
> Reasons for this are probably lost. On the patch:
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Acked-by: Thomas Hellstrom <thellstrom@vmware.com>

>
> Aside: Imo we could/should just move all the users to directly employ the
> linux hashtab instead of partially reinventing the wheel here in drm.
> -Daniel
>

Actually, in this case, the wheel was invented in drm before it was made
generic :).
It's possible to utilize part of "hashtable.h" but I don't think the
code size gain
will be major...

/Thomas

Re: [PATCH 2/3] gpu: drm: Remove unnecessary parameter from drm_ht_remove_item()

[Daniel Vetter]

On Tue, Jul 08, 2014 at 01:40:44PM +0200, Thomas Hellstrom wrote:
> On 07/08/2014 01:24 PM, Daniel Vetter wrote:
> > On Tue, Jun 24, 2014 at 10:52:13PM +0100, Masaru Nomura wrote:
> >> removed drm_open_hash from drm_ht_remove_item() as the parameter is
> >> not used within the function.
> >>
> >> Signed-off-by: Masaru Nomura <massa.nomura@gmail.com>
> >> ---
> >> Please review this patch carefully. The reason the parameter is passed
> >> might be some historical one or clarity of which drm_open_hash
> >> we remove an item from.
> > Reasons for this are probably lost. On the patch:
> >
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Acked-by: Thomas Hellstrom <thellstrom@vmware.com>
> 
> >
> > Aside: Imo we could/should just move all the users to directly employ the
> > linux hashtab instead of partially reinventing the wheel here in drm.
> > -Daniel
> >
> 
> Actually, in this case, the wheel was invented in drm before it was made
> generic :).
> It's possible to utilize part of "hashtable.h" but I don't think the
> code size gain
> will be major...

Yeah, lots of work and little gain ;-) There needs to be a terribly boring
and rainy day to get around to that ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch