Skip to content

Commit 34bce59

Browse files
kerneltoastZile995
authored andcommitted
ion: Rewrite to improve clarity and performance
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 big mutex lock is used mostly everywhere, which causes performance degradation due to unnecessary lock overhead. Instead of having a big mutex lock, multiple fine-grained locks are now used, improving performance. Additionally, ion_dup_sg_table is called very frequently, and lies within the rendering path for the display. Speed it up by copying scatterlists in page-sized chunks rather than iterating one at a time. 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 debug cruft. Signed-off-by: Sultan Alsawaf <sultan@kerneltoast.com> Signed-off-by: Zile995 <stefan.zivkovic995@gmail.com>
1 parent 669b767 commit 34bce59

File tree

10 files changed

+556
-2400
lines changed

10 files changed

+556
-2400
lines changed

drivers/staging/android/ion/ion.c

Lines changed: 375 additions & 1858 deletions
Large diffs are not rendered by default.

drivers/staging/android/ion/ion.h

Lines changed: 85 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
#define _LINUX_ION_H
2020

2121
#include <linux/err.h>
22+
#include <linux/memblock.h>
23+
#include <linux/msm_dma_iommu_mapping.h>
2224
#include "../uapi/ion.h"
2325

2426
struct ion_handle;
@@ -28,6 +30,21 @@ struct ion_mapper;
2830
struct ion_client;
2931
struct ion_buffer;
3032

33+
struct ion_buffer {
34+
struct ion_heap *heap;
35+
struct sg_table *sg_table;
36+
struct mutex kmap_lock;
37+
struct work_struct free;
38+
atomic_t refcount;
39+
void *priv_virt;
40+
void *vaddr;
41+
unsigned int flags;
42+
unsigned int private_flags;
43+
size_t size;
44+
int kmap_refcount;
45+
struct msm_iommu_data iommu_data;
46+
};
47+
3148
/* This should be removed some day when phys_addr_t's are fully
3249
plumbed in the kernel, and all instances of ion_phys_addr_t should
3350
be converted to phys_addr_t. For the time being many kernel interfaces
@@ -88,15 +105,31 @@ struct ion_platform_data {
88105
* located at specific memory addresses or of specific sizes not
89106
* managed by the kernel
90107
*/
91-
void ion_reserve(struct ion_platform_data *data);
108+
static inline void ion_reserve(struct ion_platform_data *data)
109+
{
110+
int i;
111+
112+
for (i = 0; i < data->nr; i++) {
113+
struct ion_platform_heap *pheap = &data->heaps[i];
114+
115+
if (pheap->size) {
116+
if (pheap->base)
117+
memblock_reserve(pheap->base, pheap->size);
118+
else
119+
pheap->base = memblock_alloc_base(pheap->size,
120+
pheap->align, MEMBLOCK_ALLOC_ANYWHERE);
121+
}
122+
}
123+
}
92124

93125
/**
94126
* ion_client_create() - allocate a client and returns it
95127
* @dev: the global ion device
96-
* @name: used for debugging
97128
*/
98-
struct ion_client *ion_client_create(struct ion_device *dev,
99-
const char *name);
129+
static inline struct ion_client *ion_client_create(void *idev)
130+
{
131+
return idev;
132+
}
100133

101134
/**
102135
* ion_client_destroy() - free's a client and all it's handles
@@ -105,7 +138,9 @@ struct ion_client *ion_client_create(struct ion_device *dev,
105138
* Free the provided client and all it's resources including
106139
* any handles it is holding.
107140
*/
108-
void ion_client_destroy(struct ion_client *client);
141+
static inline void ion_client_destroy(struct ion_client *client)
142+
{
143+
}
109144

110145
/**
111146
* ion_alloc - allocate ion memory
@@ -123,9 +158,14 @@ void ion_client_destroy(struct ion_client *client);
123158
* Allocate memory in one of the heaps provided in heap mask and return
124159
* an opaque handle to it.
125160
*/
126-
struct ion_handle *ion_alloc(struct ion_client *client, size_t len,
127-
size_t align, unsigned int heap_id_mask,
128-
unsigned int flags);
161+
struct ion_buffer *__ion_alloc(struct ion_device *idev, size_t len,
162+
size_t align, unsigned int heap_id_mask,
163+
unsigned int flags);
164+
static inline void *ion_alloc(void *client, size_t len, size_t align,
165+
unsigned int heap_id_mask, unsigned int flags)
166+
{
167+
return __ion_alloc(client, len, align, heap_id_mask, flags);
168+
}
129169

130170
/**
131171
* ion_free - free a handle
@@ -134,7 +174,11 @@ struct ion_handle *ion_alloc(struct ion_client *client, size_t len,
134174
*
135175
* Free the provided handle.
136176
*/
137-
void ion_free(struct ion_client *client, struct ion_handle *handle);
177+
void ion_buffer_put(struct ion_buffer *buffer);
178+
static inline void ion_free(struct ion_client *client, void *handle)
179+
{
180+
ion_buffer_put(handle);
181+
}
138182

139183
/**
140184
* ion_phys - returns the physical address and len of a handle
@@ -152,19 +196,12 @@ void ion_free(struct ion_client *client, struct ion_handle *handle);
152196
* the returned value may not be valid if the caller is not
153197
* holding a reference.
154198
*/
155-
int ion_phys(struct ion_client *client, struct ion_handle *handle,
156-
ion_phys_addr_t *addr, size_t *len);
157-
158-
/**
159-
* ion_map_dma - return an sg_table describing a handle
160-
* @client: the client
161-
* @handle: the handle
162-
*
163-
* This function returns the sg_table describing
164-
* a particular ion handle.
165-
*/
166-
struct sg_table *ion_sg_table(struct ion_client *client,
167-
struct ion_handle *handle);
199+
int __ion_phys(struct ion_buffer *buffer, ion_phys_addr_t *addr, size_t *len);
200+
static inline int ion_phys(struct ion_client *client, void *handle,
201+
ion_phys_addr_t *addr, size_t *len)
202+
{
203+
return __ion_phys(handle, addr, len);
204+
}
168205

