Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH] staging: android: ion: Remove unused rbtree for
 ion_buffer
To:     Lecopzer Chen <lecopzer.chen@mediatek.com>,
        linux-kernel@vger.kernel.org
Cc:     sumit.semwal@linaro.org, gregkh@linuxfoundation.org,
        riandrews@android.com, arve@android.com,
        devel@driverdev.osuosl.org, dri-devel@lists.freedesktop.org,
        YJ Chiang <yj.chiang@mediatek.com>
References: <20190712084717.12441-1-lecopzer.chen@mediatek.com>
From:   Laura Abbott <labbott@redhat.com>
Message-ID: <52dc3872-3d08-1d0d-6d8f-10223bc186bc@redhat.com>
Date:   Wed, 17 Jul 2019 11:07:51 -0400
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.7.2
MIME-Version: 1.0
In-Reply-To: <20190712084717.12441-1-lecopzer.chen@mediatek.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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 <lecopzer.chen@mediatek.com>
> Cc: YJ Chiang <yj.chiang@mediatek.com>
> Cc: Lecopzer Chen <lecopzer.chen@mediatek.com>
> ---
>   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 <labbott@redhat.com>