On Tue, Apr 18, 2017 at 11:27:12AM -0700, Laura Abbott wrote:
> ion_handle was introduced as an abstraction to represent a reference to
> a buffer via an ion_client. As frameworks outside of Ion evolved, the dmabuf
> emerged as the preferred standard for use in the kernel. This has made
> the ion_handle an unnecessary abstraction and prone to race
> conditions. ion_client is also now only used internally. We have enough
> mechanisms for race conditions and leaks already so just drop ion_handle
> and ion_client. This also includes ripping out most of the debugfs
> infrastructure since much of that was tied to clients and handles.
> The debugfs infrastructure was prone to give confusing data (orphaned
> allocations) so it can be replaced with something better if people
> actually want it.
> 
> Signed-off-by: Laura Abbott <labb...@redhat.com>

Yeah I think improving the dma-buf debugfs stuff (maybe with an allocator
callback to dump additional data) is the better option.

Acked-by: Daniel Vetter <daniel.vet...@ffwll.ch>
> ---
>  drivers/staging/android/ion/ion-ioctl.c |  53 +--
>  drivers/staging/android/ion/ion.c       | 701 
> ++------------------------------
>  drivers/staging/android/ion/ion.h       |  77 +---
>  drivers/staging/android/uapi/ion.h      |  25 +-
>  4 files changed, 51 insertions(+), 805 deletions(-)
> 
> diff --git a/drivers/staging/android/ion/ion-ioctl.c 
> b/drivers/staging/android/ion/ion-ioctl.c
> index 4e7bf16..76427e4 100644
> --- a/drivers/staging/android/ion/ion-ioctl.c
> +++ b/drivers/staging/android/ion/ion-ioctl.c
> @@ -21,9 +21,7 @@
>  #include "ion.h"
>  
>  union ion_ioctl_arg {
> -     struct ion_fd_data fd;
>       struct ion_allocation_data allocation;
> -     struct ion_handle_data handle;
>       struct ion_heap_query query;
>  };
>  
> @@ -48,8 +46,6 @@ static int validate_ioctl_arg(unsigned int cmd, union 
> ion_ioctl_arg *arg)
>  static unsigned int ion_ioctl_dir(unsigned int cmd)
>  {
>       switch (cmd) {
> -     case ION_IOC_FREE:
> -             return _IOC_WRITE;
>       default:
>               return _IOC_DIR(cmd);
>       }
> @@ -57,8 +53,6 @@ static unsigned int ion_ioctl_dir(unsigned int cmd)
>  
>  long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>  {
> -     struct ion_client *client = filp->private_data;
> -     struct ion_handle *cleanup_handle = NULL;
>       int ret = 0;
>       unsigned int dir;
>       union ion_ioctl_arg data;
> @@ -86,61 +80,28 @@ long ion_ioctl(struct file *filp, unsigned int cmd, 
> unsigned long arg)
>       switch (cmd) {
>       case ION_IOC_ALLOC:
>       {
> -             struct ion_handle *handle;
> +             int fd;
>  
> -             handle = ion_alloc(client, data.allocation.len,
> +             fd = ion_alloc(data.allocation.len,
>                                               data.allocation.heap_id_mask,
>                                               data.allocation.flags);
> -             if (IS_ERR(handle))
> -                     return PTR_ERR(handle);
> +             if (fd < 0)
> +                     return fd;
>  
> -             data.allocation.handle = handle->id;
> +             data.allocation.fd = fd;
>  
> -             cleanup_handle = handle;
> -             break;
> -     }
> -     case ION_IOC_FREE:
> -     {
> -             struct ion_handle *handle;
> -
> -             mutex_lock(&client->lock);
> -             handle = ion_handle_get_by_id_nolock(client,
> -                                                  data.handle.handle);
> -             if (IS_ERR(handle)) {
> -                     mutex_unlock(&client->lock);
> -                     return PTR_ERR(handle);
> -             }
> -             ion_free_nolock(client, handle);
> -             ion_handle_put_nolock(handle);
> -             mutex_unlock(&client->lock);
> -             break;
> -     }
> -     case ION_IOC_SHARE:
> -     {
> -             struct ion_handle *handle;
> -
> -             handle = ion_handle_get_by_id(client, data.handle.handle);
> -             if (IS_ERR(handle))
> -                     return PTR_ERR(handle);
> -             data.fd.fd = ion_share_dma_buf_fd(client, handle);
> -             ion_handle_put(handle);
> -             if (data.fd.fd < 0)
> -                     ret = data.fd.fd;
>               break;
>       }
>       case ION_IOC_HEAP_QUERY:
> -             ret = ion_query_heaps(client, &data.query);
> +             ret = ion_query_heaps(&data.query);
>               break;
>       default:
>               return -ENOTTY;
>       }
>  
>       if (dir & _IOC_READ) {
> -             if (copy_to_user((void __user *)arg, &data, _IOC_SIZE(cmd))) {
> -                     if (cleanup_handle)
> -                             ion_free(client, cleanup_handle);
> +             if (copy_to_user((void __user *)arg, &data, _IOC_SIZE(cmd)))
>                       return -EFAULT;
> -             }
>       }
>       return ret;
>  }
> diff --git a/drivers/staging/android/ion/ion.c 
> b/drivers/staging/android/ion/ion.c
> index 5a82bea..9eeb06f 100644
> --- a/drivers/staging/android/ion/ion.c
> +++ b/drivers/staging/android/ion/ion.c
> @@ -90,7 +90,6 @@ static struct ion_buffer *ion_buffer_create(struct ion_heap 
> *heap,
>  
>       buffer->heap = heap;
>       buffer->flags = flags;
> -     kref_init(&buffer->ref);
>  
>       ret = heap->ops->allocate(heap, buffer, len, flags);
>  
> @@ -140,9 +139,8 @@ void ion_buffer_destroy(struct ion_buffer *buffer)
>       kfree(buffer);
>  }
>  
> -static void _ion_buffer_destroy(struct kref *kref)
> +static void _ion_buffer_destroy(struct ion_buffer *buffer)
>  {
> -     struct ion_buffer *buffer = container_of(kref, struct ion_buffer, ref);
>       struct ion_heap *heap = buffer->heap;
>       struct ion_device *dev = buffer->dev;
>  
> @@ -156,255 +154,6 @@ static void _ion_buffer_destroy(struct kref *kref)
>               ion_buffer_destroy(buffer);
>  }
>  
> -static void ion_buffer_get(struct ion_buffer *buffer)
> -{
> -     kref_get(&buffer->ref);
> -}
> -
> -static int ion_buffer_put(struct ion_buffer *buffer)
> -{
> -     return kref_put(&buffer->ref, _ion_buffer_destroy);
> -}
> -
> -static void ion_buffer_add_to_handle(struct ion_buffer *buffer)
> -{
> -     mutex_lock(&buffer->lock);
> -     buffer->handle_count++;
> -     mutex_unlock(&buffer->lock);
> -}
> -
> -static void ion_buffer_remove_from_handle(struct ion_buffer *buffer)
> -{
> -     /*
> -      * when a buffer is removed from a handle, if it is not in
> -      * any other handles, copy the taskcomm and the pid of the
> -      * process it's being removed from into the buffer.  At this
> -      * point there will be no way to track what processes this buffer is
> -      * being used by, it only exists as a dma_buf file descriptor.
> -      * The taskcomm and pid can provide a debug hint as to where this fd
> -      * is in the system
> -      */
> -     mutex_lock(&buffer->lock);
> -     buffer->handle_count--;
> -     BUG_ON(buffer->handle_count < 0);
> -     if (!buffer->handle_count) {
> -             struct task_struct *task;
> -
> -             task = current->group_leader;
> -             get_task_comm(buffer->task_comm, task);
> -             buffer->pid = task_pid_nr(task);
> -     }
> -     mutex_unlock(&buffer->lock);
> -}
> -
> -static struct ion_handle *ion_handle_create(struct ion_client *client,
> -                                         struct ion_buffer *buffer)
> -{
> -     struct ion_handle *handle;
> -
> -     handle = kzalloc(sizeof(*handle), GFP_KERNEL);
> -     if (!handle)
> -             return ERR_PTR(-ENOMEM);
> -     kref_init(&handle->ref);
> -     RB_CLEAR_NODE(&handle->node);
> -     handle->client = client;
> -     ion_buffer_get(buffer);
> -     ion_buffer_add_to_handle(buffer);
> -     handle->buffer = buffer;
> -
> -     return handle;
> -}
> -
> -static void ion_handle_kmap_put(struct ion_handle *);
> -
> -static void ion_handle_destroy(struct kref *kref)
> -{
> -     struct ion_handle *handle = container_of(kref, struct ion_handle, ref);
> -     struct ion_client *client = handle->client;
> -     struct ion_buffer *buffer = handle->buffer;
> -
> -     mutex_lock(&buffer->lock);
> -     while (handle->kmap_cnt)
> -             ion_handle_kmap_put(handle);
> -     mutex_unlock(&buffer->lock);
> -
> -     idr_remove(&client->idr, handle->id);
> -     if (!RB_EMPTY_NODE(&handle->node))
> -             rb_erase(&handle->node, &client->handles);
> -
> -     ion_buffer_remove_from_handle(buffer);
> -     ion_buffer_put(buffer);
> -
> -     kfree(handle);
> -}
> -
> -static void ion_handle_get(struct ion_handle *handle)
> -{
> -     kref_get(&handle->ref);
> -}
> -
> -int ion_handle_put_nolock(struct ion_handle *handle)
> -{
> -     return kref_put(&handle->ref, ion_handle_destroy);
> -}
> -
> -int ion_handle_put(struct ion_handle *handle)
> -{
> -     struct ion_client *client = handle->client;
> -     int ret;
> -
> -     mutex_lock(&client->lock);
> -     ret = ion_handle_put_nolock(handle);
> -     mutex_unlock(&client->lock);
> -
> -     return ret;
> -}
> -
> -struct ion_handle *ion_handle_get_by_id_nolock(struct ion_client *client,
> -                                            int id)
> -{
> -     struct ion_handle *handle;
> -
> -     handle = idr_find(&client->idr, id);
> -     if (handle)
> -             ion_handle_get(handle);
> -
> -     return handle ? handle : ERR_PTR(-EINVAL);
> -}
> -
> -struct ion_handle *ion_handle_get_by_id(struct ion_client *client,
> -                                            int id)
> -{
> -     struct ion_handle *handle;
> -
> -     mutex_lock(&client->lock);
> -     handle = ion_handle_get_by_id_nolock(client, id);
> -     mutex_unlock(&client->lock);
> -
> -     return handle;
> -}
> -
> -static bool ion_handle_validate(struct ion_client *client,
> -                             struct ion_handle *handle)
> -{
> -     WARN_ON(!mutex_is_locked(&client->lock));
> -     return idr_find(&client->idr, handle->id) == handle;
> -}
> -
> -static int ion_handle_add(struct ion_client *client, struct ion_handle 
> *handle)
> -{
> -     int id;
> -     struct rb_node **p = &client->handles.rb_node;
> -     struct rb_node *parent = NULL;
> -     struct ion_handle *entry;
> -
> -     id = idr_alloc(&client->idr, handle, 1, 0, GFP_KERNEL);
> -     if (id < 0)
> -             return id;
> -
> -     handle->id = id;
> -
> -     while (*p) {
> -             parent = *p;
> -             entry = rb_entry(parent, struct ion_handle, node);
> -
> -             if (handle->buffer < entry->buffer)
> -                     p = &(*p)->rb_left;
> -             else if (handle->buffer > entry->buffer)
> -                     p = &(*p)->rb_right;
> -             else
> -                     WARN(1, "%s: buffer already found.", __func__);
> -     }
> -
> -     rb_link_node(&handle->node, parent, p);
> -     rb_insert_color(&handle->node, &client->handles);
> -
> -     return 0;
> -}
> -
> -struct ion_handle *ion_alloc(struct ion_client *client, size_t len,
> -                          unsigned int heap_id_mask,
> -                          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 heap_id_mask %u flags %x\n", __func__,
> -              len, 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
> -      */
> -     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, flags);
> -             if (!IS_ERR(buffer))
> -                     break;
> -     }
> -     up_read(&dev->lock);
> -
> -     if (buffer == NULL)
> -             return ERR_PTR(-ENODEV);
> -
> -     if (IS_ERR(buffer))
> -             return ERR_CAST(buffer);
> -
> -     handle = ion_handle_create(client, buffer);
> -
> -     /*
> -      * ion_buffer_create will create a buffer with a ref_cnt of 1,
> -      * and ion_handle_create will take a second reference, drop one here
> -      */
> -     ion_buffer_put(buffer);
> -
> -     if (IS_ERR(handle))
> -             return handle;
> -
> -     mutex_lock(&client->lock);
> -     ret = ion_handle_add(client, handle);
> -     mutex_unlock(&client->lock);
> -     if (ret) {
> -             ion_handle_put(handle);
> -             handle = ERR_PTR(ret);
> -     }
> -
> -     return handle;
> -}
> -EXPORT_SYMBOL(ion_alloc);
> -
> -void ion_free_nolock(struct ion_client *client,
> -                  struct ion_handle *handle)
> -{
> -     if (!ion_handle_validate(client, handle)) {
> -             WARN(1, "%s: invalid handle passed to free.\n", __func__);
> -             return;
> -     }
> -     ion_handle_put_nolock(handle);
> -}
> -
> -void ion_free(struct ion_client *client, struct ion_handle *handle)
> -{
> -     BUG_ON(client != handle->client);
> -
> -     mutex_lock(&client->lock);
> -     ion_free_nolock(client, handle);
> -     mutex_unlock(&client->lock);
> -}
> -EXPORT_SYMBOL(ion_free);
> -
>  static void *ion_buffer_kmap_get(struct ion_buffer *buffer)
>  {
>       void *vaddr;
> @@ -433,234 +182,6 @@ static void ion_buffer_kmap_put(struct ion_buffer 
> *buffer)
>       }
>  }
>  
> -static void ion_handle_kmap_put(struct ion_handle *handle)
> -{
> -     struct ion_buffer *buffer = handle->buffer;
> -
> -     if (!handle->kmap_cnt) {
> -             WARN(1, "%s: Double unmap detected! bailing...\n", __func__);
> -             return;
> -     }
> -     handle->kmap_cnt--;
> -     if (!handle->kmap_cnt)
> -             ion_buffer_kmap_put(buffer);
> -}
> -
> -static struct mutex debugfs_mutex;
> -static struct rb_root *ion_root_client;
> -static int is_client_alive(struct ion_client *client)
> -{
> -     struct rb_node *node;
> -     struct ion_client *tmp;
> -     struct ion_device *dev;
> -
> -     node = ion_root_client->rb_node;
> -     dev = container_of(ion_root_client, struct ion_device, clients);
> -
> -     down_read(&dev->lock);
> -     while (node) {
> -             tmp = rb_entry(node, struct ion_client, node);
> -             if (client < tmp) {
> -                     node = node->rb_left;
> -             } else if (client > tmp) {
> -                     node = node->rb_right;
> -             } else {
> -                     up_read(&dev->lock);
> -                     return 1;
> -             }
> -     }
> -
> -     up_read(&dev->lock);
> -     return 0;
> -}
> -
> -static int ion_debug_client_show(struct seq_file *s, void *unused)
> -{
> -     struct ion_client *client = s->private;
> -     struct rb_node *n;
> -     size_t sizes[ION_NUM_HEAP_IDS] = {0};
> -     const char *names[ION_NUM_HEAP_IDS] = {NULL};
> -     int i;
> -
> -     mutex_lock(&debugfs_mutex);
> -     if (!is_client_alive(client)) {
> -             seq_printf(s, "ion_client 0x%p dead, can't dump its buffers\n",
> -                        client);
> -             mutex_unlock(&debugfs_mutex);
> -             return 0;
> -     }
> -
> -     mutex_lock(&client->lock);
> -     for (n = rb_first(&client->handles); n; n = rb_next(n)) {
> -             struct ion_handle *handle = rb_entry(n, struct ion_handle,
> -                                                  node);
> -             unsigned int id = handle->buffer->heap->id;
> -
> -             if (!names[id])
> -                     names[id] = handle->buffer->heap->name;
> -             sizes[id] += handle->buffer->size;
> -     }
> -     mutex_unlock(&client->lock);
> -     mutex_unlock(&debugfs_mutex);
> -
> -     seq_printf(s, "%16.16s: %16.16s\n", "heap_name", "size_in_bytes");
> -     for (i = 0; i < ION_NUM_HEAP_IDS; i++) {
> -             if (!names[i])
> -                     continue;
> -             seq_printf(s, "%16.16s: %16zu\n", names[i], sizes[i]);
> -     }
> -     return 0;
> -}
> -
> -static int ion_debug_client_open(struct inode *inode, struct file *file)
> -{
> -     return single_open(file, ion_debug_client_show, inode->i_private);
> -}
> -
> -static const struct file_operations debug_client_fops = {
> -     .open = ion_debug_client_open,
> -     .read = seq_read,
> -     .llseek = seq_lseek,
> -     .release = single_release,
> -};
> -
> -static int ion_get_client_serial(const struct rb_root *root,
> -                              const unsigned char *name)
> -{
> -     int serial = -1;
> -     struct rb_node *node;
> -
> -     for (node = rb_first(root); node; node = rb_next(node)) {
> -             struct ion_client *client = rb_entry(node, struct ion_client,
> -                                                  node);
> -
> -             if (strcmp(client->name, name))
> -                     continue;
> -             serial = max(serial, client->display_serial);
> -     }
> -     return serial + 1;
> -}
> -
> -struct ion_client *ion_client_create(struct ion_device *dev,
> -                                  const char *name)
> -{
> -     struct ion_client *client;
> -     struct task_struct *task;
> -     struct rb_node **p;
> -     struct rb_node *parent = NULL;
> -     struct ion_client *entry;
> -     pid_t pid;
> -
> -     if (!name) {
> -             pr_err("%s: Name cannot be null\n", __func__);
> -             return ERR_PTR(-EINVAL);
> -     }
> -
> -     get_task_struct(current->group_leader);
> -     task_lock(current->group_leader);
> -     pid = task_pid_nr(current->group_leader);
> -     /*
> -      * don't bother to store task struct for kernel threads,
> -      * they can't be killed anyway
> -      */
> -     if (current->group_leader->flags & PF_KTHREAD) {
> -             put_task_struct(current->group_leader);
> -             task = NULL;
> -     } else {
> -             task = current->group_leader;
> -     }
> -     task_unlock(current->group_leader);
> -
> -     client = kzalloc(sizeof(*client), GFP_KERNEL);
> -     if (!client)
> -             goto err_put_task_struct;
> -
> -     client->dev = dev;
> -     client->handles = RB_ROOT;
> -     idr_init(&client->idr);
> -     mutex_init(&client->lock);
> -     client->task = task;
> -     client->pid = pid;
> -     client->name = kstrdup(name, GFP_KERNEL);
> -     if (!client->name)
> -             goto err_free_client;
> -
> -     down_write(&dev->lock);
> -     client->display_serial = ion_get_client_serial(&dev->clients, name);
> -     client->display_name = kasprintf(
> -             GFP_KERNEL, "%s-%d", name, client->display_serial);
> -     if (!client->display_name) {
> -             up_write(&dev->lock);
> -             goto err_free_client_name;
> -     }
> -     p = &dev->clients.rb_node;
> -     while (*p) {
> -             parent = *p;
> -             entry = rb_entry(parent, struct ion_client, node);
> -
> -             if (client < entry)
> -                     p = &(*p)->rb_left;
> -             else if (client > entry)
> -                     p = &(*p)->rb_right;
> -     }
> -     rb_link_node(&client->node, parent, p);
> -     rb_insert_color(&client->node, &dev->clients);
> -
> -     client->debug_root = debugfs_create_file(client->display_name, 0664,
> -                                              dev->clients_debug_root,
> -                                              client, &debug_client_fops);
> -     if (!client->debug_root) {
> -             char buf[256], *path;
> -
> -             path = dentry_path(dev->clients_debug_root, buf, 256);
> -             pr_err("Failed to create client debugfs at %s/%s\n",
> -                    path, client->display_name);
> -     }
> -
> -     up_write(&dev->lock);
> -
> -     return client;
> -
> -err_free_client_name:
> -     kfree(client->name);
> -err_free_client:
> -     kfree(client);
> -err_put_task_struct:
> -     if (task)
> -             put_task_struct(current->group_leader);
> -     return ERR_PTR(-ENOMEM);
> -}
> -EXPORT_SYMBOL(ion_client_create);
> -
> -void ion_client_destroy(struct ion_client *client)
> -{
> -     struct ion_device *dev = client->dev;
> -     struct rb_node *n;
> -
> -     pr_debug("%s: %d\n", __func__, __LINE__);
> -     mutex_lock(&debugfs_mutex);
> -     while ((n = rb_first(&client->handles))) {
> -             struct ion_handle *handle = rb_entry(n, struct ion_handle,
> -                                                  node);
> -             ion_handle_destroy(&handle->ref);
> -     }
> -
> -     idr_destroy(&client->idr);
> -
> -     down_write(&dev->lock);
> -     if (client->task)
> -             put_task_struct(client->task);
> -     rb_erase(&client->node, &dev->clients);
> -     debugfs_remove_recursive(client->debug_root);
> -     up_write(&dev->lock);
> -
> -     kfree(client->display_name);
> -     kfree(client->name);
> -     kfree(client);
> -     mutex_unlock(&debugfs_mutex);
> -}
> -EXPORT_SYMBOL(ion_client_destroy);
> -
>  static struct sg_table *dup_sg_table(struct sg_table *table)
>  {
>       struct sg_table *new_table;
> @@ -802,7 +323,7 @@ static void ion_dma_buf_release(struct dma_buf *dmabuf)
>  {
>       struct ion_buffer *buffer = dmabuf->priv;
>  
> -     ion_buffer_put(buffer);
> +     _ion_buffer_destroy(buffer);
>  }
>  
>  static void *ion_dma_buf_kmap(struct dma_buf *dmabuf, unsigned long offset)
> @@ -881,24 +402,44 @@ static const struct dma_buf_ops dma_buf_ops = {
>       .kunmap = ion_dma_buf_kunmap,
>  };
>  
> -struct dma_buf *ion_share_dma_buf(struct ion_client *client,
> -                               struct ion_handle *handle)
> +int ion_alloc(size_t len, unsigned int heap_id_mask, unsigned int flags)
>  {
> +     struct ion_device *dev = internal_dev;
> +     struct ion_buffer *buffer = NULL;
> +     struct ion_heap *heap;
>       DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
> -     struct ion_buffer *buffer;
> +     int fd;
>       struct dma_buf *dmabuf;
> -     bool valid_handle;
>  
> -     mutex_lock(&client->lock);
> -     valid_handle = ion_handle_validate(client, handle);
> -     if (!valid_handle) {
> -             WARN(1, "%s: invalid handle passed to share.\n", __func__);
> -             mutex_unlock(&client->lock);
> -             return ERR_PTR(-EINVAL);
> +     pr_debug("%s: len %zu heap_id_mask %u flags %x\n", __func__,
> +              len, 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
> +      */
> +     len = PAGE_ALIGN(len);
> +
> +     if (!len)
> +             return -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, flags);
> +             if (!IS_ERR(buffer))
> +                     break;
>       }
> -     buffer = handle->buffer;
> -     ion_buffer_get(buffer);
> -     mutex_unlock(&client->lock);
> +     up_read(&dev->lock);
> +
> +     if (buffer == NULL)
> +             return -ENODEV;
> +
> +     if (IS_ERR(buffer))
> +             return PTR_ERR(buffer);
>  
>       exp_info.ops = &dma_buf_ops;
>       exp_info.size = buffer->size;
> @@ -907,22 +448,9 @@ struct dma_buf *ion_share_dma_buf(struct ion_client 
> *client,
>  
>       dmabuf = dma_buf_export(&exp_info);
>       if (IS_ERR(dmabuf)) {
> -             ion_buffer_put(buffer);
> -             return dmabuf;
> -     }
> -
> -     return dmabuf;
> -}
> -EXPORT_SYMBOL(ion_share_dma_buf);
> -
> -int ion_share_dma_buf_fd(struct ion_client *client, struct ion_handle 
> *handle)
> -{
> -     struct dma_buf *dmabuf;
> -     int fd;
> -
> -     dmabuf = ion_share_dma_buf(client, handle);
> -     if (IS_ERR(dmabuf))
> +             _ion_buffer_destroy(buffer);
>               return PTR_ERR(dmabuf);
> +     }
>  
>       fd = dma_buf_fd(dmabuf, O_CLOEXEC);
>       if (fd < 0)
> @@ -930,11 +458,10 @@ int ion_share_dma_buf_fd(struct ion_client *client, 
> struct ion_handle *handle)
>  
>       return fd;
>  }
> -EXPORT_SYMBOL(ion_share_dma_buf_fd);
>  
> -int ion_query_heaps(struct ion_client *client, struct ion_heap_query *query)
> +int ion_query_heaps(struct ion_heap_query *query)
>  {
> -     struct ion_device *dev = client->dev;
> +     struct ion_device *dev = internal_dev;
>       struct ion_heap_data __user *buffer = u64_to_user_ptr(query->heaps);
>       int ret = -EINVAL, cnt = 0, max_cnt;
>       struct ion_heap *heap;
> @@ -976,137 +503,14 @@ int ion_query_heaps(struct ion_client *client, struct 
> ion_heap_query *query)
>       return ret;
>  }
>  
> -static int ion_release(struct inode *inode, struct file *file)
> -{
> -     struct ion_client *client = file->private_data;
> -
> -     pr_debug("%s: %d\n", __func__, __LINE__);
> -     ion_client_destroy(client);
> -     return 0;
> -}
> -
> -static int ion_open(struct inode *inode, struct file *file)
> -{
> -     struct miscdevice *miscdev = file->private_data;
> -     struct ion_device *dev = container_of(miscdev, struct ion_device, dev);
> -     struct ion_client *client;
> -     char debug_name[64];
> -
> -     pr_debug("%s: %d\n", __func__, __LINE__);
> -     snprintf(debug_name, 64, "%u", task_pid_nr(current->group_leader));
> -     client = ion_client_create(dev, debug_name);
> -     if (IS_ERR(client))
> -             return PTR_ERR(client);
> -     file->private_data = client;
> -
> -     return 0;
> -}
> -
>  static const struct file_operations ion_fops = {
>       .owner          = THIS_MODULE,
> -     .open           = ion_open,
> -     .release        = ion_release,
>       .unlocked_ioctl = ion_ioctl,
>  #ifdef CONFIG_COMPAT
>       .compat_ioctl   = ion_ioctl,
>  #endif
>  };
>  
> -static size_t ion_debug_heap_total(struct ion_client *client,
> -                                unsigned int id)
> -{
> -     size_t size = 0;
> -     struct rb_node *n;
> -
> -     mutex_lock(&client->lock);
> -     for (n = rb_first(&client->handles); n; n = rb_next(n)) {
> -             struct ion_handle *handle = rb_entry(n,
> -                                                  struct ion_handle,
> -                                                  node);
> -             if (handle->buffer->heap->id == id)
> -                     size += handle->buffer->size;
> -     }
> -     mutex_unlock(&client->lock);
> -     return size;
> -}
> -
> -static int ion_debug_heap_show(struct seq_file *s, void *unused)
> -{
> -     struct ion_heap *heap = s->private;
> -     struct ion_device *dev = heap->dev;
> -     struct rb_node *n;
> -     size_t total_size = 0;
> -     size_t total_orphaned_size = 0;
> -
> -     seq_printf(s, "%16s %16s %16s\n", "client", "pid", "size");
> -     seq_puts(s, "----------------------------------------------------\n");
> -
> -     mutex_lock(&debugfs_mutex);
> -     for (n = rb_first(&dev->clients); n; n = rb_next(n)) {
> -             struct ion_client *client = rb_entry(n, struct ion_client,
> -                                                  node);
> -             size_t size = ion_debug_heap_total(client, heap->id);
> -
> -             if (!size)
> -                     continue;
> -             if (client->task) {
> -                     char task_comm[TASK_COMM_LEN];
> -
> -                     get_task_comm(task_comm, client->task);
> -                     seq_printf(s, "%16s %16u %16zu\n", task_comm,
> -                                client->pid, size);
> -             } else {
> -                     seq_printf(s, "%16s %16u %16zu\n", client->name,
> -                                client->pid, size);
> -             }
> -     }
> -     mutex_unlock(&debugfs_mutex);
> -
> -     seq_puts(s, "----------------------------------------------------\n");
> -     seq_puts(s, "orphaned allocations (info is from last known client):\n");
> -     mutex_lock(&dev->buffer_lock);
> -     for (n = rb_first(&dev->buffers); n; n = rb_next(n)) {
> -             struct ion_buffer *buffer = rb_entry(n, struct ion_buffer,
> -                                                  node);
> -             if (buffer->heap->id != heap->id)
> -                     continue;
> -             total_size += buffer->size;
> -             if (!buffer->handle_count) {
> -                     seq_printf(s, "%16s %16u %16zu %d %d\n",
> -                                buffer->task_comm, buffer->pid,
> -                                buffer->size, buffer->kmap_cnt,
> -                                kref_read(&buffer->ref));
> -                     total_orphaned_size += buffer->size;
> -             }
> -     }
> -     mutex_unlock(&dev->buffer_lock);
> -     seq_puts(s, "----------------------------------------------------\n");
> -     seq_printf(s, "%16s %16zu\n", "total orphaned",
> -                total_orphaned_size);
> -     seq_printf(s, "%16s %16zu\n", "total ", total_size);
> -     if (heap->flags & ION_HEAP_FLAG_DEFER_FREE)
> -             seq_printf(s, "%16s %16zu\n", "deferred free",
> -                        heap->free_list_size);
> -     seq_puts(s, "----------------------------------------------------\n");
> -
> -     if (heap->debug_show)
> -             heap->debug_show(heap, s, unused);
> -
> -     return 0;
> -}
> -
> -static int ion_debug_heap_open(struct inode *inode, struct file *file)
> -{
> -     return single_open(file, ion_debug_heap_show, inode->i_private);
> -}
> -
> -static const struct file_operations debug_heap_fops = {
> -     .open = ion_debug_heap_open,
> -     .read = seq_read,
> -     .llseek = seq_lseek,
> -     .release = single_release,
> -};
> -
>  static int debug_shrink_set(void *data, u64 val)
>  {
>       struct ion_heap *heap = data;
> @@ -1169,29 +573,18 @@ void ion_device_add_heap(struct ion_heap *heap)
>        */
>       plist_node_init(&heap->node, -heap->id);
>       plist_add(&heap->node, &dev->heaps);
> -     debug_file = debugfs_create_file(heap->name, 0664,
> -                                      dev->heaps_debug_root, heap,
> -                                      &debug_heap_fops);
> -
> -     if (!debug_file) {
> -             char buf[256], *path;
> -
> -             path = dentry_path(dev->heaps_debug_root, buf, 256);
> -             pr_err("Failed to create heap debugfs at %s/%s\n",
> -                    path, heap->name);
> -     }
>  
>       if (heap->shrinker.count_objects && heap->shrinker.scan_objects) {
>               char debug_name[64];
>  
>               snprintf(debug_name, 64, "%s_shrink", heap->name);
>               debug_file = debugfs_create_file(
> -                     debug_name, 0644, dev->heaps_debug_root, heap,
> +                     debug_name, 0644, dev->debug_root, heap,
>                       &debug_shrink_fops);
>               if (!debug_file) {
>                       char buf[256], *path;
>  
> -                     path = dentry_path(dev->heaps_debug_root, buf, 256);
> +                     path = dentry_path(dev->debug_root, buf, 256);
>                       pr_err("Failed to create heap shrinker debugfs at 
> %s/%s\n",
>                              path, debug_name);
>               }
> @@ -1227,24 +620,12 @@ int ion_device_create(void)
>               pr_err("ion: failed to create debugfs root directory.\n");
>               goto debugfs_done;
>       }
> -     idev->heaps_debug_root = debugfs_create_dir("heaps", idev->debug_root);
> -     if (!idev->heaps_debug_root) {
> -             pr_err("ion: failed to create debugfs heaps directory.\n");
> -             goto debugfs_done;
> -     }
> -     idev->clients_debug_root = debugfs_create_dir("clients",
> -                                             idev->debug_root);
> -     if (!idev->clients_debug_root)
> -             pr_err("ion: failed to create debugfs clients directory.\n");
>  
>  debugfs_done:
>       idev->buffers = RB_ROOT;
>       mutex_init(&idev->buffer_lock);
>       init_rwsem(&idev->lock);
>       plist_head_init(&idev->heaps);
> -     idev->clients = RB_ROOT;
> -     ion_root_client = &idev->clients;
> -     mutex_init(&debugfs_mutex);
>       internal_dev = idev;
>       return 0;
>  }
> diff --git a/drivers/staging/android/ion/ion.h 
> b/drivers/staging/android/ion/ion.h
> index 36a1587..ace8416 100644
> --- a/drivers/staging/android/ion/ion.h
> +++ b/drivers/staging/android/ion/ion.h
> @@ -78,7 +78,6 @@ struct ion_platform_heap {
>   *                   handle, used for debugging
>   */
>  struct ion_buffer {
> -     struct kref ref;
>       union {
>               struct rb_node node;
>               struct list_head list;
> @@ -109,8 +108,6 @@ void ion_buffer_destroy(struct ion_buffer *buffer);
>   * @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
> - * @heaps:           list of all the heaps in the system
> - * @user_clients:    list of all the clients created from userspace
>   */
>  struct ion_device {
>       struct miscdevice dev;
> @@ -118,65 +115,11 @@ struct ion_device {
>       struct mutex buffer_lock;
>       struct rw_semaphore lock;
>       struct plist_head heaps;
> -     struct rb_root clients;
>       struct dentry *debug_root;
> -     struct dentry *heaps_debug_root;
> -     struct dentry *clients_debug_root;
>       int heap_cnt;
>  };
>  
>  /**
> - * struct ion_client - a process/hw block local address space
> - * @node:            node in the tree of all clients
> - * @dev:             backpointer to ion device
> - * @handles:         an rb tree of all the handles in this client
> - * @idr:             an idr space for allocating handle ids
> - * @lock:            lock protecting the tree of handles
> - * @name:            used for debugging
> - * @display_name:    used for debugging (unique version of @name)
> - * @display_serial:  used for debugging (to make display_name unique)
> - * @task:            used for debugging
> - *
> - * A client represents a list of buffers this client may access.
> - * The mutex stored here is used to protect both handles tree
> - * as well as the handles themselves, and should be held while modifying 
> either.
> - */
> -struct ion_client {
> -     struct rb_node node;
> -     struct ion_device *dev;
> -     struct rb_root handles;
> -     struct idr idr;
> -     struct mutex lock;
> -     const char *name;
> -     char *display_name;
> -     int display_serial;
> -     struct task_struct *task;
> -     pid_t pid;
> -     struct dentry *debug_root;
> -};
> -
> -/**
> - * ion_handle - a client local reference to a buffer
> - * @ref:             reference count
> - * @client:          back pointer to the client the buffer resides in
> - * @buffer:          pointer to the buffer
> - * @node:            node in the client's handle rbtree
> - * @kmap_cnt:                count of times this client has mapped to kernel
> - * @id:                      client-unique id allocated by client->idr
> - *
> - * Modifications to node, map_cnt or mapping should be protected by the
> - * lock in the client.  Other fields are never changed after initialization.
> - */
> -struct ion_handle {
> -     struct kref ref;
> -     struct ion_client *client;
> -     struct ion_buffer *buffer;
> -     struct rb_node node;
> -     unsigned int kmap_cnt;
> -     int id;
> -};
> -
> -/**
>   * struct ion_heap_ops - ops to operate on a given heap
>   * @allocate:                allocate memory
>   * @free:            free memory
> @@ -296,14 +239,10 @@ int ion_heap_map_user(struct ion_heap *heap, struct 
> ion_buffer *buffer,
>  int ion_heap_buffer_zero(struct ion_buffer *buffer);
>  int ion_heap_pages_zero(struct page *page, size_t size, pgprot_t pgprot);
>  
> -struct ion_handle *ion_alloc(struct ion_client *client, size_t len,
> +int ion_alloc(size_t len,
>                               unsigned int heap_id_mask,
>                               unsigned int flags);
>  
> -void ion_free(struct ion_client *client, struct ion_handle *handle);
> -
> -int ion_share_dma_buf_fd(struct ion_client *client, struct ion_handle 
> *handle);
> -
>  /**
>   * ion_heap_init_shrinker
>   * @heap:            the heap
> @@ -431,18 +370,6 @@ int ion_page_pool_shrink(struct ion_page_pool *pool, 
> gfp_t gfp_mask,
>  
>  long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg);
>  
> -struct ion_handle *ion_handle_get_by_id_nolock(struct ion_client *client,
> -                                             int id);
> -
> -void ion_free_nolock(struct ion_client *client, struct ion_handle *handle);
> -
> -int ion_handle_put_nolock(struct ion_handle *handle);
> -
> -struct ion_handle *ion_handle_get_by_id(struct ion_client *client,
> -                                             int id);
> -
> -int ion_handle_put(struct ion_handle *handle);
> -
> -int ion_query_heaps(struct ion_client *client, struct ion_heap_query *query);
> +int ion_query_heaps(struct ion_heap_query *query);
>  
>  #endif /* _ION_H */
> diff --git a/drivers/staging/android/uapi/ion.h 
> b/drivers/staging/android/uapi/ion.h
> index bba1c47..b76db1b 100644
> --- a/drivers/staging/android/uapi/ion.h
> +++ b/drivers/staging/android/uapi/ion.h
> @@ -85,31 +85,8 @@ struct ion_allocation_data {
>       __u64 len;
>       __u32 heap_id_mask;
>       __u32 flags;
> -     __u32 handle;
> -     __u32 unused;
> -};
> -
> -/**
> - * struct ion_fd_data - metadata passed to/from userspace for a handle/fd 
> pair
> - * @handle:  a handle
> - * @fd:              a file descriptor representing that handle
> - *
> - * For ION_IOC_SHARE or ION_IOC_MAP userspace populates the handle field with
> - * the handle returned from ion alloc, and the kernel returns the file
> - * descriptor to share or map in the fd field.  For ION_IOC_IMPORT, userspace
> - * provides the file descriptor and the kernel returns the handle.
> - */
> -struct ion_fd_data {
> -     __u32 handle;
>       __u32 fd;
> -};
> -
> -/**
> - * struct ion_handle_data - a handle passed to/from the kernel
> - * @handle:  a handle
> - */
> -struct ion_handle_data {
> -     __u32 handle;
> +     __u32 unused;
>  };
>  
>  #define MAX_HEAP_NAME                        32
> -- 
> 2.7.4
> 
> _______________________________________________
> Linaro-mm-sig mailing list
> linaro-mm-...@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/linaro-mm-sig

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Reply via email to