169206
/**
170207
* ion_map_kernel - create mapping for the given handle
@@ -174,29 +211,45 @@ struct sg_table *ion_sg_table(struct ion_client *client,
174211
* Map the given handle into the kernel and return a kernel address that
175212
* can be used to access this address.
176213
*/
177-
void *ion_map_kernel(struct ion_client *client, struct ion_handle *handle);
214+
void *__ion_map_kernel(struct ion_buffer *buffer);
215+
static inline void *ion_map_kernel(struct ion_client *client, void *handle)
216+
{
217+
return __ion_map_kernel(handle);
218+
}
178219

179220
/**
180221
* ion_unmap_kernel() - destroy a kernel mapping for a handle
181222
* @client: the client
182223
* @handle: handle to unmap
183224
*/
184-
void ion_unmap_kernel(struct ion_client *client, struct ion_handle *handle);
225+
void __ion_unmap_kernel(struct ion_buffer *buffer);
226+
static inline void ion_unmap_kernel(struct ion_client *client, void *handle)
227+
{
228+
__ion_unmap_kernel(handle);
229+
}
185230

186231
/**
187232
* ion_share_dma_buf() - share buffer as dma-buf
188233
* @client: the client
189234
* @handle: the handle
190235
*/
191-
struct dma_buf *ion_share_dma_buf(struct ion_client *client,
192-
struct ion_handle *handle);
236+
struct dma_buf *__ion_share_dma_buf(struct ion_buffer *buffer);
237+
static inline struct dma_buf *ion_share_dma_buf(struct ion_client *client,
238+
void *handle)
239+
{
240+
return __ion_share_dma_buf(handle);
241+
}
193242

194243
/**
195244
* ion_share_dma_buf_fd() - given an ion client, create a dma-buf fd
196245
* @client: the client
197246
* @handle: the handle
198247
*/
199-
int ion_share_dma_buf_fd(struct ion_client *client, struct ion_handle *handle);
248+
int __ion_share_dma_buf_fd(struct ion_buffer *buffer);
249+
static inline int ion_share_dma_buf_fd(struct ion_client *client, void *handle)
250+
{
251+
return __ion_share_dma_buf_fd(handle);
252+
}
200253

201254
/**
202255
* ion_import_dma_buf() - given an dma-buf fd from the ion exporter get handle
@@ -207,7 +260,11 @@ int ion_share_dma_buf_fd(struct ion_client *client, struct ion_handle *handle);
207260
* import that fd and return a handle representing it. If a dma-buf from
208261
* another exporter is passed in this function will return ERR_PTR(-EINVAL)
209262
*/
210-
struct ion_handle *ion_import_dma_buf(struct ion_client *client, int fd);
263+
struct ion_buffer *__ion_import_dma_buf(int fd);
264+
static inline void *ion_import_dma_buf(struct ion_client *client, int fd)
265+
{
266+
return __ion_import_dma_buf(fd);
267+
}
211268

212269
#else
213270
static inline void ion_reserve(struct ion_platform_data *data)

drivers/staging/android/ion/ion_chunk_heap.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ struct ion_chunk_heap {
3030
ion_phys_addr_t base;
3131
unsigned long chunk_size;
3232
unsigned long size;
33-
unsigned long allocated;
33+
atomic_long_t allocated;
3434
};
3535

3636
static int ion_chunk_heap_allocate(struct ion_heap *heap,
@@ -52,7 +52,8 @@ static int ion_chunk_heap_allocate(struct ion_heap *heap,
5252
allocated_size = ALIGN(size, chunk_heap->chunk_size);
5353
num_chunks = allocated_size / chunk_heap->chunk_size;
5454

55-
if (allocated_size > chunk_heap->size - chunk_heap->allocated)
55+
if (allocated_size >
56+
chunk_heap->size - atomic_long_read(&chunk_heap->allocated))
5657
return -ENOMEM;
5758

5859
table = kmalloc(sizeof(struct sg_table), GFP_KERNEL);
@@ -76,7 +77,7 @@ static int ion_chunk_heap_allocate(struct ion_heap *heap,
7677
}
7778

7879
buffer->priv_virt = table;
79-
chunk_heap->allocated += allocated_size;
80+
atomic_long_add(allocated_size, &chunk_heap->allocated);
8081
return 0;
8182
err:
8283
sg = table->sgl;
@@ -113,7 +114,7 @@ static void ion_chunk_heap_free(struct ion_buffer *buffer)
113114
gen_pool_free(chunk_heap->pool, page_to_phys(sg_page(sg)),
114115
sg->length);
115116
}
116-
chunk_heap->allocated -= allocated_size;
117+
atomic_long_sub(allocated_size, &chunk_heap->allocated);
117118
sg_free_table(table);
118119
kfree(table);
119120
}
@@ -169,7 +170,6 @@ struct ion_heap *ion_chunk_heap_create(struct ion_platform_heap *heap_data)
169170
}
170171
chunk_heap->base = heap_data->base;
171172
chunk_heap->size = heap_data->size;
172-
chunk_heap->allocated = 0;
173173

174174
gen_pool_add(chunk_heap->pool, chunk_heap->base, heap_data->size, -1);
175175
chunk_heap->heap.ops = &chunk_heap_ops;

0 commit comments

Comments
 (0)