2019-07-12 08:49:02

by Lecopzer Chen

[permalink] [raw]
Subject: [PATCH] staging: android: ion: Remove unused rbtree for ion_buffer

ion_buffer_add() insert ion_buffer into rbtree every time creating
an ion_buffer but never use it after ION reworking.
Also, buffer_lock protects only rbtree operation, remove it together.

Signed-off-by: Lecopzer Chen <[email protected]>
Cc: YJ Chiang <[email protected]>
Cc: Lecopzer Chen <[email protected]>
---
drivers/staging/android/ion/ion.c | 36 -------------------------------
drivers/staging/android/ion/ion.h | 10 +--------
2 files changed, 1 insertion(+), 45 deletions(-)

diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index 92c2914239e3..e6b1ca141b93 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -29,32 +29,6 @@
static struct ion_device *internal_dev;
static int heap_id;

-/* this function should only be called while dev->lock is held */
-static void ion_buffer_add(struct ion_device *dev,
- struct ion_buffer *buffer)
-{
- struct rb_node **p = &dev->buffers.rb_node;
- struct rb_node *parent = NULL;
- struct ion_buffer *entry;
-
- while (*p) {
- parent = *p;
- entry = rb_entry(parent, struct ion_buffer, node);
-
- if (buffer < entry) {
- p = &(*p)->rb_left;
- } else if (buffer > entry) {
- p = &(*p)->rb_right;
- } else {
- pr_err("%s: buffer already found.", __func__);
- BUG();
- }
- }
-
- rb_link_node(&buffer->node, parent, p);
- rb_insert_color(&buffer->node, &dev->buffers);
-}
-
/* this function should only be called while dev->lock is held */
static struct ion_buffer *ion_buffer_create(struct ion_heap *heap,
struct ion_device *dev,
@@ -100,9 +74,6 @@ static struct ion_buffer *ion_buffer_create(struct ion_heap *heap,

INIT_LIST_HEAD(&buffer->attachments);
mutex_init(&buffer->lock);
- mutex_lock(&dev->buffer_lock);
- ion_buffer_add(dev, buffer);
- mutex_unlock(&dev->buffer_lock);
return buffer;

err1:
@@ -131,11 +102,6 @@ void ion_buffer_destroy(struct ion_buffer *buffer)
static void _ion_buffer_destroy(struct ion_buffer *buffer)
{
struct ion_heap *heap = buffer->heap;
- struct ion_device *dev = buffer->dev;
-
- mutex_lock(&dev->buffer_lock);
- rb_erase(&buffer->node, &dev->buffers);
- mutex_unlock(&dev->buffer_lock);

if (heap->flags & ION_HEAP_FLAG_DEFER_FREE)
ion_heap_freelist_add(heap, buffer);
@@ -694,8 +660,6 @@ static int ion_device_create(void)
}

idev->debug_root = debugfs_create_dir("ion", NULL);
- idev->buffers = RB_ROOT;
- mutex_init(&idev->buffer_lock);
init_rwsem(&idev->lock);
plist_head_init(&idev->heaps);
internal_dev = idev;
diff --git a/drivers/staging/android/ion/ion.h b/drivers/staging/android/ion/ion.h
index e291299fd35f..74914a266e25 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -23,7 +23,6 @@

/**
* struct ion_buffer - metadata for a particular buffer
- * @node: node in the ion_device buffers tree
* @list: element in list of deferred freeable buffers
* @dev: back pointer to the ion_device
* @heap: back pointer to the heap the buffer came from
@@ -39,10 +38,7 @@
* @attachments: list of devices attached to this buffer
*/
struct ion_buffer {
- union {
- struct rb_node node;
- struct list_head list;
- };
+ struct list_head list;
struct ion_device *dev;
struct ion_heap *heap;
unsigned long flags;
@@ -61,14 +57,10 @@ void ion_buffer_destroy(struct ion_buffer *buffer);
/**
* struct ion_device - the metadata of the ion device node
* @dev: the actual misc device
- * @buffers: an rb tree of all the existing buffers
- * @buffer_lock: lock protecting the tree of buffers
* @lock: rwsem protecting the tree of heaps and clients
*/
struct ion_device {
struct miscdevice dev;
- struct rb_root buffers;
- struct mutex buffer_lock;
struct rw_semaphore lock;
struct plist_head heaps;
struct dentry *debug_root;
--
2.17.1


2019-07-17 15:08:34

by Laura Abbott

[permalink] [raw]
Subject: Re: [PATCH] staging: android: ion: Remove unused rbtree for ion_buffer

On 7/12/19 4:47 AM, Lecopzer Chen wrote:
> ion_buffer_add() insert ion_buffer into rbtree every time creating
> an ion_buffer but never use it after ION reworking.
> Also, buffer_lock protects only rbtree operation, remove it together.
>
> Signed-off-by: Lecopzer Chen <[email protected]>
> Cc: YJ Chiang <[email protected]>
> Cc: Lecopzer Chen <[email protected]>
> ---
> drivers/staging/android/ion/ion.c | 36 -------------------------------
> drivers/staging/android/ion/ion.h | 10 +--------
> 2 files changed, 1 insertion(+), 45 deletions(-)
>
> diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
> index 92c2914239e3..e6b1ca141b93 100644
> --- a/drivers/staging/android/ion/ion.c
> +++ b/drivers/staging/android/ion/ion.c
> @@ -29,32 +29,6 @@
> static struct ion_device *internal_dev;
> static int heap_id;
>
> -/* this function should only be called while dev->lock is held */
> -static void ion_buffer_add(struct ion_device *dev,
> - struct ion_buffer *buffer)
> -{
> - struct rb_node **p = &dev->buffers.rb_node;
> - struct rb_node *parent = NULL;
> - struct ion_buffer *entry;
> -
> - while (*p) {
> - parent = *p;
> - entry = rb_entry(parent, struct ion_buffer, node);
> -
> - if (buffer < entry) {
> - p = &(*p)->rb_left;
> - } else if (buffer > entry) {
> - p = &(*p)->rb_right;
> - } else {
> - pr_err("%s: buffer already found.", __func__);
> - BUG();
> - }
> - }
> -
> - rb_link_node(&buffer->node, parent, p);
> - rb_insert_color(&buffer->node, &dev->buffers);
> -}
> -
> /* this function should only be called while dev->lock is held */
> static struct ion_buffer *ion_buffer_create(struct ion_heap *heap,
> struct ion_device *dev,
> @@ -100,9 +74,6 @@ static struct ion_buffer *ion_buffer_create(struct ion_heap *heap,
>
> INIT_LIST_HEAD(&buffer->attachments);
> mutex_init(&buffer->lock);
> - mutex_lock(&dev->buffer_lock);
> - ion_buffer_add(dev, buffer);
> - mutex_unlock(&dev->buffer_lock);
> return buffer;
>
> err1:
> @@ -131,11 +102,6 @@ void ion_buffer_destroy(struct ion_buffer *buffer)
> static void _ion_buffer_destroy(struct ion_buffer *buffer)
> {
> struct ion_heap *heap = buffer->heap;
> - struct ion_device *dev = buffer->dev;
> -
> - mutex_lock(&dev->buffer_lock);
> - rb_erase(&buffer->node, &dev->buffers);
> - mutex_unlock(&dev->buffer_lock);
>
> if (heap->flags & ION_HEAP_FLAG_DEFER_FREE)
> ion_heap_freelist_add(heap, buffer);
> @@ -694,8 +660,6 @@ static int ion_device_create(void)
> }
>
> idev->debug_root = debugfs_create_dir("ion", NULL);
> - idev->buffers = RB_ROOT;
> - mutex_init(&idev->buffer_lock);
> init_rwsem(&idev->lock);
> plist_head_init(&idev->heaps);
> internal_dev = idev;
> diff --git a/drivers/staging/android/ion/ion.h b/drivers/staging/android/ion/ion.h
> index e291299fd35f..74914a266e25 100644
> --- a/drivers/staging/android/ion/ion.h
> +++ b/drivers/staging/android/ion/ion.h
> @@ -23,7 +23,6 @@
>
> /**
> * struct ion_buffer - metadata for a particular buffer
> - * @node: node in the ion_device buffers tree
> * @list: element in list of deferred freeable buffers
> * @dev: back pointer to the ion_device
> * @heap: back pointer to the heap the buffer came from
> @@ -39,10 +38,7 @@
> * @attachments: list of devices attached to this buffer
> */
> struct ion_buffer {
> - union {
> - struct rb_node node;
> - struct list_head list;
> - };
> + struct list_head list;
> struct ion_device *dev;
> struct ion_heap *heap;
> unsigned long flags;
> @@ -61,14 +57,10 @@ void ion_buffer_destroy(struct ion_buffer *buffer);
> /**
> * struct ion_device - the metadata of the ion device node
> * @dev: the actual misc device
> - * @buffers: an rb tree of all the existing buffers
> - * @buffer_lock: lock protecting the tree of buffers
> * @lock: rwsem protecting the tree of heaps and clients
> */
> struct ion_device {
> struct miscdevice dev;
> - struct rb_root buffers;
> - struct mutex buffer_lock;
> struct rw_semaphore lock;
> struct plist_head heaps;
> struct dentry *debug_root;
>

Acked-by: Laura Abbott <[email protected]>