Re: [PATCH v3] staging: android: ion: Clean unused debug_show memeber of the heap object
On 09/04/2018 10:03 PM, Alexey Skidanov wrote:
ION had supported heap debug info under /sys/kernel/debug/ion/.
This support have been removed but some leftovers (dead code) still exist.
This patch removes the existing dead code.
Acked-by: Laura Abbott
Fixes: 15c6098cfec5 ("staging: android: ion: Remove ion_handle and ion_client")
Signed-off-by: Alexey Skidanov
---
drivers/staging/android/ion/ion.h | 5 -
drivers/staging/android/ion/ion_system_heap.c | 24
2 files changed, 29 deletions(-)
diff --git a/drivers/staging/android/ion/ion.h
b/drivers/staging/android/ion/ion.h
index 876197b..0afa9cd 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -158,8 +158,6 @@ struct ion_heap_ops {
* @lock: protects the free list
* @waitqueue:queue to wait on from deferred free thread
* @task: task struct of deferred free thread
- * @debug_show:called when heap debug file is read to add any
- * heap specific debug info to output
*
* Represents a pool of memory from which buffers can be made. In some
* systems the only heap is regular system memory allocated via vmalloc.
@@ -180,9 +178,6 @@ struct ion_heap {
spinlock_t free_lock;
wait_queue_head_t waitqueue;
struct task_struct *task;
-
- int (*debug_show)(struct ion_heap *heap, struct seq_file *s,
- void *unused);
};
/**
diff --git a/drivers/staging/android/ion/ion_system_heap.c
b/drivers/staging/android/ion/ion_system_heap.c
index b5c3195..d0d0490 100644
--- a/drivers/staging/android/ion/ion_system_heap.c
+++ b/drivers/staging/android/ion/ion_system_heap.c
@@ -213,29 +213,6 @@ static struct ion_heap_ops system_heap_ops = {
.shrink = ion_system_heap_shrink,
};
-static int ion_system_heap_debug_show(struct ion_heap *heap, struct seq_file *s,
- void *unused)
-{
- struct ion_system_heap *sys_heap = container_of(heap,
- struct ion_system_heap,
- heap);
- int i;
- struct ion_page_pool *pool;
-
- for (i = 0; i < NUM_ORDERS; i++) {
- pool = sys_heap->pools[i];
-
- seq_printf(s, "%d order %u highmem pages %lu total\n",
- pool->high_count, pool->order,
- (PAGE_SIZE << pool->order) * pool->high_count);
- seq_printf(s, "%d order %u lowmem pages %lu total\n",
- pool->low_count, pool->order,
- (PAGE_SIZE << pool->order) * pool->low_count);
- }
-
- return 0;
-}
-
static void ion_system_heap_destroy_pools(struct ion_page_pool **pools)
{
int i;
@@ -282,7 +259,6 @@ static struct ion_heap *__ion_system_heap_create(void)
if (ion_system_heap_create_pools(heap->pools))
goto free_heap;
- heap->heap.debug_show = ion_system_heap_debug_show;
return &heap->heap;
free_heap:
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] android:ion: the order and count are in the wrong position
On 09/07/2018 08:20 AM, jun qian wrote: The value in the wrong position will cause misunderstanding, when the debug infomations display in the window. I think the existing order is okay, it's just not separated well. It's "$count pages of order $order". I also just acked a patch to remove all this code because it's dead on mainline anyway. For future work, we should look to make the debugfs output clearer to avoid ambiguity. Thanks, Laura Signed-off-by: jun qian --- drivers/staging/android/ion/ion_system_heap.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/android/ion/ion_system_heap.c b/drivers/staging/android/ion/ion_system_heap.c index 701eb9f3b0f1..54b8a7710958 100644 --- a/drivers/staging/android/ion/ion_system_heap.c +++ b/drivers/staging/android/ion/ion_system_heap.c @@ -225,10 +225,10 @@ static int ion_system_heap_debug_show(struct ion_heap *heap, struct seq_file *s, pool = sys_heap->pools[i]; seq_printf(s, "%d order %u highmem pages %lu total\n", - pool->high_count, pool->order, + pool->order, pool->high_count, (PAGE_SIZE << pool->order) * pool->high_count); seq_printf(s, "%d order %u lowmem pages %lu total\n", - pool->low_count, pool->order, + pool->order, pool->low_count, (PAGE_SIZE << pool->order) * pool->low_count); } ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: Add per-heap counters
On 09/10/2018 03:00 AM, Alexey Skidanov wrote:
On 09/10/2018 12:36 PM, Dan Carpenter wrote:
On Sun, Sep 09, 2018 at 01:44:31AM +0300, Alexey Skidanov wrote:
The heap statistics have been removed and currently even basics statistics
are missing.
Remind me why did we remove them? What was the git hash?
15c6098cfec5 ("staging: android: ion: Remove ion_handle and ion_client")
According to Laura, "The goal was to rip out the racy handle/buffer mess
and the associated debugfs there"
Yes, the debugfs was removed with that commit because the underlying
handle/client were being removed and the associated debugfs
tended to race with tear down. There was also a question about what
information should go in Ion vs. dma_buf since Ion is now basically
a wrapper around dma_buf.
I don't disagree that we want some kind of debugfs counters but
I'd like to see some description of what problems we can solve
with this that aren't also solved by the debugfs with dma_buf.
Thanks,
Laura
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3] staging: android: ion: Add per-heap counters
On 09/18/2018 10:50 AM, Greg KH wrote:
On Tue, Sep 11, 2018 at 02:29:19PM +0300, Alexey Skidanov wrote:
Heap statistics have been removed and currently even basics statistics
are missing.
This patch creates per heap debugfs directory /sys/kernel/debug/
and adds the following counters:
- the number of allocated buffers;
- the number of allocated bytes;
- the number of allocated bytes watermark.
Signed-off-by: Alexey Skidanov
---
It would be great to get an ack from some of the ion developers...
{hint}
I am a bit behind on reviewing. It generally looks okay but I wanted
to look a bit more before giving an Ack. Give me to the end of the
week to take a look.
Thanks,
Laura
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3] staging: android: ion: Add per-heap counters
On 09/11/2018 04:29 AM, Alexey Skidanov wrote:
Heap statistics have been removed and currently even basics statistics
are missing.
This patch creates per heap debugfs directory /sys/kernel/debug/
and adds the following counters:
- the number of allocated buffers;
- the number of allocated bytes;
- the number of allocated bytes watermark.
Signed-off-by: Alexey Skidanov
---
v3:
Removed debugfs_create_dir() return value checking
drivers/staging/android/ion/ion.c | 46 ---
drivers/staging/android/ion/ion.h | 6 ++---
2 files changed, 41 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 9907332..ba4c6e6 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -95,6 +95,11 @@ static struct ion_buffer *ion_buffer_create(struct ion_heap
*heap,
goto err1;
}
+ heap->num_of_buffers++;
+ heap->num_of_alloc_bytes += len;
+ if (heap->num_of_alloc_bytes > heap->alloc_bytes_wm)
+ heap->alloc_bytes_wm = heap->num_of_alloc_bytes;
+
INIT_LIST_HEAD(&buffer->attachments);
mutex_init(&buffer->lock);
mutex_lock(&dev->buffer_lock);
@@ -117,6 +122,9 @@ void ion_buffer_destroy(struct ion_buffer *buffer)
buffer->heap->ops->unmap_kernel(buffer->heap, buffer);
}
buffer->heap->ops->free(buffer);
+ buffer->heap->num_of_buffers--;
+ buffer->heap->num_of_alloc_bytes -= buffer->size;
+
kfree(buffer);
}
@@ -528,6 +536,8 @@ void ion_device_add_heap(struct ion_heap *heap)
{
struct ion_device *dev = internal_dev;
int ret;
+ struct dentry *heap_root;
+ char debug_name[64];
if (!heap->ops->allocate || !heap->ops->free)
pr_err("%s: can not add heap with invalid ops struct.\n",
@@ -546,6 +556,34 @@ void ion_device_add_heap(struct ion_heap *heap)
}
heap->dev = dev;
+ heap->num_of_buffers = 0;
+ heap->num_of_alloc_bytes = 0;
+ heap->alloc_bytes_wm = 0;
+
+ /* Create heap root directory */
+ heap_root = debugfs_create_dir(heap->name, dev->debug_root);
+ debugfs_create_u64("num_of_buffers",
+ 0444, heap_root,
+ &heap->num_of_buffers);
+ debugfs_create_u64("num_of_alloc_bytes",
+ 0444,
+ heap_root,
+ &heap->num_of_alloc_bytes);
+ debugfs_create_u64("alloc_bytes_wm",
+ 0444,
+ heap_root,
+ &heap->alloc_bytes_wm);
+
+ if (heap->shrinker.count_objects &&
+ heap->shrinker.scan_objects) {
+ snprintf(debug_name, 64, "%s_shrink", heap->name);
+ debugfs_create_file(debug_name,
+ 0644,
+ heap_root,
+ heap,
+ &debug_shrink_fops);
+ }
+
down_write(&dev->lock);
heap->id = heap_id++;
/*
@@ -555,14 +593,6 @@ void ion_device_add_heap(struct ion_heap *heap)
plist_node_init(&heap->node, -heap->id);
plist_add(&heap->node, &dev->heaps);
- if (heap->shrinker.count_objects && heap->shrinker.scan_objects) {
- char debug_name[64];
-
- snprintf(debug_name, 64, "%s_shrink", heap->name);
- debugfs_create_file(debug_name, 0644, dev->debug_root,
- heap, &debug_shrink_fops);
- }
-
dev->heap_cnt++;
up_write(&dev->lock);
}
diff --git a/drivers/staging/android/ion/ion.h
b/drivers/staging/android/ion/ion.h
index 16cbd38..bea84b6 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -179,9 +179,9 @@ struct ion_heap {
spinlock_t free_lock;
wait_queue_head_t waitqueue;
struct task_struct *task;
-
- int (*debug_show)(struct ion_heap *heap, struct seq_file *s,
- void *unused);
+ u64 num_of_buffers;
+ u64 num_of_alloc_bytes;
+ u64 alloc_bytes_wm;
What lock is protecting these?
};
/**
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4] staging: android: ion: Add per-heap counters
On 09/30/2018 08:24 AM, Alexey Skidanov wrote:
Heap statistics have been removed and currently even basics statistics
are missing.
This patch creates per heap debugfs directory /sys/kernel/debug/
and adds the following counters:
- the number of allocated buffers;
- the number of allocated bytes;
- the number of allocated bytes watermark.
If none of the other Android people have strong opinions
Acked-by: Laura Abbott
Signed-off-by: Alexey Skidanov
---
v3:
Removed debugfs_create_dir() return value checking
v4:
Added spinlock to protect heap statistics
drivers/staging/android/ion/ion.c | 50 ---
drivers/staging/android/ion/ion.h | 10 +++-
2 files changed, 51 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 9907332..6fd8979 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -95,6 +95,13 @@ static struct ion_buffer *ion_buffer_create(struct ion_heap
*heap,
goto err1;
}
+ spin_lock(&heap->stat_lock);
+ heap->num_of_buffers++;
+ heap->num_of_alloc_bytes += len;
+ if (heap->num_of_alloc_bytes > heap->alloc_bytes_wm)
+ heap->alloc_bytes_wm = heap->num_of_alloc_bytes;
+ spin_unlock(&heap->stat_lock);
+
INIT_LIST_HEAD(&buffer->attachments);
mutex_init(&buffer->lock);
mutex_lock(&dev->buffer_lock);
@@ -117,6 +124,11 @@ void ion_buffer_destroy(struct ion_buffer *buffer)
buffer->heap->ops->unmap_kernel(buffer->heap, buffer);
}
buffer->heap->ops->free(buffer);
+ spin_lock(&buffer->heap->stat_lock);
+ buffer->heap->num_of_buffers--;
+ buffer->heap->num_of_alloc_bytes -= buffer->size;
+ spin_unlock(&buffer->heap->stat_lock);
+
kfree(buffer);
}
@@ -528,12 +540,15 @@ void ion_device_add_heap(struct ion_heap *heap)
{
struct ion_device *dev = internal_dev;
int ret;
+ struct dentry *heap_root;
+ char debug_name[64];
if (!heap->ops->allocate || !heap->ops->free)
pr_err("%s: can not add heap with invalid ops struct.\n",
__func__);
spin_lock_init(&heap->free_lock);
+ spin_lock_init(&heap->stat_lock);
heap->free_list_size = 0;
if (heap->flags & ION_HEAP_FLAG_DEFER_FREE)
@@ -546,6 +561,33 @@ void ion_device_add_heap(struct ion_heap *heap)
}
heap->dev = dev;
+ heap->num_of_buffers = 0;
+ heap->num_of_alloc_bytes = 0;
+ heap->alloc_bytes_wm = 0;
+
+ heap_root = debugfs_create_dir(heap->name, dev->debug_root);
+ debugfs_create_u64("num_of_buffers",
+ 0444, heap_root,
+ &heap->num_of_buffers);
+ debugfs_create_u64("num_of_alloc_bytes",
+ 0444,
+ heap_root,
+ &heap->num_of_alloc_bytes);
+ debugfs_create_u64("alloc_bytes_wm",
+ 0444,
+ heap_root,
+ &heap->alloc_bytes_wm);
+
+ if (heap->shrinker.count_objects &&
+ heap->shrinker.scan_objects) {
+ snprintf(debug_name, 64, "%s_shrink", heap->name);
+ debugfs_create_file(debug_name,
+ 0644,
+ heap_root,
+ heap,
+ &debug_shrink_fops);
+ }
+
down_write(&dev->lock);
heap->id = heap_id++;
/*
@@ -555,14 +597,6 @@ void ion_device_add_heap(struct ion_heap *heap)
plist_node_init(&heap->node, -heap->id);
plist_add(&heap->node, &dev->heaps);
- if (heap->shrinker.count_objects && heap->shrinker.scan_objects) {
- char debug_name[64];
-
- snprintf(debug_name, 64, "%s_shrink", heap->name);
- debugfs_create_file(debug_name, 0644, dev->debug_root,
- heap, &debug_shrink_fops);
- }
-
dev->heap_cnt++;
up_write(&dev->lock);
}
diff --git a/drivers/staging/android/ion/ion.h
b/drivers/staging/android/ion/ion.h
index 16cbd38..f487127 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -159,6 +159,9 @@ struct ion_heap_ops {
* @task: task struct of deferred free thread
* @debug_show: called when heap debug file is read to add any
*heap specific debug info to output
+ * @num_of
Re: staging: android: ion: aligned allocation support
On 10/02/2018 07:27 AM, Alexey Skidanov wrote: Hi, Sometimes HW requires memory buffer to be aligned in order to be used properly. Of course, we may overcome the lack of aligned allocation support, but we may easily add it because CMA and gen_pool (used by several heaps) already support it. Does someone have an objection to add it? Thanks, Alexey The alignment option was removed from the allocation API before because the most common heap (system heap) didn't support it and it was causing more confusion. We've already mangled the ABI once so I really don't want to break it again. I'm not opposed to adding alignment support for the CMA via the allocation flags. I'm probably going to remove the carveout and chunk heap because nobody has stepped up to figure out how to tie allocation of those to device tree or another method. Thanks, Laura ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: staging: android: ion: aligned allocation support
On 10/03/2018 01:03 PM, Alexey Skidanov wrote: On 10/03/2018 09:07 PM, Laura Abbott wrote: On 10/02/2018 07:27 AM, Alexey Skidanov wrote: Hi, Sometimes HW requires memory buffer to be aligned in order to be used properly. Of course, we may overcome the lack of aligned allocation support, but we may easily add it because CMA and gen_pool (used by several heaps) already support it. Does someone have an objection to add it? Thanks, Alexey The alignment option was removed from the allocation API before because the most common heap (system heap) didn't support it and it was causing more confusion. We've already mangled the ABI once so I really don't want to break it again. I'm not opposed to adding alignment support for the CMA via the allocation flags. Currently, the flags member is used to define the way the buffer will be mapped - cached or uncached. So,if I understand you correct, we need to add ION_FLAG_ALIGNED flag and to share 32 bit field between flags and flags specific data (alignment value) ? Yes, that's what I was thinking. I'm probably going to remove the carveout and chunk heap because nobody has stepped up to figure out how to tie allocation of those to device tree or another method. Thanks, Laura Thanks, Alexey ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: Direct io failed with ion
On 10/11/2018 03:44 AM, Rock Lee wrote: On Thu, Oct 11, 2018 at 4:12 PM Dan Carpenter wrote: On Thu, Oct 11, 2018 at 11:24:49AM +0800, Rock Lee wrote: Hi I tested direct io with a ion allocated buffer, got a -EFAULT error. It turned out that ion_mmap() set VM_IO & VM_PFNMAP, but check_vma_flags() (do_direct_IO() calls it) doesn't allow that VMA has these flags set. Could you give me any hit that could solve this issue? You must be using an old kernel because ion_mmap() was changed in April. Yes, I am using linux-4.4 which is a little old. Even in linux-4.18, ion_mmap() still callls remap_pfn_range() if the heap is carvout/system/cma/chunk. But remap_pfn_range() set VM_IO & IO_PFNMAP as well. Yes, I don't think there's a way around this without moving away from remap_pfn_range. I thought there was a reason why we needed to use remap_pfn_range but it's escaping me. If you wanted to do the work to move away from remap_pfn_range, that would be appreciated. Thanks, Laura ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: Fixed uninitialized heap name access
On 10/22/2018 07:02 AM, Alexey Skidanov wrote:
The heap name might be uninitialized and access might crash the
kernel.
The heap name should never be null so this seems like this is being
fixed in the wrong place. Can you explain more how you are hitting
this issue?
Thanks,
Laura
Signed-off-by: Alexey Skidanov
---
drivers/staging/android/ion/ion.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 9907332..55bca92d 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -459,8 +459,11 @@ int ion_query_heaps(struct ion_heap_query *query)
max_cnt = query->cnt;
plist_for_each_entry(heap, &dev->heaps, node) {
- strncpy(hdata.name, heap->name, MAX_HEAP_NAME);
- hdata.name[sizeof(hdata.name) - 1] = '\0';
+ if (heap->name) {
+ strncpy(hdata.name, heap->name, MAX_HEAP_NAME);
+ hdata.name[sizeof(hdata.name) - 1] = '\0';
+ }
+
hdata.type = heap->type;
hdata.heap_id = heap->id;
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5] staging: android: ion: Add per-heap counters
On 10/9/18 12:08 PM, Alexey Skidanov wrote:
Heap statistics have been removed and currently even basics statistics
are missing.
This patch creates per heap debugfs directory /sys/kernel/debug/
and adds the following counters:
- the number of allocated buffers;
- the number of allocated bytes;
- the number of allocated bytes watermark.
Acked-by: Laura Abbott
Signed-off-by: Alexey Skidanov
---
v3:
Removed debugfs_create_dir() return value checking
v4:
Added spinlock to protect heap statistics
v5:
Rebased on staging-next
drivers/staging/android/ion/ion.c | 50 ---
drivers/staging/android/ion/ion.h | 9 +++
2 files changed, 51 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 9907332..6fd8979 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -95,6 +95,13 @@ static struct ion_buffer *ion_buffer_create(struct ion_heap
*heap,
goto err1;
}
+ spin_lock(&heap->stat_lock);
+ heap->num_of_buffers++;
+ heap->num_of_alloc_bytes += len;
+ if (heap->num_of_alloc_bytes > heap->alloc_bytes_wm)
+ heap->alloc_bytes_wm = heap->num_of_alloc_bytes;
+ spin_unlock(&heap->stat_lock);
+
INIT_LIST_HEAD(&buffer->attachments);
mutex_init(&buffer->lock);
mutex_lock(&dev->buffer_lock);
@@ -117,6 +124,11 @@ void ion_buffer_destroy(struct ion_buffer *buffer)
buffer->heap->ops->unmap_kernel(buffer->heap, buffer);
}
buffer->heap->ops->free(buffer);
+ spin_lock(&buffer->heap->stat_lock);
+ buffer->heap->num_of_buffers--;
+ buffer->heap->num_of_alloc_bytes -= buffer->size;
+ spin_unlock(&buffer->heap->stat_lock);
+
kfree(buffer);
}
@@ -528,12 +540,15 @@ void ion_device_add_heap(struct ion_heap *heap)
{
struct ion_device *dev = internal_dev;
int ret;
+ struct dentry *heap_root;
+ char debug_name[64];
if (!heap->ops->allocate || !heap->ops->free)
pr_err("%s: can not add heap with invalid ops struct.\n",
__func__);
spin_lock_init(&heap->free_lock);
+ spin_lock_init(&heap->stat_lock);
heap->free_list_size = 0;
if (heap->flags & ION_HEAP_FLAG_DEFER_FREE)
@@ -546,6 +561,33 @@ void ion_device_add_heap(struct ion_heap *heap)
}
heap->dev = dev;
+ heap->num_of_buffers = 0;
+ heap->num_of_alloc_bytes = 0;
+ heap->alloc_bytes_wm = 0;
+
+ heap_root = debugfs_create_dir(heap->name, dev->debug_root);
+ debugfs_create_u64("num_of_buffers",
+ 0444, heap_root,
+ &heap->num_of_buffers);
+ debugfs_create_u64("num_of_alloc_bytes",
+ 0444,
+ heap_root,
+ &heap->num_of_alloc_bytes);
+ debugfs_create_u64("alloc_bytes_wm",
+ 0444,
+ heap_root,
+ &heap->alloc_bytes_wm);
+
+ if (heap->shrinker.count_objects &&
+ heap->shrinker.scan_objects) {
+ snprintf(debug_name, 64, "%s_shrink", heap->name);
+ debugfs_create_file(debug_name,
+ 0644,
+ heap_root,
+ heap,
+ &debug_shrink_fops);
+ }
+
down_write(&dev->lock);
heap->id = heap_id++;
/*
@@ -555,14 +597,6 @@ void ion_device_add_heap(struct ion_heap *heap)
plist_node_init(&heap->node, -heap->id);
plist_add(&heap->node, &dev->heaps);
- if (heap->shrinker.count_objects && heap->shrinker.scan_objects) {
- char debug_name[64];
-
- snprintf(debug_name, 64, "%s_shrink", heap->name);
- debugfs_create_file(debug_name, 0644, dev->debug_root,
- heap, &debug_shrink_fops);
- }
-
dev->heap_cnt++;
up_write(&dev->lock);
}
diff --git a/drivers/staging/android/ion/ion.h
b/drivers/staging/android/ion/ion.h
index c006fc1..47b594c 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -157,6 +157,9 @@ struct ion_heap_ops {
* @lock: protects the free list
* @waitqueue:queue to wait on from deferred free thread
* @task: task struct of deferred free thread
+ * @num_of_buffers the number of
Re: [PATCH] Add carve out heap name initialization
On 10/22/18 2:15 PM, Alexey Skidanov wrote: Heap name is mundatory. I'm wary of this and the other change because it misses the broader problem of dealing with the carveout heaps. I still want to remove the carveout and chunk heap. I get that it's being used for out of tree work but at this point the focus needs to be on moving Ion out of staging and if we can't get an end-to-end solution for carveout/chunk heaps to be allocated in tree this needs to be removed. Thanks, Laura Signed-off-by: Alexey Skidanov --- drivers/staging/android/ion/ion_carveout_heap.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/android/ion/ion_carveout_heap.c b/drivers/staging/android/ion/ion_carveout_heap.c index e129237..e89b464 100644 --- a/drivers/staging/android/ion/ion_carveout_heap.c +++ b/drivers/staging/android/ion/ion_carveout_heap.c @@ -131,6 +131,7 @@ struct ion_heap *ion_carveout_heap_create(struct ion_platform_heap *heap_data) gen_pool_add(carveout_heap->pool, carveout_heap->base, heap_data->size, -1); carveout_heap->heap.ops = &carveout_heap_ops; + carveout_heap->heap.name = heap_data->name; carveout_heap->heap.type = ION_HEAP_TYPE_CARVEOUT; carveout_heap->heap.flags = ION_HEAP_FLAG_DEFER_FREE; ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Add carve out heap name initialization
On 11/8/18 11:34 AM, Alexey Skidanov wrote: On 11/8/18 9:15 PM, Laura Abbott wrote: On 10/22/18 2:15 PM, Alexey Skidanov wrote: Heap name is mundatory. I'm wary of this and the other change because it misses the broader problem of dealing with the carveout heaps. I still want to remove the carveout and chunk heap. I get that it's being used for out of tree work but at this point the focus needs to be on moving Ion out of staging and if we can't get an end-to-end solution for carveout/chunk heaps to be allocated in tree this needs to be removed. Thanks, Laura There are several options I would suggest: 1. The heaps may be initialized by parsing some kernel parameter, defining the contiguous chunks 2. Some functions may be exported by ION to initialize the heaps 3. CONFIG_XXX options (just like it's in CMA) We've had lots of suggestions but nobody has actually stepped up to submit patches to make this work. If you'd like to submit patches that would be great. Thanks, Laura Thanks, Alexey Signed-off-by: Alexey Skidanov --- drivers/staging/android/ion/ion_carveout_heap.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/android/ion/ion_carveout_heap.c b/drivers/staging/android/ion/ion_carveout_heap.c index e129237..e89b464 100644 --- a/drivers/staging/android/ion/ion_carveout_heap.c +++ b/drivers/staging/android/ion/ion_carveout_heap.c @@ -131,6 +131,7 @@ struct ion_heap *ion_carveout_heap_create(struct ion_platform_heap *heap_data) gen_pool_add(carveout_heap->pool, carveout_heap->base, heap_data->size, -1); carveout_heap->heap.ops = &carveout_heap_ops; + carveout_heap->heap.name = heap_data->name; carveout_heap->heap.type = ION_HEAP_TYPE_CARVEOUT; carveout_heap->heap.flags = ION_HEAP_FLAG_DEFER_FREE; ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: Add chunk heap initialization
On 11/11/18 11:29 AM, Alexey Skidanov wrote:
Create chunk heap of specified size and base address by adding
"ion_chunk_heap=size@start" kernel boot parameter.
Signed-off-by: Alexey Skidanov
---
drivers/staging/android/ion/ion_chunk_heap.c | 40
1 file changed, 40 insertions(+)
diff --git a/drivers/staging/android/ion/ion_chunk_heap.c
b/drivers/staging/android/ion/ion_chunk_heap.c
index 159d72f..67573aa4 100644
--- a/drivers/staging/android/ion/ion_chunk_heap.c
+++ b/drivers/staging/android/ion/ion_chunk_heap.c
@@ -135,6 +135,7 @@ struct ion_heap *ion_chunk_heap_create(struct
ion_platform_heap *heap_data)
}
chunk_heap->base = heap_data->base;
chunk_heap->size = heap_data->size;
+ chunk_heap->heap.name = heap_data->name;
chunk_heap->allocated = 0;
gen_pool_add(chunk_heap->pool, chunk_heap->base, heap_data->size, -1);
@@ -151,3 +152,42 @@ struct ion_heap *ion_chunk_heap_create(struct
ion_platform_heap *heap_data)
return ERR_PTR(ret);
}
+static u64 base;
+static u64 size;
+
+static int __init setup_heap(char *param)
+{
+ char *p, *pp;
+
+ size = memparse(param, &p);
+ if (param == p)
+ return -EINVAL;
+
+ if (*p == '@')
+ base = memparse(p + 1, &pp);
+ else
+ return -EINVAL;
+
+ if (p == pp)
+ return -EINVAL;
+
+ return 0;
+}
+
+__setup("ion_chunk_heap=", setup_heap);
+
+static int ion_add_chunk_heap(void)
+{
+ struct ion_heap *heap;
+ struct ion_platform_heap plat_heap = {.base = base,
+ .size = size,
+ .name = "chunk_heap",
+ .priv = (void *)PAGE_SIZE};
+ heap = ion_chunk_heap_create(&plat_heap);
+ if (heap)
+ ion_device_add_heap(heap);
+
+ return 0;
+}
+device_initcall(ion_add_chunk_heap);
+
This solves a problem but not enough of the problem.
We need to be able to support more than one chunk/carveout
heap. This also assumes that the memory has already been
reserved/placed and that you know the base and size to
pass on the command line. Part of the issue with the carveout
heaps is that we need a way to tell the kernel to reserve
the memory early enough and then get that information to
Ion. Hard coding memory locations tends to be buggy from
my past experience with Ion.
If you'd like to see about coming up with a complete solution,
feel free to resubmit but I'm still strongly considering
removing these heaps.
Thanks,
Laura
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: Add chunk heap initialization
On 11/25/18 1:22 PM, Alexey Skidanov wrote:
On 11/25/18 10:51 PM, Laura Abbott wrote:
On 11/11/18 11:29 AM, Alexey Skidanov wrote:
Create chunk heap of specified size and base address by adding
"ion_chunk_heap=size@start" kernel boot parameter.
Signed-off-by: Alexey Skidanov
---
drivers/staging/android/ion/ion_chunk_heap.c | 40
1 file changed, 40 insertions(+)
diff --git a/drivers/staging/android/ion/ion_chunk_heap.c
b/drivers/staging/android/ion/ion_chunk_heap.c
index 159d72f..67573aa4 100644
--- a/drivers/staging/android/ion/ion_chunk_heap.c
+++ b/drivers/staging/android/ion/ion_chunk_heap.c
@@ -135,6 +135,7 @@ struct ion_heap *ion_chunk_heap_create(struct
ion_platform_heap *heap_data)
}
chunk_heap->base = heap_data->base;
chunk_heap->size = heap_data->size;
+ chunk_heap->heap.name = heap_data->name;
chunk_heap->allocated = 0;
gen_pool_add(chunk_heap->pool, chunk_heap->base,
heap_data->size, -1);
@@ -151,3 +152,42 @@ struct ion_heap *ion_chunk_heap_create(struct
ion_platform_heap *heap_data)
return ERR_PTR(ret);
}
+static u64 base;
+static u64 size;
+
+static int __init setup_heap(char *param)
+{
+ char *p, *pp;
+
+ size = memparse(param, &p);
+ if (param == p)
+ return -EINVAL;
+
+ if (*p == '@')
+ base = memparse(p + 1, &pp);
+ else
+ return -EINVAL;
+
+ if (p == pp)
+ return -EINVAL;
+
+ return 0;
+}
+
+__setup("ion_chunk_heap=", setup_heap);
+
+static int ion_add_chunk_heap(void)
+{
+ struct ion_heap *heap;
+ struct ion_platform_heap plat_heap = {.base = base,
+ .size = size,
+ .name = "chunk_heap",
+ .priv = (void *)PAGE_SIZE};
+ heap = ion_chunk_heap_create(&plat_heap);
+ if (heap)
+ ion_device_add_heap(heap);
+
+ return 0;
+}
+device_initcall(ion_add_chunk_heap);
+
This solves a problem but not enough of the problem.
We need to be able to support more than one chunk/carveout
heap.
This is easy to support.
This also assumes that the memory has already been
reserved/placed and that you know the base and size to
pass on the command line. Part of the issue with the carveout
heaps is that we need a way to tell the kernel to reserve
the memory early enough and then get that information to
Ion. Hard coding memory locations tends to be buggy from
my past experience with Ion.
memmap= kernel option marks the memory region(s) as reserved (Zone
Allocator doesn't use this memory region(s)). So the heap(s) may manage
this memory region(s).
memmap= is x86 only. I really don't like using the command line for
specifying the base/size as it seems likely to conflict with platforms
that rely on devicetree for reserving memory regions.
Thanks,
Laura
If you'd like to see about coming up with a complete solution,
feel free to resubmit but I'm still strongly considering
removing these heaps.
I will add the multiple heaps support and resubmit the patch
Thanks,
Laura
Thanks,
Alexey
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: Add chunk heap initialization
On 11/25/18 2:02 PM, Alexey Skidanov wrote:
On 11/25/18 11:40 PM, Laura Abbott wrote:
On 11/25/18 1:22 PM, Alexey Skidanov wrote:
On 11/25/18 10:51 PM, Laura Abbott wrote:
On 11/11/18 11:29 AM, Alexey Skidanov wrote:
Create chunk heap of specified size and base address by adding
"ion_chunk_heap=size@start" kernel boot parameter.
Signed-off-by: Alexey Skidanov
---
drivers/staging/android/ion/ion_chunk_heap.c | 40
1 file changed, 40 insertions(+)
diff --git a/drivers/staging/android/ion/ion_chunk_heap.c
b/drivers/staging/android/ion/ion_chunk_heap.c
index 159d72f..67573aa4 100644
--- a/drivers/staging/android/ion/ion_chunk_heap.c
+++ b/drivers/staging/android/ion/ion_chunk_heap.c
@@ -135,6 +135,7 @@ struct ion_heap *ion_chunk_heap_create(struct
ion_platform_heap *heap_data)
}
chunk_heap->base = heap_data->base;
chunk_heap->size = heap_data->size;
+ chunk_heap->heap.name = heap_data->name;
chunk_heap->allocated = 0;
gen_pool_add(chunk_heap->pool, chunk_heap->base,
heap_data->size, -1);
@@ -151,3 +152,42 @@ struct ion_heap *ion_chunk_heap_create(struct
ion_platform_heap *heap_data)
return ERR_PTR(ret);
}
+static u64 base;
+static u64 size;
+
+static int __init setup_heap(char *param)
+{
+ char *p, *pp;
+
+ size = memparse(param, &p);
+ if (param == p)
+ return -EINVAL;
+
+ if (*p == '@')
+ base = memparse(p + 1, &pp);
+ else
+ return -EINVAL;
+
+ if (p == pp)
+ return -EINVAL;
+
+ return 0;
+}
+
+__setup("ion_chunk_heap=", setup_heap);
+
+static int ion_add_chunk_heap(void)
+{
+ struct ion_heap *heap;
+ struct ion_platform_heap plat_heap = {.base = base,
+ .size = size,
+ .name = "chunk_heap",
+ .priv = (void *)PAGE_SIZE};
+ heap = ion_chunk_heap_create(&plat_heap);
+ if (heap)
+ ion_device_add_heap(heap);
+
+ return 0;
+}
+device_initcall(ion_add_chunk_heap);
+
This solves a problem but not enough of the problem.
We need to be able to support more than one chunk/carveout
heap.
This is easy to support.
This also assumes that the memory has already been
reserved/placed and that you know the base and size to
pass on the command line. Part of the issue with the carveout
heaps is that we need a way to tell the kernel to reserve
the memory early enough and then get that information to
Ion. Hard coding memory locations tends to be buggy from
my past experience with Ion.
memmap= kernel option marks the memory region(s) as reserved (Zone
Allocator doesn't use this memory region(s)). So the heap(s) may manage
this memory region(s).
memmap= is x86 only. I really don't like using the command line for
specifying the base/size as it seems likely to conflict with platforms
that rely on devicetree for reserving memory regions.
Thanks,
Laura
I see ... So probably the better way is the one similar to this
https://elixir.bootlin.com/linux/latest/source/kernel/dma/contiguous.c#L245
?
Correct. For platforms that need devicetree, we need a way to specify
that a region should become an Ion heap. I went through a similar
exercise with CMA heaps before I kind of gave up on figuring out a
binding and just had Ion enumerate all CMA heaps. We do still need
a solution to work with non-DT platforms as well so whatever we
come up with needs to plausibly work for both cases. Your solution
would cover the non-DT case but I'd really like to make sure we
at least have a path forward for the devicetree case as well.
Thanks,
Laura
Thanks,
Alexey
If you'd like to see about coming up with a complete solution,
feel free to resubmit but I'm still strongly considering
removing these heaps.
I will add the multiple heaps support and resubmit the patch
Thanks,
Laura
Thanks,
Alexey
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: Add chunk heap initialization
On 11/26/18 10:43 AM, Alexey Skidanov wrote:
On 11/26/18 6:39 PM, Laura Abbott wrote:
On 11/25/18 2:02 PM, Alexey Skidanov wrote:
On 11/25/18 11:40 PM, Laura Abbott wrote:
On 11/25/18 1:22 PM, Alexey Skidanov wrote:
On 11/25/18 10:51 PM, Laura Abbott wrote:
On 11/11/18 11:29 AM, Alexey Skidanov wrote:
Create chunk heap of specified size and base address by adding
"ion_chunk_heap=size@start" kernel boot parameter.
Signed-off-by: Alexey Skidanov
---
drivers/staging/android/ion/ion_chunk_heap.c | 40
1 file changed, 40 insertions(+)
diff --git a/drivers/staging/android/ion/ion_chunk_heap.c
b/drivers/staging/android/ion/ion_chunk_heap.c
index 159d72f..67573aa4 100644
--- a/drivers/staging/android/ion/ion_chunk_heap.c
+++ b/drivers/staging/android/ion/ion_chunk_heap.c
@@ -135,6 +135,7 @@ struct ion_heap *ion_chunk_heap_create(struct
ion_platform_heap *heap_data)
}
chunk_heap->base = heap_data->base;
chunk_heap->size = heap_data->size;
+ chunk_heap->heap.name = heap_data->name;
chunk_heap->allocated = 0;
gen_pool_add(chunk_heap->pool, chunk_heap->base,
heap_data->size, -1);
@@ -151,3 +152,42 @@ struct ion_heap *ion_chunk_heap_create(struct
ion_platform_heap *heap_data)
return ERR_PTR(ret);
}
+static u64 base;
+static u64 size;
+
+static int __init setup_heap(char *param)
+{
+ char *p, *pp;
+
+ size = memparse(param, &p);
+ if (param == p)
+ return -EINVAL;
+
+ if (*p == '@')
+ base = memparse(p + 1, &pp);
+ else
+ return -EINVAL;
+
+ if (p == pp)
+ return -EINVAL;
+
+ return 0;
+}
+
+__setup("ion_chunk_heap=", setup_heap);
+
+static int ion_add_chunk_heap(void)
+{
+ struct ion_heap *heap;
+ struct ion_platform_heap plat_heap = {.base = base,
+ .size = size,
+ .name = "chunk_heap",
+ .priv = (void *)PAGE_SIZE};
+ heap = ion_chunk_heap_create(&plat_heap);
+ if (heap)
+ ion_device_add_heap(heap);
+
+ return 0;
+}
+device_initcall(ion_add_chunk_heap);
+
This solves a problem but not enough of the problem.
We need to be able to support more than one chunk/carveout
heap.
This is easy to support.
This also assumes that the memory has already been
reserved/placed and that you know the base and size to
pass on the command line. Part of the issue with the carveout
heaps is that we need a way to tell the kernel to reserve
the memory early enough and then get that information to
Ion. Hard coding memory locations tends to be buggy from
my past experience with Ion.
memmap= kernel option marks the memory region(s) as reserved (Zone
Allocator doesn't use this memory region(s)). So the heap(s) may manage
this memory region(s).
memmap= is x86 only. I really don't like using the command line for
specifying the base/size as it seems likely to conflict with platforms
that rely on devicetree for reserving memory regions.
Thanks,
Laura
I see ... So probably the better way is the one similar to this
https://elixir.bootlin.com/linux/latest/source/kernel/dma/contiguous.c#L245
?
Correct. For platforms that need devicetree, we need a way to specify
that a region should become an Ion heap. I went through a similar
exercise with CMA heaps before I kind of gave up on figuring out a
binding and just had Ion enumerate all CMA heaps. We do still need
a solution to work with non-DT platforms as well so whatever we
come up with needs to plausibly work for both cases. Your solution
would cover the non-DT case but I'd really like to make sure we
at least have a path forward for the devicetree case as well.
I would say that we have the following steps to consider:
1. Memory reservation. The suggested solution doesn't care how it's done.
2. Per-heap information passing to the Kernel. It's different for DT and
non-DT cases.
3. Heap objects instantiation. The DT and non-DT cases have different
ways/formats to pass this per-heap information. But once the parsing is
done, the rest of the code is common.
I think it clearly defines the steps covering both cases. What do you think?
Yes, that sounds about right.
Thanks,
Alexey
Thanks,
Laura
Thanks,
Alexey
If you'd like to see about coming up with a complete solution,
feel free to resubmit but I'm still strongly considering
removing these heaps.
I will add the multiple heaps support and resubmit the patch
Thanks,
Laura
Thanks,
Alexey
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: Add chunk heap initialization
On 11/27/18 12:07 PM, Alexey Skidanov wrote:
On 11/27/18 9:20 PM, Laura Abbott wrote:
On 11/26/18 10:43 AM, Alexey Skidanov wrote:
On 11/26/18 6:39 PM, Laura Abbott wrote:
On 11/25/18 2:02 PM, Alexey Skidanov wrote:
On 11/25/18 11:40 PM, Laura Abbott wrote:
On 11/25/18 1:22 PM, Alexey Skidanov wrote:
On 11/25/18 10:51 PM, Laura Abbott wrote:
On 11/11/18 11:29 AM, Alexey Skidanov wrote:
Create chunk heap of specified size and base address by adding
"ion_chunk_heap=size@start" kernel boot parameter.
Signed-off-by: Alexey Skidanov
---
drivers/staging/android/ion/ion_chunk_heap.c | 40
1 file changed, 40 insertions(+)
diff --git a/drivers/staging/android/ion/ion_chunk_heap.c
b/drivers/staging/android/ion/ion_chunk_heap.c
index 159d72f..67573aa4 100644
--- a/drivers/staging/android/ion/ion_chunk_heap.c
+++ b/drivers/staging/android/ion/ion_chunk_heap.c
@@ -135,6 +135,7 @@ struct ion_heap *ion_chunk_heap_create(struct
ion_platform_heap *heap_data)
}
chunk_heap->base = heap_data->base;
chunk_heap->size = heap_data->size;
+ chunk_heap->heap.name = heap_data->name;
chunk_heap->allocated = 0;
gen_pool_add(chunk_heap->pool, chunk_heap->base,
heap_data->size, -1);
@@ -151,3 +152,42 @@ struct ion_heap *ion_chunk_heap_create(struct
ion_platform_heap *heap_data)
return ERR_PTR(ret);
}
+static u64 base;
+static u64 size;
+
+static int __init setup_heap(char *param)
+{
+ char *p, *pp;
+
+ size = memparse(param, &p);
+ if (param == p)
+ return -EINVAL;
+
+ if (*p == '@')
+ base = memparse(p + 1, &pp);
+ else
+ return -EINVAL;
+
+ if (p == pp)
+ return -EINVAL;
+
+ return 0;
+}
+
+__setup("ion_chunk_heap=", setup_heap);
+
+static int ion_add_chunk_heap(void)
+{
+ struct ion_heap *heap;
+ struct ion_platform_heap plat_heap = {.base = base,
+ .size = size,
+ .name = "chunk_heap",
+ .priv = (void *)PAGE_SIZE};
+ heap = ion_chunk_heap_create(&plat_heap);
+ if (heap)
+ ion_device_add_heap(heap);
+
+ return 0;
+}
+device_initcall(ion_add_chunk_heap);
+
This solves a problem but not enough of the problem.
We need to be able to support more than one chunk/carveout
heap.
This is easy to support.
This also assumes that the memory has already been
reserved/placed and that you know the base and size to
pass on the command line. Part of the issue with the carveout
heaps is that we need a way to tell the kernel to reserve
the memory early enough and then get that information to
Ion. Hard coding memory locations tends to be buggy from
my past experience with Ion.
memmap= kernel option marks the memory region(s) as reserved (Zone
Allocator doesn't use this memory region(s)). So the heap(s) may
manage
this memory region(s).
memmap= is x86 only. I really don't like using the command line for
specifying the base/size as it seems likely to conflict with platforms
that rely on devicetree for reserving memory regions.
Thanks,
Laura
I see ... So probably the better way is the one similar to this
https://elixir.bootlin.com/linux/latest/source/kernel/dma/contiguous.c#L245
?
Correct. For platforms that need devicetree, we need a way to specify
that a region should become an Ion heap. I went through a similar
exercise with CMA heaps before I kind of gave up on figuring out a
binding and just had Ion enumerate all CMA heaps. We do still need
a solution to work with non-DT platforms as well so whatever we
come up with needs to plausibly work for both cases. Your solution
would cover the non-DT case but I'd really like to make sure we
at least have a path forward for the devicetree case as well.
I would say that we have the following steps to consider:
1. Memory reservation. The suggested solution doesn't care how it's done.
2. Per-heap information passing to the Kernel. It's different for DT and
non-DT cases.
3. Heap objects instantiation. The DT and non-DT cases have different
ways/formats to pass this per-heap information. But once the parsing is
done, the rest of the code is common.
I think it clearly defines the steps covering both cases. What do you
think?
Yes, that sounds about right.
So, in this patch step #2 - is setup_heap() and step #3 - is
ion_add_chunk_heap(). The only missing part is to support several heap
instances creation, correct?
Mostly yes. I'd like to see struct ion_platform_heap go away since
it really isn't used for anything else but we need another
way to get the reserved memory information into Ion.
Thanks,
Laura
Thanks,
Alexey
Thanks,
Alexey
Thanks,
Laura
Thanks,
Alexey
If you'd like to see about coming up with a complete solution,
feel free to resubmit but I'm still stro
Re: [PATCH] staging: android: ion: Remove unused header files
On 11/30/18 6:43 AM, Yangtao Li wrote: seq_file.h does not need to be included,so remove it. Acked-by: Laura Abbott Signed-off-by: Yangtao Li --- drivers/staging/android/ion/ion.c | 1 - drivers/staging/android/ion/ion_system_heap.c | 1 - 2 files changed, 2 deletions(-) diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c index 99073325b0c0..0d61e9cd0887 100644 --- a/drivers/staging/android/ion/ion.c +++ b/drivers/staging/android/ion/ion.c @@ -23,7 +23,6 @@ #include #include #include -#include #include #include #include diff --git a/drivers/staging/android/ion/ion_system_heap.c b/drivers/staging/android/ion/ion_system_heap.c index 548bb02c0ca6..9ce2c0d7ac17 100644 --- a/drivers/staging/android/ion/ion_system_heap.c +++ b/drivers/staging/android/ion/ion_system_heap.c @@ -11,7 +11,6 @@ #include #include #include -#include #include #include #include "ion.h" ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: add buffer flag update ioctl
On 12/19/18 9:19 AM, Zeng Tao wrote:
In some usecases, the buffer cached attribute is not determined at
allocation time, it's determined just before the real cpu mapping.
And from the memory view of point, a buffer should not have the cached
attribute util is really mapped by the cpu. So in this patch, we
introduced the new ioctl command to target the requirement.
This is racy and error prone. Can you explain more what
problem you are trying to solve?
Signed-off-by: Zeng Tao
---
drivers/staging/android/ion/ion-ioctl.c | 4
drivers/staging/android/ion/ion.c | 17 +
drivers/staging/android/ion/ion.h | 1 +
drivers/staging/android/uapi/ion.h | 22 ++
4 files changed, 44 insertions(+)
diff --git a/drivers/staging/android/ion/ion-ioctl.c
b/drivers/staging/android/ion/ion-ioctl.c
index a8d3cc4..60bb702 100644
--- a/drivers/staging/android/ion/ion-ioctl.c
+++ b/drivers/staging/android/ion/ion-ioctl.c
@@ -12,6 +12,7 @@
union ion_ioctl_arg {
struct ion_allocation_data allocation;
+ struct ion_buffer_flag_data update;
struct ion_heap_query query;
};
@@ -83,6 +84,9 @@ long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
break;
}
+ case ION_IOC_BUFFER_UPDATE:
+ ret = ion_buffer_update(data.update.fd, data.update.flags);
+ break;
case ION_IOC_HEAP_QUERY:
ret = ion_query_heaps(&data.query);
break;
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 9907332..f1404dc 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -436,6 +436,23 @@ int ion_alloc(size_t len, unsigned int heap_id_mask,
unsigned int flags)
return fd;
}
+int ion_buffer_update(unsigned int fd, unsigned int flags)
+{
+ struct dma_buf *dmabuf;
+ struct ion_buffer *buffer;
+
+ dmabuf = dma_buf_get(fd);
+
+ if (!dmabuf)
+ return -EINVAL;
+
+ buffer = dmabuf->priv;
+ buffer->flags = flags;
+ dma_buf_put(dmabuf);
+
+ return 0;
+}
+
int ion_query_heaps(struct ion_heap_query *query)
{
struct ion_device *dev = internal_dev;
diff --git a/drivers/staging/android/ion/ion.h
b/drivers/staging/android/ion/ion.h
index c006fc1..99bf9ab 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -199,6 +199,7 @@ int ion_heap_pages_zero(struct page *page, size_t size,
pgprot_t pgprot);
int ion_alloc(size_t len,
unsigned int heap_id_mask,
unsigned int flags);
+int ion_buffer_update(unsigned int fd, unsigned int flags);
/**
* ion_heap_init_shrinker
diff --git a/drivers/staging/android/uapi/ion.h
b/drivers/staging/android/uapi/ion.h
index 5d70098..99753fc 100644
--- a/drivers/staging/android/uapi/ion.h
+++ b/drivers/staging/android/uapi/ion.h
@@ -74,6 +74,20 @@ struct ion_allocation_data {
__u32 unused;
};
+/**
+ * struct ion_buffer_flag_data - metadata passed from userspace for update
+ * buffer flags
+ * @fd:file descriptor of the buffer
+ * @flags: flags passed to the buffer
+ *
+ * Provided by userspace as an argument to the ioctl
+ */
+
+struct ion_buffer_flag_data {
+ __u32 fd;
+ __u32 flags;
+}
+
#define MAX_HEAP_NAME 32
/**
@@ -116,6 +130,14 @@ struct ion_heap_query {
struct ion_allocation_data)
/**
+ * DOC: ION_IOC_BUFFER_UPDATE - update the specified ion buffer flags
+ *
+ * Takes an ion_buffer_flag_data structure and returns the result of the
+ * buffer flag update operation.
+ */
+#define ION_IOC_BUFFER_UPDATE _IOWR(ION_IOC_MAGIC, 1, \
+ struct ion_buffer_flag_data)
+/**
* DOC: ION_IOC_HEAP_QUERY - information about available heaps
*
* Takes an ion_heap_query structure and populates information about
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: Add chunk heaps instantiation
On 12/16/18 2:46 AM, Alexey Skidanov wrote:
Chunk heap instantiation should be supported for device tree platforms and
non device tree platforms. For device tree platforms, it's a platform
specific code responsibility to retrieve the heap configuration data
and to call the appropriate API for heap creation. For non device tree
platforms, there is no defined way to create the heaps.
This patch provides the way of chunk heaps creation using
"ion_chunk_heap=name:size@start" kernel boot parameter.
I've been thinking about this and I think it works but
I'm still kind of torn about not having devicetree bindings.
It doesn't _preclude_ devicetree bindings but I'd hate to
merge this and then end up with something incompatible.
I do want to support non-Android use cases too.
I'm also curious about the value of this heap with just PAGE_SIZE.
The original purpose of the chunk heap was a carveout where
you could easily get allocations large than PAGE_SIZE to
reduce TLB pressure. Do you have another use case for the
chunk heap?
Sumit, do you have any thoughts?
Thanks,
Laura
Link:
http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-November/128495.html
Signed-off-by: Alexey Skidanov
---
drivers/staging/android/ion/ion_chunk_heap.c | 96 +---
include/linux/ion.h | 18 ++
2 files changed, 105 insertions(+), 9 deletions(-)
create mode 100644 include/linux/ion.h
diff --git a/drivers/staging/android/ion/ion_chunk_heap.c
b/drivers/staging/android/ion/ion_chunk_heap.c
index 102c093..1a8e3ad 100644
--- a/drivers/staging/android/ion/ion_chunk_heap.c
+++ b/drivers/staging/android/ion/ion_chunk_heap.c
@@ -21,8 +21,12 @@
#include
#include
#include
+#include
#include "ion.h"
+static struct ion_chunk_heap_cfg chunk_heap_cfg[MAX_NUM_OF_CHUNK_HEAPS];
+static unsigned int num_of_req_chunk_heaps;
+
struct ion_chunk_heap {
struct ion_heap heap;
struct gen_pool *pool;
@@ -117,15 +121,15 @@ static struct ion_heap_ops chunk_heap_ops = {
.unmap_kernel = ion_heap_unmap_kernel,
};
-struct ion_heap *ion_chunk_heap_create(struct ion_platform_heap *heap_data)
+static struct ion_heap *ion_chunk_heap_create(struct ion_chunk_heap_cfg
*heap_cfg)
{
struct ion_chunk_heap *chunk_heap;
int ret;
struct page *page;
size_t size;
- page = pfn_to_page(PFN_DOWN(heap_data->base));
- size = heap_data->size;
+ page = pfn_to_page(PFN_DOWN(heap_cfg->base));
+ size = heap_cfg->size;
ret = ion_heap_pages_zero(page, size, pgprot_writecombine(PAGE_KERNEL));
if (ret)
@@ -135,23 +139,27 @@ struct ion_heap *ion_chunk_heap_create(struct
ion_platform_heap *heap_data)
if (!chunk_heap)
return ERR_PTR(-ENOMEM);
- chunk_heap->chunk_size = (unsigned long)heap_data->priv;
+ chunk_heap->chunk_size = heap_cfg->chunk_size;
chunk_heap->pool = gen_pool_create(get_order(chunk_heap->chunk_size) +
PAGE_SHIFT, -1);
if (!chunk_heap->pool) {
ret = -ENOMEM;
goto error_gen_pool_create;
}
- chunk_heap->base = heap_data->base;
- chunk_heap->size = heap_data->size;
+ chunk_heap->base = heap_cfg->base;
+ chunk_heap->size = heap_cfg->size;
+ chunk_heap->heap.name = heap_cfg->heap_name;
chunk_heap->allocated = 0;
- gen_pool_add(chunk_heap->pool, chunk_heap->base, heap_data->size, -1);
+ gen_pool_add(chunk_heap->pool, chunk_heap->base, heap_cfg->size, -1);
chunk_heap->heap.ops = &chunk_heap_ops;
chunk_heap->heap.type = ION_HEAP_TYPE_CHUNK;
chunk_heap->heap.flags = ION_HEAP_FLAG_DEFER_FREE;
- pr_debug("%s: base %pa size %zu\n", __func__,
-&chunk_heap->base, heap_data->size);
+
+ pr_info("%s: name %s base %pa size %zu\n", __func__,
+ heap_cfg->heap_name,
+ &heap_cfg->base,
+ heap_cfg->size);
return &chunk_heap->heap;
@@ -160,3 +168,73 @@ struct ion_heap *ion_chunk_heap_create(struct ion_platform_heap *heap_data)
return ERR_PTR(ret);
}
+static int __init setup_heap(char *param)
+{
+ char *at_sign, *coma, *colon;
+ size_t size_to_copy;
+ struct ion_chunk_heap_cfg *cfg;
+
+ do {
+ cfg = &chunk_heap_cfg[num_of_req_chunk_heaps];
+
+ /* heap name */
+ colon = strchr(param, ':');
+ if (!colon)
+ return -EINVAL;
+
+ size_to_copy = min_t(size_t, MAX_CHUNK_HEAP_NAME_SIZE - 1,
+(colon - param));
+ strncpy(cfg->heap_name, param, size_to_copy);
+ cfg->heap_name[size_to_copy] = '\0';
+
+ /* heap size */
+ cfg->size = memparse((colon + 1), &at_sign);
+ if ((colon + 1) == at_sign)
+
Re: [PATCH] staging: android: ion: add buffer flag update ioctl
On 12/19/18 5:39 PM, Zengtao (B) wrote:
Hi laura:
-Original Message-
From: Laura Abbott [mailto:labb...@redhat.com]
Sent: Thursday, December 20, 2018 2:10 AM
To: Zengtao (B) ; sumit.sem...@linaro.org
Cc: Greg Kroah-Hartman ; Arve Hjønnevåg
; Todd Kjos ; Martijn Coenen
; Joel Fernandes ;
de...@driverdev.osuosl.org; dri-de...@lists.freedesktop.org;
linaro-mm-...@lists.linaro.org; linux-ker...@vger.kernel.org
Subject: Re: [PATCH] staging: android: ion: add buffer flag update ioctl
On 12/19/18 9:19 AM, Zeng Tao wrote:
In some usecases, the buffer cached attribute is not determined at
allocation time, it's determined just before the real cpu mapping.
And from the memory view of point, a buffer should not have the
cached
attribute util is really mapped by the cpu. So in this patch, we
introduced the new ioctl command to target the requirement.
This is racy and error prone. Can you explain more what problem you are
trying to solve?
My use case is like this:
1. There are two process A and B, A takes case of ion buffer allocation, and
pass the buffer fd to B, then B maps and uses it.
2. Process B need to map the buffer with different cached attribute for
different use case, for example, if the buffer is used for pure software
algorithm,
then we need to map it as cached, otherwise non-cached, and B needs to deal
with both cases.
And unfortunately the mmap syscall takes no cached flags and we can't decide
the cache attribute when we are doing the mmap, so I introduce new the ioctl
even though I think the solution is not as good.
Thanks for the explanation, this was about the use case I expected.
I'm pretty sure I had this exact problem once upon a time and we
didn't come up with a solution. I'd still like to get rid of
uncached buffers in general and just use cached buffers
(see
http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-November/128842.html)
What's your usecase for uncached buffers?
Signed-off-by: Zeng Tao
---
drivers/staging/android/ion/ion-ioctl.c | 4
drivers/staging/android/ion/ion.c | 17 +
drivers/staging/android/ion/ion.h | 1 +
drivers/staging/android/uapi/ion.h | 22
++
4 files changed, 44 insertions(+)
diff --git a/drivers/staging/android/ion/ion-ioctl.c
b/drivers/staging/android/ion/ion-ioctl.c
index a8d3cc4..60bb702 100644
--- a/drivers/staging/android/ion/ion-ioctl.c
+++ b/drivers/staging/android/ion/ion-ioctl.c
@@ -12,6 +12,7 @@
union ion_ioctl_arg {
struct ion_allocation_data allocation;
+ struct ion_buffer_flag_data update;
struct ion_heap_query query;
};
@@ -83,6 +84,9 @@ long ion_ioctl(struct file *filp, unsigned int cmd,
unsigned long arg)
break;
}
+ case ION_IOC_BUFFER_UPDATE:
+ ret = ion_buffer_update(data.update.fd, data.update.flags);
+ break;
case ION_IOC_HEAP_QUERY:
ret = ion_query_heaps(&data.query);
break;
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 9907332..f1404dc 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -436,6 +436,23 @@ int ion_alloc(size_t len, unsigned int
heap_id_mask, unsigned int flags)
return fd;
}
+int ion_buffer_update(unsigned int fd, unsigned int flags) {
+ struct dma_buf *dmabuf;
+ struct ion_buffer *buffer;
+
+ dmabuf = dma_buf_get(fd);
+
+ if (!dmabuf)
+ return -EINVAL;
+
+ buffer = dmabuf->priv;
+ buffer->flags = flags;
+ dma_buf_put(dmabuf);
+
+ return 0;
+}
+
int ion_query_heaps(struct ion_heap_query *query)
{
struct ion_device *dev = internal_dev; diff --git
a/drivers/staging/android/ion/ion.h
b/drivers/staging/android/ion/ion.h
index c006fc1..99bf9ab 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -199,6 +199,7 @@ int ion_heap_pages_zero(struct page *page,
size_t size, pgprot_t pgprot);
int ion_alloc(size_t len,
unsigned int heap_id_mask,
unsigned int flags);
+int ion_buffer_update(unsigned int fd, unsigned int flags);
/**
* ion_heap_init_shrinker
diff --git a/drivers/staging/android/uapi/ion.h
b/drivers/staging/android/uapi/ion.h
index 5d70098..99753fc 100644
--- a/drivers/staging/android/uapi/ion.h
+++ b/drivers/staging/android/uapi/ion.h
@@ -74,6 +74,20 @@ struct ion_allocation_data {
__u32 unused;
};
+/**
+ * struct ion_buffer_flag_data - metadata passed from userspace for
+update
+ * buffer flags
+ * @fd:file descriptor of the buffer
+ * @flags: flags passed to the buffer
+ *
+ * Provided by userspace as an argument to the ioctl */
+
+struct ion_buffer_flag_data {
+ __u32 fd;
+ __u32 flags;
+}
+
#define MAX_HEAP_NAME
Re: [PATCH] staging: android: ion: add buffer flag update ioctl
On 12/23/18 6:47 PM, Zengtao (B) wrote:
Hi laura:
-Original Message-
From: Laura Abbott [mailto:labb...@redhat.com]
Sent: Friday, December 21, 2018 4:50 AM
To: Zengtao (B) ; sumit.sem...@linaro.org
Cc: Greg Kroah-Hartman ; Arve Hjønnevåg
; Todd Kjos ; Martijn Coenen
; Joel Fernandes ;
de...@driverdev.osuosl.org; dri-de...@lists.freedesktop.org;
linaro-mm-...@lists.linaro.org; linux-ker...@vger.kernel.org
Subject: Re: [PATCH] staging: android: ion: add buffer flag update ioctl
On 12/19/18 5:39 PM, Zengtao (B) wrote:
Hi laura:
-Original Message-
From: Laura Abbott [mailto:labb...@redhat.com]
Sent: Thursday, December 20, 2018 2:10 AM
To: Zengtao (B) ;
sumit.sem...@linaro.org
Cc: Greg Kroah-Hartman ; Arve
Hjønnevåg
; Todd Kjos ; Martijn
Coenen
; Joel Fernandes ;
de...@driverdev.osuosl.org; dri-de...@lists.freedesktop.org;
linaro-mm-...@lists.linaro.org; linux-ker...@vger.kernel.org
Subject: Re: [PATCH] staging: android: ion: add buffer flag update
ioctl
On 12/19/18 9:19 AM, Zeng Tao wrote:
In some usecases, the buffer cached attribute is not determined at
allocation time, it's determined just before the real cpu mapping.
And from the memory view of point, a buffer should not have the
cached
attribute util is really mapped by the cpu. So in this patch, we
introduced the new ioctl command to target the requirement.
This is racy and error prone. Can you explain more what problem you
are trying to solve?
My use case is like this:
1. There are two process A and B, A takes case of ion buffer allocation,
and
pass the buffer fd to B, then B maps and uses it.
2. Process B need to map the buffer with different cached attribute
for different use case, for example, if the buffer is used for pure
software algorithm, then we need to map it as cached, otherwise
non-cached, and B needs to deal with both cases.
And unfortunately the mmap syscall takes no cached flags and we can't
decide the cache attribute when we are doing the mmap, so I introduce
new the ioctl even though I think the solution is not as good.
Thanks for the explanation, this was about the use case I expected.
I'm pretty sure I had this exact problem once upon a time and we didn't
come up with a solution. I'd still like to get rid of uncached buffers in
general and just use cached buffers (see
http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-N
ovember/128842.html)
What's your usecase for uncached buffers?
Some buffers are only used by HW, in this case, we tend to use uncached buffers.
But sometimes the SW need to read few buffer contents for debug purpose and
no synchronization is needed.
If this is primarily for debug, that's not really a compelling reason to
support uncached buffers. It's incredibly difficult to do uncached
buffers correctly I'd rather spend effort on making the cached
use cases work better.
Thanks,
Laura
Signed-off-by: Zeng Tao
---
drivers/staging/android/ion/ion-ioctl.c | 4
drivers/staging/android/ion/ion.c | 17
+
drivers/staging/android/ion/ion.h | 1 +
drivers/staging/android/uapi/ion.h | 22
++
4 files changed, 44 insertions(+)
diff --git a/drivers/staging/android/ion/ion-ioctl.c
b/drivers/staging/android/ion/ion-ioctl.c
index a8d3cc4..60bb702 100644
--- a/drivers/staging/android/ion/ion-ioctl.c
+++ b/drivers/staging/android/ion/ion-ioctl.c
@@ -12,6 +12,7 @@
union ion_ioctl_arg {
struct ion_allocation_data allocation;
+ struct ion_buffer_flag_data update;
struct ion_heap_query query;
};
@@ -83,6 +84,9 @@ long ion_ioctl(struct file *filp, unsigned int
cmd, unsigned long arg)
break;
}
+ case ION_IOC_BUFFER_UPDATE:
+ ret = ion_buffer_update(data.update.fd, data.update.flags);
+ break;
case ION_IOC_HEAP_QUERY:
ret = ion_query_heaps(&data.query);
break;
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 9907332..f1404dc 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -436,6 +436,23 @@ int ion_alloc(size_t len, unsigned int
heap_id_mask, unsigned int flags)
return fd;
}
+int ion_buffer_update(unsigned int fd, unsigned int flags) {
+ struct dma_buf *dmabuf;
+ struct ion_buffer *buffer;
+
+ dmabuf = dma_buf_get(fd);
+
+ if (!dmabuf)
+ return -EINVAL;
+
+ buffer = dmabuf->priv;
+ buffer->flags = flags;
+ dma_buf_put(dmabuf);
+
+ return 0;
+}
+
int ion_query_heaps(struct ion_heap_query *query)
{
struct ion_device *dev = internal_dev; diff --git
a/drivers/staging/android/ion/ion.h
b/drivers/staging/android/ion/ion.h
index c006fc1..99bf9ab 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
Re: [PATCH] staging: android: ion: move map_kernel to ion_dma_buf_kmap
On 12/24/18 12:19 AM, Qing Xia wrote:
Now, as Google's user guide, if userspace need clean ION buffer's
cache, they should call ioctl(fd, DMA_BUF_IOCTL_SYNC, sync). Then
we found that ion_dma_buf_begin_cpu_access/ion_dma_buf_end_cpu_access
will do ION buffer's map_kernel, it's not necessary. And if usersapce
only need clean cache, they will call ion_dma_buf_end_cpu_access by
dmabuf's ioctl, ION buffer's kmap_cnt will be wrong value -1, then
driver could not get right kernel vaddr by dma_buf_kmap.
The problem is this subtly violates how dma_buf is supposed
to work. All setup is supposed to happen in begin_cpu_access.
I agree calling map kernel each time isn't great but I think
this needs to be worked out with dma_buf.
Thanks,
Laura
Signed-off-by: Qing Xia
---
drivers/staging/android/ion/ion.c | 46 ++-
1 file changed, 21 insertions(+), 25 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 9907332..f7e2812 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -303,45 +303,47 @@ static void ion_dma_buf_release(struct dma_buf *dmabuf)
static void *ion_dma_buf_kmap(struct dma_buf *dmabuf, unsigned long offset)
{
struct ion_buffer *buffer = dmabuf->priv;
+ void *vaddr;
- return buffer->vaddr + offset * PAGE_SIZE;
+ if (buffer->heap->ops->map_kernel) {
+ mutex_lock(&buffer->lock);
+ vaddr = ion_buffer_kmap_get(buffer);
+ mutex_unlock(&buffer->lock);
+ if (IS_ERR(vaddr))
+ return vaddr;
+
+ return vaddr + offset * PAGE_SIZE;
+ }
+
+ return NULL;
}
static void ion_dma_buf_kunmap(struct dma_buf *dmabuf, unsigned long offset,
void *ptr)
{
+ struct ion_buffer *buffer = dmabuf->priv;
+
+ if (buffer->heap->ops->map_kernel) {
+ mutex_lock(&buffer->lock);
+ ion_buffer_kmap_put(buffer);
+ mutex_unlock(&buffer->lock);
+ }
}
static int ion_dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
enum dma_data_direction direction)
{
struct ion_buffer *buffer = dmabuf->priv;
- void *vaddr;
struct ion_dma_buf_attachment *a;
- int ret = 0;
-
- /*
-* TODO: Move this elsewhere because we don't always need a vaddr
-*/
- if (buffer->heap->ops->map_kernel) {
- mutex_lock(&buffer->lock);
- vaddr = ion_buffer_kmap_get(buffer);
- if (IS_ERR(vaddr)) {
- ret = PTR_ERR(vaddr);
- goto unlock;
- }
- mutex_unlock(&buffer->lock);
- }
mutex_lock(&buffer->lock);
list_for_each_entry(a, &buffer->attachments, list) {
dma_sync_sg_for_cpu(a->dev, a->table->sgl, a->table->nents,
direction);
}
-
-unlock:
mutex_unlock(&buffer->lock);
- return ret;
+
+ return 0;
}
static int ion_dma_buf_end_cpu_access(struct dma_buf *dmabuf,
@@ -350,12 +352,6 @@ static int ion_dma_buf_end_cpu_access(struct dma_buf
*dmabuf,
struct ion_buffer *buffer = dmabuf->priv;
struct ion_dma_buf_attachment *a;
- if (buffer->heap->ops->map_kernel) {
- mutex_lock(&buffer->lock);
- ion_buffer_kmap_put(buffer);
- mutex_unlock(&buffer->lock);
- }
-
mutex_lock(&buffer->lock);
list_for_each_entry(a, &buffer->attachments, list) {
dma_sync_sg_for_device(a->dev, a->table->sgl, a->table->nents,
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: Add chunk heaps instantiation
On 12/20/18 1:29 PM, Alexey Skidanov wrote:
On 12/20/18 10:36 PM, Laura Abbott wrote:
On 12/16/18 2:46 AM, Alexey Skidanov wrote:
Chunk heap instantiation should be supported for device tree platforms
and
non device tree platforms. For device tree platforms, it's a platform
specific code responsibility to retrieve the heap configuration data
and to call the appropriate API for heap creation. For non device tree
platforms, there is no defined way to create the heaps.
This patch provides the way of chunk heaps creation using
"ion_chunk_heap=name:size@start" kernel boot parameter.
I've been thinking about this and I think it works but
I'm still kind of torn about not having devicetree bindings.
It doesn't _preclude_ devicetree bindings but I'd hate to
merge this and then end up with something incompatible.
I do want to support non-Android use cases too.
Sorry, what do you mean by non-Android cases?
Any user of Ion that isn't tied to Android. This includes other
userspace frameworks as well as non-devicetree targets.
I'm also curious about the value of this heap with just PAGE_SIZE.
The original purpose of the chunk heap was a carveout where
you could easily get allocations large than PAGE_SIZE to
reduce TLB pressure. Do you have another use case for the
chunk heap?
It need to be fixed ... Obviously ... The minimum allocation size should
be parametrized
Sumit, do you have any thoughts?
Thanks,
Laura
Link:
http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-November/128495.html
Signed-off-by: Alexey Skidanov
---
drivers/staging/android/ion/ion_chunk_heap.c | 96
+---
include/linux/ion.h | 18 ++
2 files changed, 105 insertions(+), 9 deletions(-)
create mode 100644 include/linux/ion.h
diff --git a/drivers/staging/android/ion/ion_chunk_heap.c
b/drivers/staging/android/ion/ion_chunk_heap.c
index 102c093..1a8e3ad 100644
--- a/drivers/staging/android/ion/ion_chunk_heap.c
+++ b/drivers/staging/android/ion/ion_chunk_heap.c
@@ -21,8 +21,12 @@
#include
#include
#include
+#include
#include "ion.h"
+static struct ion_chunk_heap_cfg
chunk_heap_cfg[MAX_NUM_OF_CHUNK_HEAPS];
+static unsigned int num_of_req_chunk_heaps;
+
struct ion_chunk_heap {
struct ion_heap heap;
struct gen_pool *pool;
@@ -117,15 +121,15 @@ static struct ion_heap_ops chunk_heap_ops = {
.unmap_kernel = ion_heap_unmap_kernel,
};
-struct ion_heap *ion_chunk_heap_create(struct ion_platform_heap
*heap_data)
+static struct ion_heap *ion_chunk_heap_create(struct
ion_chunk_heap_cfg *heap_cfg)
{
struct ion_chunk_heap *chunk_heap;
int ret;
struct page *page;
size_t size;
- page = pfn_to_page(PFN_DOWN(heap_data->base));
- size = heap_data->size;
+ page = pfn_to_page(PFN_DOWN(heap_cfg->base));
+ size = heap_cfg->size;
ret = ion_heap_pages_zero(page, size,
pgprot_writecombine(PAGE_KERNEL));
if (ret)
@@ -135,23 +139,27 @@ struct ion_heap *ion_chunk_heap_create(struct
ion_platform_heap *heap_data)
if (!chunk_heap)
return ERR_PTR(-ENOMEM);
- chunk_heap->chunk_size = (unsigned long)heap_data->priv;
+ chunk_heap->chunk_size = heap_cfg->chunk_size;
chunk_heap->pool =
gen_pool_create(get_order(chunk_heap->chunk_size) +
PAGE_SHIFT, -1);
if (!chunk_heap->pool) {
ret = -ENOMEM;
goto error_gen_pool_create;
}
- chunk_heap->base = heap_data->base;
- chunk_heap->size = heap_data->size;
+ chunk_heap->base = heap_cfg->base;
+ chunk_heap->size = heap_cfg->size;
+ chunk_heap->heap.name = heap_cfg->heap_name;
chunk_heap->allocated = 0;
- gen_pool_add(chunk_heap->pool, chunk_heap->base,
heap_data->size, -1);
+ gen_pool_add(chunk_heap->pool, chunk_heap->base, heap_cfg->size,
-1);
chunk_heap->heap.ops = &chunk_heap_ops;
chunk_heap->heap.type = ION_HEAP_TYPE_CHUNK;
chunk_heap->heap.flags = ION_HEAP_FLAG_DEFER_FREE;
- pr_debug("%s: base %pa size %zu\n", __func__,
- &chunk_heap->base, heap_data->size);
+
+ pr_info("%s: name %s base %pa size %zu\n", __func__,
+ heap_cfg->heap_name,
+ &heap_cfg->base,
+ heap_cfg->size);
return &chunk_heap->heap;
@@ -160,3 +168,73 @@ struct ion_heap *ion_chunk_heap_create(struct
ion_platform_heap *heap_data)
return ERR_PTR(ret);
}
+static int __init setup_heap(char *param)
+{
+ char *at_sign, *coma, *colon;
+ size_t size_to_copy;
+ struct ion_chunk_heap_cfg *cfg;
+
+ do {
+ cfg = &chunk_heap_cfg[num_of_req_chunk_heaps];
+
+ /* heap name */
+ colon = strchr(param, ':');
+ if (!colon)
+
Re: [PATCH] staging: android: ion: add buffer flag update ioctl
On 1/3/19 5:42 PM, Zengtao (B) wrote: -Original Message- From: Laura Abbott [mailto:labb...@redhat.com] Sent: Thursday, January 03, 2019 6:31 AM To: Zengtao (B) ; sumit.sem...@linaro.org Cc: Greg Kroah-Hartman ; Arve Hjønnevåg ; Todd Kjos ; Martijn Coenen ; Joel Fernandes ; de...@driverdev.osuosl.org; dri-de...@lists.freedesktop.org; linaro-mm-...@lists.linaro.org; linux-ker...@vger.kernel.org Subject: Re: [PATCH] staging: android: ion: add buffer flag update ioctl On 12/23/18 6:47 PM, Zengtao (B) wrote: Hi laura: -Original Message- From: Laura Abbott [mailto:labb...@redhat.com] Sent: Friday, December 21, 2018 4:50 AM To: Zengtao (B) ; sumit.sem...@linaro.org Cc: Greg Kroah-Hartman ; Arve Hjønnevåg ; Todd Kjos ; Martijn Coenen ; Joel Fernandes ; de...@driverdev.osuosl.org; dri-de...@lists.freedesktop.org; linaro-mm-...@lists.linaro.org; linux-ker...@vger.kernel.org Subject: Re: [PATCH] staging: android: ion: add buffer flag update ioctl On 12/19/18 5:39 PM, Zengtao (B) wrote: Hi laura: -Original Message- From: Laura Abbott [mailto:labb...@redhat.com] Sent: Thursday, December 20, 2018 2:10 AM To: Zengtao (B) ; sumit.sem...@linaro.org Cc: Greg Kroah-Hartman ; Arve Hjønnevåg ; Todd Kjos ; Martijn Coenen ; Joel Fernandes ; de...@driverdev.osuosl.org; dri-de...@lists.freedesktop.org; linaro-mm-...@lists.linaro.org; linux-ker...@vger.kernel.org Subject: Re: [PATCH] staging: android: ion: add buffer flag update ioctl On 12/19/18 9:19 AM, Zeng Tao wrote: In some usecases, the buffer cached attribute is not determined at allocation time, it's determined just before the real cpu mapping. And from the memory view of point, a buffer should not have the cached attribute util is really mapped by the cpu. So in this patch, we introduced the new ioctl command to target the requirement. This is racy and error prone. Can you explain more what problem you are trying to solve? My use case is like this: 1. There are two process A and B, A takes case of ion buffer allocation, and pass the buffer fd to B, then B maps and uses it. 2. Process B need to map the buffer with different cached attribute for different use case, for example, if the buffer is used for pure software algorithm, then we need to map it as cached, otherwise non-cached, and B needs to deal with both cases. And unfortunately the mmap syscall takes no cached flags and we can't decide the cache attribute when we are doing the mmap, so I introduce new the ioctl even though I think the solution is not as good. Thanks for the explanation, this was about the use case I expected. I'm pretty sure I had this exact problem once upon a time and we didn't come up with a solution. I'd still like to get rid of uncached buffers in general and just use cached buffers (see http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/201 8-N ovember/128842.html) What's your usecase for uncached buffers? Some buffers are only used by HW, in this case, we tend to use uncached buffers. But sometimes the SW need to read few buffer contents for debug purpose and no synchronization is needed. If this is primarily for debug, that's not really a compelling reason to support uncached buffers. It's incredibly difficult to do uncached buffers correctly I'd rather spend effort on making the cached use cases work better. No, not only for debug, I meant if the buffers are uncached, the SW will only mmap them for debug or even don't need to mmap them. So for the two kinds of buffers: 1. uncached: only the HW need to access it, the SW don't need to mmap it or just for debug. 2. cached: both the HW and the SW need to access it. The ION is designed as a generalized memory manager, I think we should cover as many requirements as we can in the ION design, so I 'd rather that we keep the uncached support. And I don’t quite understand the difficulty you mentioned to support the uncached buffers, would you explain a little more about it. We end up with aliasing problems. Each kernel page still has a cached mapping so it's very difficult to keep the two mappings in sync. https://lore.kernel.org/lkml/20181120164636.jcw7li2uaa3cmwc3@DESKTOP-E1NTVVP.localdomain/ This thread does a better job of explaining the problem and the objections to uncached buffers. And if the software never touches the buffer, why does it matter if the buffer is uncached? Laura Thanks Zengtao Signed-off-by: Zeng Tao --- drivers/staging/android/ion/ion-ioctl.c | 4 drivers/staging/android/ion/ion.c | 17 + drivers/staging/android/ion/ion.h | 1 + drivers/staging/android/uapi/ion.h | 22 ++ 4 files changed, 44 insertions(+) diff --git a/drivers/staging/android/ion/ion-ioctl.c b/drivers/staging/android/ion/ion-ioctl.c index a8d3cc4..60bb702 100644 --- a/drivers/staging/andr
Re: [PATCH] staging: android: ion: Add chunk heaps instantiation
On 1/2/19 10:28 PM, Alexey Skidanov wrote: On 1/3/19 12:37 AM, Laura Abbott wrote: On 12/20/18 1:29 PM, Alexey Skidanov wrote: On 12/20/18 10:36 PM, Laura Abbott wrote: On 12/16/18 2:46 AM, Alexey Skidanov wrote: Chunk heap instantiation should be supported for device tree platforms and non device tree platforms. For device tree platforms, it's a platform specific code responsibility to retrieve the heap configuration data and to call the appropriate API for heap creation. For non device tree platforms, there is no defined way to create the heaps. This patch provides the way of chunk heaps creation using "ion_chunk_heap=name:size@start" kernel boot parameter. I've been thinking about this and I think it works but I'm still kind of torn about not having devicetree bindings. It doesn't _preclude_ devicetree bindings but I'd hate to merge this and then end up with something incompatible. I do want to support non-Android use cases too. Sorry, what do you mean by non-Android cases? Any user of Ion that isn't tied to Android. This includes other userspace frameworks as well as non-devicetree targets. Sorry, don't follow you ... I tested the patch on Ubuntu machine - so the non-Android and non-devicetree case is obviously supported :) This patch _doesn't_ provide the required support for devicetree bindings and Android use case. I'm trying to balance wanting to support this use case with knowing that the Android use case is still unsolved. Laura ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 14/14] staging: android: ion: Add UNMAPPED heap type and helper
On 1/11/19 10:05 AM, Andrew F. Davis wrote:
The "unmapped" heap is very similar to the carveout heap except
the backing memory is presumed to be unmappable by the host, in
my specific case due to firewalls. This memory can still be
allocated from and used by devices that do have access to the
backing memory.
Based originally on the secure/unmapped heap from Linaro for
the OP-TEE SDP implementation, this was re-written to match
the carveout heap helper code.
Suggested-by: Etienne Carriere
Signed-off-by: Andrew F. Davis
---
drivers/staging/android/ion/Kconfig | 10 ++
drivers/staging/android/ion/Makefile | 1 +
drivers/staging/android/ion/ion.h | 16 +++
.../staging/android/ion/ion_unmapped_heap.c | 123 ++
drivers/staging/android/uapi/ion.h| 3 +
5 files changed, 153 insertions(+)
create mode 100644 drivers/staging/android/ion/ion_unmapped_heap.c
diff --git a/drivers/staging/android/ion/Kconfig
b/drivers/staging/android/ion/Kconfig
index 0fdda6f62953..a117b8b91b14 100644
--- a/drivers/staging/android/ion/Kconfig
+++ b/drivers/staging/android/ion/Kconfig
@@ -42,3 +42,13 @@ config ION_CMA_HEAP
Choose this option to enable CMA heaps with Ion. This heap is backed
by the Contiguous Memory Allocator (CMA). If your system has these
regions, you should say Y here.
+
+config ION_UNMAPPED_HEAP
+ bool "ION unmapped heap support"
+ depends on ION
+ help
+ Choose this option to enable UNMAPPED heaps with Ion. This heap is
+ backed in specific memory pools, carveout from the Linux memory.
+ Unlike carveout heaps these are assumed to be not mappable by
+ kernel or user-space.
+ Unless you know your system has these regions, you should say N here.
diff --git a/drivers/staging/android/ion/Makefile
b/drivers/staging/android/ion/Makefile
index 17f3a7569e3d..c71a1f3de581 100644
--- a/drivers/staging/android/ion/Makefile
+++ b/drivers/staging/android/ion/Makefile
@@ -4,3 +4,4 @@ obj-$(CONFIG_ION_SYSTEM_HEAP) += ion_system_heap.o
ion_page_pool.o
obj-$(CONFIG_ION_CARVEOUT_HEAP) += ion_carveout_heap.o
obj-$(CONFIG_ION_CHUNK_HEAP) += ion_chunk_heap.o
obj-$(CONFIG_ION_CMA_HEAP) += ion_cma_heap.o
+obj-$(CONFIG_ION_UNMAPPED_HEAP) += ion_unmapped_heap.o
diff --git a/drivers/staging/android/ion/ion.h
b/drivers/staging/android/ion/ion.h
index 97b2876b165a..ce74332018ba 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -341,4 +341,20 @@ static inline struct ion_heap
*ion_chunk_heap_create(phys_addr_t base, size_t si
}
#endif
+#ifdef CONFIG_ION_UNMAPPED_HEAP
+/**
+ * ion_unmapped_heap_create
+ * @base: base address of carveout memory
+ * @size: size of carveout memory region
+ *
+ * Creates an unmapped ion_heap using the passed in data
+ */
+struct ion_heap *ion_unmapped_heap_create(phys_addr_t base, size_t size);
+#else
+struct ion_heap *ion_unmapped_heap_create(phys_addr_t base, size_t size)
+{
+ return ERR_PTR(-ENODEV);
+}
+#endif
+
#endif /* _ION_H */
diff --git a/drivers/staging/android/ion/ion_unmapped_heap.c
b/drivers/staging/android/ion/ion_unmapped_heap.c
new file mode 100644
index ..7602b659c2ec
--- /dev/null
+++ b/drivers/staging/android/ion/ion_unmapped_heap.c
@@ -0,0 +1,123 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ION Memory Allocator unmapped heap helper
+ *
+ * Copyright (C) 2015-2016 Texas Instruments Incorporated - http://www.ti.com/
+ * Andrew F. Davis
+ *
+ * ION "unmapped" heaps are physical memory heaps not by default mapped into
+ * a virtual address space. The buffer owner can explicitly request kernel
+ * space mappings but the underlying memory may still not be accessible for
+ * various reasons, such as firewalls.
+ */
+
+#include
+#include
+#include
+#include
+
+#include "ion.h"
+
+#define ION_UNMAPPED_ALLOCATE_FAIL -1
+
+struct ion_unmapped_heap {
+ struct ion_heap heap;
+ struct gen_pool *pool;
+};
+
+static phys_addr_t ion_unmapped_allocate(struct ion_heap *heap,
+unsigned long size)
+{
+ struct ion_unmapped_heap *unmapped_heap =
+ container_of(heap, struct ion_unmapped_heap, heap);
+ unsigned long offset;
+
+ offset = gen_pool_alloc(unmapped_heap->pool, size);
+ if (!offset)
+ return ION_UNMAPPED_ALLOCATE_FAIL;
+
+ return offset;
+}
+
+static void ion_unmapped_free(struct ion_heap *heap, phys_addr_t addr,
+ unsigned long size)
+{
+ struct ion_unmapped_heap *unmapped_heap =
+ container_of(heap, struct ion_unmapped_heap, heap);
+
+ gen_pool_free(unmapped_heap->pool, addr, size);
+}
+
+static int ion_unmapped_heap_allocate(struct ion_heap *heap,
+ struct ion_buffer *buffer,
+ unsigned long siz
Re: [PATCH 00/14] Misc ION cleanups and adding unmapped heap
On 1/11/19 10:05 AM, Andrew F. Davis wrote: Hello all, This is a set of (hopefully) non-controversial cleanups for the ION framework and current set of heaps. These were found as I start to familiarize myself with the framework to help in whatever way I can in getting all this up to the standards needed for de-staging. I would like to get some ideas of what is left to work on to get ION out of staging. Has there been some kind of agreement on what ION should eventually end up being? To me it looks like it is being whittled away at to it's most core functions. To me that is looking like being a DMA-BUF user-space front end, simply advertising available memory backings in a system and providing allocations as DMA-BUF handles. If this is the case then it looks close to being ready to me at least, but I would love to hear any other opinions and concerns. Yes, at this point the only functionality that people are really depending on is the ability to allocate a dma_buf easily from userspace. Back to this patchset, the last patch may be a bit different than the others, it adds an unmapped heaps type and creation helper. I wanted to get this in to show off another heap type and maybe some issues we may have with the current ION framework. The unmapped heap is used when the backing memory should not (or cannot) be touched. Currently this kind of heap is used for firewalled secure memory that can be allocated like normal heap memory but only used by secure devices (OP-TEE, crypto HW, etc). It is basically just copied from the "carveout" heap type with the only difference being it is not mappable to userspace and we do not clear the memory (as we should not map it either). So should this really be a new heap type? Or maybe advertised as a carveout heap but with an additional allocation flag? Perhaps we do away with "types" altogether and just have flags, coherent/non-coherent, mapped/unmapped, etc. Maybe more thinking will be needed afterall.. So the cleanup looks okay (I need to finish reviewing) but I'm not a fan of adding another heaptype without solving the problem of adding some sort of devicetree binding or other method of allocating and placing Ion heaps. That plus uncached buffers are one of the big open problems that need to be solved for destaging Ion. See https://lore.kernel.org/lkml/20181120164636.jcw7li2uaa3cmwc3@DESKTOP-E1NTVVP.localdomain/ for some background on that problem. Thanks, Laura Thanks, Andrew Andrew F. Davis (14): staging: android: ion: Add proper header information staging: android: ion: Remove empty ion_ioctl_dir() function staging: android: ion: Merge ion-ioctl.c into ion.c staging: android: ion: Remove leftover comment staging: android: ion: Remove struct ion_platform_heap staging: android: ion: Fixup some white-space issues staging: android: ion: Sync comment docs with struct ion_buffer staging: android: ion: Remove base from ion_carveout_heap staging: android: ion: Remove base from ion_chunk_heap staging: android: ion: Remove unused headers staging: android: ion: Allow heap name to be null staging: android: ion: Declare helpers for carveout and chunk heaps staging: android: ion: Do not sync CPU cache on map/unmap staging: android: ion: Add UNMAPPED heap type and helper drivers/staging/android/ion/Kconfig | 10 ++ drivers/staging/android/ion/Makefile | 3 +- drivers/staging/android/ion/ion-ioctl.c | 98 -- drivers/staging/android/ion/ion.c | 93 +++-- drivers/staging/android/ion/ion.h | 87 - .../staging/android/ion/ion_carveout_heap.c | 19 +-- drivers/staging/android/ion/ion_chunk_heap.c | 25 ++-- drivers/staging/android/ion/ion_cma_heap.c| 6 +- drivers/staging/android/ion/ion_heap.c| 8 +- drivers/staging/android/ion/ion_page_pool.c | 2 +- drivers/staging/android/ion/ion_system_heap.c | 8 +- .../staging/android/ion/ion_unmapped_heap.c | 123 ++ drivers/staging/android/uapi/ion.h| 3 + 13 files changed, 307 insertions(+), 178 deletions(-) delete mode 100644 drivers/staging/android/ion/ion-ioctl.c create mode 100644 drivers/staging/android/ion/ion_unmapped_heap.c ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 14/14] staging: android: ion: Add UNMAPPED heap type and helper
On 1/15/19 7:58 AM, Andrew F. Davis wrote:
On 1/14/19 8:32 PM, Laura Abbott wrote:
On 1/11/19 10:05 AM, Andrew F. Davis wrote:
The "unmapped" heap is very similar to the carveout heap except
the backing memory is presumed to be unmappable by the host, in
my specific case due to firewalls. This memory can still be
allocated from and used by devices that do have access to the
backing memory.
Based originally on the secure/unmapped heap from Linaro for
the OP-TEE SDP implementation, this was re-written to match
the carveout heap helper code.
Suggested-by: Etienne Carriere
Signed-off-by: Andrew F. Davis
---
drivers/staging/android/ion/Kconfig | 10 ++
drivers/staging/android/ion/Makefile | 1 +
drivers/staging/android/ion/ion.h | 16 +++
.../staging/android/ion/ion_unmapped_heap.c | 123 ++
drivers/staging/android/uapi/ion.h | 3 +
5 files changed, 153 insertions(+)
create mode 100644 drivers/staging/android/ion/ion_unmapped_heap.c
diff --git a/drivers/staging/android/ion/Kconfig
b/drivers/staging/android/ion/Kconfig
index 0fdda6f62953..a117b8b91b14 100644
--- a/drivers/staging/android/ion/Kconfig
+++ b/drivers/staging/android/ion/Kconfig
@@ -42,3 +42,13 @@ config ION_CMA_HEAP
Choose this option to enable CMA heaps with Ion. This heap is
backed
by the Contiguous Memory Allocator (CMA). If your system has
these
regions, you should say Y here.
+
+config ION_UNMAPPED_HEAP
+ bool "ION unmapped heap support"
+ depends on ION
+ help
+ Choose this option to enable UNMAPPED heaps with Ion. This heap is
+ backed in specific memory pools, carveout from the Linux memory.
+ Unlike carveout heaps these are assumed to be not mappable by
+ kernel or user-space.
+ Unless you know your system has these regions, you should say N
here.
diff --git a/drivers/staging/android/ion/Makefile
b/drivers/staging/android/ion/Makefile
index 17f3a7569e3d..c71a1f3de581 100644
--- a/drivers/staging/android/ion/Makefile
+++ b/drivers/staging/android/ion/Makefile
@@ -4,3 +4,4 @@ obj-$(CONFIG_ION_SYSTEM_HEAP) += ion_system_heap.o
ion_page_pool.o
obj-$(CONFIG_ION_CARVEOUT_HEAP) += ion_carveout_heap.o
obj-$(CONFIG_ION_CHUNK_HEAP) += ion_chunk_heap.o
obj-$(CONFIG_ION_CMA_HEAP) += ion_cma_heap.o
+obj-$(CONFIG_ION_UNMAPPED_HEAP) += ion_unmapped_heap.o
diff --git a/drivers/staging/android/ion/ion.h
b/drivers/staging/android/ion/ion.h
index 97b2876b165a..ce74332018ba 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -341,4 +341,20 @@ static inline struct ion_heap
*ion_chunk_heap_create(phys_addr_t base, size_t si
}
#endif
+#ifdef CONFIG_ION_UNMAPPED_HEAP
+/**
+ * ion_unmapped_heap_create
+ * @base: base address of carveout memory
+ * @size: size of carveout memory region
+ *
+ * Creates an unmapped ion_heap using the passed in data
+ */
+struct ion_heap *ion_unmapped_heap_create(phys_addr_t base, size_t
size);
+#else
+struct ion_heap *ion_unmapped_heap_create(phys_addr_t base, size_t size)
+{
+ return ERR_PTR(-ENODEV);
+}
+#endif
+
#endif /* _ION_H */
diff --git a/drivers/staging/android/ion/ion_unmapped_heap.c
b/drivers/staging/android/ion/ion_unmapped_heap.c
new file mode 100644
index ..7602b659c2ec
--- /dev/null
+++ b/drivers/staging/android/ion/ion_unmapped_heap.c
@@ -0,0 +1,123 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ION Memory Allocator unmapped heap helper
+ *
+ * Copyright (C) 2015-2016 Texas Instruments Incorporated -
http://www.ti.com/
+ * Andrew F. Davis
+ *
+ * ION "unmapped" heaps are physical memory heaps not by default
mapped into
+ * a virtual address space. The buffer owner can explicitly request
kernel
+ * space mappings but the underlying memory may still not be
accessible for
+ * various reasons, such as firewalls.
+ */
+
+#include
+#include
+#include
+#include
+
+#include "ion.h"
+
+#define ION_UNMAPPED_ALLOCATE_FAIL -1
+
+struct ion_unmapped_heap {
+ struct ion_heap heap;
+ struct gen_pool *pool;
+};
+
+static phys_addr_t ion_unmapped_allocate(struct ion_heap *heap,
+ unsigned long size)
+{
+ struct ion_unmapped_heap *unmapped_heap =
+ container_of(heap, struct ion_unmapped_heap, heap);
+ unsigned long offset;
+
+ offset = gen_pool_alloc(unmapped_heap->pool, size);
+ if (!offset)
+ return ION_UNMAPPED_ALLOCATE_FAIL;
+
+ return offset;
+}
+
+static void ion_unmapped_free(struct ion_heap *heap, phys_addr_t addr,
+ unsigned long size)
+{
+ struct ion_unmapped_heap *unmapped_heap =
+ container_of(heap, struct ion_unmapped_heap, heap);
+
+ gen_pool_free(unmapped_heap->pool, addr, size);
+}
+
+static int ion_unmapped_heap_allocate(struct ion_heap *heap,
+ struct ion_buffer *buffer,
+ unsigned long si
Re: [PATCH 00/14] Misc ION cleanups and adding unmapped heap
On 1/15/19 9:47 AM, Andrew F. Davis wrote: On 1/14/19 8:39 PM, Laura Abbott wrote: On 1/11/19 10:05 AM, Andrew F. Davis wrote: Hello all, This is a set of (hopefully) non-controversial cleanups for the ION framework and current set of heaps. These were found as I start to familiarize myself with the framework to help in whatever way I can in getting all this up to the standards needed for de-staging. I would like to get some ideas of what is left to work on to get ION out of staging. Has there been some kind of agreement on what ION should eventually end up being? To me it looks like it is being whittled away at to it's most core functions. To me that is looking like being a DMA-BUF user-space front end, simply advertising available memory backings in a system and providing allocations as DMA-BUF handles. If this is the case then it looks close to being ready to me at least, but I would love to hear any other opinions and concerns. Yes, at this point the only functionality that people are really depending on is the ability to allocate a dma_buf easily from userspace. Back to this patchset, the last patch may be a bit different than the others, it adds an unmapped heaps type and creation helper. I wanted to get this in to show off another heap type and maybe some issues we may have with the current ION framework. The unmapped heap is used when the backing memory should not (or cannot) be touched. Currently this kind of heap is used for firewalled secure memory that can be allocated like normal heap memory but only used by secure devices (OP-TEE, crypto HW, etc). It is basically just copied from the "carveout" heap type with the only difference being it is not mappable to userspace and we do not clear the memory (as we should not map it either). So should this really be a new heap type? Or maybe advertised as a carveout heap but with an additional allocation flag? Perhaps we do away with "types" altogether and just have flags, coherent/non-coherent, mapped/unmapped, etc. Maybe more thinking will be needed afterall.. So the cleanup looks okay (I need to finish reviewing) but I'm not a fan of adding another heaptype without solving the problem of adding some sort of devicetree binding or other method of allocating and placing Ion heaps. That plus uncached buffers are one of the big open problems that need to be solved for destaging Ion. See https://lore.kernel.org/lkml/20181120164636.jcw7li2uaa3cmwc3@DESKTOP-E1NTVVP.localdomain/ for some background on that problem. I'm under the impression that adding heaps like carveouts/chunk will be rather system specific and so do not lend themselves well to a universal DT style exporter. For instance a carveout memory space can be reported by a device at runtime, then the driver managing that device should go and use the carveout heap helpers to export that heap. If this is the case then I'm not sure it is a problem for the ION core framework to solve, but rather the users of it to figure out how best to create the various heaps. All Ion needs to do is allow exporting and advertising them IMHO. I think it is a problem for the Ion core framework to take care of. Ion is useless if you don't actually have the heaps. Nobody has actually gotten a full Ion solution end-to-end with a carveout heap working in mainline because any proposals have been rejected. I think we need at least one example in mainline of how creating a carveout heap would work. Thanks for the background thread link, I've been looking for some info on current status of all this and "ion" is a bit hard to search the lists for. The core reason for posting this cleanup series is to throw my hat into the ring of all this Ion work and start getting familiar with the pending issues. The last two patches are not all that important to get in right now. In that thread you linked above, it seems we may have arrived at a similar problem for different reasons. I think the root issue is the Ion core makes too many assumptions about the heap memory. My proposal would be to allow the heap exporters more control over the DMA-BUF ops, maybe even going as far as letting them provide their own complete struct dma_buf_ops. Let me give an example where I think this is going to be useful. We have the classic constraint solving problem on our SoCs. Our SoCs are full of various coherent and non-coherent devices, some require contiguous memory allocations, others have in-line IOMMUs so can operate on non-contiguous, etc.. DMA-BUF has a solution designed in for this we can use, namely allocation at map time after all the attachments have come in. The checking of each attached device to find the right backing memory is something the DMA-BUF exporter has to do, and so this SoC specific logic would have to be added to each exporting framework (DRM, V4L2, etc), unless we have one unified system exporter everyone uses, Ion. That's how dmabuf is
Re: [PATCH 13/14] staging: android: ion: Do not sync CPU cache on map/unmap
On 1/15/19 10:38 AM, Andrew F. Davis wrote:
On 1/15/19 11:45 AM, Liam Mark wrote:
On Tue, 15 Jan 2019, Andrew F. Davis wrote:
On 1/14/19 11:13 AM, Liam Mark wrote:
On Fri, 11 Jan 2019, Andrew F. Davis wrote:
Buffers may not be mapped from the CPU so skip cache maintenance here.
Accesses from the CPU to a cached heap should be bracketed with
{begin,end}_cpu_access calls so maintenance should not be needed anyway.
Signed-off-by: Andrew F. Davis
---
drivers/staging/android/ion/ion.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 14e48f6eb734..09cb5a8e2b09 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -261,8 +261,8 @@ static struct sg_table *ion_map_dma_buf(struct
dma_buf_attachment *attachment,
table = a->table;
- if (!dma_map_sg(attachment->dev, table->sgl, table->nents,
- direction))
+ if (!dma_map_sg_attrs(attachment->dev, table->sgl, table->nents,
+ direction, DMA_ATTR_SKIP_CPU_SYNC))
Unfortunately I don't think you can do this for a couple reasons.
You can't rely on {begin,end}_cpu_access calls to do cache maintenance.
If the calls to {begin,end}_cpu_access were made before the call to
dma_buf_attach then there won't have been a device attached so the calls
to {begin,end}_cpu_access won't have done any cache maintenance.
That should be okay though, if you have no attachments (or all
attachments are IO-coherent) then there is no need for cache
maintenance. Unless you mean a sequence where a non-io-coherent device
is attached later after data has already been written. Does that
sequence need supporting?
Yes, but also I think there are cases where CPU access can happen before
in Android, but I will focus on later for now.
DMA-BUF doesn't have to allocate the backing
memory until map_dma_buf() time, and that should only happen after all
the devices have attached so it can know where to put the buffer. So we
shouldn't expect any CPU access to buffers before all the devices are
attached and mapped, right?
Here is an example where CPU access can happen later in Android.
Camera device records video -> software post processing -> video device
(who does compression of raw data) and writes to a file
In this example assume the buffer is cached and the devices are not
IO-coherent (quite common).
This is the start of the problem, having cached mappings of memory that
is also being accessed non-coherently is going to cause issues one way
or another. On top of the speculative cache fills that have to be
constantly fought back against with CMOs like below; some coherent
interconnects behave badly when you mix coherent and non-coherent access
(snoop filters get messed up).
The solution is to either always have the addresses marked non-coherent
(like device memory, no-map carveouts), or if you really want to use
regular system memory allocated at runtime, then all cached mappings of
it need to be dropped, even the kernel logical address (area as painful
as that would be).
I agree it's broken, hence my desire to remove it :)
The other problem is that uncached buffers are being used for
performance reason so anything that would involve getting
rid of the logical address would probably negate any performance
benefit.
ION buffer is allocated.
//Camera device records video
dma_buf_attach
dma_map_attachment (buffer needs to be cleaned)
Why does the buffer need to be cleaned here? I just got through reading
the thread linked by Laura in the other reply. I do like +Brian's
suggestion of tracking if the buffer has had CPU access since the last
time and only flushing the cache if it has. As unmapped heaps never get
CPU mapped this would never be the case for unmapped heaps, it solves my
problem.
[camera device writes to buffer]
dma_buf_unmap_attachment (buffer needs to be invalidated)
It doesn't know there will be any further CPU access, it could get freed
after this for all we know, the invalidate can be saved until the CPU
requests access again.
dma_buf_detach (device cannot stay attached because it is being sent down
the pipeline and Camera doesn't know the end of the use case)
This seems like a broken use-case, I understand the desire to keep
everything as modular as possible and separate the steps, but at this
point no one owns this buffers backing memory, not the CPU or any
device. I would go as far as to say DMA-BUF should be free now to
de-allocate the backing storage if it wants, that way it could get ready
for the next attachment, which may change the required backing memory
completely.
All devices should attach before the first mapping, and only let go
after the task is complete, otherwise this buffers data needs copied off
to a different location or the CPU needs to take ownership in-between.
Maybe it's broken but it's the status quo and we spent a good
am
Re: [PATCH 14/14] staging: android: ion: Add UNMAPPED heap type and helper
On 1/15/19 10:43 AM, Laura Abbott wrote:
On 1/15/19 7:58 AM, Andrew F. Davis wrote:
On 1/14/19 8:32 PM, Laura Abbott wrote:
On 1/11/19 10:05 AM, Andrew F. Davis wrote:
The "unmapped" heap is very similar to the carveout heap except
the backing memory is presumed to be unmappable by the host, in
my specific case due to firewalls. This memory can still be
allocated from and used by devices that do have access to the
backing memory.
Based originally on the secure/unmapped heap from Linaro for
the OP-TEE SDP implementation, this was re-written to match
the carveout heap helper code.
Suggested-by: Etienne Carriere
Signed-off-by: Andrew F. Davis
---
drivers/staging/android/ion/Kconfig | 10 ++
drivers/staging/android/ion/Makefile | 1 +
drivers/staging/android/ion/ion.h | 16 +++
.../staging/android/ion/ion_unmapped_heap.c | 123 ++
drivers/staging/android/uapi/ion.h | 3 +
5 files changed, 153 insertions(+)
create mode 100644 drivers/staging/android/ion/ion_unmapped_heap.c
diff --git a/drivers/staging/android/ion/Kconfig
b/drivers/staging/android/ion/Kconfig
index 0fdda6f62953..a117b8b91b14 100644
--- a/drivers/staging/android/ion/Kconfig
+++ b/drivers/staging/android/ion/Kconfig
@@ -42,3 +42,13 @@ config ION_CMA_HEAP
Choose this option to enable CMA heaps with Ion. This heap is
backed
by the Contiguous Memory Allocator (CMA). If your system has
these
regions, you should say Y here.
+
+config ION_UNMAPPED_HEAP
+ bool "ION unmapped heap support"
+ depends on ION
+ help
+ Choose this option to enable UNMAPPED heaps with Ion. This heap is
+ backed in specific memory pools, carveout from the Linux memory.
+ Unlike carveout heaps these are assumed to be not mappable by
+ kernel or user-space.
+ Unless you know your system has these regions, you should say N
here.
diff --git a/drivers/staging/android/ion/Makefile
b/drivers/staging/android/ion/Makefile
index 17f3a7569e3d..c71a1f3de581 100644
--- a/drivers/staging/android/ion/Makefile
+++ b/drivers/staging/android/ion/Makefile
@@ -4,3 +4,4 @@ obj-$(CONFIG_ION_SYSTEM_HEAP) += ion_system_heap.o
ion_page_pool.o
obj-$(CONFIG_ION_CARVEOUT_HEAP) += ion_carveout_heap.o
obj-$(CONFIG_ION_CHUNK_HEAP) += ion_chunk_heap.o
obj-$(CONFIG_ION_CMA_HEAP) += ion_cma_heap.o
+obj-$(CONFIG_ION_UNMAPPED_HEAP) += ion_unmapped_heap.o
diff --git a/drivers/staging/android/ion/ion.h
b/drivers/staging/android/ion/ion.h
index 97b2876b165a..ce74332018ba 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -341,4 +341,20 @@ static inline struct ion_heap
*ion_chunk_heap_create(phys_addr_t base, size_t si
}
#endif
+#ifdef CONFIG_ION_UNMAPPED_HEAP
+/**
+ * ion_unmapped_heap_create
+ * @base: base address of carveout memory
+ * @size: size of carveout memory region
+ *
+ * Creates an unmapped ion_heap using the passed in data
+ */
+struct ion_heap *ion_unmapped_heap_create(phys_addr_t base, size_t
size);
+#else
+struct ion_heap *ion_unmapped_heap_create(phys_addr_t base, size_t size)
+{
+ return ERR_PTR(-ENODEV);
+}
+#endif
+
#endif /* _ION_H */
diff --git a/drivers/staging/android/ion/ion_unmapped_heap.c
b/drivers/staging/android/ion/ion_unmapped_heap.c
new file mode 100644
index ..7602b659c2ec
--- /dev/null
+++ b/drivers/staging/android/ion/ion_unmapped_heap.c
@@ -0,0 +1,123 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ION Memory Allocator unmapped heap helper
+ *
+ * Copyright (C) 2015-2016 Texas Instruments Incorporated -
http://www.ti.com/
+ * Andrew F. Davis
+ *
+ * ION "unmapped" heaps are physical memory heaps not by default
mapped into
+ * a virtual address space. The buffer owner can explicitly request
kernel
+ * space mappings but the underlying memory may still not be
accessible for
+ * various reasons, such as firewalls.
+ */
+
+#include
+#include
+#include
+#include
+
+#include "ion.h"
+
+#define ION_UNMAPPED_ALLOCATE_FAIL -1
+
+struct ion_unmapped_heap {
+ struct ion_heap heap;
+ struct gen_pool *pool;
+};
+
+static phys_addr_t ion_unmapped_allocate(struct ion_heap *heap,
+ unsigned long size)
+{
+ struct ion_unmapped_heap *unmapped_heap =
+ container_of(heap, struct ion_unmapped_heap, heap);
+ unsigned long offset;
+
+ offset = gen_pool_alloc(unmapped_heap->pool, size);
+ if (!offset)
+ return ION_UNMAPPED_ALLOCATE_FAIL;
+
+ return offset;
+}
+
+static void ion_unmapped_free(struct ion_heap *heap, phys_addr_t addr,
+ unsigned long size)
+{
+ struct ion_unmapped_heap *unmapped_heap =
+ container_of(heap, struct ion_unmapped_heap, heap);
+
+ gen_pool_free(unmapped_heap->pool, addr, size);
+}
+
+static int ion_unmapped_heap_allocate(struct ion_heap *heap,
+ struct ion_buffer *buf
Re: [PATCH 11/14] staging: android: ion: Allow heap name to be null
On 1/16/19 9:12 AM, Andrew F. Davis wrote: On 1/16/19 9:28 AM, Brian Starkey wrote: Hi Andrew, On Fri, Jan 11, 2019 at 12:05:20PM -0600, Andrew F. Davis wrote: The heap name can be used for debugging but otherwise does not seem to be required and no other part of the code will fail if left NULL except here. We can make it required and check for it at some point, for now lets just prevent this from causing a NULL pointer exception. I'm not so keen on this one. In the "new" API with heap querying, the name string is the only way to identify the heap. I think Laura mentioned at XDC2017 that it was expected that userspace should use the strings to find the heap they want. Right now the names are only for debug. I accidentally left the name null once and got a kernel crash. This is the only spot where it is needed so I fixed it up. The other option is to make the name mandatory and properly error out, I don't want to do that right now until the below discussion is had to see if names really do matter or not. Yes, the heap names are part of the query API and are the expected way to identify individual heaps for the API at the moment so having a null heap name is incorrect. The heap name seemed like the best way to identify heaps to userspace but if you have an alternative proposal I'd be interested. Thanks, Laura ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 12/14] staging: android: ion: Declare helpers for carveout and chunk heaps
On 1/18/19 1:59 AM, Greg Kroah-Hartman wrote: On Fri, Jan 11, 2019 at 12:05:21PM -0600, Andrew F. Davis wrote: When enabled the helpers functions for creating carveout and chunk heaps should have declarations in the ION header. Why? No one calls these from what I can tell. Which makes me believe we should just delete the drivers/staging/android/ion/ion_carveout_heap.c and drivers/staging/android/ion/ion_chunk_heap.c files as there are no in-tree users? Any objection to me doing that? thanks, greg k-h I'd rather not delete it quite yet. Part of this entire thread is a discussion on how to let those heaps and associated function actually be called in some way in tree. I expect them to either get called in tree or be replaced. Thanks, Laura ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 00/14] Misc ION cleanups and adding unmapped heap
On 1/16/19 8:05 AM, Andrew F. Davis wrote: On 1/15/19 12:58 PM, Laura Abbott wrote: On 1/15/19 9:47 AM, Andrew F. Davis wrote: On 1/14/19 8:39 PM, Laura Abbott wrote: On 1/11/19 10:05 AM, Andrew F. Davis wrote: Hello all, This is a set of (hopefully) non-controversial cleanups for the ION framework and current set of heaps. These were found as I start to familiarize myself with the framework to help in whatever way I can in getting all this up to the standards needed for de-staging. I would like to get some ideas of what is left to work on to get ION out of staging. Has there been some kind of agreement on what ION should eventually end up being? To me it looks like it is being whittled away at to it's most core functions. To me that is looking like being a DMA-BUF user-space front end, simply advertising available memory backings in a system and providing allocations as DMA-BUF handles. If this is the case then it looks close to being ready to me at least, but I would love to hear any other opinions and concerns. Yes, at this point the only functionality that people are really depending on is the ability to allocate a dma_buf easily from userspace. Back to this patchset, the last patch may be a bit different than the others, it adds an unmapped heaps type and creation helper. I wanted to get this in to show off another heap type and maybe some issues we may have with the current ION framework. The unmapped heap is used when the backing memory should not (or cannot) be touched. Currently this kind of heap is used for firewalled secure memory that can be allocated like normal heap memory but only used by secure devices (OP-TEE, crypto HW, etc). It is basically just copied from the "carveout" heap type with the only difference being it is not mappable to userspace and we do not clear the memory (as we should not map it either). So should this really be a new heap type? Or maybe advertised as a carveout heap but with an additional allocation flag? Perhaps we do away with "types" altogether and just have flags, coherent/non-coherent, mapped/unmapped, etc. Maybe more thinking will be needed afterall.. So the cleanup looks okay (I need to finish reviewing) but I'm not a fan of adding another heaptype without solving the problem of adding some sort of devicetree binding or other method of allocating and placing Ion heaps. That plus uncached buffers are one of the big open problems that need to be solved for destaging Ion. See https://lore.kernel.org/lkml/20181120164636.jcw7li2uaa3cmwc3@DESKTOP-E1NTVVP.localdomain/ for some background on that problem. I'm under the impression that adding heaps like carveouts/chunk will be rather system specific and so do not lend themselves well to a universal DT style exporter. For instance a carveout memory space can be reported by a device at runtime, then the driver managing that device should go and use the carveout heap helpers to export that heap. If this is the case then I'm not sure it is a problem for the ION core framework to solve, but rather the users of it to figure out how best to create the various heaps. All Ion needs to do is allow exporting and advertising them IMHO. I think it is a problem for the Ion core framework to take care of. Ion is useless if you don't actually have the heaps. Nobody has actually gotten a full Ion solution end-to-end with a carveout heap working in mainline because any proposals have been rejected. I think we need at least one example in mainline of how creating a carveout heap would work. In our evil vendor trees we have several examples. The issue being that Ion is still staging and attempts for generic DT heap definitions haven't seemed to go so well. So for now we just keep it specific to our platforms until upstream makes a direction decision. Yeah, it's been a bit of a chicken and egg in that this has been blocking Ion getting out of staging but we don't actually have in-tree users because it's still in staging. Thanks for the background thread link, I've been looking for some info on current status of all this and "ion" is a bit hard to search the lists for. The core reason for posting this cleanup series is to throw my hat into the ring of all this Ion work and start getting familiar with the pending issues. The last two patches are not all that important to get in right now. In that thread you linked above, it seems we may have arrived at a similar problem for different reasons. I think the root issue is the Ion core makes too many assumptions about the heap memory. My proposal would be to allow the heap exporters more control over the DMA-BUF ops, maybe even going as far as letting them provide their own complete struct dma_buf_ops. Let me give an example where I think this is going to be useful. We have the classic constraint solving problem on our SoCs. Our SoCs are full of various coherent and non-coherent de
Re: [PATCH 13/14] staging: android: ion: Do not sync CPU cache on map/unmap
On 1/17/19 8:13 AM, Andrew F. Davis wrote:
On 1/16/19 4:48 PM, Liam Mark wrote:
On Wed, 16 Jan 2019, Andrew F. Davis wrote:
On 1/15/19 1:05 PM, Laura Abbott wrote:
On 1/15/19 10:38 AM, Andrew F. Davis wrote:
On 1/15/19 11:45 AM, Liam Mark wrote:
On Tue, 15 Jan 2019, Andrew F. Davis wrote:
On 1/14/19 11:13 AM, Liam Mark wrote:
On Fri, 11 Jan 2019, Andrew F. Davis wrote:
Buffers may not be mapped from the CPU so skip cache maintenance
here.
Accesses from the CPU to a cached heap should be bracketed with
{begin,end}_cpu_access calls so maintenance should not be needed
anyway.
Signed-off-by: Andrew F. Davis
---
drivers/staging/android/ion/ion.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 14e48f6eb734..09cb5a8e2b09 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -261,8 +261,8 @@ static struct sg_table *ion_map_dma_buf(struct
dma_buf_attachment *attachment,
table = a->table;
- if (!dma_map_sg(attachment->dev, table->sgl, table->nents,
- direction))
+ if (!dma_map_sg_attrs(attachment->dev, table->sgl, table->nents,
+ direction, DMA_ATTR_SKIP_CPU_SYNC))
Unfortunately I don't think you can do this for a couple reasons.
You can't rely on {begin,end}_cpu_access calls to do cache
maintenance.
If the calls to {begin,end}_cpu_access were made before the call to
dma_buf_attach then there won't have been a device attached so the
calls
to {begin,end}_cpu_access won't have done any cache maintenance.
That should be okay though, if you have no attachments (or all
attachments are IO-coherent) then there is no need for cache
maintenance. Unless you mean a sequence where a non-io-coherent device
is attached later after data has already been written. Does that
sequence need supporting?
Yes, but also I think there are cases where CPU access can happen before
in Android, but I will focus on later for now.
DMA-BUF doesn't have to allocate the backing
memory until map_dma_buf() time, and that should only happen after all
the devices have attached so it can know where to put the buffer. So we
shouldn't expect any CPU access to buffers before all the devices are
attached and mapped, right?
Here is an example where CPU access can happen later in Android.
Camera device records video -> software post processing -> video device
(who does compression of raw data) and writes to a file
In this example assume the buffer is cached and the devices are not
IO-coherent (quite common).
This is the start of the problem, having cached mappings of memory that
is also being accessed non-coherently is going to cause issues one way
or another. On top of the speculative cache fills that have to be
constantly fought back against with CMOs like below; some coherent
interconnects behave badly when you mix coherent and non-coherent access
(snoop filters get messed up).
The solution is to either always have the addresses marked non-coherent
(like device memory, no-map carveouts), or if you really want to use
regular system memory allocated at runtime, then all cached mappings of
it need to be dropped, even the kernel logical address (area as painful
as that would be).
I agree it's broken, hence my desire to remove it :)
The other problem is that uncached buffers are being used for
performance reason so anything that would involve getting
rid of the logical address would probably negate any performance
benefit.
I wouldn't go as far as to remove them just yet.. Liam seems pretty
adamant that they have valid uses. I'm just not sure performance is one
of them, maybe in the case of software locks between devices or
something where there needs to be a lot of back and forth interleaved
access on small amounts of data?
I wasn't aware that ARM considered this not supported, I thought it was
supported but they advised against it because of the potential performance
impact.
Not sure what you mean by "this" being not supported, do you mean mixed
attribute mappings? If so, it will certainly cause problems, and the
problems will change from platform to platform, avoid at all costs is my
understanding of ARM's position.
This is after all supported in the DMA APIs and up until now devices have
been successfully commercializing with this configurations, and I think
they will continue to commercialize with these configurations for quite a
while.
Use of uncached memory mappings are almost always wrong in my experience
and are used to work around some bug or because the user doesn't want to
implement proper CMOs. Counter examples welcome.
It would be really unfortunate if support was removed as I think that
would drive clients away from using upstream ION.
I'm not petitioning to remove support, but at very least lets reve
Re: [PATCH 1/4] staging: android: ion: Support cpu access during dma_buf_detach
On 1/18/19 10:37 AM, Liam Mark wrote:
Often userspace doesn't know when the kernel will be calling dma_buf_detach
on the buffer.
If userpace starts its CPU access at the same time as the sg list is being
freed it could end up accessing the sg list after it has been freed.
Thread AThread B
- DMA_BUF_IOCTL_SYNC IOCT
- ion_dma_buf_begin_cpu_access
- list_for_each_entry
- ion_dma_buf_detatch
- free_duped_table
- dma_sync_sg_for_cpu
Fix this by getting the ion_buffer lock before freeing the sg table memory.
Fixes: 2a55e7b5e544 ("staging: android: ion: Call dma_map_sg for syncing and
mapping")
Signed-off-by: Liam Mark
---
drivers/staging/android/ion/ion.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index a0802de8c3a1..6f5afab7c1a1 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -248,10 +248,10 @@ static void ion_dma_buf_detatch(struct dma_buf *dmabuf,
struct ion_dma_buf_attachment *a = attachment->priv;
struct ion_buffer *buffer = dmabuf->priv;
- free_duped_table(a->table);
mutex_lock(&buffer->lock);
list_del(&a->list);
mutex_unlock(&buffer->lock);
+ free_duped_table(a->table);
kfree(a);
}
Acked-by: Laura Abbott
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 13/14] staging: android: ion: Do not sync CPU cache on map/unmap
On 1/18/19 12:43 PM, Andrew F. Davis wrote:
On 1/18/19 2:31 PM, Laura Abbott wrote:
On 1/17/19 8:13 AM, Andrew F. Davis wrote:
On 1/16/19 4:48 PM, Liam Mark wrote:
On Wed, 16 Jan 2019, Andrew F. Davis wrote:
On 1/15/19 1:05 PM, Laura Abbott wrote:
On 1/15/19 10:38 AM, Andrew F. Davis wrote:
On 1/15/19 11:45 AM, Liam Mark wrote:
On Tue, 15 Jan 2019, Andrew F. Davis wrote:
On 1/14/19 11:13 AM, Liam Mark wrote:
On Fri, 11 Jan 2019, Andrew F. Davis wrote:
Buffers may not be mapped from the CPU so skip cache maintenance
here.
Accesses from the CPU to a cached heap should be bracketed with
{begin,end}_cpu_access calls so maintenance should not be needed
anyway.
Signed-off-by: Andrew F. Davis
---
drivers/staging/android/ion/ion.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 14e48f6eb734..09cb5a8e2b09 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -261,8 +261,8 @@ static struct sg_table
*ion_map_dma_buf(struct
dma_buf_attachment *attachment,
table = a->table;
- if (!dma_map_sg(attachment->dev, table->sgl,
table->nents,
- direction))
+ if (!dma_map_sg_attrs(attachment->dev, table->sgl,
table->nents,
+ direction, DMA_ATTR_SKIP_CPU_SYNC))
Unfortunately I don't think you can do this for a couple reasons.
You can't rely on {begin,end}_cpu_access calls to do cache
maintenance.
If the calls to {begin,end}_cpu_access were made before the
call to
dma_buf_attach then there won't have been a device attached so the
calls
to {begin,end}_cpu_access won't have done any cache maintenance.
That should be okay though, if you have no attachments (or all
attachments are IO-coherent) then there is no need for cache
maintenance. Unless you mean a sequence where a non-io-coherent
device
is attached later after data has already been written. Does that
sequence need supporting?
Yes, but also I think there are cases where CPU access can happen
before
in Android, but I will focus on later for now.
DMA-BUF doesn't have to allocate the backing
memory until map_dma_buf() time, and that should only happen
after all
the devices have attached so it can know where to put the
buffer. So we
shouldn't expect any CPU access to buffers before all the
devices are
attached and mapped, right?
Here is an example where CPU access can happen later in Android.
Camera device records video -> software post processing -> video
device
(who does compression of raw data) and writes to a file
In this example assume the buffer is cached and the devices are not
IO-coherent (quite common).
This is the start of the problem, having cached mappings of memory
that
is also being accessed non-coherently is going to cause issues one
way
or another. On top of the speculative cache fills that have to be
constantly fought back against with CMOs like below; some coherent
interconnects behave badly when you mix coherent and non-coherent
access
(snoop filters get messed up).
The solution is to either always have the addresses marked
non-coherent
(like device memory, no-map carveouts), or if you really want to use
regular system memory allocated at runtime, then all cached
mappings of
it need to be dropped, even the kernel logical address (area as
painful
as that would be).
I agree it's broken, hence my desire to remove it :)
The other problem is that uncached buffers are being used for
performance reason so anything that would involve getting
rid of the logical address would probably negate any performance
benefit.
I wouldn't go as far as to remove them just yet.. Liam seems pretty
adamant that they have valid uses. I'm just not sure performance is one
of them, maybe in the case of software locks between devices or
something where there needs to be a lot of back and forth interleaved
access on small amounts of data?
I wasn't aware that ARM considered this not supported, I thought it was
supported but they advised against it because of the potential
performance
impact.
Not sure what you mean by "this" being not supported, do you mean mixed
attribute mappings? If so, it will certainly cause problems, and the
problems will change from platform to platform, avoid at all costs is my
understanding of ARM's position.
This is after all supported in the DMA APIs and up until now devices
have
been successfully commercializing with this configurations, and I think
they will continue to commercialize with these configurations for
quite a
while.
Use of uncached memory mappings are almost always wrong in my experience
and are used to work around some bug or because the user doesn't want to
implement proper CMOs. Counter examples welcome.
It would be really unfortunate if support was removed as I think that
would drive clients away from
Re: [PATCH 3/4] dma-buf: add support for mapping with dma mapping attributes
On 1/18/19 10:37 AM, Liam Mark wrote:
Add support for configuring dma mapping attributes when mapping
and unmapping memory through dma_buf_map_attachment and
dma_buf_unmap_attachment.
Signed-off-by: Liam Mark
---
include/linux/dma-buf.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
index 58725f890b5b..59bf33e09e2d 100644
--- a/include/linux/dma-buf.h
+++ b/include/linux/dma-buf.h
@@ -308,6 +308,8 @@ struct dma_buf {
* @dev: device attached to the buffer.
* @node: list of dma_buf_attachment.
* @priv: exporter specific attachment data.
+ * @dma_map_attrs: DMA mapping attributes to be used in
+ *dma_buf_map_attachment() and dma_buf_unmap_attachment().
*
* This structure holds the attachment information between the dma_buf buffer
* and its user device(s). The list contains one attachment struct per device
@@ -323,6 +325,7 @@ struct dma_buf_attachment {
struct device *dev;
struct list_head node;
void *priv;
+ unsigned long dma_map_attrs;
};
/**
Did you miss part of this patch? This only adds it to the structure but doesn't
add it to any API. The same commment applies to the follow up patch,
I don't quite see how it's being used.
Thanks,
Laura
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/4] dma-buf: add support for mapping with dma mapping attributes
On 1/18/19 1:32 PM, Liam Mark wrote:
On Fri, 18 Jan 2019, Laura Abbott wrote:
On 1/18/19 10:37 AM, Liam Mark wrote:
Add support for configuring dma mapping attributes when mapping
and unmapping memory through dma_buf_map_attachment and
dma_buf_unmap_attachment.
Signed-off-by: Liam Mark
---
include/linux/dma-buf.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
index 58725f890b5b..59bf33e09e2d 100644
--- a/include/linux/dma-buf.h
+++ b/include/linux/dma-buf.h
@@ -308,6 +308,8 @@ struct dma_buf {
* @dev: device attached to the buffer.
* @node: list of dma_buf_attachment.
* @priv: exporter specific attachment data.
+ * @dma_map_attrs: DMA mapping attributes to be used in
+ *dma_buf_map_attachment() and dma_buf_unmap_attachment().
*
* This structure holds the attachment information between the dma_buf
buffer
* and its user device(s). The list contains one attachment struct per
device
@@ -323,6 +325,7 @@ struct dma_buf_attachment {
struct device *dev;
struct list_head node;
void *priv;
+ unsigned long dma_map_attrs;
};
/**
Did you miss part of this patch? This only adds it to the structure but
doesn't
add it to any API. The same commment applies to the follow up patch,
I don't quite see how it's being used.
Were you asking for a cleaner DMA-buf API to set this field or were you
asking for a change to an upstream client to make use of this field?
I have clients set the dma_map_attrs field directly on their
dma_buf_attachment struct before calling dma_buf_map_attachment (if they
need this functionality).
Of course this is all being used in Android for out of tree drivers, but
I assume it is just as useful to everyone else who has cached ION buffers
which aren't always accessed by the CPU.
My understanding is that AOSP Android on Hikey 960 also is currently
suffering from too many CMOs due to dma_map_attachemnt always applying
CMOs, so this support should help them avoid it.
A I see how you intend this to be used now! I was missing
that clients would do attachment->dma_map_attrs = blah
and that was how it would be stored as opposed to passing
it in at the top level for dma_buf_map. I'll give this some
more thought but I think it could work if Sumit is okay
with the approach.
Thanks,
Laura
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/4] dma-buf: add support for mapping with dma mapping attributes
On 1/19/19 2:25 AM, Christoph Hellwig wrote: On Fri, Jan 18, 2019 at 10:37:46AM -0800, Liam Mark wrote: Add support for configuring dma mapping attributes when mapping and unmapping memory through dma_buf_map_attachment and dma_buf_unmap_attachment. Signed-off-by: Liam Mark And who is going to decide which ones to pass? And who documents which ones are safe? I'd much rather have explicit, well documented dma-buf flags that might get translated to the DMA API flags, which are not error checked, not very well documented and way to easy to get wrong. I'm not sure having flags in dma-buf really solves anything given drivers can use the attributes directly with dma_map anyway, which is what we're looking to do. The intention is for the driver creating the dma_buf attachment to have the knowledge of which flags to use. Thanks, Laura ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: Use low_order_gfp_flags for smaller allocations
On 2/11/19 11:09 PM, Jing Xia wrote:
gfp_flags is always set high_order_gfp_flags even if allocations of
order 0 are made.But for smaller allocations, the system should be able
to reclaim some memory.
Signed-off-by: Jing Xia
Reviewed-by: Yuming Han
Reviewed-by: Zhaoyang Huang
Reviewed-by: Orson Zhai
---
drivers/staging/android/ion/ion_system_heap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/android/ion/ion_system_heap.c
b/drivers/staging/android/ion/ion_system_heap.c
index 0383f75..20f2103 100644
--- a/drivers/staging/android/ion/ion_system_heap.c
+++ b/drivers/staging/android/ion/ion_system_heap.c
@@ -223,10 +223,10 @@ static void ion_system_heap_destroy_pools(struct
ion_page_pool **pools)
static int ion_system_heap_create_pools(struct ion_page_pool **pools)
{
int i;
- gfp_t gfp_flags = low_order_gfp_flags;
for (i = 0; i < NUM_ORDERS; i++) {
struct ion_page_pool *pool;
+ gfp_t gfp_flags = low_order_gfp_flags;
if (orders[i] > 4)
gfp_flags = high_order_gfp_flags;
This was already submitted in
https://lore.kernel.org/lkml/1549004386-38778-1-git-send-email-saberlily@hisilicon.com/T/#u
(I'm also very behind on Ion e-mail and need to catch up...)
Laura
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: android: ion: Allocate from heap ID directly without mask
On 2/15/19 11:01 AM, John Stultz wrote: On Fri, Feb 15, 2019 at 2:51 AM Brian Starkey wrote: Hi John, On Thu, Feb 14, 2019 at 09:38:29AM -0800, John Stultz wrote: [snip] Some thoughts, as this ABI break has the potential to be pretty painful. 1) Unfortunately, this ABI is exposed *through* libion via ion_alloc/ion_alloc_fd out to gralloc implementations. Which means it will have a wider impact to vendor userland code. I figured libion could fairly easily loop through all the set bits in heap_mask and call the ioctl for each until it succeeds. That preserves the old behaviour from the libion clients' perspective. Potentially, though that implicitly still caps the heaps to 32. So I'm not sure what the net benefit would be. 2) For patches that cause ABI breaks, it might be good to make it clear in the commit what the userland impact looks like in userspace, possibly with an example, so the poor folks who bisect down the change as breaking their system in a year or so have a clear example as to what they need to change in their code. 3) Also, its not clear how a given userland should distinguish between the different ABIs. We already have logic in libion to distinguish between pre-4.12 legacy and post-4.12 implementations (using implicit ion_free() behavior). I don't see any such check we can make with this code. Adding another ABI version may require we provide an actual interface version ioctl. A slightly fragile/ugly approach might be to attempt a small allocation with a heap_mask of 0x. On an "old" implementation, you'd expect that to succeed, whereas it would/could be made to fail in the "new" one. Yea I think having a proper ION_IOC_VERSION is going to be necessary. I'm hoping to send out an ugly first stab at the kernel side for switching to per-heap devices (with a config for keeping /dev/ion for legacy compat), which I hope will address the core issue this patch does (moving away from heap masks to specifically requested heaps). thanks -john Arnd/Greg said no to this last time I tried back in 2016 https://lore.kernel.org/lkml/1472769644-11039-4-git-send-email-labb...@redhat.com/T/#u ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: fix sys heap pool's gfp_flags
On 1/31/19 10:59 PM, Qing Xia wrote:
In the first loop, gfp_flags will be modified to high_order_gfp_flags,
and there will be no chance to change back to low_order_gfp_flags.
Fixes: e7f63771 ("ION: Sys_heap: Add cached pool to spead up cached buffer
alloc")
Signed-off-by: Qing Xia
---
drivers/staging/android/ion/ion_system_heap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/android/ion/ion_system_heap.c
b/drivers/staging/android/ion/ion_system_heap.c
index 0383f75..20f2103 100644
--- a/drivers/staging/android/ion/ion_system_heap.c
+++ b/drivers/staging/android/ion/ion_system_heap.c
@@ -223,10 +223,10 @@ static void ion_system_heap_destroy_pools(struct
ion_page_pool **pools)
static int ion_system_heap_create_pools(struct ion_page_pool **pools)
{
int i;
- gfp_t gfp_flags = low_order_gfp_flags;
for (i = 0; i < NUM_ORDERS; i++) {
struct ion_page_pool *pool;
+ gfp_t gfp_flags = low_order_gfp_flags;
if (orders[i] > 4)
gfp_flags = high_order_gfp_flags;
Acked-by: Laura Abbott
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 13/14] staging: android: ion: Do not sync CPU cache on map/unmap
On 1/24/19 8:44 AM, Brian Starkey wrote:
On Thu, Jan 24, 2019 at 10:04:46AM -0600, Andrew F. Davis wrote:
On 1/23/19 11:11 AM, Brian Starkey wrote:
[snip]
I'm very new to all this, so any pointers to history in this area are
appreciated.
[snip]
In case you didn't come across it already, the effort which seems to
have gained the most "air-time" recently is
https://github.com/cubanismo/allocator, which is still a userspace
module (perhaps some concepts from there could go into the kernel?),
but makes some attempts at generic constraint solving. It's also not
really moving anywhere at the moment.
Very interesting, I'm going to have to stare at this for a bit.
In which case, some reading material that might be of interest :-)
https://www.x.org/wiki/Events/XDC2016/Program/Unix_Device_Memory_Allocation.pdf
https://www.x.org/wiki/Events/XDC2017/jones_allocator.pdf
https://lists.freedesktop.org/archives/mesa-dev/2017-November/177632.html
-Brian
In some respects this is more a question of "what is the purpose
of Ion". Once upon a time, Ion was going to do constraint solving
but that never really happened and I figured Ion would get deprecated.
People keep coming out of the woodwork with new uses for Ion so
its stuck around. This is why I've primarily focused on Ion as a
framework for exposing available memory types to userspace and leave
the constraint solving to someone else, since that's what most
users seem to want out of Ion ("I know I want memory type X please
give it to me"). That's not to say that this was a perfect or
even the correct approach, just what made the most sense based
on users.
Thanks,
Laura
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] driver : staging : ion: optimization for decreasing memory fragmentaion
On 3/20/19 7:23 AM, Vlastimil Babka wrote:
You should have CC'd the ION maintainers/lists per
./scripts/get_maintainer.pl - CCing now.
On 3/14/19 12:06 PM, Zhaoyang Huang wrote:
From: Zhaoyang Huang
Two action for this patch:
1. set a batch size for system heap's shrinker, which can have it buffer
reasonable page blocks in pool for future allocation.
2. reverse the order sequence when free page blocks, the purpose is also
to have system heap keep as more big blocks as it can.
By testing on an android system with 2G RAM, the changes with setting
batch = 48MB can help reduce the fragmentation obviously and improve
big block allocation speed for 15%.
Signed-off-by: Zhaoyang Huang
---
drivers/staging/android/ion/ion_heap.c| 12 +++-
drivers/staging/android/ion/ion_system_heap.c | 2 +-
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/android/ion/ion_heap.c
b/drivers/staging/android/ion/ion_heap.c
index 31db510..9e9caf2 100644
--- a/drivers/staging/android/ion/ion_heap.c
+++ b/drivers/staging/android/ion/ion_heap.c
@@ -16,6 +16,8 @@
#include
#include "ion.h"
+unsigned long ion_heap_batch = 0;
+
void *ion_heap_map_kernel(struct ion_heap *heap,
struct ion_buffer *buffer)
{
@@ -303,7 +305,15 @@ int ion_heap_init_shrinker(struct ion_heap *heap)
heap->shrinker.count_objects = ion_heap_shrink_count;
heap->shrinker.scan_objects = ion_heap_shrink_scan;
heap->shrinker.seeks = DEFAULT_SEEKS;
- heap->shrinker.batch = 0;
+ heap->shrinker.batch = ion_heap_batch;
return register_shrinker(&heap->shrinker);
}
+
+static int __init ion_system_heap_batch_init(char *arg)
+{
+ion_heap_batch = memparse(arg, NULL);
+
+ return 0;
+}
+early_param("ion_batch", ion_system_heap_batch_init);
diff --git a/drivers/staging/android/ion/ion_system_heap.c
b/drivers/staging/android/ion/ion_system_heap.c
index 701eb9f..d249f8d 100644
--- a/drivers/staging/android/ion/ion_system_heap.c
+++ b/drivers/staging/android/ion/ion_system_heap.c
@@ -182,7 +182,7 @@ static int ion_system_heap_shrink(struct ion_heap *heap,
gfp_t gfp_mask,
if (!nr_to_scan)
only_scan = 1;
- for (i = 0; i < NUM_ORDERS; i++) {
+ for (i = NUM_ORDERS - 1; i >= 0; i--) {
pool = sys_heap->pools[i];
if (only_scan) {
We're in the process of significantly reworking Ion so I
don't think it makes sense to take these as we work to
get things out of staging. You can resubmit this later,
but when you do please split this into two separate
patches since it's actually two independent changes.
Thanks,
Laura
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: refactory ion_alloc for kernel driver use
On 3/29/19 11:40 AM, Zeng Tao wrote:
There are two reasons for this patch:
1. There are some potential requirements for ion_alloc in kernel space,
some media drivers need to allocate media buffers from ion instead of
buddy or dma framework, this is more convient and clean very for media
drivers. And In that case, ion is the only media buffer provider, it's
more easier to maintain.
2. Fd is only needed by user processes, not the kernel space, so dma_buf
should be returned instead of fd for kernel space, and dma_buf_fd should
be called only for userspace api.
I really want to just NAK this because it doesn't seem like something
that's necessary. The purpose of Ion is to provide buffers to userspace
because there's no other way for userspace to get access to the memory.
The kernel already has other APIs to access the memory. This also
complicates the re-work that's been happening where the requirement
is only userspace.
Can you be more detailed about which media drivers you are referring
to and why they can't just use other APIs?
Signed-off-by: Zeng Tao
---
drivers/staging/android/ion/ion.c | 32 +---
1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 92c2914..e93fb49 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -387,13 +387,13 @@ static const struct dma_buf_ops dma_buf_ops = {
.unmap = ion_dma_buf_kunmap,
};
-static int ion_alloc(size_t len, unsigned int heap_id_mask, unsigned int flags)
+struct dma_buf *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);
- int fd;
struct dma_buf *dmabuf;
pr_debug("%s: len %zu heap_id_mask %u flags %x\n", __func__,
@@ -407,7 +407,7 @@ static int ion_alloc(size_t len, unsigned int heap_id_mask,
unsigned int flags)
len = PAGE_ALIGN(len);
if (!len)
- return -EINVAL;
+ return ERR_PTR(-EINVAL);
down_read(&dev->lock);
plist_for_each_entry(heap, &dev->heaps, node) {
@@ -421,10 +421,10 @@ static int ion_alloc(size_t len, unsigned int
heap_id_mask, unsigned int flags)
up_read(&dev->lock);
if (!buffer)
- return -ENODEV;
+ return ERR_PTR(-ENODEV);
if (IS_ERR(buffer))
- return PTR_ERR(buffer);
+ return ERR_PTR(PTR_ERR(buffer));
exp_info.ops = &dma_buf_ops;
exp_info.size = buffer->size;
@@ -432,17 +432,12 @@ static int ion_alloc(size_t len, unsigned int
heap_id_mask, unsigned int flags)
exp_info.priv = buffer;
dmabuf = dma_buf_export(&exp_info);
- if (IS_ERR(dmabuf)) {
+ if (IS_ERR(dmabuf))
_ion_buffer_destroy(buffer);
- return PTR_ERR(dmabuf);
- }
- fd = dma_buf_fd(dmabuf, O_CLOEXEC);
- if (fd < 0)
- dma_buf_put(dmabuf);
-
- return fd;
+ return dmabuf;
}
+EXPORT_SYMBOL(ion_alloc);
static int ion_query_heaps(struct ion_heap_query *query)
{
@@ -539,12 +534,19 @@ static long ion_ioctl(struct file *filp, unsigned int
cmd, unsigned long arg)
case ION_IOC_ALLOC:
{
int fd;
+ struct dma_buf *dmabuf;
- fd = ion_alloc(data.allocation.len,
+ dmabuf = ion_alloc(data.allocation.len,
data.allocation.heap_id_mask,
data.allocation.flags);
- if (fd < 0)
+ if (IS_ERR(dmabuf))
+ return PTR_ERR(dmabuf);
+
+ fd = dma_buf_fd(dmabuf, O_CLOEXEC);
+ if (fd < 0) {
+ dma_buf_put(dmabuf);
return fd;
+ }
data.allocation.fd = fd;
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: refactory ion_alloc for kernel driver use
On 3/29/19 7:26 PM, Zengtao (B) wrote:
Hi laura:
-Original Message-
From: Laura Abbott [mailto:labb...@redhat.com]
Sent: Friday, March 29, 2019 9:27 PM
To: Zengtao (B) ; sumit.sem...@linaro.org
Cc: Greg Kroah-Hartman ; Arve Hjønnevåg
; Todd Kjos ; Martijn Coenen
; Joel Fernandes ;
Christian Brauner ; de...@driverdev.osuosl.org;
dri-de...@lists.freedesktop.org; linaro-mm-...@lists.linaro.org;
linux-ker...@vger.kernel.org
Subject: Re: [PATCH] staging: android: ion: refactory ion_alloc for kernel
driver use
On 3/29/19 11:40 AM, Zeng Tao wrote:
There are two reasons for this patch:
1. There are some potential requirements for ion_alloc in kernel
space, some media drivers need to allocate media buffers from ion
instead of buddy or dma framework, this is more convient and clean
very for media drivers. And In that case, ion is the only media buffer
provider, it's more easier to maintain.
2. Fd is only needed by user processes, not the kernel space, so
dma_buf should be returned instead of fd for kernel space, and
dma_buf_fd should be called only for userspace api.
I really want to just NAK this because it doesn't seem like something
that's necessary. The purpose of Ion is to provide buffers to userspace
because there's no other way for userspace to get access to the memory.
The kernel already has other APIs to access the memory. This also
complicates the re-work that's been happening where the requirement is
only userspace.
Can you be more detailed about which media drivers you are referring to
and why they can't just use other APIs?
I think I 've got your point, the ION is designed for usespace, but for kernel
space, we are really lacking of someone which plays the same role,(allocate
media memory, share the memory using dma_buf, provide debug and statistics
for media memory).
In fact, for kernel space, we have the dma framework, dma-buf, etc..
And we can work on top of such apis, but some duplicate jobs(everyone has
to maintain its own buffer sharing, debug and statistics).
So we need to have some to do the common things(ION's the best choice now)
Keep in mind that Ion is a thin shell of what it was as most of the
debugging and statistics was removed because it was buggy. Most of that
should end up going at the dma_buf layer since it's really a dma_buf allocation
API.
When the ION was introduced, a lot of media memory frameworks existed, the
dma framework was not so good, so ION heaps, integrated buffer sharing,
statistics
and usespace api were the required features, but now dma framework is more
powerful,
we don't even need ION heaps now, but the userspace api, buffer sharing,
statistics are
still needed, and the buffer sharing, statistics can be re-worked and export to
kernel space,
not only used by userspace, , and that is my point.
I see what you are getting at but I don't think the same thing
applies to the kernel as it does userspace. We can enforce a
single way of using the dma_buf fd in userspace but the kernel
has a variety of ways to use dma_buf because each driver and
framework has its own needs. I'm still not convinced that adding
Ion APIs in the kernel is the right option since as you point out
we don't really need the heaps. That mostly leaves Ion as a wrapper
to handle doing the export. Maybe we could benefit from that
but I think it might require more thought.
I'd rather see a proposal in the media API itself showing what
you think is necessary but without using Ion. That would be
a good start so we could fully review what might make sense to
pull out of Ion into something common.
Thanks,
Laura
Signed-off-by: Zeng Tao
---
drivers/staging/android/ion/ion.c | 32
+---
1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 92c2914..e93fb49 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -387,13 +387,13 @@ static const struct dma_buf_ops
dma_buf_ops = {
.unmap = ion_dma_buf_kunmap,
};
-static int ion_alloc(size_t len, unsigned int heap_id_mask, unsigned
int flags)
+struct dma_buf *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);
- int fd;
struct dma_buf *dmabuf;
pr_debug("%s: len %zu heap_id_mask %u flags %x\n", __func__,
@@
-407,7 +407,7 @@ static int ion_alloc(size_t len, unsigned int
heap_id_mask, unsigned int flags)
len = PAGE_ALIGN(len);
if (!len)
- return -EINVAL;
+ return ERR_PTR(-EINVAL);
down_read(&dev->lock);
plist_for_each_entry(heap, &dev->heaps, node) { @@ -421,1
Re: [PATCH] staging: android: ion: cma heap: Limit size of allocated buffer
On 8/23/19 10:28 PM, Alexey Skidanov wrote:
In ion_cma_heap, the allocated buffer is represented by a single
struct scatterlist instance. The length field of this struct is
32 bit, hence the maximal size of requested buffer should be
less than 4GB.
The len paramer of the allocation function is 64 bit (on 64 bit systems).
Hence the requested size might be greater than 4GB and in this case
the field length of the struct scatterlist is initialized incorrectly.
To fix this, we check that requested size may fit into
the field length of the struct scatterlist
Is this a real issue that's actually possible to hit? Allocating
more than a 4GB region of CMA seems ill advised and likely to
throw off all the accounting.
Signed-off-by: Alexey Skidanov
---
drivers/staging/android/ion/ion.h | 5 +
drivers/staging/android/ion/ion_cma_heap.c | 3 +++
2 files changed, 8 insertions(+)
diff --git a/drivers/staging/android/ion/ion.h
b/drivers/staging/android/ion/ion.h
index e291299..9dd7e20 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -21,6 +21,11 @@
#include "../uapi/ion.h"
+#define MAX_SCATTERLIST_LEN ({\
+ typeof(((struct scatterlist *)0)->length) v;\
+ v = -1;\
+ })
+
/**
* struct ion_buffer - metadata for a particular buffer
* @node: node in the ion_device buffers tree
diff --git a/drivers/staging/android/ion/ion_cma_heap.c
b/drivers/staging/android/ion/ion_cma_heap.c
index bf65e67..d069719 100644
--- a/drivers/staging/android/ion/ion_cma_heap.c
+++ b/drivers/staging/android/ion/ion_cma_heap.c
@@ -36,6 +36,9 @@ static int ion_cma_allocate(struct ion_heap *heap, struct
ion_buffer *buffer,
unsigned long align = get_order(size);
int ret;
+ if (size > MAX_SCATTERLIST_LEN)
+ return -EINVAL;
+
if (align > CONFIG_CMA_ALIGNMENT)
align = CONFIG_CMA_ALIGNMENT;
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: cma heap: Limit size of allocated buffer
On 8/26/19 3:55 AM, Alexey Skidanov wrote:
On 8/26/19 11:36 AM, Laura Abbott wrote:
On 8/23/19 10:28 PM, Alexey Skidanov wrote:
In ion_cma_heap, the allocated buffer is represented by a single
struct scatterlist instance. The length field of this struct is
32 bit, hence the maximal size of requested buffer should be
less than 4GB.
The len paramer of the allocation function is 64 bit (on 64 bit systems).
Hence the requested size might be greater than 4GB and in this case
the field length of the struct scatterlist is initialized incorrectly.
To fix this, we check that requested size may fit into
the field length of the struct scatterlist
Is this a real issue that's actually possible to hit? Allocating
more than a 4GB region of CMA seems ill advised and likely to
throw off all the accounting.
Yes. Not sure why it seems ill advised - most of the buffers are small or
middle size ones, but sometimes really huge one is requested.
Mostly I'm surprised allocating 4GB of CMA actually succeeds. When
I was last doing work on CMA, the reliability just wasn't there
for larger regions. It's been several years since then so maybe
things have changed. My concern about the accounting is that
most of the math is done such that there are a few CMA pages
vs more pages across the rest of the system which seems likely to
hit performance issues. CMA was designed with targets with smaller
memory footprints so anything that has 4GB of memory to go around is
much larger than what CMA was originally designed for.
The check added should only be applicable if we can reliably
allocate more than 4GB of CMA. If you can confirm you have a setup
where you are actually able to allocate 4GB of CMA, I'd rather
just have the check be for 4GB explicitly instead of tying
it to just the scatterlist length. It's a reasonable restriction
to make and it's easier to review.
Signed-off-by: Alexey Skidanov
---
drivers/staging/android/ion/ion.h | 5 +
drivers/staging/android/ion/ion_cma_heap.c | 3 +++
2 files changed, 8 insertions(+)
diff --git a/drivers/staging/android/ion/ion.h
b/drivers/staging/android/ion/ion.h
index e291299..9dd7e20 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -21,6 +21,11 @@
#include "../uapi/ion.h"
+#define MAX_SCATTERLIST_LEN ({\
+ typeof(((struct scatterlist *)0)->length) v;\
+ v = -1;\
+ })
+
/**
* struct ion_buffer - metadata for a particular buffer
* @node: node in the ion_device buffers tree
diff --git a/drivers/staging/android/ion/ion_cma_heap.c
b/drivers/staging/android/ion/ion_cma_heap.c
index bf65e67..d069719 100644
--- a/drivers/staging/android/ion/ion_cma_heap.c
+++ b/drivers/staging/android/ion/ion_cma_heap.c
@@ -36,6 +36,9 @@ static int ion_cma_allocate(struct ion_heap *heap, struct
ion_buffer *buffer,
unsigned long align = get_order(size);
int ret;
+ if (size > MAX_SCATTERLIST_LEN)
+ return -EINVAL;
+
if (align > CONFIG_CMA_ALIGNMENT)
align = CONFIG_CMA_ALIGNMENT;
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] ion_system_heap: support X86 archtecture
On 9/29/19 3:28 AM, jun.zh...@intel.com wrote:
From: zhang jun
we see tons of warning like:
[ 45.846872] x86/PAT: NDK MediaCodec_:3753 map pfn RAM range req
write-combining for [mem 0x1e7a8-0x1e7a87fff], got write-back
[ 45.848827] x86/PAT: .vorbis.decoder:4088 map pfn RAM range req
write-combining for [mem 0x1e7a58000-0x1e7a58fff], got write-back
[ 45.848875] x86/PAT: NDK MediaCodec_:3753 map pfn RAM range req
write-combining for [mem 0x1e7a48000-0x1e7a4], got write-back
[ 45.849403] x86/PAT: .vorbis.decoder:4088 map pfn RAM range
req write-combining for [mem 0x1e7a7-0x1e7a70fff], got write-back
check the kernel Documentation/x86/pat.txt, it says:
A. Exporting pages to users with remap_pfn_range, io_remap_pfn_range,
vm_insert_pfn
Drivers wanting to export some pages to userspace do it by using
mmap interface and a combination of
1) pgprot_noncached()
2) io_remap_pfn_range() or remap_pfn_range() or vm_insert_pfn()
With PAT support, a new API pgprot_writecombine is being added.
So, drivers can continue to use the above sequence, with either
pgprot_noncached() or pgprot_writecombine() in step 1, followed by step 2.
In addition, step 2 internally tracks the region as UC or WC in
memtype list in order to ensure no conflicting mapping.
Note that this set of APIs only works with IO (non RAM) regions.
If driver ants to export a RAM region, it has to do set_memory_uc() or
set_memory_wc() as step 0 above and also track the usage of those pages
and use set_memory_wb() before the page is freed to free pool.
the fix follow the pat document, do set_memory_wc() as step 0 and
use the set_memory_wb() before the page is freed.
All this work needs to be done on the new dma-buf heap rework and I
don't think it makes sense to put it on the staging version
https://lore.kernel.org/lkml/20190906184712.91980-1-john.stu...@linaro.org/
(I also continue to question the value of uncached buffers, especially on
x86)
Signed-off-by: he, bo
Signed-off-by: zhang jun
Signed-off-by: Bai, Jie A
---
drivers/staging/android/ion/ion_system_heap.c | 28 ++-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/android/ion/ion_system_heap.c
b/drivers/staging/android/ion/ion_system_heap.c
index b83a1d16bd89..d298b8194820 100644
--- a/drivers/staging/android/ion/ion_system_heap.c
+++ b/drivers/staging/android/ion/ion_system_heap.c
@@ -13,6 +13,7 @@
#include
#include
#include
+#include
#include "ion.h"
@@ -134,6 +135,13 @@ static int ion_system_heap_allocate(struct ion_heap *heap,
sg = table->sgl;
list_for_each_entry_safe(page, tmp_page, &pages, lru) {
sg_set_page(sg, page, page_size(page), 0);
+
+#ifdef CONFIG_X86
+ if (!(buffer->flags & ION_FLAG_CACHED))
+ set_memory_wc((unsigned long)page_address(sg_page(sg)),
+ PAGE_ALIGN(sg->length) >> PAGE_SHIFT);
+#endif
+
sg = sg_next(sg);
list_del(&page->lru);
}
@@ -162,8 +170,15 @@ static void ion_system_heap_free(struct ion_buffer *buffer)
if (!(buffer->private_flags & ION_PRIV_FLAG_SHRINKER_FREE))
ion_heap_buffer_zero(buffer);
- for_each_sg(table->sgl, sg, table->nents, i)
+ for_each_sg(table->sgl, sg, table->nents, i) {
+#ifdef CONFIG_X86
+ if (!(buffer->flags & ION_FLAG_CACHED))
+ set_memory_wb((unsigned long)page_address(sg_page(sg)),
+ PAGE_ALIGN(sg->length) >> PAGE_SHIFT);
+#endif
+
free_buffer_page(sys_heap, buffer, sg_page(sg));
+ }
sg_free_table(table);
kfree(table);
}
@@ -316,6 +331,12 @@ static int ion_system_contig_heap_allocate(struct ion_heap
*heap,
buffer->sg_table = table;
+#ifdef CONFIG_X86
+ if (!(buffer->flags & ION_FLAG_CACHED))
+ set_memory_wc((unsigned long)page_address(page),
+ PAGE_ALIGN(len) >> PAGE_SHIFT);
+#endif
+
return 0;
free_table:
@@ -334,6 +355,11 @@ static void ion_system_contig_heap_free(struct ion_buffer
*buffer)
unsigned long pages = PAGE_ALIGN(buffer->size) >> PAGE_SHIFT;
unsigned long i;
+#ifdef CONFIG_X86
+ if (!(buffer->flags & ION_FLAG_CACHED))
+ set_memory_wb((unsigned long)page_address(page), pages);
+#endif
+
for (i = 0; i < pages; i++)
__free_page(page + i);
sg_free_table(table);
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 04/15] staging/android/ion: delete dma_buf->kmap/unmap implemenation
On 11/18/19 5:35 AM, Daniel Vetter wrote:
There's no callers in-tree anymore.
For merging probably best to stuff this into drm-misc, since that's
where the dma-buf heaps will land too. And the resulting conflict
hopefully ensures that dma-buf heaps wont have a new ->kmap/unmap
implemenation.
Signed-off-by: Daniel Vetter
Cc: Laura Abbott
Cc: Sumit Semwal
Cc: de...@driverdev.osuosl.org
Cc: linaro-mm-...@lists.linaro.org
---
drivers/staging/android/ion/ion.c | 14 --
1 file changed, 14 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index e6b1ca141b93..bb4ededc1150 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -274,18 +274,6 @@ static void ion_dma_buf_release(struct dma_buf *dmabuf)
_ion_buffer_destroy(buffer);
}
-static void *ion_dma_buf_kmap(struct dma_buf *dmabuf, unsigned long offset)
-{
- struct ion_buffer *buffer = dmabuf->priv;
-
- return buffer->vaddr + offset * PAGE_SIZE;
-}
-
-static void ion_dma_buf_kunmap(struct dma_buf *dmabuf, unsigned long offset,
- void *ptr)
-{
-}
-
static int ion_dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
enum dma_data_direction direction)
{
@@ -349,8 +337,6 @@ static const struct dma_buf_ops dma_buf_ops = {
.detach = ion_dma_buf_detatch,
.begin_cpu_access = ion_dma_buf_begin_cpu_access,
.end_cpu_access = ion_dma_buf_end_cpu_access,
- .map = ion_dma_buf_kmap,
- .unmap = ion_dma_buf_kunmap,
};
static int ion_alloc(size_t len, unsigned int heap_id_mask, unsigned int flags)
Acked-by: Laura Abbott
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: Why does ion_cma_allocate need GFP_HIGHUSER | __GFP_ZERO flags?
On 11/12/2014 11:18 PM, Gioh Kim wrote: > Hi, > > I'm wondering why dma_alloc_coherent needs GFP_HIGHUSER | __GFP_ZERO flags in > ion_cma_allocate(). > > I think dma_alloc_coherent can go to __dma_alloc if the system has CMA area. > If __GFP_WAIT flag is set, it allocates memory from atomic pool. > So I think calling dma_alloc_coherent with __GFP_WAIT is enough. > > And can I get zero-filled-memory from CMA if I set __GFP_ZERO? > > If we are actually using CMA, the GFP flags don't make much of a difference. If we didn't have CMA, using GFP_HIGHUSER would still be appropriate. The memory is going to userspace so the user part is necessary and there should be no need for a lowmem mapping so highmem can be used. __GFP_ZERO might be redundant since the dma layer zeros memory anyway but it's probably safe. I think the flags should be fine as is. Thanks, Laura -- Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: ion : ion_cma_heap: fix bug for highmem cma
On 11/28/2014 5:31 PM, Zeng Tao wrote:
when cma is located in highmem, virt_to_page will not
work the right way, use pfn_to_page instead.
Signed-off-by: Zeng Tao
---
drivers/staging/android/ion/ion_cma_heap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/android/ion/ion_cma_heap.c
b/drivers/staging/android/ion/ion_cma_heap.c
index f8cabcb..cc45fc0 100644
--- a/drivers/staging/android/ion/ion_cma_heap.c
+++ b/drivers/staging/android/ion/ion_cma_heap.c
@@ -47,7 +47,7 @@ struct ion_cma_buffer_info {
static int ion_cma_get_sgtable(struct device *dev, struct sg_table *sgt,
void *cpu_addr, dma_addr_t handle, size_t size)
{
- struct page *page = virt_to_page(cpu_addr);
+ struct page *page = pfn_to_page(handle >> PAGE_SHIFT);
There's a comment right above this that notes this function could be replaced
with dma_common_get_sgtable when it's available. That function is now
present in drivers/base/dma-mapping.c although it has the same bug. I'd
suggest fixing the bug in dma_common_get_sgtable and switching Ion to use
that function instead.
int ret;
ret = sg_alloc_table(sgt, 1, GFP_KERNEL);
Thanks,
Laura
--
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] staging: android: ion: Remove file ion_carveout_heap.c
On 7/3/19 5:50 AM, Daniel Vetter wrote: On Wed, Jul 3, 2019 at 10:37 AM Greg KH wrote: On Wed, Jul 03, 2019 at 01:48:41PM +0530, Nishka Dasgupta wrote: Remove file ion_carveout_heap.c as its functions and definitions are not used anywhere. Issue found with Coccinelle. Signed-off-by: Nishka Dasgupta --- drivers/staging/android/ion/Kconfig | 9 -- drivers/staging/android/ion/Makefile | 1 - .../staging/android/ion/ion_carveout_heap.c | 133 -- I keep trying to do this, but others point out that the ion code is "going to be fixed up soon" and that people rely on this interface now. Well, "code outside of the kernel tree" relies on this, which is not ok, but the "soon" people keep insisting on it... Odds are I should just delete all of ION, as there hasn't been any forward progress on it in a long time. Hopefully that wakes some people up... John Stultz has done a steady stream on ion destaging patch series past few months, und the heading of "DMA-BUF Heaps", targeting drivers/dma-buf. I'm not following the details, and it seems a bit a crawl, but there's definitely work going on ... Just probably not in-place in staging I think. -Daniel https://lists.freedesktop.org/archives/dri-devel/2019-June/223705.html It is making slow and steady progress. Part of this is trying to make sure we actually get this right before moving anything out of staging. That said, I think we're at the point where nobody wants the carveout and chunk heaps so I'd actually be okay with removing those files. Just to be explicit: Acked-by: Laura Abbott Thanks, Laura ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] staging: android: ion: Remove file ion_chunk_heap.c
On 7/3/19 4:18 AM, Nishka Dasgupta wrote:
Remove file ion_chunk_heap.c as its functions and definitions are not
used anywhere else.
Issue found with Coccinelle.
Acked-by: Laura Abbott
Signed-off-by: Nishka Dasgupta
---
drivers/staging/android/ion/Kconfig | 9 --
drivers/staging/android/ion/Makefile | 1 -
drivers/staging/android/ion/ion_chunk_heap.c | 147 ---
3 files changed, 157 deletions(-)
delete mode 100644 drivers/staging/android/ion/ion_chunk_heap.c
diff --git a/drivers/staging/android/ion/Kconfig
b/drivers/staging/android/ion/Kconfig
index dff641451a89..989fe84a9f9d 100644
--- a/drivers/staging/android/ion/Kconfig
+++ b/drivers/staging/android/ion/Kconfig
@@ -18,15 +18,6 @@ config ION_SYSTEM_HEAP
Choose this option to enable the Ion system heap. The system heap
is backed by pages from the buddy allocator. If in doubt, say Y.
-config ION_CHUNK_HEAP
- bool "Ion chunk heap support"
- depends on ION
- help
- Choose this option to enable chunk heaps with Ion. This heap is
- similar in function the carveout heap but memory is broken down
- into smaller chunk sizes, typically corresponding to a TLB size.
- Unless you know your system has these regions, you should say N here.
-
config ION_CMA_HEAP
bool "Ion CMA heap support"
depends on ION && DMA_CMA
diff --git a/drivers/staging/android/ion/Makefile
b/drivers/staging/android/ion/Makefile
index 0ac5465e2841..5f4487b1a224 100644
--- a/drivers/staging/android/ion/Makefile
+++ b/drivers/staging/android/ion/Makefile
@@ -1,5 +1,4 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_ION) += ion.o ion_heap.o
obj-$(CONFIG_ION_SYSTEM_HEAP) += ion_system_heap.o ion_page_pool.o
-obj-$(CONFIG_ION_CHUNK_HEAP) += ion_chunk_heap.o
obj-$(CONFIG_ION_CMA_HEAP) += ion_cma_heap.o
diff --git a/drivers/staging/android/ion/ion_chunk_heap.c
b/drivers/staging/android/ion/ion_chunk_heap.c
deleted file mode 100644
index 1e869f4bad45..
--- a/drivers/staging/android/ion/ion_chunk_heap.c
+++ /dev/null
@@ -1,147 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * ION memory allocator chunk heap helper
- *
- * Copyright (C) 2012 Google, Inc.
- */
-
-#include
-#include
-#include
-#include
-#include
-#include
-
-#include "ion.h"
-
-struct ion_chunk_heap {
- struct ion_heap heap;
- struct gen_pool *pool;
- unsigned long chunk_size;
- unsigned long size;
- unsigned long allocated;
-};
-
-static int ion_chunk_heap_allocate(struct ion_heap *heap,
- struct ion_buffer *buffer,
- unsigned long size,
- unsigned long flags)
-{
- struct ion_chunk_heap *chunk_heap =
- container_of(heap, struct ion_chunk_heap, heap);
- struct sg_table *table;
- struct scatterlist *sg;
- int ret, i;
- unsigned long num_chunks;
- unsigned long allocated_size;
-
- allocated_size = ALIGN(size, chunk_heap->chunk_size);
- num_chunks = allocated_size / chunk_heap->chunk_size;
-
- if (allocated_size > chunk_heap->size - chunk_heap->allocated)
- return -ENOMEM;
-
- table = kmalloc(sizeof(*table), GFP_KERNEL);
- if (!table)
- return -ENOMEM;
- ret = sg_alloc_table(table, num_chunks, GFP_KERNEL);
- if (ret) {
- kfree(table);
- return ret;
- }
-
- sg = table->sgl;
- for (i = 0; i < num_chunks; i++) {
- unsigned long paddr = gen_pool_alloc(chunk_heap->pool,
-chunk_heap->chunk_size);
- if (!paddr)
- goto err;
- sg_set_page(sg, pfn_to_page(PFN_DOWN(paddr)),
- chunk_heap->chunk_size, 0);
- sg = sg_next(sg);
- }
-
- buffer->sg_table = table;
- chunk_heap->allocated += allocated_size;
- return 0;
-err:
- sg = table->sgl;
- for (i -= 1; i >= 0; i--) {
- gen_pool_free(chunk_heap->pool, page_to_phys(sg_page(sg)),
- sg->length);
- sg = sg_next(sg);
- }
- sg_free_table(table);
- kfree(table);
- return -ENOMEM;
-}
-
-static void ion_chunk_heap_free(struct ion_buffer *buffer)
-{
- struct ion_heap *heap = buffer->heap;
- struct ion_chunk_heap *chunk_heap =
- container_of(heap, struct ion_chunk_heap, heap);
- struct sg_table *table = buffer->sg_table;
- struct scatterlist *sg;
- int i;
- unsigned long allocated_size;
-
- allocated_size = ALIGN(buffer->size, chunk_heap->chunk_size);
-
- ion_heap_buffer_zero(buffer);
-
- for_each_sg(table-&g
Re: [PATCH] staging: android: ion: Remove unused rbtree for ion_buffer
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
Cc: YJ Chiang
Cc: Lecopzer Chen
---
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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: Limits for ION Memory Allocator
On 7/17/19 12:31 PM, Alexander Popov wrote:
Hello!
The syzkaller [1] has a trouble with fuzzing the Linux kernel with ION Memory
Allocator.
Syzkaller uses several methods [2] to limit memory consumption of the userspace
processes calling the syscalls for testing the kernel:
- setrlimit(),
- cgroups,
- various sysctl.
But these methods don't work for ION Memory Allocator, so any userspace process
that has access to /dev/ion can bring the system to the out-of-memory state.
An example of a program doing that:
#include
#include
#include
#include
#include
#include
#define ION_IOC_MAGIC 'I'
#define ION_IOC_ALLOC _IOWR(ION_IOC_MAGIC, 0, \
struct ion_allocation_data)
struct ion_allocation_data {
__u64 len;
__u32 heap_id_mask;
__u32 flags;
__u32 fd;
__u32 unused;
};
int main(void)
{
unsigned long i = 0;
int fd = -1;
struct ion_allocation_data data = {
.len = 0x13f65d8c,
.heap_id_mask = 1,
.flags = 0,
.fd = -1,
.unused = 0
};
fd = open("/dev/ion", 0);
if (fd == -1) {
perror("[-] open /dev/ion");
return 1;
}
while (1) {
printf("iter %lu\n", i);
ioctl(fd, ION_IOC_ALLOC, &data);
i++;
}
return 0;
}
I looked through the code of ion_alloc() and didn't find any limit checks.
Is it currently possible to limit ION kernel allocations for some process?
If not, is it a right idea to do that?
Thanks!
Yes, I do think that's the right approach. We're working on moving Ion
out of staging and this is something I mentioned to John Stultz. I don't
think we've thought too hard about how to do the actual limiting so
suggestions are welcome.
Thanks,
Laura
Best regards,
Alexander
[1]: https://github.com/google/syzkaller
[2]: https://github.com/google/syzkaller/blob/master/executor/common_linux.h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 2/2] staging: ion: create one device entry per heap
On 09/19/2017 04:55 AM, Benjamin Gaignard wrote:
2017-09-19 13:02 GMT+02:00 Greg KH :
On Tue, Sep 19, 2017 at 12:25:38PM +0200, Benjamin Gaignard wrote:
Instead a getting one common device "/dev/ion" for
all the heaps this patch allow to create one device
entry ("/dev/ionX") per heap.
Getting an entry per heap could allow to set security rules
per heap and global ones for all heaps.
Allocation requests will be only allowed if the mask_id
match with device minor.
Query request could be done on any of the devices.
Signed-off-by: Benjamin Gaignard
---
drivers/staging/android/ion/ion-ioctl.c | 11 +--
drivers/staging/android/ion/ion.c | 23 +--
drivers/staging/android/ion/ion.h | 10 +++---
3 files changed, 33 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/android/ion/ion-ioctl.c
b/drivers/staging/android/ion/ion-ioctl.c
index e26b786..c8c906c 100644
--- a/drivers/staging/android/ion/ion-ioctl.c
+++ b/drivers/staging/android/ion/ion-ioctl.c
@@ -25,8 +25,11 @@ union ion_ioctl_arg {
struct ion_heap_query query;
};
-static int validate_ioctl_arg(unsigned int cmd, union ion_ioctl_arg *arg)
+static int validate_ioctl_arg(struct file *filp,
+ unsigned int cmd, union ion_ioctl_arg *arg)
{
+ int mask = 1 << iminor(filp->f_inode);
+
switch (cmd) {
case ION_IOC_HEAP_QUERY:
if (arg->query.reserved0 ||
@@ -34,6 +37,10 @@ static int validate_ioctl_arg(unsigned int cmd, union
ion_ioctl_arg *arg)
arg->query.reserved2 )
return -EINVAL;
break;
+ case ION_IOC_ALLOC:
+ if (!(arg->allocation.heap_id_mask & mask))
+ return -EINVAL;
+ break;
default:
break;
}
@@ -69,7 +76,7 @@ long ion_ioctl(struct file *filp, unsigned int cmd, unsigned
long arg)
if (copy_from_user(&data, (void __user *)arg, _IOC_SIZE(cmd)))
return -EFAULT;
- ret = validate_ioctl_arg(cmd, &data);
+ ret = validate_ioctl_arg(filp, cmd, &data);
if (WARN_ON_ONCE(ret))
return ret;
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 93e2c90..3f8b595 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -40,6 +40,8 @@
#include "ion.h"
+#define ION_DEV_MAX 32
+
static struct ion_device *internal_dev;
static int heap_id;
@@ -541,11 +543,24 @@ void ion_device_add_heap(struct ion_heap *heap)
{
struct dentry *debug_file;
struct ion_device *dev = internal_dev;
+ int ret;
if (!heap->ops->allocate || !heap->ops->free)
pr_err("%s: can not add heap with invalid ops struct.\n",
__func__);
+ if (heap_id >= ION_DEV_MAX)
+ return -EBUSY;
+
+ heap->ddev.devt = MKDEV(MAJOR(dev->devt), heap_id);
+ dev_set_name(&heap->ddev, "ion%d", heap_id);
+ device_initialize(&heap->ddev);
+ cdev_init(&heap->chrdev, &ion_fops);
+ heap->chrdev.owner = THIS_MODULE;
+ ret = cdev_device_add(&heap->chrdev, &heap->ddev);
+ if (ret < 0)
+ return;
No cleanup needed? No reporting an error happened back up the chain?
Not nice :(
I will change that
+
spin_lock_init(&heap->free_lock);
heap->free_list_size = 0;
@@ -595,13 +610,9 @@ static int ion_device_create(void)
if (!idev)
return -ENOMEM;
- idev->dev.minor = MISC_DYNAMIC_MINOR;
- idev->dev.name = "ion";
- idev->dev.fops = &ion_fops;
- idev->dev.parent = NULL;
- ret = misc_register(&idev->dev);
+ ret = alloc_chrdev_region(&idev->devt, 0, ION_DEV_MAX, "ion");
Did you just change the major number for the device node as well?
My understanding of alloc_chrdev_region() is that major number is chosen
dynamically but I don't understand the link with device node, sorry.
Wow, that's a lot of userspace breakage (both major number, and name),
how did you test this?
I had to write a test by myself:
https://git.linaro.org/people/benjamin.gaignard/ion_test_application.git/log/?h=one_device_per_heap
Laura have tried to push a test VGEM but I believe it hasn't be
accepted yet (I will check)
Have you gotten "upstream" to agree to these changes? We can't take
these until they think it's ok as well.
Split /dev/ion into multiple nodes is one of the task listed in
staging/android/TODO
file before been able to de-stage ion.
Since it has been a big bang in ion ABI on 4.12 and the fact that
ion.h is still in
staging/android/uapi/ directory I do believe that userland is still unstable.
I hope this kind of patch will help to clarify what is still need to
be done to de-stage ion
even if this patch is NACK-ed we can at least the item from the TODO list.
Thanks for sending this Benjamin.
At plumbers, it was requested to not break the ABI too much or do it in
one last big bang
Re: [PATCH v3 2/2] staging: ion: create one device entry per heap
On 09/20/2017 01:45 AM, Benjamin Gaignard wrote:
Instead a getting one common device "/dev/ion" for
all the heaps this patch allow to create one device
entry ("/dev/ionX") per heap.
Getting an entry per heap could allow to set security rules
per heap and global ones for all heaps.
Allocation requests will be only allowed if the mask_id
match with device minor.
Query request could be done on any of the devices.
Deivce node major will also change and that may impact init scripts.
We should start Cc linux-api for future changes since we're going
to have more than just /dev/ion.
Thinking about this some more, I think we need to allow backwards
compatibility. It's just not feasible to keep throwing workarounds
into userspace if we can avoid it. I'd propose keeping the old /dev/ion
misc interface available under a Kconfig and then creating the new
split heaps in parallel.
On a somewhat related note, there was some interest in possibly
having a sysfs interface for Ion long term. I don't think this
needs to happen right now but I'd like whatever we do to not
make adding that harder.
Thanks,
Laura
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 2/2] staging: ion: create one device entry per heap
On 09/25/2017 11:56 PM, Greg KH wrote:
On Tue, Sep 26, 2017 at 07:09:14AM +0200, Daniel Vetter wrote:
On Mon, Sep 25, 2017 at 11:26:27AM -0700, Laura Abbott wrote:
On 09/20/2017 01:45 AM, Benjamin Gaignard wrote:
Instead a getting one common device "/dev/ion" for
all the heaps this patch allow to create one device
entry ("/dev/ionX") per heap.
Getting an entry per heap could allow to set security rules
per heap and global ones for all heaps.
Allocation requests will be only allowed if the mask_id
match with device minor.
Query request could be done on any of the devices.
Deivce node major will also change and that may impact init scripts.
We should start Cc linux-api for future changes since we're going
to have more than just /dev/ion.
Thinking about this some more, I think we need to allow backwards
compatibility. It's just not feasible to keep throwing workarounds
into userspace if we can avoid it. I'd propose keeping the old /dev/ion
misc interface available under a Kconfig and then creating the new
split heaps in parallel.
On a somewhat related note, there was some interest in possibly
having a sysfs interface for Ion long term. I don't think this
needs to happen right now but I'd like whatever we do to not
make adding that harder.
Not sure sysfs is a good idea for allocating buffers. The backlight
interface is in sysfs, which defacto makes it a root-only interface.
Distros really hate that, because it makes unpriviledged X/wayland really
hard to pull of. Passing a device file otoh from logind to the compositor
is dead simple (and how X et al get at e.g. the drm/input devices
already).
sysfs is not a good idea for allocating buffers. We already have some
out-of-tree drm drivers doing horrid things in sysfs in ways that
totally abuse that api, and vendors have to do crazy things with selinux
rules to try to lock it down because of that. A device node is fine, we
are used to that for graphics stuff :)
thanks,
greg k-h
I wasn't thinking of sysfs for allocation, this was for exposure of
other Ion information that might make more sense than debugfs. Like
I said, this was mostly forward thinking to make sure we aren't
stuck later.
Thanks,
Laura
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 2/2] staging: ion: create one device entry per heap
On 09/26/2017 09:17 AM, Mark Brown wrote: On Tue, Sep 26, 2017 at 02:07:05PM +0200, Benjamin Gaignard wrote: version 4: - add a configuration flag to switch between legacy Ion misc device and one device per heap version. Should this be a switch or should it just be enabling and disabling the legacy device with the per heap ones always availalbe? I can't see that the new devices would do any harm or have trouble interacting with the per heap ones. Being able to have both enabled would make things easier for userspaces that are moving to the device per heap interface. Agreed. We should be enabling the new interface unconditionally. The old /dev/ion interface should coexist to allow for backwards compatibility but keep it under a Kconfig to allow it to be turned off for security or other reasons. Thanks, Laura ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 2/2] staging: ion: create one device entry per heap
On 09/27/2017 06:20 AM, Benjamin Gaignard wrote:
> Instead a getting only one common device "/dev/ion" for
> all the heaps this patch allow to create one device
> entry ("/dev/ionX") per heap.
> Getting an entry per heap could allow to set security rules
> per heap and global ones for all heaps.
>
> Allocation requests will be only allowed if the mask_id
> match with device minor.
> Query request could be done on any of the devices.
>
Thinking about this a bit more, I'm not 100% sure if this
will allow the security rules we want. Heap ids are assigned
dynamically and therefore so will the /dev/ionX designation.
>From my understanding, security rules like selinux need to
be fully specified at boot time so I'm not sure how you would
be able to write rules to differentiate between /dev/ionX and
/dev/ionY without knowing the values at boottime.
So I think we need a different way to match the heap ids to
get the security we want unless my understanding of security
policies is wrong and we can dynamically specify permissions.
Thanks,
Laura
> Signed-off-by: Benjamin Gaignard
> ---
> version 5:
> - create a configuration flag to keep legacy Ion misc device
>
> version 4:
> - add a configuration flag to switch between legacy Ion misc device
> and one device per heap version.
>
> version 3:
> - change ion_device_add_heap prototype to return a possible error.
>
> version 2:
> - simplify ioctl check like propose by Dan
> - make sure that we don't register more than ION_DEV_MAX heaps.
>
> drivers/staging/android/TODO| 1 -
> drivers/staging/android/ion/Kconfig | 7 +++
> drivers/staging/android/ion/ion-ioctl.c | 18 --
> drivers/staging/android/ion/ion.c | 31 ++-
> drivers/staging/android/ion/ion.h | 15 +--
> 5 files changed, 66 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/android/TODO b/drivers/staging/android/TODO
> index 5f14247..d770ffa 100644
> --- a/drivers/staging/android/TODO
> +++ b/drivers/staging/android/TODO
> @@ -9,7 +9,6 @@ TODO:
> ion/
> - Add dt-bindings for remaining heaps (chunk and carveout heaps). This would
> involve putting appropriate bindings in a memory node for Ion to find.
> - - Split /dev/ion up into multiple nodes (e.g. /dev/ion/heap0)
> - Better test framework (integration with VGEM was suggested)
>
> Please send patches to Greg Kroah-Hartman and Cc:
> diff --git a/drivers/staging/android/ion/Kconfig
> b/drivers/staging/android/ion/Kconfig
> index a517b2d..cb4666e 100644
> --- a/drivers/staging/android/ion/Kconfig
> +++ b/drivers/staging/android/ion/Kconfig
> @@ -10,6 +10,13 @@ menuconfig ION
> If you're not using Android its probably safe to
> say N here.
>
> +config ION_LEGACY_DEVICE_API
> + bool "Keep using Ion legacy misc device API"
> + depends on ION
> + help
> + Choose this option to keep using Ion legacy misc device API
> + i.e. /dev/ion
> +
> config ION_SYSTEM_HEAP
> bool "Ion system heap"
> depends on ION
> diff --git a/drivers/staging/android/ion/ion-ioctl.c
> b/drivers/staging/android/ion/ion-ioctl.c
> index e26b786..bb5c77b 100644
> --- a/drivers/staging/android/ion/ion-ioctl.c
> +++ b/drivers/staging/android/ion/ion-ioctl.c
> @@ -25,7 +25,8 @@ union ion_ioctl_arg {
> struct ion_heap_query query;
> };
>
> -static int validate_ioctl_arg(unsigned int cmd, union ion_ioctl_arg *arg)
> +static int validate_ioctl_arg(struct file *filp,
> + unsigned int cmd, union ion_ioctl_arg *arg)
> {
> switch (cmd) {
> case ION_IOC_HEAP_QUERY:
> @@ -34,6 +35,19 @@ static int validate_ioctl_arg(unsigned int cmd, union
> ion_ioctl_arg *arg)
> arg->query.reserved2 )
> return -EINVAL;
> break;
> +
> + case ION_IOC_ALLOC:
> + {
> + int mask = 1 << iminor(filp->f_inode);
> +
> +#ifdef CONFIG_ION_LEGACY_DEVICE_API
> + if (imajor(filp->f_inode) == MISC_MAJOR)
> + return 0;
> +#endif
> + if (!(arg->allocation.heap_id_mask & mask))
> + return -EINVAL;
> + break;
> + }
> default:
> break;
> }
> @@ -69,7 +83,7 @@ long ion_ioctl(struct file *filp, unsigned int cmd,
> unsigned long arg)
> if (copy_from_user(&data, (void __user *)arg, _IOC_SIZE(cmd)))
> return -EFAULT;
>
> - ret = validate_ioctl_arg(cmd, &data);
> + ret = validate_ioctl_arg(filp, cmd, &data);
> if (WARN_ON_ONCE(ret))
> return ret;
>
> diff --git a/drivers/staging/android/ion/ion.c
> b/drivers/staging/android/ion/ion.c
> index 93e2c90..092b24c 100644
> --- a/drivers/staging/android/ion/ion.c
> +++ b/drivers/staging/android/ion/ion.c
> @@ -40,6 +40,8 @@
>
> #include "ion.h"
>
> +#define ION_DEV_MAX 32
> +
> static struct ion_device *internal_dev;
> static int heap_id;
>
>
Re: [PATCH v5 2/2] staging: ion: create one device entry per heap
On 10/03/2017 09:48 AM, Mark Brown wrote: > On Mon, Oct 02, 2017 at 11:07:48AM -0700, Laura Abbott wrote: > >> Thinking about this a bit more, I'm not 100% sure if this >> will allow the security rules we want. Heap ids are assigned >> dynamically and therefore so will the /dev/ionX designation. >> From my understanding, security rules like selinux need to >> be fully specified at boot time so I'm not sure how you would >> be able to write rules to differentiate between /dev/ionX and >> /dev/ionY without knowing the values at boottime. > > Isn't this something that should be managable via udev rules that ensure > stable names in the same way as for things like disks or ethernet > controllers (even if it just ends up doing something like /dev/ion-gpu > or whatever)? If we're not giving it enough information to assign stable > names where needed we probably need to fix that anyway. > Android doesn't use a standard udev so we'd need something that would work there. My understanding was that android needs everything specified at boot unless that's changed. There would be enough information to assign stable names (e.g. /dev/ion-system) if we used the query ioctl to find out what's actually available. Is just the ioctl useful though? Thanks, Laura ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 2/2] staging: ion: create one device entry per heap
On 10/03/2017 04:08 PM, Sandeep Patil wrote: > On Tue, Oct 03, 2017 at 02:42:32PM -0700, Laura Abbott wrote: >> On 10/03/2017 09:48 AM, Mark Brown wrote: >>> On Mon, Oct 02, 2017 at 11:07:48AM -0700, Laura Abbott wrote: >>> >>>> Thinking about this a bit more, I'm not 100% sure if this >>>> will allow the security rules we want. Heap ids are assigned >>>> dynamically and therefore so will the /dev/ionX designation. >>>> From my understanding, security rules like selinux need to >>>> be fully specified at boot time so I'm not sure how you would >>>> be able to write rules to differentiate between /dev/ionX and >>>> /dev/ionY without knowing the values at boottime. >>> >>> Isn't this something that should be managable via udev rules that ensure >>> stable names in the same way as for things like disks or ethernet >>> controllers (even if it just ends up doing something like /dev/ion-gpu >>> or whatever)? If we're not giving it enough information to assign stable >>> names where needed we probably need to fix that anyway. >>> >> >> Android doesn't use a standard udev so we'd need something that >> would work there. My understanding was that android needs everything >> specified at boot unless that's changed. >> >> There would be enough information to assign stable names >> (e.g. /dev/ion-system) if we used the query ioctl to find out >> what's actually available. Is just the ioctl useful though? > > Wouldn't this new ioctl() to query the heap name also result in special case > handling of all ion devices in user space? > I'm not quite sure what you are referring to. If you mean we have to match on the heap name then yes that's going to happen but we can't just zero knowledge which heap to allocate from and matching on heap names seemed like the easiest way to make that happen. > If the devices are named with their corresponding heap names like ion-system, > ion-cma etc. > It is entirely possible and easy in android/ueventd to create those nodes > under "/dev/ion/". (assuming the heap 'subsystem' for these new devices will > point to 'ion'). > > Something like the following should work if added in ueventd.rc > > subsystem ion > devname uevent_devname > dirname /dev/ion > > Also, makes SElinux labelling easier. > That's useful to know, thanks. > (Also FWIW, the SELinux permissions are also possible with the current ion > implementation by adding rules to disallow specific ioctls instead of adding > permissions to access device node as this change would do) > Can selinux disallow access within the ioctls though? The access control wanted is at a heap granularity and disallowing certain ioctls won't fix that. > > - ssp > Thanks, Laura ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 1/2] staging: ion: simplify ioctl args checking function
On 10/09/2017 02:21 AM, Benjamin Gaignard wrote:
> 2017-09-27 15:20 GMT+02:00 Benjamin Gaignard :
>> Make arguments checking more easy to read.
>>
>
> Hi Laura,
>
> Even if we don't have found a solution for the second patch I believe
> this one could be useful.
> May I ask you your point of view on those few lines ?
>
> Benjamin
>
Yes, this looks better.
Acked-by: Laura Abbott
>> Signed-off-by: Benjamin Gaignard
>> ---
>> drivers/staging/android/ion/ion-ioctl.c | 11 +--
>> 1 file changed, 5 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/staging/android/ion/ion-ioctl.c
>> b/drivers/staging/android/ion/ion-ioctl.c
>> index d9f8b14..e26b786 100644
>> --- a/drivers/staging/android/ion/ion-ioctl.c
>> +++ b/drivers/staging/android/ion/ion-ioctl.c
>> @@ -27,19 +27,18 @@ union ion_ioctl_arg {
>>
>> static int validate_ioctl_arg(unsigned int cmd, union ion_ioctl_arg *arg)
>> {
>> - int ret = 0;
>> -
>> switch (cmd) {
>> case ION_IOC_HEAP_QUERY:
>> - ret = arg->query.reserved0 != 0;
>> - ret |= arg->query.reserved1 != 0;
>> - ret |= arg->query.reserved2 != 0;
>> + if (arg->query.reserved0 ||
>> + arg->query.reserved1 ||
>> + arg->query.reserved2)
>> + return -EINVAL;
>> break;
>> default:
>> break;
>> }
>>
>> - return ret ? -EINVAL : 0;
>> + return 0;
>> }
>>
>> /* fix up the cases where the ioctl direction bits are incorrect */
>> --
>> 2.7.4
>>
>
>
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 2/2] staging: ion: create one device entry per heap
On 10/05/2017 06:06 AM, Benjamin Gaignard wrote: > 2017-10-04 12:17 GMT+02:00 Mark Brown : >> On Tue, Oct 03, 2017 at 04:08:30PM -0700, Sandeep Patil wrote: >> >>> It is entirely possible and easy in android/ueventd to create those nodes >>> under "/dev/ion/". (assuming the heap 'subsystem' for these new devices >>> will >>> point to 'ion'). > > I think it is the same problem than for webcam under v4l framework. > Each time you plug a webcam you got a v4l node but android/uevent rules > the plug order doesn't have impact. > The same think will happen for ion nodes it may be even easier because > the heap will always being created in the smae order for a given product > configuration. > Relying on the heap being created in the same order seems troublesome. If for some reason it changes in the kernel we might break something in userspace. Anyway, to move this forward I think we need to see a proof of concept of using selinux to protect access to specific heaps. Thanks, Laura >> >> The reason I didn't say /dev/ion/foo initially is that if people want to >> keep the existing /dev/ion around for compatibility reasons then the >> /dev/ion name isn't available which might cause issues. Otherwise just >> dumping everything under a directory (perhaps with a different name) was >> my first thought as well. >> >>> (Also FWIW, the SELinux permissions are also possible with the current ion >>> implementation by adding rules to disallow specific ioctls instead of adding >>> permissions to access device node as this change would do) >> >> AIUI the request is to limit access to specific heaps, and obviously not >> everyone wants to deal with SELinux at all. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 2/2] staging: ion: create one device entry per heap
On 10/09/2017 03:08 PM, Mark Brown wrote: > On Mon, Oct 09, 2017 at 02:25:47PM -0700, Laura Abbott wrote: > >> Anyway, to move this forward I think we need to see a proof of concept >> of using selinux to protect access to specific heaps. > > Aren't Unix permissions enough with separate files or am I > misunderstanding what you're looking to see a proof of concept for? > The goal is to be able to restrict heap access to certain services and selinux groups on Android so straight unix permissions aren't sufficient. Thanks, Laura ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 2/2] staging: ion: create one device entry per heap
On 10/10/2017 02:11 AM, Mark Brown wrote: > On Mon, Oct 09, 2017 at 05:10:37PM -0700, Laura Abbott wrote: >> On 10/09/2017 03:08 PM, Mark Brown wrote: >>> On Mon, Oct 09, 2017 at 02:25:47PM -0700, Laura Abbott wrote: > >>>> Anyway, to move this forward I think we need to see a proof of concept >>>> of using selinux to protect access to specific heaps. > >>> Aren't Unix permissions enough with separate files or am I >>> misunderstanding what you're looking to see a proof of concept for? > >> The goal is to be able to restrict heap access to certain services >> and selinux groups on Android so straight unix permissions aren't >> sufficient. > > Oh, there's Android users for this? The users I was aware of were > non-Android. Though even so I'd have thought that given that SELinux is > a superset of Unix file permissions it ought to be sufficient to be able > to use them. I'd been thinking people were suggesting SELinux as a > replacement for file permissions, using the single file and the greater > capabilities of SELinux. > Unix file permissions are necessary but not sufficient, they can be used separately. Mostly what I want to see before merging this is an example that splitting the Ion heaps provides more protection than just keeping /dev/ion. Thanks, Laura ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 2/2] staging: ion: create one device entry per heap
On 09/27/2017 06:20 AM, Benjamin Gaignard wrote:
> diff --git a/drivers/staging/android/ion/ion.c
> b/drivers/staging/android/ion/ion.c
> index 93e2c90..092b24c 100644
> --- a/drivers/staging/android/ion/ion.c
> +++ b/drivers/staging/android/ion/ion.c
> @@ -40,6 +40,8 @@
>
> #include "ion.h"
>
> +#define ION_DEV_MAX 32
> +
> static struct ion_device *internal_dev;
> static int heap_id;
>
> @@ -537,15 +539,28 @@ static int debug_shrink_get(void *data, u64 *val)
> DEFINE_SIMPLE_ATTRIBUTE(debug_shrink_fops, debug_shrink_get,
> debug_shrink_set, "%llu\n");
>
> -void ion_device_add_heap(struct ion_heap *heap)
> +int ion_device_add_heap(struct ion_heap *heap)
> {
> struct dentry *debug_file;
> struct ion_device *dev = internal_dev;
> + int ret = 0;
>
> if (!heap->ops->allocate || !heap->ops->free)
> pr_err("%s: can not add heap with invalid ops struct.\n",
> __func__);
>
> + if (heap_id >= ION_DEV_MAX)
> + return -EBUSY;
> +
> + heap->ddev.devt = MKDEV(MAJOR(dev->devt), heap_id);
> + dev_set_name(&heap->ddev, "ion%d", heap_id);
> + device_initialize(&heap->ddev);
> + cdev_init(&heap->chrdev, &ion_fops);
> + heap->chrdev.owner = THIS_MODULE;
> + ret = cdev_device_add(&heap->chrdev, &heap->ddev);
> + if (ret < 0)
> + return ret;
> +
> spin_lock_init(&heap->free_lock);
> heap->free_list_size = 0;
>
> @@ -583,6 +598,8 @@ void ion_device_add_heap(struct ion_heap *heap)
>
> dev->heap_cnt++;
> up_write(&dev->lock);
> +
> + return ret;
> }
> EXPORT_SYMBOL(ion_device_add_heap);
>
> @@ -595,6 +612,7 @@ static int ion_device_create(void)
> if (!idev)
> return -ENOMEM;
>
> +#ifdef CONFIG_ION_LEGACY_DEVICE_API
> idev->dev.minor = MISC_DYNAMIC_MINOR;
> idev->dev.name = "ion";
> idev->dev.fops = &ion_fops;
> @@ -605,6 +623,17 @@ static int ion_device_create(void)
> kfree(idev);
> return ret;
> }
> +#endif
> +
> + ret = alloc_chrdev_region(&idev->devt, 0, ION_DEV_MAX, "ion");
> + if (ret) {
> + pr_err("ion: unable to allocate device\n");
> +#ifdef CONFIG_ION_LEGACY_DEVICE_API
> + misc_deregister(&idev->dev);
> +#endif
> + kfree(idev);
> + return ret;
> + }
>
> idev->debug_root = debugfs_create_dir("ion", NULL);
> if (!idev->debug_root) {
I'm not 100% sure about the device hierarchy here. We're
ending up with devices at the root of /sys/devices
/sys/devices # ls
breakpoint ion0 ion1 ion2 platform software system virtual
and the Android init system doesn't pick this up. I'll
admit to being out of my area here but I don't think
this looks quite right.
Thanks,
Laura
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v6 2/2] staging: ion: create one device entry per heap
On 10/23/2017 08:55 AM, Benjamin Gaignard wrote:
> Instead a getting only one common device "/dev/ion" for
> all the heaps this patch allow to create one device
> entry ("/dev/ionX") per heap.
> Getting an entry per heap could allow to set security rules
> per heap and global ones for all heaps.
>
> Allocation requests will be only allowed if the mask_id
> match with device minor.
> Query request could be done on any of the devices.
>
I'm wondering if we should always keep /dev/ion for the query
ioctl and just disallow allocation from /dev/ion. I guess
running the query ioctl on /dev/ion0 always wouldn't be too
bad? Anyone else have strong opinions?
> Ion devices are parentless so it is need to add platform_bus as
> parent and platform_bus_type as bus to be put in /sys/device/paltform.
> Those two parameters need platform_device.h to be included but
> include files weren't in alphabetic order so I reorder them correctly.
>
I'm not a fan of the platform bus but I have mixed feelings about
creating a dedicated bus type. I guess if we really need a bus
type we can do it later?
Thanks,
Laura
> Signed-off-by: Benjamin Gaignard
> ---
> drivers/staging/android/TODO| 1 -
> drivers/staging/android/ion/Kconfig | 7 +
> drivers/staging/android/ion/ion-ioctl.c | 18 +--
> drivers/staging/android/ion/ion.c | 53
> ++---
> drivers/staging/android/ion/ion.h | 15 --
> 5 files changed, 79 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/staging/android/TODO b/drivers/staging/android/TODO
> index 5f14247..d770ffa 100644
> --- a/drivers/staging/android/TODO
> +++ b/drivers/staging/android/TODO
> @@ -9,7 +9,6 @@ TODO:
> ion/
> - Add dt-bindings for remaining heaps (chunk and carveout heaps). This would
> involve putting appropriate bindings in a memory node for Ion to find.
> - - Split /dev/ion up into multiple nodes (e.g. /dev/ion/heap0)
> - Better test framework (integration with VGEM was suggested)
>
> Please send patches to Greg Kroah-Hartman and Cc:
> diff --git a/drivers/staging/android/ion/Kconfig
> b/drivers/staging/android/ion/Kconfig
> index a517b2d..cb4666e 100644
> --- a/drivers/staging/android/ion/Kconfig
> +++ b/drivers/staging/android/ion/Kconfig
> @@ -10,6 +10,13 @@ menuconfig ION
> If you're not using Android its probably safe to
> say N here.
>
> +config ION_LEGACY_DEVICE_API
> + bool "Keep using Ion legacy misc device API"
> + depends on ION
> + help
> + Choose this option to keep using Ion legacy misc device API
> + i.e. /dev/ion
> +
> config ION_SYSTEM_HEAP
> bool "Ion system heap"
> depends on ION
> diff --git a/drivers/staging/android/ion/ion-ioctl.c
> b/drivers/staging/android/ion/ion-ioctl.c
> index e26b786..bb5c77b 100644
> --- a/drivers/staging/android/ion/ion-ioctl.c
> +++ b/drivers/staging/android/ion/ion-ioctl.c
> @@ -25,7 +25,8 @@ union ion_ioctl_arg {
> struct ion_heap_query query;
> };
>
> -static int validate_ioctl_arg(unsigned int cmd, union ion_ioctl_arg *arg)
> +static int validate_ioctl_arg(struct file *filp,
> + unsigned int cmd, union ion_ioctl_arg *arg)
> {
> switch (cmd) {
> case ION_IOC_HEAP_QUERY:
> @@ -34,6 +35,19 @@ static int validate_ioctl_arg(unsigned int cmd, union
> ion_ioctl_arg *arg)
> arg->query.reserved2 )
> return -EINVAL;
> break;
> +
> + case ION_IOC_ALLOC:
> + {
> + int mask = 1 << iminor(filp->f_inode);
> +
> +#ifdef CONFIG_ION_LEGACY_DEVICE_API
> + if (imajor(filp->f_inode) == MISC_MAJOR)
> + return 0;
> +#endif
> + if (!(arg->allocation.heap_id_mask & mask))
> + return -EINVAL;
> + break;
> + }
> default:
> break;
> }
> @@ -69,7 +83,7 @@ long ion_ioctl(struct file *filp, unsigned int cmd,
> unsigned long arg)
> if (copy_from_user(&data, (void __user *)arg, _IOC_SIZE(cmd)))
> return -EFAULT;
>
> - ret = validate_ioctl_arg(cmd, &data);
> + ret = validate_ioctl_arg(filp, cmd, &data);
> if (WARN_ON_ONCE(ret))
> return ret;
>
> diff --git a/drivers/staging/android/ion/ion.c
> b/drivers/staging/android/ion/ion.c
> index 93e2c90..dd66f55 100644
> --- a/drivers/staging/android/ion/ion.c
> +++ b/drivers/staging/android/ion/ion.c
> @@ -15,31 +15,35 @@
> *
> */
>
> +#include
> +#include
> #include
> +#include
> #include
> +#include
> #include
> #include
> #include
> -#include
> +#include
> #include
> #include
> #include
> #include
> -#include
> #include
> #include
> +#include
> #include
> -#include
> +#include
> #include
> +#include
> #include
> #include
> -#include
> -#include
> -#include
> -#include
>
> #include "ion.h"
>
> +#define ION_DEV_MAX 32
> +#define ION_NAME "ion"
Re: [PATCH v6 2/2] staging: ion: create one device entry per heap
On 10/31/2017 12:11 PM, Mark Brown wrote: > On Tue, Oct 31, 2017 at 12:03:35PM -0700, Laura Abbott wrote: > >> I'm not a fan of the platform bus but I have mixed feelings about >> creating a dedicated bus type. I guess if we really need a bus >> type we can do it later? > > There was a discussion a while ago in the context of I2C/SPI MFDs > which concluded that if you need a bus and it's going to be effectively > noop then you should just use the platform bus as anything else will > consist almost entirely of cut'n'paste from the platform bus with some > light sed usage and code duplication is bad. It's not super lovely as > it's not actually a memory mapped device but it's the best idea we've > got. > Thanks for the pointer. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: remove redundant variable table
On 10/31/2017 07:02 AM, Colin King wrote:
> From: Colin Ian King
>
> Variable table is being set but is never read, it is therefore
> redundant and can be removed. Cleans up clang warning:
>
> drivers/staging/android/ion/ion.c:112:2: warning: Value stored to 'table' is
> never read
>
Acked-by: Laura Abbott
> Signed-off-by: Colin Ian King
> ---
> drivers/staging/android/ion/ion.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/staging/android/ion/ion.c
> b/drivers/staging/android/ion/ion.c
> index 93e2c90fa77d..a7d9b0e98572 100644
> --- a/drivers/staging/android/ion/ion.c
> +++ b/drivers/staging/android/ion/ion.c
> @@ -81,7 +81,6 @@ static struct ion_buffer *ion_buffer_create(struct ion_heap
> *heap,
> unsigned long flags)
> {
> struct ion_buffer *buffer;
> - struct sg_table *table;
> int ret;
>
> buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
> @@ -109,7 +108,6 @@ static struct ion_buffer *ion_buffer_create(struct
> ion_heap *heap,
> goto err1;
> }
>
> - table = buffer->sg_table;
> buffer->dev = dev;
> buffer->size = len;
>
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 6/9] staging: android: add SPDX identifiers to all android driver files
On 11/07/2017 12:07 PM, Greg Kroah-Hartman wrote: It's good to have SPDX identifiers in all files to make it easier to audit the kernel tree for correct licenses. Update the android driver files with the correct SPDX license identifier based on the license text in the file itself. The SPDX identifier is a legally binding shorthand, which can be used instead of the full boiler plate text. This work is based on a script and data from Thomas Gleixner, Philippe Ombredanne, and Kate Stewart. For the Ion parts, Acked-by: Laura Abbott Cc: "Arve Hjønnevåg" Cc: Riley Andrews Cc: Laura Abbott Cc: Sumit Semwal Cc: Thomas Gleixner Cc: Kate Stewart Cc: Philippe Ombredanne Signed-off-by: Greg Kroah-Hartman --- drivers/staging/android/ashmem.c| 1 + drivers/staging/android/ashmem.h| 1 + drivers/staging/android/ion/ion-ioctl.c | 1 + drivers/staging/android/ion/ion.c | 1 + drivers/staging/android/ion/ion.h | 1 + drivers/staging/android/ion/ion_carveout_heap.c | 1 + drivers/staging/android/ion/ion_chunk_heap.c| 1 + drivers/staging/android/ion/ion_cma_heap.c | 1 + drivers/staging/android/ion/ion_heap.c | 1 + drivers/staging/android/ion/ion_page_pool.c | 1 + drivers/staging/android/ion/ion_system_heap.c | 1 + drivers/staging/android/uapi/ashmem.h | 1 + drivers/staging/android/uapi/ion.h | 1 + 13 files changed, 13 insertions(+) diff --git a/drivers/staging/android/ashmem.c b/drivers/staging/android/ashmem.c index 0f695df14c9d..24298077a3ba 100644 --- a/drivers/staging/android/ashmem.c +++ b/drivers/staging/android/ashmem.c @@ -1,3 +1,4 @@ +// SPDX-License-Identifier: GPL-2.0 /* mm/ashmem.c * * Anonymous Shared Memory Subsystem, ashmem diff --git a/drivers/staging/android/ashmem.h b/drivers/staging/android/ashmem.h index 5abcfd7aa706..60d7208f110a 100644 --- a/drivers/staging/android/ashmem.h +++ b/drivers/staging/android/ashmem.h @@ -1,3 +1,4 @@ +// SPDX-License-Identifier: (GPL-2.0 OR Apache-2.0) /* * include/linux/ashmem.h * diff --git a/drivers/staging/android/ion/ion-ioctl.c b/drivers/staging/android/ion/ion-ioctl.c index c78989351f9c..4eda17713d82 100644 --- a/drivers/staging/android/ion/ion-ioctl.c +++ b/drivers/staging/android/ion/ion-ioctl.c @@ -1,3 +1,4 @@ +// SPDX-License-Identifier: GPL-2.0 /* * * Copyright (C) 2011 Google, Inc. diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c index a7d9b0e98572..eac43db27b60 100644 --- a/drivers/staging/android/ion/ion.c +++ b/drivers/staging/android/ion/ion.c @@ -1,3 +1,4 @@ +// SPDX-License-Identifier: GPL-2.0 /* * * drivers/staging/android/ion/ion.c diff --git a/drivers/staging/android/ion/ion.h b/drivers/staging/android/ion/ion.h index f5f9cd63f8e9..3e9327625426 100644 --- a/drivers/staging/android/ion/ion.h +++ b/drivers/staging/android/ion/ion.h @@ -1,3 +1,4 @@ +// SPDX-License-Identifier: GPL-2.0 /* * drivers/staging/android/ion/ion.h * diff --git a/drivers/staging/android/ion/ion_carveout_heap.c b/drivers/staging/android/ion/ion_carveout_heap.c index fee7650d6fbb..ff1345d81564 100644 --- a/drivers/staging/android/ion/ion_carveout_heap.c +++ b/drivers/staging/android/ion/ion_carveout_heap.c @@ -1,3 +1,4 @@ +// SPDX-License-Identifier: GPL-2.0 /* * drivers/staging/android/ion/ion_carveout_heap.c * diff --git a/drivers/staging/android/ion/ion_chunk_heap.c b/drivers/staging/android/ion/ion_chunk_heap.c index 102c09398317..b23694f6bf7f 100644 --- a/drivers/staging/android/ion/ion_chunk_heap.c +++ b/drivers/staging/android/ion/ion_chunk_heap.c @@ -1,3 +1,4 @@ +// SPDX-License-Identifier: GPL-2.0 /* * drivers/staging/android/ion/ion_chunk_heap.c * diff --git a/drivers/staging/android/ion/ion_cma_heap.c b/drivers/staging/android/ion/ion_cma_heap.c index dd5545d9990a..6f6b1e208ec5 100644 --- a/drivers/staging/android/ion/ion_cma_heap.c +++ b/drivers/staging/android/ion/ion_cma_heap.c @@ -1,3 +1,4 @@ +// SPDX-License-Identifier: GPL-2.0 /* * drivers/staging/android/ion/ion_cma_heap.c * diff --git a/drivers/staging/android/ion/ion_heap.c b/drivers/staging/android/ion/ion_heap.c index 91faa7f035b9..97344eba611c 100644 --- a/drivers/staging/android/ion/ion_heap.c +++ b/drivers/staging/android/ion/ion_heap.c @@ -1,3 +1,4 @@ +// SPDX-License-Identifier: GPL-2.0 /* * drivers/staging/android/ion/ion_heap.c * diff --git a/drivers/staging/android/ion/ion_page_pool.c b/drivers/staging/android/ion/ion_page_pool.c index 817849df9de3..4c63e8f25b3f 100644 --- a/drivers/staging/android/ion/ion_page_pool.c +++ b/drivers/staging/android/ion/ion_page_pool.c @@ -1,3 +1,4 @@ +// SPDX-License-Identifier: GPL-2.0 /* * drivers/staging/android/ion/ion_mem_pool.c * diff --git a/drivers/staging/android/ion/ion_system_heap.c b/drivers/staging/android/ion/ion_system_heap.c index 4dc5d7a589c2..9ff940095e3b 10064
Re: [PATCH 9/9] staging: android: Remove redundant license text
On 11/07/2017 12:07 PM, Greg Kroah-Hartman wrote: Now that the SPDX tag is in all android files, that identifies the license in a specific and legally-defined manner. So the extra GPL text wording can be removed as it is no longer needed at all. This is done on a quest to remove the 700+ different ways that files in the kernel describe the GPL license text. And there's unneeded stuff like the address (sometimes incorrect) for the FSF which is never needed. No copyright headers or other non-license-description text was removed. For the Ion portion, Acked-by: Laura Abbott Cc: "Arve Hjønnevåg" Cc: Riley Andrews Cc: Laura Abbott Cc: Sumit Semwal Signed-off-by: Greg Kroah-Hartman --- drivers/staging/android/ashmem.c| 9 - drivers/staging/android/ion/ion-ioctl.c | 11 --- drivers/staging/android/ion/ion.c | 11 --- drivers/staging/android/ion/ion.h | 10 -- drivers/staging/android/ion/ion_carveout_heap.c | 10 -- drivers/staging/android/ion/ion_chunk_heap.c| 10 -- drivers/staging/android/ion/ion_cma_heap.c | 10 -- drivers/staging/android/ion/ion_heap.c | 10 -- drivers/staging/android/ion/ion_page_pool.c | 10 -- drivers/staging/android/ion/ion_system_heap.c | 10 -- drivers/staging/android/uapi/ion.h | 10 -- 11 files changed, 111 deletions(-) diff --git a/drivers/staging/android/ashmem.c b/drivers/staging/android/ashmem.c index 24298077a3ba..4e8947923904 100644 --- a/drivers/staging/android/ashmem.c +++ b/drivers/staging/android/ashmem.c @@ -6,15 +6,6 @@ * Copyright (C) 2008 Google, Inc. * * Robert Love - * - * This software is licensed under the terms of the GNU General Public - * License version 2, as published by the Free Software Foundation, and - * may be copied, distributed, and modified under those terms. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. */ #define pr_fmt(fmt) "ashmem: " fmt diff --git a/drivers/staging/android/ion/ion-ioctl.c b/drivers/staging/android/ion/ion-ioctl.c index 4eda17713d82..6ed2cc15c8c0 100644 --- a/drivers/staging/android/ion/ion-ioctl.c +++ b/drivers/staging/android/ion/ion-ioctl.c @@ -1,17 +1,6 @@ // SPDX-License-Identifier: GPL-2.0 /* - * * Copyright (C) 2011 Google, Inc. - * - * This software is licensed under the terms of the GNU General Public - * License version 2, as published by the Free Software Foundation, and - * may be copied, distributed, and modified under those terms. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * */ #include diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c index eac43db27b60..7630432a7de8 100644 --- a/drivers/staging/android/ion/ion.c +++ b/drivers/staging/android/ion/ion.c @@ -1,19 +1,8 @@ // SPDX-License-Identifier: GPL-2.0 /* - * * drivers/staging/android/ion/ion.c * * Copyright (C) 2011 Google, Inc. - * - * This software is licensed under the terms of the GNU General Public - * License version 2, as published by the Free Software Foundation, and - * may be copied, distributed, and modified under those terms. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * */ #include diff --git a/drivers/staging/android/ion/ion.h b/drivers/staging/android/ion/ion.h index 3e9327625426..7409437ffdd8 100644 --- a/drivers/staging/android/ion/ion.h +++ b/drivers/staging/android/ion/ion.h @@ -3,16 +3,6 @@ * drivers/staging/android/ion/ion.h * * Copyright (C) 2011 Google, Inc. - * - * This software is licensed under the terms of the GNU General Public - * License version 2, as published by the Free Software Foundation, and - * may be copied, distributed, and modified under those terms. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * */ #ifndef _ION_H diff --git a/drivers/staging/android/ion/ion_carveout_heap.c b/drivers/staging/android/ion/ion_carveout_heap.c index ff1345d81564..e129237a0417 100644 --- a/drivers/staging/android/ion/ion_carveout_heap.c +++ b/drivers/staging/android/ion/ion_carv
Re: [PATCH v6 1/2] staging: ion: reorder include
On 11/06/2017 07:59 AM, Benjamin Gaignard wrote: Put include in alphabetic order Acked-by: Laura Abbott Signed-off-by: Benjamin Gaignard --- drivers/staging/android/ion/ion.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c index a7d9b0e..fda9756 100644 --- a/drivers/staging/android/ion/ion.c +++ b/drivers/staging/android/ion/ion.c @@ -15,28 +15,28 @@ * */ +#include +#include #include +#include #include +#include #include #include #include -#include +#include #include #include #include #include -#include #include #include #include -#include +#include #include +#include #include #include -#include -#include -#include -#include #include "ion.h" ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v6 2/2] staging: ion: create one device entry per heap
On 11/06/2017 07:59 AM, Benjamin Gaignard wrote:
Instead a getting only one common device "/dev/ion" for
all the heaps this patch allow to create one device
entry ("/dev/ionX") per heap.
Getting an entry per heap could allow to set security rules
per heap and global ones for all heaps.
Allocation requests will be only allowed if the mask_id
match with device minor.
Query request could be done on any of the devices.
With this patch, sysfs looks like:
$ ls /sys/devices/
breakpoint ion platform software system virtual
From an Ion perspective, you can have
Acked-by: Laura Abbott
Another Ack for the device model stuff would be good but I'll
assume deafening silence means nobody hates it.
Thanks,
Laura
Signed-off-by: Benjamin Gaignard
---
drivers/staging/android/TODO| 1 -
drivers/staging/android/ion/Kconfig | 7
drivers/staging/android/ion/ion-ioctl.c | 18 --
drivers/staging/android/ion/ion.c | 62 +
drivers/staging/android/ion/ion.h | 15 ++--
5 files changed, 91 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/android/TODO b/drivers/staging/android/TODO
index 687e0ea..8a11931 100644
--- a/drivers/staging/android/TODO
+++ b/drivers/staging/android/TODO
@@ -8,7 +8,6 @@ TODO:
ion/
- Add dt-bindings for remaining heaps (chunk and carveout heaps). This would
involve putting appropriate bindings in a memory node for Ion to find.
- - Split /dev/ion up into multiple nodes (e.g. /dev/ion/heap0)
- Better test framework (integration with VGEM was suggested)
Please send patches to Greg Kroah-Hartman and Cc:
diff --git a/drivers/staging/android/ion/Kconfig
b/drivers/staging/android/ion/Kconfig
index a517b2d..cb4666e 100644
--- a/drivers/staging/android/ion/Kconfig
+++ b/drivers/staging/android/ion/Kconfig
@@ -10,6 +10,13 @@ menuconfig ION
If you're not using Android its probably safe to
say N here.
+config ION_LEGACY_DEVICE_API
+ bool "Keep using Ion legacy misc device API"
+ depends on ION
+ help
+ Choose this option to keep using Ion legacy misc device API
+ i.e. /dev/ion
+
config ION_SYSTEM_HEAP
bool "Ion system heap"
depends on ION
diff --git a/drivers/staging/android/ion/ion-ioctl.c
b/drivers/staging/android/ion/ion-ioctl.c
index e26b786..bb5c77b 100644
--- a/drivers/staging/android/ion/ion-ioctl.c
+++ b/drivers/staging/android/ion/ion-ioctl.c
@@ -25,7 +25,8 @@ union ion_ioctl_arg {
struct ion_heap_query query;
};
-static int validate_ioctl_arg(unsigned int cmd, union ion_ioctl_arg *arg)
+static int validate_ioctl_arg(struct file *filp,
+ unsigned int cmd, union ion_ioctl_arg *arg)
{
switch (cmd) {
case ION_IOC_HEAP_QUERY:
@@ -34,6 +35,19 @@ static int validate_ioctl_arg(unsigned int cmd, union
ion_ioctl_arg *arg)
arg->query.reserved2 )
return -EINVAL;
break;
+
+ case ION_IOC_ALLOC:
+ {
+ int mask = 1 << iminor(filp->f_inode);
+
+#ifdef CONFIG_ION_LEGACY_DEVICE_API
+ if (imajor(filp->f_inode) == MISC_MAJOR)
+ return 0;
+#endif
+ if (!(arg->allocation.heap_id_mask & mask))
+ return -EINVAL;
+ break;
+ }
default:
break;
}
@@ -69,7 +83,7 @@ long ion_ioctl(struct file *filp, unsigned int cmd, unsigned
long arg)
if (copy_from_user(&data, (void __user *)arg, _IOC_SIZE(cmd)))
return -EFAULT;
- ret = validate_ioctl_arg(cmd, &data);
+ ret = validate_ioctl_arg(filp, cmd, &data);
if (WARN_ON_ONCE(ret))
return ret;
diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index fda9756..2c2568b 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -40,6 +40,9 @@
#include "ion.h"
+#define ION_DEV_MAX 32
+#define ION_NAME "ion"
+
static struct ion_device *internal_dev;
static int heap_id;
@@ -535,15 +538,38 @@ static int debug_shrink_get(void *data, u64 *val)
DEFINE_SIMPLE_ATTRIBUTE(debug_shrink_fops, debug_shrink_get,
debug_shrink_set, "%llu\n");
-void ion_device_add_heap(struct ion_heap *heap)
+static struct device ion_bus = {
+ .init_name = ION_NAME,
+};
+
+static struct bus_type ion_bus_type = {
+ .name = ION_NAME,
+};
+
+int ion_device_add_heap(struct ion_heap *heap)
{
struct dentry *debug_file;
struct ion_device *dev = internal_dev;
+ int ret = 0;
if (!heap->ops->allocate || !heap->ops->free)
pr_err("%s: can not add heap with invalid ops struct.\n",
__func__);
+ if
Re: [PATCH v6 2/2] staging: ion: create one device entry per heap
On 12/02/2017 07:53 AM, Greg KH wrote: This is one of the item in the TODO list before been able to unstage ION which is my real need. Why does it matter where in the tree this code is? Don't go adding new things to it that are not needed. Who needs this? What userspace code wants this type of multiple ion devices? Requirements came in from several places to split /dev/ion -> /dev/ion0 and /dev/ion1 so that security policy (i.e. selinux) could be used to protect access to certain heaps. I wanted the ABI to be settled before trying to move out of staging, hence the line in the TODO list about doing the split. thanks, greg k-h Thanks, Laura ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: Fix dma direction for dma_sync_sg_for_cpu/device
On 12/15/2017 12:59 PM, Sushmita Susheelendra wrote:
Use the direction argument passed into begin_cpu_access
and end_cpu_access when calling the dma_sync_sg_for_cpu/device.
The actual cache primitive called depends on the direction
passed in.
Acked-by: Laura Abbott
Signed-off-by: Sushmita Susheelendra
---
drivers/staging/android/ion/ion.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index a7d9b0e..f480885 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -346,7 +346,7 @@ static int ion_dma_buf_begin_cpu_access(struct dma_buf
*dmabuf,
mutex_lock(&buffer->lock);
list_for_each_entry(a, &buffer->attachments, list) {
dma_sync_sg_for_cpu(a->dev, a->table->sgl, a->table->nents,
- DMA_BIDIRECTIONAL);
+ direction);
}
mutex_unlock(&buffer->lock);
@@ -368,7 +368,7 @@ static int ion_dma_buf_end_cpu_access(struct dma_buf *dmabuf,
mutex_lock(&buffer->lock);
list_for_each_entry(a, &buffer->attachments, list) {
dma_sync_sg_for_device(a->dev, a->table->sgl, a->table->nents,
- DMA_BIDIRECTIONAL);
+ direction);
}
mutex_unlock(&buffer->lock);
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: android: ion: Clean unused debug_show memeber of the heap object
On 08/26/2018 01:08 PM, Alexey Skidanov wrote:
ION had supported heap debug info under /sys/kernel/debug/ion/.
This support have been removed but some leftovers (dead code) still exist.
This patch removes the existing dead code.
This was actually an unintended side effect. The goal was to rip out
the racy handle/buffer mess and the associated debugfs there. I do
think the extra heap_show for system heap is useful but I'd rather
see it reimplemented with a full debugfs vs. trying to just tack it
on.
I'd like to see the exact commit where this was orphaned (15c6098cfec5
("staging: android: ion: Remove ion_handle and ion_client")) specified
somewhere in the commit text so if you add that, you can add
Acked-by: Laura Abbott
Signed-off-by: Alexey Skidanov
---
drivers/staging/android/ion/ion.h | 5 -
drivers/staging/android/ion/ion_system_heap.c | 24
2 files changed, 29 deletions(-)
diff --git a/drivers/staging/android/ion/ion.h
b/drivers/staging/android/ion/ion.h
index 876197b..0afa9cd 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -158,8 +158,6 @@ struct ion_heap_ops {
* @lock: protects the free list
* @waitqueue:queue to wait on from deferred free thread
* @task: task struct of deferred free thread
- * @debug_show:called when heap debug file is read to add any
- * heap specific debug info to output
*
* Represents a pool of memory from which buffers can be made. In some
* systems the only heap is regular system memory allocated via vmalloc.
@@ -180,9 +178,6 @@ struct ion_heap {
spinlock_t free_lock;
wait_queue_head_t waitqueue;
struct task_struct *task;
-
- int (*debug_show)(struct ion_heap *heap, struct seq_file *s,
- void *unused);
};
/**
diff --git a/drivers/staging/android/ion/ion_system_heap.c
b/drivers/staging/android/ion/ion_system_heap.c
index b5c3195..d0d0490 100644
--- a/drivers/staging/android/ion/ion_system_heap.c
+++ b/drivers/staging/android/ion/ion_system_heap.c
@@ -213,29 +213,6 @@ static struct ion_heap_ops system_heap_ops = {
.shrink = ion_system_heap_shrink,
};
-static int ion_system_heap_debug_show(struct ion_heap *heap, struct seq_file *s,
- void *unused)
-{
- struct ion_system_heap *sys_heap = container_of(heap,
- struct ion_system_heap,
- heap);
- int i;
- struct ion_page_pool *pool;
-
- for (i = 0; i < NUM_ORDERS; i++) {
- pool = sys_heap->pools[i];
-
- seq_printf(s, "%d order %u highmem pages %lu total\n",
- pool->high_count, pool->order,
- (PAGE_SIZE << pool->order) * pool->high_count);
- seq_printf(s, "%d order %u lowmem pages %lu total\n",
- pool->low_count, pool->order,
- (PAGE_SIZE << pool->order) * pool->low_count);
- }
-
- return 0;
-}
-
static void ion_system_heap_destroy_pools(struct ion_page_pool **pools)
{
int i;
@@ -282,7 +259,6 @@ static struct ion_heap *__ion_system_heap_create(void)
if (ion_system_heap_create_pools(heap->pools))
goto free_heap;
- heap->heap.debug_show = ion_system_heap_debug_show;
return &heap->heap;
free_heap:
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: android: ion: fix ION_IOC_{MAP,SHARE} use-after-free
On 09/04/2018 09:33 AM, Greg Hackmann wrote:
The ION_IOC_{MAP,SHARE} ioctls drop and reacquire client->lock several
times while operating on one of the client's ion_handles. This creates
windows where userspace can call ION_IOC_FREE on the same client with
the same handle, and effectively make the kernel drop its own reference.
For example:
- thread A: ION_IOC_ALLOC creates an ion_handle with refcount 1
- thread A: starts ION_IOC_MAP and increments the refcount to 2
- thread B: ION_IOC_FREE decrements the refcount to 1
- thread B: ION_IOC_FREE decrements the refcount to 0 and frees the
handle
- thread A: continues ION_IOC_MAP with a dangling ion_handle * to
freed memory
Fix this by holding client->lock for the duration of
ION_IOC_{MAP,SHARE}, preventing the concurrent ION_IOC_FREE. Also
remove ion_handle_get_by_id(), since there's literally no way to use it
safely.
This patch is applied on top of 4.4.y, and applies to older kernels
too. 4.9.y was fixed separately. Kernels 4.12 and later are
unaffected, since all the underlying ion_handle infrastructure has been
ripped out.
Acked-by: Laura Abbott
Cc: sta...@vger.kernel.org # v4.4-
Signed-off-by: Greg Hackmann
---
v2: remove Change-Id line from commit message
drivers/staging/android/ion/ion.c | 60 +++
1 file changed, 37 insertions(+), 23 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 47cb163da9a0..4adb1138af09 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -449,18 +449,6 @@ static struct ion_handle
*ion_handle_get_by_id_nolock(struct ion_client *client,
return 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)
{
@@ -1138,24 +1126,28 @@ static 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)
+static struct dma_buf *__ion_share_dma_buf(struct ion_client *client,
+ struct ion_handle *handle,
+ bool lock_client)
{
DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
struct ion_buffer *buffer;
struct dma_buf *dmabuf;
bool valid_handle;
- mutex_lock(&client->lock);
+ if (lock_client)
+ 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);
+ if (lock_client)
+ mutex_unlock(&client->lock);
return ERR_PTR(-EINVAL);
}
buffer = handle->buffer;
ion_buffer_get(buffer);
- mutex_unlock(&client->lock);
+ if (lock_client)
+ mutex_unlock(&client->lock);
exp_info.ops = &dma_buf_ops;
exp_info.size = buffer->size;
@@ -1170,14 +1162,21 @@ struct dma_buf *ion_share_dma_buf(struct ion_client
*client,
return dmabuf;
}
+
+struct dma_buf *ion_share_dma_buf(struct ion_client *client,
+ struct ion_handle *handle)
+{
+ return __ion_share_dma_buf(client, handle, true);
+}
EXPORT_SYMBOL(ion_share_dma_buf);
-int ion_share_dma_buf_fd(struct ion_client *client, struct ion_handle *handle)
+static int __ion_share_dma_buf_fd(struct ion_client *client,
+ struct ion_handle *handle, bool lock_client)
{
struct dma_buf *dmabuf;
int fd;
- dmabuf = ion_share_dma_buf(client, handle);
+ dmabuf = __ion_share_dma_buf(client, handle, lock_client);
if (IS_ERR(dmabuf))
return PTR_ERR(dmabuf);
@@ -1187,8 +1186,19 @@ int ion_share_dma_buf_fd(struct ion_client *client, struct ion_handle *handle)
return fd;
}
+
+int ion_share_dma_buf_fd(struct ion_client *client, struct ion_handle *handle)
+{
+ return __ion_share_dma_buf_fd(client, handle, true);
+}
EXPORT_SYMBOL(ion_share_dma_buf_fd);
+static int ion_share_dma_buf_fd_nolock(struct ion_client *client,
+ struct ion_handle *handle)
+{
+ return __ion_share_dma_buf_fd(client, handle, false);
+}
+
struct ion_handle *ion_import_dma_buf(struct ion_client *client, int fd)
{
struct dma_buf
Re: [Linaro-mm-sig] [RFCv3][PATCH 3/5] arm64: Implement ARCH_HAS_FORCE_CACHE
On 02/20/2017 10:05 PM, Chen Feng wrote: > Hi Laura, > > When we enable kernel v4.4 or newer version on our platform, we meet the issue > of flushing cache without reference device. It seems that this patch set is > a solution. I'm curious the progress of the discussion. Do you have any plan > to fix it in v4.4 and newer kernel verison? > No, I've abandoned this approach based on feedback. The APIs had too much potential for incorrect usage. I'm ripping out the implicit caching in Ion and switching it to a model where there should always be a device available. What's your use case where you don't have a device structure? Thanks, Laura > On 2016/9/14 2:41, Laura Abbott wrote: >> On 09/13/2016 08:14 AM, Will Deacon wrote: >>> On Tue, Sep 13, 2016 at 08:02:20AM -0700, Laura Abbott wrote: >>>> On 09/13/2016 02:19 AM, Will Deacon wrote: >>>>> On Mon, Sep 12, 2016 at 02:32:56PM -0700, Laura Abbott wrote: >>>>>> >>>>>> arm64 may need to guarantee the caches are synced. Implement versions of >>>>>> the kernel_force_cache API to allow this. >>>>>> >>>>>> Signed-off-by: Laura Abbott >>>>>> --- >>>>>> v3: Switch to calling cache operations directly instead of relying on >>>>>> DMA mapping. >>>>>> --- >>>>>> arch/arm64/include/asm/cacheflush.h | 8 >>>>>> arch/arm64/mm/cache.S | 24 >>>>>> arch/arm64/mm/flush.c | 11 +++ >>>>>> 3 files changed, 39 insertions(+), 4 deletions(-) >>>>> >>>>> I'm really hesitant to expose these cache routines as an API solely to >>>>> support a driver sitting in staging/. I appreciate that there's a chicken >>>>> and egg problem here, but we *really* don't want people using these >>>>> routines >>>>> in preference to the DMA API, and I fear that we'll simply grow a bunch >>>>> more users of these things if we promote it as an API like you're >>>>> proposing. >>>>> >>>>> Can the code not be contained under staging/, as part of ion? >>>>> >>>> >>>> I proposed that in V1 and it was suggested I make it a proper API >>>> >>>> http://www.mail-archive.com/driverdev-devel@linuxdriverproject.org/msg47654.html >>>> http://www.mail-archive.com/driverdev-devel@linuxdriverproject.org/msg47672.html >>> >>> :/ then I guess we're in disagreement. If ion really needs this stuff >>> (which I don't fully grok), perhaps we should be exposing something at >>> a higher level from the architecture, so it really can't be used for >>> anything other than ion. >> >> I talked/complained about this at a past plumbers. The gist is that Ion >> ends up acting as a fake DMA layer for clients. It doesn't match nicely >> because clients can allocate both coherent and non-coherent memory. >> Trying to use dma_map doesn't work because a) a device for coherency isn't >> known at allocation time b) it kills performance. Part of the motivation >> for taking this approach is to avoid the need to rework the existing >> Android userspace and keep the existing behavior, as terrible as it >> is. Having Ion out of staging and not actually usable isn't helpful. >> >> I'll give this all some more thought and hopefully have one or two more >> proposals before Connect/Plumbers. >> >>> >>> Will >>> >> >> Thanks, >> Laura >> ___ >> Linaro-mm-sig mailing list >> linaro-mm-...@lists.linaro.org >> https://lists.linaro.org/mailman/listinfo/linaro-mm-sig > > > ___ > linux-arm-kernel mailing list > linux-arm-ker...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [Linaro-mm-sig] [RFCv3][PATCH 3/5] arm64: Implement ARCH_HAS_FORCE_CACHE
On 02/22/2017 05:01 PM, Chen Feng wrote: > > > On 2017/2/22 3:29, Laura Abbott wrote: >> On 02/20/2017 10:05 PM, Chen Feng wrote: >>> Hi Laura, >>> >>> When we enable kernel v4.4 or newer version on our platform, we meet the >>> issue >>> of flushing cache without reference device. It seems that this patch set is >>> a solution. I'm curious the progress of the discussion. Do you have any plan >>> to fix it in v4.4 and newer kernel verison? >>> >> >> No, I've abandoned this approach based on feedback. The APIs had too much >> potential for incorrect usage. I'm ripping out the implicit caching in Ion >> and switching it to a model where there should always be a device available. >> >> What's your use case where you don't have a device structure? >> > Userspace use ioctl to flush cache for device. > > ion_sync_for_device > dma_sync_sg_for_device(NULL, buffer->sg_table->sgl, >buffer->sg_table->nents, DMA_BIDIRECTIONAL); > > And sys-heap when allocate a zero buffer flush zero data to ddr. > alloc_buffer_page > ion_pages_sync_for_device(NULL, page, PAGE_SIZE << order, > DMA_BIDIRECTIONAL); > > Yes, those calls are being removed. This is what I proposed back in December https://marc.info/?l=linux-kernel&m=148176054902921&w=2 I never heard any feedback on it so I assume everyone was okay with the general direction. I plan on pushing a revised and expanded version of that series once the merge window closes. Thanks, Laura > >> Thanks, >> Laura >> >>> On 2016/9/14 2:41, Laura Abbott wrote: >>>> On 09/13/2016 08:14 AM, Will Deacon wrote: >>>>> On Tue, Sep 13, 2016 at 08:02:20AM -0700, Laura Abbott wrote: >>>>>> On 09/13/2016 02:19 AM, Will Deacon wrote: >>>>>>> On Mon, Sep 12, 2016 at 02:32:56PM -0700, Laura Abbott wrote: >>>>>>>> >>>>>>>> arm64 may need to guarantee the caches are synced. Implement versions >>>>>>>> of >>>>>>>> the kernel_force_cache API to allow this. >>>>>>>> >>>>>>>> Signed-off-by: Laura Abbott >>>>>>>> --- >>>>>>>> v3: Switch to calling cache operations directly instead of relying on >>>>>>>> DMA mapping. >>>>>>>> --- >>>>>>>> arch/arm64/include/asm/cacheflush.h | 8 >>>>>>>> arch/arm64/mm/cache.S | 24 >>>>>>>> arch/arm64/mm/flush.c | 11 +++ >>>>>>>> 3 files changed, 39 insertions(+), 4 deletions(-) >>>>>>> >>>>>>> I'm really hesitant to expose these cache routines as an API solely to >>>>>>> support a driver sitting in staging/. I appreciate that there's a >>>>>>> chicken >>>>>>> and egg problem here, but we *really* don't want people using these >>>>>>> routines >>>>>>> in preference to the DMA API, and I fear that we'll simply grow a bunch >>>>>>> more users of these things if we promote it as an API like you're >>>>>>> proposing. >>>>>>> >>>>>>> Can the code not be contained under staging/, as part of ion? >>>>>>> >>>>>> >>>>>> I proposed that in V1 and it was suggested I make it a proper API >>>>>> >>>>>> http://www.mail-archive.com/driverdev-devel@linuxdriverproject.org/msg47654.html >>>>>> http://www.mail-archive.com/driverdev-devel@linuxdriverproject.org/msg47672.html >>>>> >>>>> :/ then I guess we're in disagreement. If ion really needs this stuff >>>>> (which I don't fully grok), perhaps we should be exposing something at >>>>> a higher level from the architecture, so it really can't be used for >>>>> anything other than ion. >>>> >>>> I talked/complained about this at a past plumbers. The gist is that Ion >>>> ends up acting as a fake DMA layer for clients. It doesn't match nicely >>>> because clients can allocate both coherent and non-coherent memory. >>>> Trying to use dma_map doesn't work because a) a device for coherency isn't >>>> known at allocation time b) it kills performance. Part of the motivation >>>> for taking this approach is to avoid the need to rework the existing >>>> Android userspace and keep the existing behavior, as terrible as it >>>> is. Having Ion out of staging and not actually usable isn't helpful. >>>> >>>> I'll give this all some more thought and hopefully have one or two more >>>> proposals before Connect/Plumbers. >>>> >>>>> >>>>> Will >>>>> >>>> >>>> Thanks, >>>> Laura >>>> ___ >>>> Linaro-mm-sig mailing list >>>> linaro-mm-...@lists.linaro.org >>>> https://lists.linaro.org/mailman/listinfo/linaro-mm-sig >>> >>> >>> ___ >>> linux-arm-kernel mailing list >>> linux-arm-ker...@lists.infradead.org >>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >>> >> >> >> . >> > > > ___ > linux-arm-kernel mailing list > linux-arm-ker...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging
Hi, There's been some recent discussions[1] about Ion-like frameworks. There's apparently interest in just keeping Ion since it works reasonablly well. This series does what should be the final clean ups for it to possibly be moved out of staging. This includes the following: - Some general clean up and removal of features that never got a lot of use as far as I can tell. - Fixing up the caching. This is the series I proposed back in December[2] but never heard any feedback on. It will certainly break existing applications that rely on the implicit caching. I'd rather make an effort to move to a model that isn't going directly against the establishement though. - Fixing up the platform support. The devicetree approach was never well recieved by DT maintainers. The proposal here is to think of Ion less as specifying requirements and more of a framework for exposing memory to userspace. - CMA allocations now happen without the need of a dummy device structure. This fixes a bunch of the reasons why I attempted to add devicetree support before. I've had problems getting feedback in the past so if I don't hear any major objections I'm going to send out with the RFC dropped to be picked up. The only reason there isn't a patch to come out of staging is to discuss any other changes to the ABI people might want. Once this comes out of staging, I really don't want to mess with the ABI. Feedback appreciated. Thanks, Laura [1] https://marc.info/?l=linux-kernel&m=148699712602105&w=2 [2] https://marc.info/?l=linaro-mm-sig&m=148176050802908&w=2 Laura Abbott (12): staging: android: ion: Remove dmap_cnt staging: android: ion: Remove alignment from allocation field staging: android: ion: Duplicate sg_table staging: android: ion: Call dma_map_sg for syncing and mapping staging: android: ion: Remove page faulting support staging: android: ion: Remove crufty cache support staging: android: ion: Remove old platform support cma: Store a name in the cma structure cma: Introduce cma_for_each_area staging: android: ion: Use CMA APIs directly staging: android: ion: Make Ion heaps selectable staging; android: ion: Enumerate all available heaps drivers/base/dma-contiguous.c | 5 +- drivers/staging/android/ion/Kconfig| 51 ++-- drivers/staging/android/ion/Makefile | 14 +- drivers/staging/android/ion/hisilicon/Kconfig | 5 - drivers/staging/android/ion/hisilicon/Makefile | 1 - drivers/staging/android/ion/hisilicon/hi6220_ion.c | 113 - drivers/staging/android/ion/ion-ioctl.c| 6 - drivers/staging/android/ion/ion.c | 282 ++--- drivers/staging/android/ion/ion.h | 5 +- drivers/staging/android/ion/ion_carveout_heap.c| 16 +- drivers/staging/android/ion/ion_chunk_heap.c | 15 +- drivers/staging/android/ion/ion_cma_heap.c | 102 ++-- drivers/staging/android/ion/ion_dummy_driver.c | 156 drivers/staging/android/ion/ion_enumerate.c| 89 +++ drivers/staging/android/ion/ion_of.c | 184 -- drivers/staging/android/ion/ion_of.h | 37 --- drivers/staging/android/ion/ion_page_pool.c| 3 - drivers/staging/android/ion/ion_priv.h | 57 - drivers/staging/android/ion/ion_system_heap.c | 14 +- drivers/staging/android/ion/tegra/Makefile | 1 - drivers/staging/android/ion/tegra/tegra_ion.c | 80 -- include/linux/cma.h| 6 +- mm/cma.c | 25 +- mm/cma.h | 1 + mm/cma_debug.c | 2 +- 25 files changed, 312 insertions(+), 958 deletions(-) delete mode 100644 drivers/staging/android/ion/hisilicon/Kconfig delete mode 100644 drivers/staging/android/ion/hisilicon/Makefile delete mode 100644 drivers/staging/android/ion/hisilicon/hi6220_ion.c delete mode 100644 drivers/staging/android/ion/ion_dummy_driver.c create mode 100644 drivers/staging/android/ion/ion_enumerate.c delete mode 100644 drivers/staging/android/ion/ion_of.c delete mode 100644 drivers/staging/android/ion/ion_of.h delete mode 100644 drivers/staging/android/ion/tegra/Makefile delete mode 100644 drivers/staging/android/ion/tegra/tegra_ion.c -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[RFC PATCH 01/12] staging: android: ion: Remove dmap_cnt
The reference counting of dma_map calls was removed. Remove the
associated counter field as well.
Signed-off-by: Laura Abbott
---
drivers/staging/android/ion/ion_priv.h | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/staging/android/ion/ion_priv.h
b/drivers/staging/android/ion/ion_priv.h
index 5b3059c..46d3ff5 100644
--- a/drivers/staging/android/ion/ion_priv.h
+++ b/drivers/staging/android/ion/ion_priv.h
@@ -44,7 +44,6 @@
* @lock: protects the buffers cnt fields
* @kmap_cnt: number of times the buffer is mapped to the kernel
* @vaddr: the kernel mapping if kmap_cnt is not zero
- * @dmap_cnt: number of times the buffer is mapped for dma
* @sg_table: the sg table for the buffer if dmap_cnt is not zero
* @pages: flat array of pages in the buffer -- used by fault
* handler and only valid for buffers that are faulted in
@@ -70,7 +69,6 @@ struct ion_buffer {
struct mutex lock;
int kmap_cnt;
void *vaddr;
- int dmap_cnt;
struct sg_table *sg_table;
struct page **pages;
struct list_head vmas;
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[RFC PATCH 02/12] staging: android: ion: Remove alignment from allocation field
The align field was supposed to be used to specify the alignment of
the allocation. Nobody actually does anything with it except to check
if the alignment specified is out of bounds. Since this has no effect
on the actual allocation, just remove it.
Signed-off-by: Laura Abbott
---
drivers/staging/android/ion/ion-ioctl.c | 1 -
drivers/staging/android/ion/ion.c | 14 ++
drivers/staging/android/ion/ion.h | 5 +
drivers/staging/android/ion/ion_carveout_heap.c | 10 +++---
drivers/staging/android/ion/ion_chunk_heap.c| 9 +++--
drivers/staging/android/ion/ion_cma_heap.c | 5 +
drivers/staging/android/ion/ion_priv.h | 2 +-
drivers/staging/android/ion/ion_system_heap.c | 9 +
8 files changed, 16 insertions(+), 39 deletions(-)
diff --git a/drivers/staging/android/ion/ion-ioctl.c
b/drivers/staging/android/ion/ion-ioctl.c
index 9ff815a..5b2e93f 100644
--- a/drivers/staging/android/ion/ion-ioctl.c
+++ b/drivers/staging/android/ion/ion-ioctl.c
@@ -95,7 +95,6 @@ long ion_ioctl(struct file *filp, unsigned int cmd, unsigned
long arg)
struct ion_handle *handle;
handle = ion_alloc(client, data.allocation.len,
- data.allocation.align,
data.allocation.heap_id_mask,
data.allocation.flags);
if (IS_ERR(handle))
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 9696007..94a498e 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -102,7 +102,6 @@ static void ion_buffer_add(struct ion_device *dev,
static struct ion_buffer *ion_buffer_create(struct ion_heap *heap,
struct ion_device *dev,
unsigned long len,
- unsigned long align,
unsigned long flags)
{
struct ion_buffer *buffer;
@@ -118,15 +117,14 @@ static struct ion_buffer *ion_buffer_create(struct
ion_heap *heap,
buffer->flags = flags;
kref_init(&buffer->ref);
- ret = heap->ops->allocate(heap, buffer, len, align, flags);
+ ret = heap->ops->allocate(heap, buffer, len, flags);
if (ret) {
if (!(heap->flags & ION_HEAP_FLAG_DEFER_FREE))
goto err2;
ion_heap_freelist_drain(heap, 0);
- ret = heap->ops->allocate(heap, buffer, len, align,
- flags);
+ ret = heap->ops->allocate(heap, buffer, len, flags);
if (ret)
goto err2;
}
@@ -400,7 +398,7 @@ static int ion_handle_add(struct ion_client *client, struct
ion_handle *handle)
}
struct ion_handle *ion_alloc(struct ion_client *client, size_t len,
-size_t align, unsigned int heap_id_mask,
+unsigned int heap_id_mask,
unsigned int flags)
{
struct ion_handle *handle;
@@ -409,8 +407,8 @@ struct ion_handle *ion_alloc(struct ion_client *client,
size_t len,
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);
+ 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
@@ -427,7 +425,7 @@ struct ion_handle *ion_alloc(struct ion_client *client,
size_t len,
/* 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);
+ buffer = ion_buffer_create(heap, dev, len, flags);
if (!IS_ERR(buffer))
break;
}
diff --git a/drivers/staging/android/ion/ion.h
b/drivers/staging/android/ion/ion.h
index 93dafb4..3b4bff5 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -45,7 +45,6 @@ struct ion_buffer;
* @name: used for debug purposes
* @base: base address of heap in physical memory if applicable
* @size: size of the heap in bytes if applicable
- * @align: required alignment in physical memory if applicable
* @priv: private info passed from the board file
*
* Provided by the board file.
@@ -93,8 +92,6 @@ void ion_client_destroy(struct ion_client *client);
* ion_alloc - allocate ion memory
[RFC PATCH 03/12] staging: android: ion: Duplicate sg_table
Ion currently returns a single sg_table on each dma_map call. This is
incorrect for later usage.
Signed-off-by: Laura Abbott
---
drivers/staging/android/ion/ion.c | 30 +-
1 file changed, 29 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 94a498e..ce4adac 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -799,6 +799,32 @@ static void ion_buffer_sync_for_device(struct ion_buffer
*buffer,
struct device *dev,
enum dma_data_direction direction);
+static struct sg_table *dup_sg_table(struct sg_table *table)
+{
+ struct sg_table *new_table;
+ int ret, i;
+ struct scatterlist *sg, *new_sg;
+
+ new_table = kzalloc(sizeof(*new_table), GFP_KERNEL);
+ if (!new_table)
+ return ERR_PTR(-ENOMEM);
+
+ ret = sg_alloc_table(new_table, table->nents, GFP_KERNEL);
+ if (ret) {
+ kfree(table);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ new_sg = new_table->sgl;
+ for_each_sg(table->sgl, sg, table->nents, i) {
+ memcpy(new_sg, sg, sizeof(*sg));
+ sg->dma_address = 0;
+ new_sg = sg_next(new_sg);
+ }
+
+ return new_table;
+}
+
static struct sg_table *ion_map_dma_buf(struct dma_buf_attachment *attachment,
enum dma_data_direction direction)
{
@@ -806,13 +832,15 @@ static struct sg_table *ion_map_dma_buf(struct
dma_buf_attachment *attachment,
struct ion_buffer *buffer = dmabuf->priv;
ion_buffer_sync_for_device(buffer, attachment->dev, direction);
- return buffer->sg_table;
+ return dup_sg_table(buffer->sg_table);
}
static void ion_unmap_dma_buf(struct dma_buf_attachment *attachment,
struct sg_table *table,
enum dma_data_direction direction)
{
+ sg_free_table(table);
+ kfree(table);
}
void ion_pages_sync_for_device(struct device *dev, struct page *page,
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[RFC PATCH 07/12] staging: android: ion: Remove old platform support
Device specific platform support has been haphazard for Ion. There have been several independent attempts and there are still objections to what bindings exist right now. Just remove everything for a fresh start. Signed-off-by: Laura Abbott --- drivers/staging/android/ion/Kconfig| 35 drivers/staging/android/ion/Makefile | 6 - drivers/staging/android/ion/hisilicon/Kconfig | 5 - drivers/staging/android/ion/hisilicon/Makefile | 1 - drivers/staging/android/ion/hisilicon/hi6220_ion.c | 113 - drivers/staging/android/ion/ion_dummy_driver.c | 156 - drivers/staging/android/ion/ion_of.c | 184 - drivers/staging/android/ion/ion_of.h | 37 - drivers/staging/android/ion/tegra/Makefile | 1 - drivers/staging/android/ion/tegra/tegra_ion.c | 80 - 10 files changed, 618 deletions(-) delete mode 100644 drivers/staging/android/ion/hisilicon/Kconfig delete mode 100644 drivers/staging/android/ion/hisilicon/Makefile delete mode 100644 drivers/staging/android/ion/hisilicon/hi6220_ion.c delete mode 100644 drivers/staging/android/ion/ion_dummy_driver.c delete mode 100644 drivers/staging/android/ion/ion_of.c delete mode 100644 drivers/staging/android/ion/ion_of.h delete mode 100644 drivers/staging/android/ion/tegra/Makefile delete mode 100644 drivers/staging/android/ion/tegra/tegra_ion.c diff --git a/drivers/staging/android/ion/Kconfig b/drivers/staging/android/ion/Kconfig index c8fb413..0c91b2b 100644 --- a/drivers/staging/android/ion/Kconfig +++ b/drivers/staging/android/ion/Kconfig @@ -17,38 +17,3 @@ config ION_TEST Choose this option to create a device that can be used to test the kernel and device side ION functions. -config ION_DUMMY - bool "Dummy Ion driver" - depends on ION - help - Provides a dummy ION driver that registers the - /dev/ion device and some basic heaps. This can - be used for testing the ION infrastructure if - one doesn't have access to hardware drivers that - use ION. - -config ION_TEGRA - tristate "Ion for Tegra" - depends on ARCH_TEGRA && ION - help - Choose this option if you wish to use ion on an nVidia Tegra. - -config ION_HISI - tristate "Ion for Hisilicon" - depends on ARCH_HISI && ION - select ION_OF - help - Choose this option if you wish to use ion on Hisilicon Platform. - -source "drivers/staging/android/ion/hisilicon/Kconfig" - -config ION_OF - bool "Devicetree support for Ion" - depends on ION && OF_ADDRESS - help - Provides base support for defining Ion heaps in devicetree - and setting them up. Also includes functions for platforms - to parse the devicetree and expand for their own custom - extensions - - If using Ion and devicetree, you should say Y here diff --git a/drivers/staging/android/ion/Makefile b/drivers/staging/android/ion/Makefile index 5d630a0..9457090 100644 --- a/drivers/staging/android/ion/Makefile +++ b/drivers/staging/android/ion/Makefile @@ -5,9 +5,3 @@ obj-$(CONFIG_ION_TEST) += ion_test.o ifdef CONFIG_COMPAT obj-$(CONFIG_ION) += compat_ion.o endif - -obj-$(CONFIG_ION_DUMMY) += ion_dummy_driver.o -obj-$(CONFIG_ION_TEGRA) += tegra/ -obj-$(CONFIG_ION_HISI) += hisilicon/ -obj-$(CONFIG_ION_OF) += ion_of.o - diff --git a/drivers/staging/android/ion/hisilicon/Kconfig b/drivers/staging/android/ion/hisilicon/Kconfig deleted file mode 100644 index 2b4bd07..000 --- a/drivers/staging/android/ion/hisilicon/Kconfig +++ /dev/null @@ -1,5 +0,0 @@ -config HI6220_ION -bool "Hi6220 ION Driver" -depends on ARCH_HISI && ION -help - Build the Hisilicon Hi6220 ion driver. diff --git a/drivers/staging/android/ion/hisilicon/Makefile b/drivers/staging/android/ion/hisilicon/Makefile deleted file mode 100644 index 2a89414..000 --- a/drivers/staging/android/ion/hisilicon/Makefile +++ /dev/null @@ -1 +0,0 @@ -obj-$(CONFIG_HI6220_ION) += hi6220_ion.o diff --git a/drivers/staging/android/ion/hisilicon/hi6220_ion.c b/drivers/staging/android/ion/hisilicon/hi6220_ion.c deleted file mode 100644 index 0de7897..000 --- a/drivers/staging/android/ion/hisilicon/hi6220_ion.c +++ /dev/null @@ -1,113 +0,0 @@ -/* - * Hisilicon Hi6220 ION Driver - * - * Copyright (c) 2015 Hisilicon Limited. - * - * Author: Chen Feng - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 as - * published by the Free Software Foundation. - */ - -#define pr_fmt(fmt) "Ion: " fmt - -#include -#include -#include -#include -#include -#include "../ion_priv.h" -#include "../ion.h" -#include "../i
[RFC PATCH 05/12] staging: android: ion: Remove page faulting support
The new method of syncing with dma_map means that the page faulting sync
implementation is no longer applicable. Remove it.
Signed-off-by: Laura Abbott
---
drivers/staging/android/ion/ion.c | 117 --
1 file changed, 117 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index a931b30..8eef1d7 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -41,37 +41,11 @@
#include "ion_priv.h"
#include "compat_ion.h"
-bool ion_buffer_fault_user_mappings(struct ion_buffer *buffer)
-{
- return (buffer->flags & ION_FLAG_CACHED) &&
- !(buffer->flags & ION_FLAG_CACHED_NEEDS_SYNC);
-}
-
bool ion_buffer_cached(struct ion_buffer *buffer)
{
return !!(buffer->flags & ION_FLAG_CACHED);
}
-static inline struct page *ion_buffer_page(struct page *page)
-{
- return (struct page *)((unsigned long)page & ~(1UL));
-}
-
-static inline bool ion_buffer_page_is_dirty(struct page *page)
-{
- return !!((unsigned long)page & 1UL);
-}
-
-static inline void ion_buffer_page_dirty(struct page **page)
-{
- *page = (struct page *)((unsigned long)(*page) | 1UL);
-}
-
-static inline void ion_buffer_page_clean(struct page **page)
-{
- *page = (struct page *)((unsigned long)(*page) & ~(1UL));
-}
-
/* this function should only be called while dev->lock is held */
static void ion_buffer_add(struct ion_device *dev,
struct ion_buffer *buffer)
@@ -139,25 +113,6 @@ static struct ion_buffer *ion_buffer_create(struct
ion_heap *heap,
buffer->dev = dev;
buffer->size = len;
- if (ion_buffer_fault_user_mappings(buffer)) {
- int num_pages = PAGE_ALIGN(buffer->size) / PAGE_SIZE;
- struct scatterlist *sg;
- int i, j, k = 0;
-
- buffer->pages = vmalloc(sizeof(struct page *) * num_pages);
- if (!buffer->pages) {
- ret = -ENOMEM;
- goto err1;
- }
-
- for_each_sg(table->sgl, sg, table->nents, i) {
- struct page *page = sg_page(sg);
-
- for (j = 0; j < sg->length / PAGE_SIZE; j++)
- buffer->pages[k++] = page++;
- }
- }
-
buffer->dev = dev;
buffer->size = len;
INIT_LIST_HEAD(&buffer->vmas);
@@ -876,69 +831,6 @@ void ion_pages_sync_for_device(struct device *dev, struct
page *page,
dma_sync_sg_for_device(dev, &sg, 1, dir);
}
-struct ion_vma_list {
- struct list_head list;
- struct vm_area_struct *vma;
-};
-
-static int ion_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
-{
- struct ion_buffer *buffer = vma->vm_private_data;
- unsigned long pfn;
- int ret;
-
- mutex_lock(&buffer->lock);
- ion_buffer_page_dirty(buffer->pages + vmf->pgoff);
- BUG_ON(!buffer->pages || !buffer->pages[vmf->pgoff]);
-
- pfn = page_to_pfn(ion_buffer_page(buffer->pages[vmf->pgoff]));
- ret = vm_insert_pfn(vma, vmf->address, pfn);
- mutex_unlock(&buffer->lock);
- if (ret)
- return VM_FAULT_ERROR;
-
- return VM_FAULT_NOPAGE;
-}
-
-static void ion_vm_open(struct vm_area_struct *vma)
-{
- struct ion_buffer *buffer = vma->vm_private_data;
- struct ion_vma_list *vma_list;
-
- vma_list = kmalloc(sizeof(*vma_list), GFP_KERNEL);
- if (!vma_list)
- return;
- vma_list->vma = vma;
- mutex_lock(&buffer->lock);
- list_add(&vma_list->list, &buffer->vmas);
- mutex_unlock(&buffer->lock);
- pr_debug("%s: adding %p\n", __func__, vma);
-}
-
-static void ion_vm_close(struct vm_area_struct *vma)
-{
- struct ion_buffer *buffer = vma->vm_private_data;
- struct ion_vma_list *vma_list, *tmp;
-
- pr_debug("%s\n", __func__);
- mutex_lock(&buffer->lock);
- list_for_each_entry_safe(vma_list, tmp, &buffer->vmas, list) {
- if (vma_list->vma != vma)
- continue;
- list_del(&vma_list->list);
- kfree(vma_list);
- pr_debug("%s: deleting %p\n", __func__, vma);
- break;
- }
- mutex_unlock(&buffer->lock);
-}
-
-static const struct vm_operations_struct ion_vma_ops = {
- .open = ion_vm_open,
- .close = ion_vm_close,
- .fault = ion_vm_fault,
-};
-
static int ion_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma)
{
struct ion_buffer *buffer = dmabuf->priv;
@@ -950,15 +842,6 @@ static int ion_mmap(struct dma_buf *dmabuf, struct
vm_area_struct *vma)
[RFC PATCH 08/12] cma: Store a name in the cma structure
Frameworks that may want to enumerate CMA heaps (e.g. Ion) will find it
useful to have an explicit name attached to each region. Store the name
in each CMA structure.
Signed-off-by: Laura Abbott
---
drivers/base/dma-contiguous.c | 5 +++--
include/linux/cma.h | 4 +++-
mm/cma.c | 11 +--
mm/cma.h | 1 +
mm/cma_debug.c| 2 +-
5 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
index e167a1e1..4f638ab 100644
--- a/drivers/base/dma-contiguous.c
+++ b/drivers/base/dma-contiguous.c
@@ -165,7 +165,8 @@ int __init dma_contiguous_reserve_area(phys_addr_t size,
phys_addr_t base,
{
int ret;
- ret = cma_declare_contiguous(base, size, limit, 0, 0, fixed, res_cma);
+ ret = cma_declare_contiguous(base, size, limit, 0, 0, fixed,
+ "reserved", res_cma);
if (ret)
return ret;
@@ -257,7 +258,7 @@ static int __init rmem_cma_setup(struct reserved_mem *rmem)
return -EINVAL;
}
- err = cma_init_reserved_mem(rmem->base, rmem->size, 0, &cma);
+ err = cma_init_reserved_mem(rmem->base, rmem->size, 0, rmem->name,
&cma);
if (err) {
pr_err("Reserved memory: unable to setup CMA region\n");
return err;
diff --git a/include/linux/cma.h b/include/linux/cma.h
index 6f0a91b..49f98ea 100644
--- a/include/linux/cma.h
+++ b/include/linux/cma.h
@@ -21,13 +21,15 @@ struct cma;
extern unsigned long totalcma_pages;
extern phys_addr_t cma_get_base(const struct cma *cma);
extern unsigned long cma_get_size(const struct cma *cma);
+extern const char *cma_get_name(const struct cma *cma);
extern int __init cma_declare_contiguous(phys_addr_t base,
phys_addr_t size, phys_addr_t limit,
phys_addr_t alignment, unsigned int order_per_bit,
- bool fixed, struct cma **res_cma);
+ bool fixed, const char *name, struct cma **res_cma);
extern int cma_init_reserved_mem(phys_addr_t base, phys_addr_t size,
unsigned int order_per_bit,
+ const char *name,
struct cma **res_cma);
extern struct page *cma_alloc(struct cma *cma, size_t count, unsigned int
align);
extern bool cma_release(struct cma *cma, const struct page *pages, unsigned
int count);
diff --git a/mm/cma.c b/mm/cma.c
index 94b3460..4a93d2b 100644
--- a/mm/cma.c
+++ b/mm/cma.c
@@ -53,6 +53,11 @@ unsigned long cma_get_size(const struct cma *cma)
return cma->count << PAGE_SHIFT;
}
+const char *cma_get_name(const struct cma *cma)
+{
+ return cma->name ? cma->name : "(undefined)";
+}
+
static unsigned long cma_bitmap_aligned_mask(const struct cma *cma,
int align_order)
{
@@ -168,6 +173,7 @@ core_initcall(cma_init_reserved_areas);
*/
int __init cma_init_reserved_mem(phys_addr_t base, phys_addr_t size,
unsigned int order_per_bit,
+const char *name,
struct cma **res_cma)
{
struct cma *cma;
@@ -201,6 +207,7 @@ int __init cma_init_reserved_mem(phys_addr_t base,
phys_addr_t size,
cma->base_pfn = PFN_DOWN(base);
cma->count = size >> PAGE_SHIFT;
cma->order_per_bit = order_per_bit;
+ cma->name = name;
*res_cma = cma;
cma_area_count++;
totalcma_pages += (size / PAGE_SIZE);
@@ -229,7 +236,7 @@ int __init cma_init_reserved_mem(phys_addr_t base,
phys_addr_t size,
int __init cma_declare_contiguous(phys_addr_t base,
phys_addr_t size, phys_addr_t limit,
phys_addr_t alignment, unsigned int order_per_bit,
- bool fixed, struct cma **res_cma)
+ bool fixed, const char *name, struct cma **res_cma)
{
phys_addr_t memblock_end = memblock_end_of_DRAM();
phys_addr_t highmem_start;
@@ -335,7 +342,7 @@ int __init cma_declare_contiguous(phys_addr_t base,
base = addr;
}
- ret = cma_init_reserved_mem(base, size, order_per_bit, res_cma);
+ ret = cma_init_reserved_mem(base, size, order_per_bit, name, res_cma);
if (ret)
goto err;
diff --git a/mm/cma.h b/mm/cma.h
index 17c75a4..4986128 100644
--- a/mm/cma.h
+++ b/mm/cma.h
@@ -11,6 +11,7 @@ struct cma {
struct hlist_head mem_head;
spinlock_t mem_head_lock;
#endif
+ const char *name;
};
extern struct cma cma_areas[MAX_CMA_AREAS];
diff --git a/mm/cma_debug.c b/mm/cma_debug.c
index f8e4b60..4742efd 100644
--- a/mm/cma_debug.c
+++ b/mm/cma_debug.c
@@ -167,7 +1
[RFC PATCH 04/12] staging: android: ion: Call dma_map_sg for syncing and mapping
Technically, calling dma_buf_map_attachment should return a buffer
properly dma_mapped. Add calls to dma_map_sg to begin_cpu_access to
ensure this happens. As a side effect, this lets Ion buffers take
advantage of the dma_buf sync ioctls.
Signed-off-by: Laura Abbott
---
drivers/staging/android/ion/ion.c | 101 +++---
1 file changed, 50 insertions(+), 51 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index ce4adac..a931b30 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -795,10 +795,6 @@ void ion_client_destroy(struct ion_client *client)
}
EXPORT_SYMBOL(ion_client_destroy);
-static void ion_buffer_sync_for_device(struct ion_buffer *buffer,
- struct device *dev,
- enum dma_data_direction direction);
-
static struct sg_table *dup_sg_table(struct sg_table *table)
{
struct sg_table *new_table;
@@ -825,22 +821,43 @@ static struct sg_table *dup_sg_table(struct sg_table
*table)
return new_table;
}
+static void free_duped_table(struct sg_table *table)
+{
+ sg_free_table(table);
+ kfree(table);
+}
+
static struct sg_table *ion_map_dma_buf(struct dma_buf_attachment *attachment,
enum dma_data_direction direction)
{
struct dma_buf *dmabuf = attachment->dmabuf;
struct ion_buffer *buffer = dmabuf->priv;
+ struct sg_table *table;
+ int ret;
+
+ /*
+* TODO: Need to sync wrt CPU or device completely owning?
+*/
+
+ table = dup_sg_table(buffer->sg_table);
- ion_buffer_sync_for_device(buffer, attachment->dev, direction);
- return dup_sg_table(buffer->sg_table);
+ if (!dma_map_sg(attachment->dev, table->sgl, table->nents,
+ direction)){
+ ret = -ENOMEM;
+ goto err;
+ }
+
+err:
+ free_duped_table(table);
+ return ERR_PTR(ret);
}
static void ion_unmap_dma_buf(struct dma_buf_attachment *attachment,
struct sg_table *table,
enum dma_data_direction direction)
{
- sg_free_table(table);
- kfree(table);
+ dma_unmap_sg(attachment->dev, table->sgl, table->nents, direction);
+ free_duped_table(table);
}
void ion_pages_sync_for_device(struct device *dev, struct page *page,
@@ -864,38 +881,6 @@ struct ion_vma_list {
struct vm_area_struct *vma;
};
-static void ion_buffer_sync_for_device(struct ion_buffer *buffer,
- struct device *dev,
- enum dma_data_direction dir)
-{
- struct ion_vma_list *vma_list;
- int pages = PAGE_ALIGN(buffer->size) / PAGE_SIZE;
- int i;
-
- pr_debug("%s: syncing for device %s\n", __func__,
-dev ? dev_name(dev) : "null");
-
- if (!ion_buffer_fault_user_mappings(buffer))
- return;
-
- mutex_lock(&buffer->lock);
- for (i = 0; i < pages; i++) {
- struct page *page = buffer->pages[i];
-
- if (ion_buffer_page_is_dirty(page))
- ion_pages_sync_for_device(dev, ion_buffer_page(page),
- PAGE_SIZE, dir);
-
- ion_buffer_page_clean(buffer->pages + i);
- }
- list_for_each_entry(vma_list, &buffer->vmas, list) {
- struct vm_area_struct *vma = vma_list->vma;
-
- zap_page_range(vma, vma->vm_start, vma->vm_end - vma->vm_start);
- }
- mutex_unlock(&buffer->lock);
-}
-
static int ion_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
{
struct ion_buffer *buffer = vma->vm_private_data;
@@ -1014,16 +999,24 @@ static int ion_dma_buf_begin_cpu_access(struct dma_buf
*dmabuf,
struct ion_buffer *buffer = dmabuf->priv;
void *vaddr;
- if (!buffer->heap->ops->map_kernel) {
- pr_err("%s: map kernel is not implemented by this heap.\n",
- __func__);
- return -ENODEV;
+ /*
+* TODO: Move this elsewhere because we don't always need a vaddr
+*/
+ if (buffer->heap->ops->map_kernel) {
+ mutex_lock(&buffer->lock);
+ vaddr = ion_buffer_kmap_get(buffer);
+ mutex_unlock(&buffer->lock);
}
- mutex_lock(&buffer->lock);
- vaddr = ion_buffer_kmap_get(buffer);
- mutex_unlock(&buffer->lock);
- return PTR_ERR_OR_ZERO(vaddr);
+ /*
+* Close enough right now? Flag to skip sync?
+*/
+ if (!dma_map_sg(buffer->dev->dev.this_device, buffer->sg_
[RFC PATCH 06/12] staging: android: ion: Remove crufty cache support
Now that we call dma_map in the dma_buf API callbacks there is no need
to use the existing cache APIs. Remove the sync ioctl and the existing
bad dma_sync calls. Explicit caching can be handled with the dma_buf
sync API.
Signed-off-by: Laura Abbott
---
drivers/staging/android/ion/ion-ioctl.c | 5
drivers/staging/android/ion/ion.c | 40 -
drivers/staging/android/ion/ion_carveout_heap.c | 6
drivers/staging/android/ion/ion_chunk_heap.c| 6
drivers/staging/android/ion/ion_page_pool.c | 3 --
drivers/staging/android/ion/ion_system_heap.c | 5
6 files changed, 65 deletions(-)
diff --git a/drivers/staging/android/ion/ion-ioctl.c
b/drivers/staging/android/ion/ion-ioctl.c
index 5b2e93f..f820d77 100644
--- a/drivers/staging/android/ion/ion-ioctl.c
+++ b/drivers/staging/android/ion/ion-ioctl.c
@@ -146,11 +146,6 @@ long ion_ioctl(struct file *filp, unsigned int cmd,
unsigned long arg)
data.handle.handle = handle->id;
break;
}
- case ION_IOC_SYNC:
- {
- ret = ion_sync_for_device(client, data.fd.fd);
- break;
- }
case ION_IOC_CUSTOM:
{
if (!dev->custom_ioctl)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 8eef1d7..c3c316f 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -815,22 +815,6 @@ static void ion_unmap_dma_buf(struct dma_buf_attachment
*attachment,
free_duped_table(table);
}
-void ion_pages_sync_for_device(struct device *dev, struct page *page,
- size_t size, enum dma_data_direction dir)
-{
- struct scatterlist sg;
-
- sg_init_table(&sg, 1);
- sg_set_page(&sg, page, size, 0);
- /*
-* This is not correct - sg_dma_address needs a dma_addr_t that is valid
-* for the targeted device, but this works on the currently targeted
-* hardware.
-*/
- sg_dma_address(&sg) = page_to_phys(page);
- dma_sync_sg_for_device(dev, &sg, 1, dir);
-}
-
static int ion_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma)
{
struct ion_buffer *buffer = dmabuf->priv;
@@ -1042,30 +1026,6 @@ struct ion_handle *ion_import_dma_buf_fd(struct
ion_client *client, int fd)
}
EXPORT_SYMBOL(ion_import_dma_buf_fd);
-int ion_sync_for_device(struct ion_client *client, int fd)
-{
- struct dma_buf *dmabuf;
- struct ion_buffer *buffer;
-
- dmabuf = dma_buf_get(fd);
- if (IS_ERR(dmabuf))
- return PTR_ERR(dmabuf);
-
- /* if this memory came from ion */
- if (dmabuf->ops != &dma_buf_ops) {
- pr_err("%s: can not sync dmabuf from another exporter\n",
- __func__);
- dma_buf_put(dmabuf);
- return -EINVAL;
- }
- buffer = dmabuf->priv;
-
- dma_sync_sg_for_device(NULL, buffer->sg_table->sgl,
- buffer->sg_table->nents, DMA_BIDIRECTIONAL);
- dma_buf_put(dmabuf);
- return 0;
-}
-
int ion_query_heaps(struct ion_client *client, struct ion_heap_query *query)
{
struct ion_device *dev = client->dev;
diff --git a/drivers/staging/android/ion/ion_carveout_heap.c
b/drivers/staging/android/ion/ion_carveout_heap.c
index 9bf8e98..e0e360f 100644
--- a/drivers/staging/android/ion/ion_carveout_heap.c
+++ b/drivers/staging/android/ion/ion_carveout_heap.c
@@ -100,10 +100,6 @@ static void ion_carveout_heap_free(struct ion_buffer
*buffer)
ion_heap_buffer_zero(buffer);
- if (ion_buffer_cached(buffer))
- dma_sync_sg_for_device(NULL, table->sgl, table->nents,
- DMA_BIDIRECTIONAL);
-
ion_carveout_free(heap, paddr, buffer->size);
sg_free_table(table);
kfree(table);
@@ -128,8 +124,6 @@ struct ion_heap *ion_carveout_heap_create(struct
ion_platform_heap *heap_data)
page = pfn_to_page(PFN_DOWN(heap_data->base));
size = heap_data->size;
- ion_pages_sync_for_device(NULL, page, size, DMA_BIDIRECTIONAL);
-
ret = ion_heap_pages_zero(page, size, pgprot_writecombine(PAGE_KERNEL));
if (ret)
return ERR_PTR(ret);
diff --git a/drivers/staging/android/ion/ion_chunk_heap.c
b/drivers/staging/android/ion/ion_chunk_heap.c
index 8c41889..46e13f6 100644
--- a/drivers/staging/android/ion/ion_chunk_heap.c
+++ b/drivers/staging/android/ion/ion_chunk_heap.c
@@ -101,10 +101,6 @@ static void ion_chunk_heap_free(struct ion_buffer *buffer)
ion_heap_buffer_zero(buffer);
- if (ion_buffer_cached(buffer))
- dma_sync_sg_for_device(NULL, table->sgl, table->nents,
- DMA_BIDIRECTIONAL);
-
for_each_sg(table->sgl, sg, table-
[RFC PATCH 10/12] staging: android: ion: Use CMA APIs directly
When CMA was first introduced, its primary use was for DMA allocation
and the only way to get CMA memory was to call dma_alloc_coherent. This
put Ion in an awkward position since there was no device structure
readily available and setting one up messed up the coherency model.
These days, CMA can be allocated directly from the APIs. Switch to using
this model to avoid needing a dummy device. This also avoids awkward
caching questions.
Signed-off-by: Laura Abbott
---
drivers/staging/android/ion/ion_cma_heap.c | 97 --
1 file changed, 26 insertions(+), 71 deletions(-)
diff --git a/drivers/staging/android/ion/ion_cma_heap.c
b/drivers/staging/android/ion/ion_cma_heap.c
index d562fd7..6838825 100644
--- a/drivers/staging/android/ion/ion_cma_heap.c
+++ b/drivers/staging/android/ion/ion_cma_heap.c
@@ -19,24 +19,19 @@
#include
#include
#include
-#include
+#include
+#include
#include "ion.h"
#include "ion_priv.h"
struct ion_cma_heap {
struct ion_heap heap;
- struct device *dev;
+ struct cma *cma;
};
#define to_cma_heap(x) container_of(x, struct ion_cma_heap, heap)
-struct ion_cma_buffer_info {
- void *cpu_addr;
- dma_addr_t handle;
- struct sg_table *table;
-};
-
/* ION CMA heap operations functions */
static int ion_cma_allocate(struct ion_heap *heap, struct ion_buffer *buffer,
@@ -44,93 +39,53 @@ static int ion_cma_allocate(struct ion_heap *heap, struct
ion_buffer *buffer,
unsigned long flags)
{
struct ion_cma_heap *cma_heap = to_cma_heap(heap);
- struct device *dev = cma_heap->dev;
- struct ion_cma_buffer_info *info;
-
- dev_dbg(dev, "Request buffer allocation len %ld\n", len);
-
- if (buffer->flags & ION_FLAG_CACHED)
- return -EINVAL;
+ struct sg_table *table;
+ struct page *pages;
+ int ret;
- info = kzalloc(sizeof(*info), GFP_KERNEL);
- if (!info)
+ pages = cma_alloc(cma_heap->cma, len, 0);
+ if (!pages)
return -ENOMEM;
- info->cpu_addr = dma_alloc_coherent(dev, len, &(info->handle),
- GFP_HIGHUSER | __GFP_ZERO);
-
- if (!info->cpu_addr) {
- dev_err(dev, "Fail to allocate buffer\n");
+ table = kmalloc(sizeof(struct sg_table), GFP_KERNEL);
+ if (!table)
goto err;
- }
- info->table = kmalloc(sizeof(*info->table), GFP_KERNEL);
- if (!info->table)
+ ret = sg_alloc_table(table, 1, GFP_KERNEL);
+ if (ret)
goto free_mem;
- if (dma_get_sgtable(dev, info->table, info->cpu_addr, info->handle,
- len))
- goto free_table;
- /* keep this for memory release */
- buffer->priv_virt = info;
- buffer->sg_table = info->table;
- dev_dbg(dev, "Allocate buffer %p\n", buffer);
+ sg_set_page(table->sgl, pages, len, 0);
+
+ buffer->priv_virt = pages;
+ buffer->sg_table = table;
return 0;
-free_table:
- kfree(info->table);
free_mem:
- dma_free_coherent(dev, len, info->cpu_addr, info->handle);
+ kfree(table);
err:
- kfree(info);
+ cma_release(cma_heap->cma, pages, buffer->size);
return -ENOMEM;
}
static void ion_cma_free(struct ion_buffer *buffer)
{
struct ion_cma_heap *cma_heap = to_cma_heap(buffer->heap);
- struct device *dev = cma_heap->dev;
- struct ion_cma_buffer_info *info = buffer->priv_virt;
+ struct page *pages = buffer->priv_virt;
- dev_dbg(dev, "Release buffer %p\n", buffer);
/* release memory */
- dma_free_coherent(dev, buffer->size, info->cpu_addr, info->handle);
+ cma_release(cma_heap->cma, pages, buffer->size);
/* release sg table */
- sg_free_table(info->table);
- kfree(info->table);
- kfree(info);
-}
-
-static int ion_cma_mmap(struct ion_heap *mapper, struct ion_buffer *buffer,
- struct vm_area_struct *vma)
-{
- struct ion_cma_heap *cma_heap = to_cma_heap(buffer->heap);
- struct device *dev = cma_heap->dev;
- struct ion_cma_buffer_info *info = buffer->priv_virt;
-
- return dma_mmap_coherent(dev, vma, info->cpu_addr, info->handle,
-buffer->size);
-}
-
-static void *ion_cma_map_kernel(struct ion_heap *heap,
- struct ion_buffer *buffer)
-{
- struct ion_cma_buffer_info *info = buffer->priv_virt;
- /* kernel memory mapping has been done at allocation time */
- return info->cpu_addr;
-}
-
-static void ion_cma_unmap_kernel(struct ion_heap *heap,
-struct ion_buffer *buffer
[RFC PATCH 09/12] cma: Introduce cma_for_each_area
Frameworks (e.g. Ion) may want to iterate over each possible CMA area to
allow for enumeration. Introduce a function to allow a callback.
Signed-off-by: Laura Abbott
---
include/linux/cma.h | 2 ++
mm/cma.c| 14 ++
2 files changed, 16 insertions(+)
diff --git a/include/linux/cma.h b/include/linux/cma.h
index 49f98ea..b521e3c 100644
--- a/include/linux/cma.h
+++ b/include/linux/cma.h
@@ -33,4 +33,6 @@ extern int cma_init_reserved_mem(phys_addr_t base,
phys_addr_t size,
struct cma **res_cma);
extern struct page *cma_alloc(struct cma *cma, size_t count, unsigned int
align);
extern bool cma_release(struct cma *cma, const struct page *pages, unsigned
int count);
+
+extern int cma_for_each_area(int (*it)(struct cma *cma, void *data), void
*data);
#endif
diff --git a/mm/cma.c b/mm/cma.c
index 4a93d2b..a430df0 100644
--- a/mm/cma.c
+++ b/mm/cma.c
@@ -464,3 +464,17 @@ bool cma_release(struct cma *cma, const struct page
*pages, unsigned int count)
return true;
}
+
+int cma_for_each_area(int (*it)(struct cma *cma, void *data), void *data)
+{
+ int i;
+
+ for (i = 0; i < cma_area_count; i++) {
+ int ret = it(&cma_areas[i], data);
+
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[RFC PATCH 11/12] staging: android: ion: Make Ion heaps selectable
Currently, all heaps are compiled in all the time. In switching to
a better platform model, let's allow these to be compiled out for good
measure.
Signed-off-by: Laura Abbott
---
drivers/staging/android/ion/Kconfig| 32
drivers/staging/android/ion/Makefile | 8 +++--
drivers/staging/android/ion/ion_priv.h | 53 --
3 files changed, 87 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/android/ion/Kconfig
b/drivers/staging/android/ion/Kconfig
index 0c91b2b..2e97990 100644
--- a/drivers/staging/android/ion/Kconfig
+++ b/drivers/staging/android/ion/Kconfig
@@ -17,3 +17,35 @@ config ION_TEST
Choose this option to create a device that can be used to test the
kernel and device side ION functions.
+config ION_SYSTEM_HEAP
+ bool "Ion system heap"
+ depends on ION
+ help
+ Choose this option to enable the Ion system heap. The system heap
+ is backed by pages from the buddy allocator. If in doubt, say Y.
+
+config ION_CARVEOUT_HEAP
+ bool "Ion carveout heap support"
+ depends on ION
+ help
+ Choose this option to enable carveout heaps with Ion. Carveout heaps
+ are backed by memory reserved from the system. Allocation times are
+ typically faster at the cost of memory not being used. Unless you
+ know your system has these regions, you should say N here.
+
+config ION_CHUNK_HEAP
+ bool "Ion chunk heap support"
+ depends on ION
+ help
+ Choose this option to enable chunk heaps with Ion. This heap is
+ similar in function the carveout heap but memory is broken down
+ into smaller chunk sizes, typically corresponding to a TLB size.
+ Unless you know your system has these regions, you should say N here.
+
+config ION_CMA_HEAP
+ bool "Ion CMA heap support"
+ depends on ION && CMA
+ help
+ Choose this option to enable CMA heaps with Ion. This heap is backed
+ by the Contiguous Memory Allocator (CMA). If your system has these
+ regions, you should say Y here.
diff --git a/drivers/staging/android/ion/Makefile
b/drivers/staging/android/ion/Makefile
index 9457090..eef022b 100644
--- a/drivers/staging/android/ion/Makefile
+++ b/drivers/staging/android/ion/Makefile
@@ -1,6 +1,8 @@
-obj-$(CONFIG_ION) += ion.o ion-ioctl.o ion_heap.o \
- ion_page_pool.o ion_system_heap.o \
- ion_carveout_heap.o ion_chunk_heap.o ion_cma_heap.o
+obj-$(CONFIG_ION) += ion.o ion-ioctl.o ion_heap.o
+obj-$(CONFIG_ION_SYSTEM_HEAP) += ion_system_heap.o ion_page_pool.o
+obj-$(CONFIG_ION_CARVEOUT_HEAP) += ion_carveout_heap.o
+obj-$(CONFIG_ION_CHUNK_HEAP) += ion_chunk_heap.o
+obj-$(CONFIG_ION_CMA_HEAP) += ion_cma_heap.o
obj-$(CONFIG_ION_TEST) += ion_test.o
ifdef CONFIG_COMPAT
obj-$(CONFIG_ION) += compat_ion.o
diff --git a/drivers/staging/android/ion/ion_priv.h
b/drivers/staging/android/ion/ion_priv.h
index b09bc7c..6eafe0d 100644
--- a/drivers/staging/android/ion/ion_priv.h
+++ b/drivers/staging/android/ion/ion_priv.h
@@ -369,21 +369,68 @@ size_t ion_heap_freelist_size(struct ion_heap *heap);
* heaps as appropriate.
*/
+
struct ion_heap *ion_heap_create(struct ion_platform_heap *heap_data);
void ion_heap_destroy(struct ion_heap *heap);
+
+#ifdef CONFIG_ION_SYSTEM_HEAP
struct ion_heap *ion_system_heap_create(struct ion_platform_heap *unused);
void ion_system_heap_destroy(struct ion_heap *heap);
-
struct ion_heap *ion_system_contig_heap_create(struct ion_platform_heap *heap);
void ion_system_contig_heap_destroy(struct ion_heap *heap);
-
+#else
+static inline struct ion_heap * ion_system_heap_create(
+ struct ion_platform_heap *unused)
+{
+ return ERR_PTR(-ENODEV);
+}
+static inline void ion_system_heap_destroy(struct ion_heap *heap) { }
+
+static inline struct ion_heap *ion_system_contig_heap_create(
+ struct ion_platform_heap *heap)
+{
+ return ERR_PTR(-ENODEV);
+}
+
+static inline void ion_system_contig_heap_destroy(struct ion_heap *heap) { }
+#endif
+
+#ifdef CONFIG_ION_CARVEOUT_HEAP
struct ion_heap *ion_carveout_heap_create(struct ion_platform_heap *heap_data);
void ion_carveout_heap_destroy(struct ion_heap *heap);
-
+#else
+static inline struct ion_heap *ion_carveout_heap_create(
+ struct ion_platform_heap *heap_data)
+{
+ return ERR_PTR(-ENODEV);
+}
+static inline void ion_carveout_heap_destroy(struct ion_heap *heap) { }
+#endif
+
+#ifdef CONFIG_ION_CHUNK_HEAP
struct ion_heap *ion_chunk_heap_create(struct ion_platform_heap *heap_data);
void ion_chunk_heap_destroy(struct ion_heap *heap);
+#else
+static inline struct ion_heap *ion_chunk_heap_create(
+ struct ion_platform_heap *heap_data)
+{
+ return ERR_PTR(-ENODEV);
+}
+static inline void ion_chunk_heap_destroy(struct ion_heap *heap) { }
+
+#endif
+
+#ifdef CONFIG_ION_
[RFC PATCH 12/12] staging; android: ion: Enumerate all available heaps
Practiaclly speaking, most Ion heaps are either going to be available
all the time (system heaps) or found based off of the reserved-memory
node. Parse the CMA and reserved-memory nodes to assign the heaps.
Signed-off-by: Laura Abbott
---
drivers/staging/android/ion/Makefile| 2 +-
drivers/staging/android/ion/ion_enumerate.c | 89 +
2 files changed, 90 insertions(+), 1 deletion(-)
create mode 100644 drivers/staging/android/ion/ion_enumerate.c
diff --git a/drivers/staging/android/ion/Makefile
b/drivers/staging/android/ion/Makefile
index eef022b..4ebf655 100644
--- a/drivers/staging/android/ion/Makefile
+++ b/drivers/staging/android/ion/Makefile
@@ -1,4 +1,4 @@
-obj-$(CONFIG_ION) += ion.o ion-ioctl.o ion_heap.o
+obj-$(CONFIG_ION) += ion.o ion-ioctl.o ion_heap.o ion_enumerate.o
obj-$(CONFIG_ION_SYSTEM_HEAP) += ion_system_heap.o ion_page_pool.o
obj-$(CONFIG_ION_CARVEOUT_HEAP) += ion_carveout_heap.o
obj-$(CONFIG_ION_CHUNK_HEAP) += ion_chunk_heap.o
diff --git a/drivers/staging/android/ion/ion_enumerate.c
b/drivers/staging/android/ion/ion_enumerate.c
new file mode 100644
index 000..21344c7
--- /dev/null
+++ b/drivers/staging/android/ion/ion_enumerate.c
@@ -0,0 +1,89 @@
+#include
+#include
+
+#include "ion.h"
+#include "ion_priv.h"
+
+static struct ion_device *internal_dev;
+static int heap_id = 2;
+
+static int ion_add_system_heap(void)
+{
+#ifdef CONFIG_ION_SYSTEM_HEAP
+ struct ion_platform_heap pheap;
+ struct ion_heap *heap;
+
+ pheap.type = ION_HEAP_TYPE_SYSTEM;
+ pheap.id = heap_id++;
+ pheap.name = "ion_system_heap";
+
+ heap = ion_heap_create(&pheap);
+ if (!heap)
+ return -ENODEV;
+
+ ion_device_add_heap(internal_dev, heap);
+#endif
+ return 0;
+}
+
+static int ion_add_system_contig_heap(void)
+{
+#ifdef CONFIG_ION_SYSTEM_HEAP
+ struct ion_platform_heap pheap;
+ struct ion_heap *heap;
+
+ pheap.type = ION_HEAP_TYPE_SYSTEM_CONTIG;
+ pheap.id = heap_id++;
+ pheap.name = "ion_system_contig_heap";
+
+ heap = ion_heap_create(&pheap);
+ if (!heap)
+ return -ENODEV;
+
+ ion_device_add_heap(internal_dev, heap);
+#endif
+ return 0;
+}
+
+#ifdef CONFIG_ION_CMA_HEAP
+int __ion_add_cma_heaps(struct cma *cma, void *data)
+{
+ struct ion_heap *heap;
+ struct ion_platform_heap pheap;
+
+ pheap.type = ION_HEAP_TYPE_DMA;
+ pheap.id = heap_id++;
+ pheap.name = cma_get_name(cma);
+ pheap.priv = cma;
+
+ heap = ion_heap_create(&pheap);
+ if (!heap)
+ return -ENODEV;
+
+ ion_device_add_heap(internal_dev, heap);
+ return 0;
+}
+#endif
+
+
+static int ion_add_cma_heaps(void)
+{
+#ifdef CONFIG_ION_CMA_HEAP
+ cma_for_each_area(__ion_add_cma_heaps, NULL);
+#endif
+ return 0;
+}
+
+int ion_enumerate(void)
+{
+ internal_dev = ion_device_create(NULL);
+ if (IS_ERR(internal_dev))
+ return PTR_ERR(internal_dev);
+
+ ion_add_system_heap();
+ ion_add_system_contig_heap();
+
+ ion_add_cma_heaps();
+ return 0;
+}
+subsys_initcall(ion_enumerate);
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging
On 03/03/2017 05:29 AM, Michal Hocko wrote: > On Thu 02-03-17 13:44:32, Laura Abbott wrote: >> Hi, >> >> There's been some recent discussions[1] about Ion-like frameworks. There's >> apparently interest in just keeping Ion since it works reasonablly well. >> This series does what should be the final clean ups for it to possibly be >> moved out of staging. >> >> This includes the following: >> - Some general clean up and removal of features that never got a lot of use >> as far as I can tell. >> - Fixing up the caching. This is the series I proposed back in December[2] >> but never heard any feedback on. It will certainly break existing >> applications that rely on the implicit caching. I'd rather make an effort >> to move to a model that isn't going directly against the establishement >> though. >> - Fixing up the platform support. The devicetree approach was never well >> recieved by DT maintainers. The proposal here is to think of Ion less as >> specifying requirements and more of a framework for exposing memory to >> userspace. >> - CMA allocations now happen without the need of a dummy device structure. >> This fixes a bunch of the reasons why I attempted to add devicetree >> support before. >> >> I've had problems getting feedback in the past so if I don't hear any major >> objections I'm going to send out with the RFC dropped to be picked up. >> The only reason there isn't a patch to come out of staging is to discuss any >> other changes to the ABI people might want. Once this comes out of staging, >> I really don't want to mess with the ABI. > > Could you recapitulate concerns preventing the code being merged > normally rather than through the staging tree and how they were > addressed? > Sorry, I'm really not understanding your question here, can you clarify? Thanks, Laura ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC PATCH 04/12] staging: android: ion: Call dma_map_sg for syncing and mapping
On 03/03/2017 08:37 AM, Laurent Pinchart wrote:
> Hi Laura,
>
> Thank you for the patch.
>
> On Thursday 02 Mar 2017 13:44:36 Laura Abbott wrote:
>> Technically, calling dma_buf_map_attachment should return a buffer
>> properly dma_mapped. Add calls to dma_map_sg to begin_cpu_access to
>> ensure this happens. As a side effect, this lets Ion buffers take
>> advantage of the dma_buf sync ioctls.
>>
>> Signed-off-by: Laura Abbott
>> ---
>> drivers/staging/android/ion/ion.c | 101 +--
>> 1 file changed, 50 insertions(+), 51 deletions(-)
>>
>> diff --git a/drivers/staging/android/ion/ion.c
>> b/drivers/staging/android/ion/ion.c index ce4adac..a931b30 100644
>> --- a/drivers/staging/android/ion/ion.c
>> +++ b/drivers/staging/android/ion/ion.c
>> @@ -795,10 +795,6 @@ void ion_client_destroy(struct ion_client *client)
>> }
>> EXPORT_SYMBOL(ion_client_destroy);
>>
>> -static void ion_buffer_sync_for_device(struct ion_buffer *buffer,
>> - struct device *dev,
>> - enum dma_data_direction direction);
>> -
>> static struct sg_table *dup_sg_table(struct sg_table *table)
>> {
>> struct sg_table *new_table;
>> @@ -825,22 +821,43 @@ static struct sg_table *dup_sg_table(struct sg_table
>> *table) return new_table;
>> }
>>
>> +static void free_duped_table(struct sg_table *table)
>> +{
>> +sg_free_table(table);
>> +kfree(table);
>> +}
>> +
>> static struct sg_table *ion_map_dma_buf(struct dma_buf_attachment
>> *attachment, enum dma_data_direction direction)
>> {
>> struct dma_buf *dmabuf = attachment->dmabuf;
>> struct ion_buffer *buffer = dmabuf->priv;
>> +struct sg_table *table;
>> +int ret;
>> +
>> +/*
>> + * TODO: Need to sync wrt CPU or device completely owning?
>> + */
>> +
>> +table = dup_sg_table(buffer->sg_table);
>>
>> -ion_buffer_sync_for_device(buffer, attachment->dev, direction);
>> -return dup_sg_table(buffer->sg_table);
>> +if (!dma_map_sg(attachment->dev, table->sgl, table->nents,
>> +direction)){
>> +ret = -ENOMEM;
>> +goto err;
>> +}
>> +
>> +err:
>> +free_duped_table(table);
>> +return ERR_PTR(ret);
>> }
>>
>> static void ion_unmap_dma_buf(struct dma_buf_attachment *attachment,
>>struct sg_table *table,
>>enum dma_data_direction direction)
>> {
>> -sg_free_table(table);
>> -kfree(table);
>> +dma_unmap_sg(attachment->dev, table->sgl, table->nents, direction);
>> +free_duped_table(table);
>> }
>>
>> void ion_pages_sync_for_device(struct device *dev, struct page *page,
>> @@ -864,38 +881,6 @@ struct ion_vma_list {
>> struct vm_area_struct *vma;
>> };
>>
>> -static void ion_buffer_sync_for_device(struct ion_buffer *buffer,
>> - struct device *dev,
>> - enum dma_data_direction dir)
>> -{
>> -struct ion_vma_list *vma_list;
>> -int pages = PAGE_ALIGN(buffer->size) / PAGE_SIZE;
>> -int i;
>> -
>> -pr_debug("%s: syncing for device %s\n", __func__,
>> - dev ? dev_name(dev) : "null");
>> -
>> -if (!ion_buffer_fault_user_mappings(buffer))
>> -return;
>> -
>> -mutex_lock(&buffer->lock);
>> -for (i = 0; i < pages; i++) {
>> -struct page *page = buffer->pages[i];
>> -
>> -if (ion_buffer_page_is_dirty(page))
>> -ion_pages_sync_for_device(dev, ion_buffer_page(page),
>> - PAGE_SIZE, dir);
>> -
>> -ion_buffer_page_clean(buffer->pages + i);
>> -}
>> -list_for_each_entry(vma_list, &buffer->vmas, list) {
>> -struct vm_area_struct *vma = vma_list->vma;
>> -
>> -zap_page_range(vma, vma->vm_start, vma->vm_end - vma-
>> vm_start);
>> -}
>> -mutex_unlock(&buffer->lock);
>> -}
>> -
>> static int ion_vm_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
>> {
>> struct ion_buffer *buffer = vma->vm_private_data;
>> @@ -1014,16 +999,24 @@ static int ion_dma_buf_begin_cpu_access
Re: [RFC PATCH 03/12] staging: android: ion: Duplicate sg_table
On 03/03/2017 12:18 AM, Hillf Danton wrote:
>
> On March 03, 2017 5:45 AM Laura Abbott wrote:
>>
>> +static struct sg_table *dup_sg_table(struct sg_table *table)
>> +{
>> +struct sg_table *new_table;
>> +int ret, i;
>> +struct scatterlist *sg, *new_sg;
>> +
>> +new_table = kzalloc(sizeof(*new_table), GFP_KERNEL);
>> +if (!new_table)
>> +return ERR_PTR(-ENOMEM);
>> +
>> +ret = sg_alloc_table(new_table, table->nents, GFP_KERNEL);
>> +if (ret) {
>> +kfree(table);
>
> Free new table?
>
>> +return ERR_PTR(-ENOMEM);
>> +}
>> +
>> +new_sg = new_table->sgl;
>> +for_each_sg(table->sgl, sg, table->nents, i) {
>> +memcpy(new_sg, sg, sizeof(*sg));
>> +sg->dma_address = 0;
>> +new_sg = sg_next(new_sg);
>> +}
>> +
>
> Do we need a helper, sg_copy_table(dst_table, src_table)?
>
>> +return new_table;
>> +}
>> +
Yes, that would probably be good since I've seen this
code elsewhere.
Thanks,
Laura
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
