| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20160608131356.GL1165@e106497-lin.cambridge.arm.com>
Date: Wed, 8 Jun 2016 14:13:56 +0100
From: Liviu Dudau <Liviu.Dudau@....com>
To: Laura Abbott <labbott@...hat.com>
Cc: Sumit Semwal <sumit.semwal@...aro.org>,
John Stultz <john.stultz@...aro.org>,
Arve Hjønnevåg <arve@...roid.com>,
Riley Andrews <riandrews@...roid.com>,
Laura Abbott <labbott@...oraproject.org>,
Daniel Vetter <daniel.vetter@...ll.ch>,
linaro-mm-sig@...ts.linaro.org, devel@...verdev.osuosl.org,
linux-kernel@...r.kernel.org,
Eun Taik Lee <eun.taik.lee@...sung.com>,
Jon Medhurst <tixy@...aro.org>,
Mitchel Humpherys <mitchelh@...eaurora.org>,
Jeremy Gebben <jgebben@...eaurora.org>,
Bryan Huntsman <bryanh@...eaurora.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Android Kernel Team <kernel-team@...roid.com>
Subject: Re: [RFC][PATCH 2/6] staging: android: ion: Switch to using an idr
to manage heaps
On Mon, Jun 06, 2016 at 11:23:29AM -0700, Laura Abbott wrote:
> From: Laura Abbott <labbott@...oraproject.org>
>
>
> In anticipation of dynamic registration of heaps, switch to using
> an idr for heaps. The idr makes it easier to control the assignment
> and management + lookup of heap numbers.
>
> Signed-off-by: Laura Abbott <labbott@...hat.com>
> ---
> drivers/staging/android/ion/ion.c | 83 +++++++++++++++++++++++++--------------
> 1 file changed, 53 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
> index 306340a..739060f 100644
> --- a/drivers/staging/android/ion/ion.c
> +++ b/drivers/staging/android/ion/ion.c
> @@ -55,13 +55,14 @@ struct ion_device {
> struct rb_root buffers;
> struct mutex buffer_lock;
> struct rw_semaphore lock;
> - struct plist_head heaps;
> long (*custom_ioctl)(struct ion_client *client, unsigned int cmd,
> unsigned long arg);
> struct rb_root clients;
> struct dentry *debug_root;
> struct dentry *heaps_debug_root;
> struct dentry *clients_debug_root;
> + struct idr idr;
> + int heap_cnt;
> };
>
> /**
> @@ -488,39 +489,22 @@ static int ion_handle_add(struct ion_client *client, struct ion_handle *handle)
> return 0;
> }
>
> -struct ion_handle *ion_alloc(struct ion_client *client, size_t len,
> - size_t align, unsigned int heap_id_mask,
> +static struct ion_handle *__ion_alloc(struct ion_client *client, size_t len,
> + size_t align, struct ion_heap *heap,
> unsigned int flags)
> {
> struct ion_handle *handle;
> struct ion_device *dev = client->dev;
> struct ion_buffer *buffer = NULL;
> - struct ion_heap *heap;
> int ret;
>
> - pr_debug("%s: len %zu align %zu heap_id_mask %u flags %x\n", __func__,
> - len, align, heap_id_mask, flags);
> - /*
> - * traverse the list of heaps available in this system in priority
> - * order. If the heap type is supported by the client, and matches the
> - * request of the caller allocate from it. Repeat until allocate has
> - * succeeded or all heaps have been tried
> - */
> +
Drop the (second) empty line here.
> len = PAGE_ALIGN(len);
>
> if (!len)
> return ERR_PTR(-EINVAL);
>
> - down_read(&dev->lock);
> - plist_for_each_entry(heap, &dev->heaps, node) {
> - /* if the caller didn't specify this heap id */
> - if (!((1 << heap->id) & heap_id_mask))
> - continue;
> - buffer = ion_buffer_create(heap, dev, len, align, flags);
> - if (!IS_ERR(buffer))
> - break;
> - }
> - up_read(&dev->lock);
> + buffer = ion_buffer_create(heap, dev, len, align, flags);
>
> if (buffer == NULL)
> return ERR_PTR(-ENODEV);
> @@ -549,6 +533,41 @@ struct ion_handle *ion_alloc(struct ion_client *client, size_t len,
>
> return handle;
> }
> +
> +struct ion_handle *ion_alloc(struct ion_client *client, size_t len,
> + size_t align, unsigned int heap_mask,
> + unsigned int flags)
> +{
> + int bit;
> + struct ion_handle *handle = ERR_PTR(-ENODEV);
> +
> + pr_debug("%s: len %zu align %zu heap_id_mask %u flags %x\n", __func__,
> + len, align, heap_mask, flags);
> +
> + down_read(&client->dev->lock);
> + /*
> + * traverse the list of heaps available in this system in priority
> + * order. If the heap type is supported by the client, and matches the
> + * request of the caller allocate from it. Repeat until allocate has
> + * succeeded or all heaps have been tried
> + */
> + for_each_set_bit(bit, (unsigned long *)&heap_mask, 32) {
> + struct ion_heap *heap;
> +
> + heap = idr_find(&client->dev->idr, bit);
> + if (!heap)
> + continue;
> +
> + handle = __ion_alloc(client, len, align, heap, flags);
> + if (IS_ERR(handle))
> + continue;
> + else
> + break;
> + }
> +
> + up_read(&client->dev->lock);
> + return handle;
> +}
> EXPORT_SYMBOL(ion_alloc);
>
> static void ion_free_nolock(struct ion_client *client, struct ion_handle *handle)
> @@ -1587,6 +1606,7 @@ DEFINE_SIMPLE_ATTRIBUTE(debug_shrink_fops, debug_shrink_get,
> int ion_device_add_heap(struct ion_device *dev, struct ion_heap *heap)
> {
> struct dentry *debug_file;
> + int ret;
>
> if (!heap->ops->allocate || !heap->ops->free || !heap->ops->map_dma ||
> !heap->ops->unmap_dma) {
> @@ -1606,12 +1626,15 @@ int ion_device_add_heap(struct ion_device *dev, struct ion_heap *heap)
>
> heap->dev = dev;
> down_write(&dev->lock);
> - /*
> - * use negative heap->id to reverse the priority -- when traversing
> - * the list later attempt higher id numbers first
> - */
> - plist_node_init(&heap->node, -heap->id);
> - plist_add(&heap->node, &dev->heaps);
> +
> + ret = idr_alloc(&dev->idr, heap, heap->id, heap->id + 1, GFP_KERNEL);
> + if (ret < 0 || ret != heap->id) {
> + pr_info("%s: Failed to add heap id, expected %d got %d\n",
> + __func__, heap->id, ret);
> + up_write(&dev->lock);
> + return ret < 0 ? ret : -EINVAL;
> + }
> +
> debug_file = debugfs_create_file(heap->name, 0664,
> dev->heaps_debug_root, heap,
> &debug_heap_fops);
> @@ -1639,7 +1662,7 @@ int ion_device_add_heap(struct ion_device *dev, struct ion_heap *heap)
> path, debug_name);
> }
> }
> -
> + dev->heap_cnt++;
> up_write(&dev->lock);
> return 0;
> }
> @@ -1689,7 +1712,7 @@ debugfs_done:
> idev->buffers = RB_ROOT;
> mutex_init(&idev->buffer_lock);
> init_rwsem(&idev->lock);
> - plist_head_init(&idev->heaps);
> + idr_init(&idev->idr);
> idev->clients = RB_ROOT;
> ion_root_client = &idev->clients;
> mutex_init(&debugfs_mutex);
> --
> 2.5.5
>
Reviewed-by: Liviu Dudau <Liviu.Dudau@....com>
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
Powered by blists - more mailing lists