Skip to content
This repository has been archived by the owner on Jul 7, 2021. It is now read-only.

Commit

Permalink
ion: Rewrite for improved clarity and performance
Browse files Browse the repository at this point in the history
The ION driver suffers from massive code bloat caused by excessive
debug features, as well as poor lock usage as a result of that. Multiple
locks in ION exist to make the debug features thread-safe, which hurts
ION's actual performance when doing its job.

There are numerous code paths in ION that hold mutexes for no reason and
hold them for longer than necessary. This results in not only unwanted
lock contention, but also long delays when a mutex lock results in the
calling thread getting preempted for a while. All lock usage in ION
follows this pattern, which causes poor performance across the board.
Furthermore, a single big lock is used mostly everywhere rather than
multiple fine-grained locks.

Most of the mutex locks can be replaced with simple atomic operations.
Where a mutex lock can't be eliminated completely, a spinlock or rwlock
can be used instead for quick operations, thereby avoiding long delays
due to preemption. Fine-grained locks are also now used in place of the
single big lock that was used before.

Additionally, ion_dupe_sg_table is called very frequently, and lies
within the rendering path for the display. Speed it up by reserving
caches for its sg_table and page-sized scatterlist allocations, as well
as by improving the sg copy process. Note that sg_alloc_table zeroes
out `table`, so there's no need to zero it out using the memory
allocator.

Overall, just rewrite ION entirely to fix its deficiencies. This
optimizes ION for excellent performance and discards its rarely-used
debug bloat.

Signed-off-by: Sultan Alsawaf <sultan@kerneltoast.com>
Signed-off-by: Adam W. Willis <return.of.octobot@gmail.com>
  • Loading branch information
kerneltoast authored and 0ctobot committed Jan 19, 2020
1 parent 025d551 commit d3712a4
Show file tree
Hide file tree
Showing 9 changed files with 245 additions and 921 deletions.
12 changes: 0 additions & 12 deletions drivers/staging/android/ion/Kconfig
Expand Up @@ -44,18 +44,6 @@ config ION_CMA_HEAP
by the Contiguous Memory Allocator (CMA). If your system has these
regions, you should say Y here.

config ION_FORCE_DMA_SYNC
bool "Force ION to always DMA sync buffer memory"
depends on ION
help
Force ION to DMA sync buffer memory when it is allocated and to
always DMA sync the buffer memory on calls to begin/end cpu
access. This makes ION DMA sync behavior similar to that of the
older version of ION.
We generally don't want to enable this config as it breaks the
cache maintenance model.
If you're not sure say N here.

config ION_DEFER_FREE_NO_SCHED_IDLE
bool "Increases the priority of ION defer free thead"
depends on ION
Expand Down
40 changes: 9 additions & 31 deletions drivers/staging/android/ion/ion-ioctl.c
Expand Up @@ -44,22 +44,11 @@ static int validate_ioctl_arg(unsigned int cmd, union ion_ioctl_arg *arg)
return ret ? -EINVAL : 0;
}

/* fix up the cases where the ioctl direction bits are incorrect */
static unsigned int ion_ioctl_dir(unsigned int cmd)
{
switch (cmd) {
default:
return _IOC_DIR(cmd);
}
}

long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
{
int ret = 0;
unsigned int dir;
unsigned int dir = _IOC_DIR(cmd);
union ion_ioctl_arg data;

dir = ion_ioctl_dir(cmd);
int ret = 0;

if (_IOC_SIZE(cmd) > sizeof(data))
return -EINVAL;
Expand All @@ -73,59 +62,47 @@ long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
return -EFAULT;

ret = validate_ioctl_arg(cmd, &data);
if (ret) {
pr_warn_once("%s: ioctl validate failed\n", __func__);
if (ret)
return ret;
}

if (!(dir & _IOC_WRITE))
memset(&data, 0, sizeof(data));

switch (cmd) {
case ION_IOC_ALLOC:
{
int fd;

fd = ion_alloc_fd(data.allocation.len,
data.allocation.heap_id_mask,
data.allocation.flags);
int fd = ion_alloc_fd(data.allocation.len,
data.allocation.heap_id_mask,
data.allocation.flags);
if (fd < 0)
return fd;

data.allocation.fd = fd;

break;
}
case ION_IOC_HEAP_QUERY:
ret = ion_query_heaps(&data.query);
break;
case ION_IOC_PREFETCH:
{
int ret;

ret = ion_walk_heaps(data.prefetch_data.heap_id,
(enum ion_heap_type)
ION_HEAP_TYPE_SYSTEM_SECURE,
(void *)&data.prefetch_data,
ion_system_secure_heap_prefetch);
if (ret)
return ret;

break;
}
case ION_IOC_DRAIN:
{
int ret;

ret = ion_walk_heaps(data.prefetch_data.heap_id,
(enum ion_heap_type)
ION_HEAP_TYPE_SYSTEM_SECURE,
(void *)&data.prefetch_data,
ion_system_secure_heap_drain);

if (ret)
return ret;

break;
}
default:
return -ENOTTY;
}
Expand All @@ -134,5 +111,6 @@ long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
if (copy_to_user((void __user *)arg, &data, _IOC_SIZE(cmd)))
return -EFAULT;
}

return ret;
}

0 comments on commit d3712a4

Please sign in to comment.