v1->v2 Sync to Aug 30 common.git
v0->v1: 1, Change gen_pool_create(12, -1) to gen_pool_create(PAGE_SHIFT, -1), suggested by Haojian 2. move ion_shrink out of mutex, suggested by Nishanth 3. check error flag of ERR_PTR(-ENOMEM) 4. add msleep to allow schedule out.
Base on common.git, android-3.4 branch
Patch 2: Add support page wised cache flush for carveout_heap There is only one nents for carveout heap, as well as dirty bit. As a result, cache flush only takes effect for total carve heap.
Patch 3: Add oom killer. Once heap is used off, SIGKILL is send to all tasks refered the buffer with descending oom_socre_adj
Zhangfei Gao (3): gpu: ion: update carveout_heap_ops gpu: ion: carveout_heap page wised cache flush gpu: ion: oom killer
drivers/gpu/ion/ion.c | 118 +++++++++++++++++++++++++++++++++- drivers/gpu/ion/ion_carveout_heap.c | 25 ++++++-- 2 files changed, 133 insertions(+), 10 deletions(-)
ion_device_add_heap fail without map_dma & unmap_dma
Signed-off-by: Zhangfei Gao zhangfei.gao@marvell.com --- drivers/gpu/ion/ion_carveout_heap.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/ion/ion_carveout_heap.c b/drivers/gpu/ion/ion_carveout_heap.c index 5b6255b..13f6e8d 100644 --- a/drivers/gpu/ion/ion_carveout_heap.c +++ b/drivers/gpu/ion/ion_carveout_heap.c @@ -145,6 +145,8 @@ static struct ion_heap_ops carveout_heap_ops = { .map_user = ion_carveout_heap_map_user, .map_kernel = ion_carveout_heap_map_kernel, .unmap_kernel = ion_carveout_heap_unmap_kernel, + .map_dma = ion_carveout_heap_map_dma, + .unmap_dma = ion_carveout_heap_unmap_dma, };
struct ion_heap *ion_carveout_heap_create(struct ion_platform_heap *heap_data)
Extend dirty bit per PAGE_SIZE Page wised cache flush is supported and only takes effect for dirty buffer
Signed-off-by: Zhangfei Gao zhangfei.gao@marvell.com --- drivers/gpu/ion/ion_carveout_heap.c | 23 +++++++++++++++++------ 1 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/ion/ion_carveout_heap.c b/drivers/gpu/ion/ion_carveout_heap.c index 13f6e8d..60e97e5 100644 --- a/drivers/gpu/ion/ion_carveout_heap.c +++ b/drivers/gpu/ion/ion_carveout_heap.c @@ -88,25 +88,36 @@ struct sg_table *ion_carveout_heap_map_dma(struct ion_heap *heap, struct ion_buffer *buffer) { struct sg_table *table; - int ret; + struct scatterlist *sg; + int ret, i; + int nents = PAGE_ALIGN(buffer->size) / PAGE_SIZE; + struct page *page = phys_to_page(buffer->priv_phys);
table = kzalloc(sizeof(struct sg_table), GFP_KERNEL); if (!table) return ERR_PTR(-ENOMEM); - ret = sg_alloc_table(table, 1, GFP_KERNEL); + + ret = sg_alloc_table(table, nents, GFP_KERNEL); if (ret) { kfree(table); return ERR_PTR(ret); } - sg_set_page(table->sgl, phys_to_page(buffer->priv_phys), buffer->size, - 0); + + sg = table->sgl; + for (i = 0; i < nents; i++) { + sg_set_page(sg, page + i, PAGE_SIZE, 0); + sg = sg_next(sg); + } + return table; }
void ion_carveout_heap_unmap_dma(struct ion_heap *heap, struct ion_buffer *buffer) { - sg_free_table(buffer->sg_table); + if (buffer->sg_table) + sg_free_table(buffer->sg_table); + kfree(buffer->sg_table); }
void *ion_carveout_heap_map_kernel(struct ion_heap *heap, @@ -157,7 +168,7 @@ struct ion_heap *ion_carveout_heap_create(struct ion_platform_heap *heap_data) if (!carveout_heap) return ERR_PTR(-ENOMEM);
- carveout_heap->pool = gen_pool_create(12, -1); + carveout_heap->pool = gen_pool_create(PAGE_SHIFT, -1); if (!carveout_heap->pool) { kfree(carveout_heap); return ERR_PTR(-ENOMEM);
On Tue, Sep 4, 2012 at 1:52 PM, Zhangfei Gao zhangfei.gao@marvell.com wrote:
Extend dirty bit per PAGE_SIZE Page wised cache flush is supported and only takes effect for dirty buffer
Signed-off-by: Zhangfei Gao zhangfei.gao@marvell.com
drivers/gpu/ion/ion_carveout_heap.c | 23 +++++++++++++++++------ 1 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/ion/ion_carveout_heap.c b/drivers/gpu/ion/ion_carveout_heap.c index 13f6e8d..60e97e5 100644 --- a/drivers/gpu/ion/ion_carveout_heap.c +++ b/drivers/gpu/ion/ion_carveout_heap.c @@ -88,25 +88,36 @@ struct sg_table *ion_carveout_heap_map_dma(struct ion_heap *heap, struct ion_buffer *buffer) { struct sg_table *table;
int ret;
struct scatterlist *sg;int ret, i;int nents = PAGE_ALIGN(buffer->size) / PAGE_SIZE;struct page *page = phys_to_page(buffer->priv_phys); table = kzalloc(sizeof(struct sg_table), GFP_KERNEL); if (!table) return ERR_PTR(-ENOMEM);
ret = sg_alloc_table(table, 1, GFP_KERNEL);
ret = sg_alloc_table(table, nents, GFP_KERNEL); if (ret) { kfree(table); return ERR_PTR(ret); }
sg_set_page(table->sgl, phys_to_page(buffer->priv_phys), buffer->size,0);
sg = table->sgl;for (i = 0; i < nents; i++) {sg_set_page(sg, page + i, PAGE_SIZE, 0);sg = sg_next(sg);}return table;}
It would need page-wise sg_table only if buffer is allocated as cached.
void ion_carveout_heap_unmap_dma(struct ion_heap *heap, struct ion_buffer *buffer) {
sg_free_table(buffer->sg_table);
if (buffer->sg_table)sg_free_table(buffer->sg_table);kfree(buffer->sg_table);}
void *ion_carveout_heap_map_kernel(struct ion_heap *heap, @@ -157,7 +168,7 @@ struct ion_heap *ion_carveout_heap_create(struct ion_platform_heap *heap_data) if (!carveout_heap) return ERR_PTR(-ENOMEM);
carveout_heap->pool = gen_pool_create(12, -1);
carveout_heap->pool = gen_pool_create(PAGE_SHIFT, -1); if (!carveout_heap->pool) { kfree(carveout_heap); return ERR_PTR(-ENOMEM);-- 1.7.1
Linaro-mm-sig mailing list Linaro-mm-sig@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-mm-sig
@@ -88,25 +88,36 @@ struct sg_table *ion_carveout_heap_map_dma(struct ion_heap *heap, struct ion_buffer *buffer) { struct sg_table *table;
int ret;
struct scatterlist *sg;int ret, i;int nents = PAGE_ALIGN(buffer->size) / PAGE_SIZE;struct page *page = phys_to_page(buffer->priv_phys); table = kzalloc(sizeof(struct sg_table), GFP_KERNEL); if (!table) return ERR_PTR(-ENOMEM);
ret = sg_alloc_table(table, 1, GFP_KERNEL);
ret = sg_alloc_table(table, nents, GFP_KERNEL); if (ret) { kfree(table); return ERR_PTR(ret); }
sg_set_page(table->sgl, phys_to_page(buffer->priv_phys), buffer->size,0);
sg = table->sgl;for (i = 0; i < nents; i++) {sg_set_page(sg, page + i, PAGE_SIZE, 0);sg = sg_next(sg);}return table;}
It would need page-wise sg_table only if buffer is allocated as cached.
This refers to ion_system_heap.c, personally, I think it is fine.
On Tue, Sep 4, 2012 at 3:40 PM, zhangfei gao zhangfei.gao@gmail.com wrote:
@@ -88,25 +88,36 @@ struct sg_table *ion_carveout_heap_map_dma(struct ion_heap *heap, struct ion_buffer *buffer) { struct sg_table *table;
int ret;
struct scatterlist *sg;int ret, i;int nents = PAGE_ALIGN(buffer->size) / PAGE_SIZE;struct page *page = phys_to_page(buffer->priv_phys); table = kzalloc(sizeof(struct sg_table), GFP_KERNEL); if (!table) return ERR_PTR(-ENOMEM);
ret = sg_alloc_table(table, 1, GFP_KERNEL);
ret = sg_alloc_table(table, nents, GFP_KERNEL); if (ret) { kfree(table); return ERR_PTR(ret); }
sg_set_page(table->sgl, phys_to_page(buffer->priv_phys), buffer->size,0);
sg = table->sgl;for (i = 0; i < nents; i++) {sg_set_page(sg, page + i, PAGE_SIZE, 0);sg = sg_next(sg);}return table;}
It would need page-wise sg_table only if buffer is allocated as cached.
This refers to ion_system_heap.c, personally, I think it is fine.
AFAIU the purpose of making page-wise sg_table is for dirty page tracking by ion and so is needed only if ION_FLAG_CACHED is set for the buffer. If buffer->flags does not have ION_FLAG_CACHED, sg_table with single nents should be fine. Right?
On Tue, Sep 4, 2012 at 6:59 PM, Nishanth Peethambaran nishanth.peethu@gmail.com wrote:
On Tue, Sep 4, 2012 at 3:40 PM, zhangfei gao zhangfei.gao@gmail.com wrote:
@@ -88,25 +88,36 @@ struct sg_table *ion_carveout_heap_map_dma(struct ion_heap *heap, struct ion_buffer *buffer) { struct sg_table *table;
int ret;
struct scatterlist *sg;int ret, i;int nents = PAGE_ALIGN(buffer->size) / PAGE_SIZE;struct page *page = phys_to_page(buffer->priv_phys); table = kzalloc(sizeof(struct sg_table), GFP_KERNEL); if (!table) return ERR_PTR(-ENOMEM);
ret = sg_alloc_table(table, 1, GFP_KERNEL);
ret = sg_alloc_table(table, nents, GFP_KERNEL); if (ret) { kfree(table); return ERR_PTR(ret); }
sg_set_page(table->sgl, phys_to_page(buffer->priv_phys), buffer->size,0);
sg = table->sgl;for (i = 0; i < nents; i++) {sg_set_page(sg, page + i, PAGE_SIZE, 0);sg = sg_next(sg);}return table;}
It would need page-wise sg_table only if buffer is allocated as cached.
This refers to ion_system_heap.c, personally, I think it is fine.
AFAIU the purpose of making page-wise sg_table is for dirty page tracking by ion and so is needed only if ION_FLAG_CACHED is set for the buffer. If buffer->flags does not have ION_FLAG_CACHED, sg_table with single nents should be fine. Right?
Yes, it is workable to use single nents when no ION_FLAG_CACHED. However, do you really think it is meaningful to distinguish them?
ion_shrink is called when ION_CARVEOUT_ALLOCATE_FAIL
How to test: mount -t debugfs none /mnt cat /mnt/ion/carveout_heap echo adj > /mnt/ion/carveout_heap send_sig SIGKILL to the task with higher adj and using ion buffer Also kill all others tasks refered the buffers, in case using empty buffer
Example:
cat /mnt/ion/carveout_heap client pid size oom_score_adj ion_test 191 4096 0 ion_test 192 4096 0 ion_test 193 4096 0
echo -1 > /mnt/ion/carveout_heap [ 1333.689318] SIGKILL pid: 191 [ 1333.692192] SIGKILL pid: 192 [ 1333.695436] SIGKILL pid: 193 [ 1333.698312] SIGKILL pid: 193 size: 4096 adj: 0 [1]+ Killed ./ion_test 2
cat /mnt/ion/carveout_heap client pid size oom_score_adj
Signed-off-by: Zhangfei Gao zhangfei.gao@marvell.com --- drivers/gpu/ion/ion.c | 118 +++++++++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 114 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/ion/ion.c b/drivers/gpu/ion/ion.c index 207d00f..5ead74c 100644 --- a/drivers/gpu/ion/ion.c +++ b/drivers/gpu/ion/ion.c @@ -32,6 +32,8 @@ #include <linux/uaccess.h> #include <linux/debugfs.h> #include <linux/dma-buf.h> +#include <linux/oom.h> +#include <linux/delay.h>
#include "ion_priv.h"
@@ -367,6 +369,8 @@ static void ion_handle_add(struct ion_client *client, struct ion_handle *handle) rb_insert_color(&handle->node, &client->handles); }
+static int ion_shrink(struct ion_heap *heap, int kill_adj); + struct ion_handle *ion_alloc(struct ion_client *client, size_t len, size_t align, unsigned int heap_mask, unsigned int flags) @@ -375,6 +379,7 @@ struct ion_handle *ion_alloc(struct ion_client *client, size_t len, struct ion_handle *handle; struct ion_device *dev = client->dev; struct ion_buffer *buffer = NULL; + struct ion_heap *heap = NULL;
pr_debug("%s: len %d align %d heap_mask %u flags %x\n", __func__, len, align, heap_mask, flags); @@ -389,9 +394,10 @@ struct ion_handle *ion_alloc(struct ion_client *client, size_t len,
len = PAGE_ALIGN(len);
+retry: mutex_lock(&dev->lock); for (n = rb_first(&dev->heaps); n != NULL; n = rb_next(n)) { - struct ion_heap *heap = rb_entry(n, struct ion_heap, node); + heap = rb_entry(n, struct ion_heap, node); /* if the client doesn't support this heap type */ if (!((1 << heap->type) & client->heap_mask)) continue; @@ -404,6 +410,11 @@ struct ion_handle *ion_alloc(struct ion_client *client, size_t len, } mutex_unlock(&dev->lock);
+ if (buffer == ERR_PTR(-ENOMEM)) { + if (!ion_shrink(heap, 0)) + goto retry; + } + if (buffer == NULL) return ERR_PTR(-ENODEV);
@@ -1178,7 +1189,8 @@ static int ion_debug_heap_show(struct seq_file *s, void *unused) size_t total_size = 0; size_t total_orphaned_size = 0;
- seq_printf(s, "%16.s %16.s %16.s\n", "client", "pid", "size"); + seq_printf(s, "%16.s %16.s %16.s %16.s\n", + "client", "pid", "size", "oom_score_adj"); seq_printf(s, "----------------------------------------------------\n");
for (n = rb_first(&dev->clients); n; n = rb_next(n)) { @@ -1191,8 +1203,9 @@ static int ion_debug_heap_show(struct seq_file *s, void *unused) char task_comm[TASK_COMM_LEN];
get_task_comm(task_comm, client->task); - seq_printf(s, "%16.s %16u %16u\n", task_comm, - client->pid, size); + seq_printf(s, "%16.s %16u %16u %16d\n", task_comm, + client->pid, size, + client->task->signal->oom_score_adj); } else { seq_printf(s, "%16.s %16u %16u\n", client->name, client->pid, size); @@ -1227,13 +1240,110 @@ static int ion_debug_heap_open(struct inode *inode, struct file *file) return single_open(file, ion_debug_heap_show, inode->i_private); }
+static ssize_t +ion_debug_heap_write(struct file *file, const char __user *ubuf, + size_t count, loff_t *ppos) +{ + struct ion_heap *heap = + ((struct seq_file *)file->private_data)->private; + char buf[16]; + long kill_adj, ret; + + memset(buf, 0, sizeof(buf)); + + if (copy_from_user(&buf, ubuf, min_t(size_t, sizeof(buf) - 1, count))) + return -EFAULT; + + ret = kstrtol(buf, 10, &kill_adj); + if (ret) + return ret; + + ion_shrink(heap, kill_adj); + + return count; +} static const struct file_operations debug_heap_fops = { .open = ion_debug_heap_open, + .write = ion_debug_heap_write, .read = seq_read, .llseek = seq_lseek, .release = single_release, };
+/* + * ion_shrink + * kill all tasks referd the buffer by selected task + */ +static int ion_shrink(struct ion_heap *heap, int kill_adj) +{ + struct rb_node *n; + struct ion_client *client = NULL; + struct ion_device *dev = heap->dev; + struct task_struct *selected = NULL; + int selected_size = 0; + int selected_oom_score_adj = 0; + + for (n = rb_first(&dev->clients); n; n = rb_next(n)) { + size_t size; + struct task_struct *p; + + client = rb_entry(n, struct ion_client, node); + if (!client->task) + continue; + + p = client->task; + + if ((p->signal->oom_score_adj <= kill_adj) || + (p->signal->oom_score_adj < selected_oom_score_adj)) + continue; + + size = ion_debug_heap_total(client, heap->type); + if (!size) + continue; + if (size < selected_size) + continue; + + selected = p; + selected_size = size; + selected_oom_score_adj = p->signal->oom_score_adj; + } + + if (selected) { + /* kill all proeces refer buffer shared with this client */ + mutex_lock(&client->lock); + for (n = rb_first(&client->handles); n; n = rb_next(n)) { + struct rb_node *r; + struct ion_client *c; + struct ion_handle *handle = rb_entry(n, + struct ion_handle, + node); + + for (r = rb_first(&dev->clients); r; r = rb_next(r)) { + struct ion_handle *h; + + c = rb_entry(r, struct ion_client, node); + h = ion_handle_lookup(c, handle->buffer); + if (!IS_ERR_OR_NULL(h)) { + send_sig(SIGKILL, c->task, 0); + pr_info("SIGKILL pid: %u\n", + c->task->pid); + } + + } + } + mutex_unlock(&client->lock); + + send_sig(SIGKILL, selected, 0); + set_tsk_thread_flag(selected, TIF_MEMDIE); + pr_info("SIGKILL pid: %u size: %u adj: %u\n", + selected->pid, selected_size, + selected_oom_score_adj); + msleep(20); + return 0; + } + return -EAGAIN; +} + void ion_device_add_heap(struct ion_device *dev, struct ion_heap *heap) { struct rb_node **p = &dev->heaps.rb_node;
On Tue, Sep 4, 2012 at 1:52 PM, Zhangfei Gao zhangfei.gao@marvell.com wrote:
ion_shrink is called when ION_CARVEOUT_ALLOCATE_FAIL
How to test: mount -t debugfs none /mnt cat /mnt/ion/carveout_heap echo adj > /mnt/ion/carveout_heap send_sig SIGKILL to the task with higher adj and using ion buffer Also kill all others tasks refered the buffers, in case using empty buffer
Example:
cat /mnt/ion/carveout_heap client pid size oom_score_adj ion_test 191 4096 0 ion_test 192 4096 0 ion_test 193 4096 0
echo -1 > /mnt/ion/carveout_heap [ 1333.689318] SIGKILL pid: 191 [ 1333.692192] SIGKILL pid: 192 [ 1333.695436] SIGKILL pid: 193 [ 1333.698312] SIGKILL pid: 193 size: 4096 adj: 0 [1]+ Killed ./ion_test 2
cat /mnt/ion/carveout_heap client pid size oom_score_adj
Signed-off-by: Zhangfei Gao zhangfei.gao@marvell.com
drivers/gpu/ion/ion.c | 118 +++++++++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 114 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/ion/ion.c b/drivers/gpu/ion/ion.c index 207d00f..5ead74c 100644 --- a/drivers/gpu/ion/ion.c +++ b/drivers/gpu/ion/ion.c @@ -32,6 +32,8 @@ #include <linux/uaccess.h> #include <linux/debugfs.h> #include <linux/dma-buf.h> +#include <linux/oom.h> +#include <linux/delay.h>
#include "ion_priv.h"
@@ -367,6 +369,8 @@ static void ion_handle_add(struct ion_client *client, struct ion_handle *handle) rb_insert_color(&handle->node, &client->handles); }
+static int ion_shrink(struct ion_heap *heap, int kill_adj);
struct ion_handle *ion_alloc(struct ion_client *client, size_t len, size_t align, unsigned int heap_mask, unsigned int flags) @@ -375,6 +379,7 @@ struct ion_handle *ion_alloc(struct ion_client *client, size_t len, struct ion_handle *handle; struct ion_device *dev = client->dev; struct ion_buffer *buffer = NULL;
struct ion_heap *heap = NULL; pr_debug("%s: len %d align %d heap_mask %u flags %x\n", __func__, len, align, heap_mask, flags);@@ -389,9 +394,10 @@ struct ion_handle *ion_alloc(struct ion_client *client, size_t len,
len = PAGE_ALIGN(len);+retry: mutex_lock(&dev->lock); for (n = rb_first(&dev->heaps); n != NULL; n = rb_next(n)) {
struct ion_heap *heap = rb_entry(n, struct ion_heap, node);
heap = rb_entry(n, struct ion_heap, node); /* if the client doesn't support this heap type */ if (!((1 << heap->type) & client->heap_mask)) continue;@@ -404,6 +410,11 @@ struct ion_handle *ion_alloc(struct ion_client *client, size_t len, } mutex_unlock(&dev->lock);
if (buffer == ERR_PTR(-ENOMEM)) {if (!ion_shrink(heap, 0))goto retry;}
The heap which is attempted for shrink in this patch would be the last heap registered by platform. It should be the last heap user mentioned to try as per heap_mask. Else, it could go into infinite loop also. The last cma/carveout heap attempted for ion_buffer_create() would be a better candidate for ion_shrink(). Alos, exiting retry after a few iterations and returning allocation failure is a safe choice.
if (buffer == NULL) return ERR_PTR(-ENODEV);@@ -1178,7 +1189,8 @@ static int ion_debug_heap_show(struct seq_file *s, void *unused) size_t total_size = 0; size_t total_orphaned_size = 0;
seq_printf(s, "%16.s %16.s %16.s\n", "client", "pid", "size");
seq_printf(s, "%16.s %16.s %16.s %16.s\n","client", "pid", "size", "oom_score_adj"); seq_printf(s, "----------------------------------------------------\n"); for (n = rb_first(&dev->clients); n; n = rb_next(n)) {@@ -1191,8 +1203,9 @@ static int ion_debug_heap_show(struct seq_file *s, void *unused) char task_comm[TASK_COMM_LEN];
get_task_comm(task_comm, client->task);
seq_printf(s, "%16.s %16u %16u\n", task_comm,client->pid, size);
seq_printf(s, "%16.s %16u %16u %16d\n", task_comm,client->pid, size,client->task->signal->oom_score_adj); } else { seq_printf(s, "%16.s %16u %16u\n", client->name, client->pid, size);@@ -1227,13 +1240,110 @@ static int ion_debug_heap_open(struct inode *inode, struct file *file) return single_open(file, ion_debug_heap_show, inode->i_private); }
+static ssize_t +ion_debug_heap_write(struct file *file, const char __user *ubuf,
size_t count, loff_t *ppos)+{
struct ion_heap *heap =((struct seq_file *)file->private_data)->private;char buf[16];long kill_adj, ret;memset(buf, 0, sizeof(buf));if (copy_from_user(&buf, ubuf, min_t(size_t, sizeof(buf) - 1, count)))return -EFAULT;ret = kstrtol(buf, 10, &kill_adj);if (ret)return ret;ion_shrink(heap, kill_adj);return count;+} static const struct file_operations debug_heap_fops = { .open = ion_debug_heap_open,
.write = ion_debug_heap_write, .read = seq_read, .llseek = seq_lseek, .release = single_release,};
+/*
- ion_shrink
- kill all tasks referd the buffer by selected task
- */
+static int ion_shrink(struct ion_heap *heap, int kill_adj) +{
struct rb_node *n;struct ion_client *client = NULL;struct ion_device *dev = heap->dev;struct task_struct *selected = NULL;int selected_size = 0;int selected_oom_score_adj = 0;for (n = rb_first(&dev->clients); n; n = rb_next(n)) {size_t size;struct task_struct *p;client = rb_entry(n, struct ion_client, node);if (!client->task)continue;p = client->task;if ((p->signal->oom_score_adj <= kill_adj) ||(p->signal->oom_score_adj < selected_oom_score_adj))continue;size = ion_debug_heap_total(client, heap->type);if (!size)continue;if (size < selected_size)continue;selected = p;selected_size = size;selected_oom_score_adj = p->signal->oom_score_adj;}if (selected) {/* kill all proeces refer buffer shared with this client */mutex_lock(&client->lock);for (n = rb_first(&client->handles); n; n = rb_next(n)) {struct rb_node *r;struct ion_client *c;struct ion_handle *handle = rb_entry(n,struct ion_handle,node);for (r = rb_first(&dev->clients); r; r = rb_next(r)) {struct ion_handle *h;c = rb_entry(r, struct ion_client, node);h = ion_handle_lookup(c, handle->buffer);if (!IS_ERR_OR_NULL(h)) {send_sig(SIGKILL, c->task, 0);pr_info("SIGKILL pid: %u\n",c->task->pid);}}}mutex_unlock(&client->lock);send_sig(SIGKILL, selected, 0);set_tsk_thread_flag(selected, TIF_MEMDIE);pr_info("SIGKILL pid: %u size: %u adj: %u\n",selected->pid, selected_size,selected_oom_score_adj);msleep(20);
The msleep() might not be the right way to do it. Signalling from client_destroy/buffer_free may be better.
return 0;}return -EAGAIN;+}
void ion_device_add_heap(struct ion_device *dev, struct ion_heap *heap) { struct rb_node **p = &dev->heaps.rb_node; -- 1.7.1
Linaro-mm-sig mailing list Linaro-mm-sig@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-mm-sig
+retry: mutex_lock(&dev->lock); for (n = rb_first(&dev->heaps); n != NULL; n = rb_next(n)) {
struct ion_heap *heap = rb_entry(n, struct ion_heap, node);
heap = rb_entry(n, struct ion_heap, node); /* if the client doesn't support this heap type */ if (!((1 << heap->type) & client->heap_mask)) continue;@@ -404,6 +410,11 @@ struct ion_handle *ion_alloc(struct ion_client *client, size_t len, } mutex_unlock(&dev->lock);
if (buffer == ERR_PTR(-ENOMEM)) {if (!ion_shrink(heap, 0))goto retry;}The heap which is attempted for shrink in this patch would be the last heap registered by platform.
The heap is searched out according to id and type.
It should be the last heap user mentioned to try as per heap_mask. Else, it could go into infinite loop also. The last cma/carveout heap attempted for ion_buffer_create() would be a better candidate for ion_shrink(). Alos, exiting retry after a few iterations and returning allocation failure is a safe choice.
1. ion_shrink happens only buffer == ERR_PTR(-ENOMEM), which happens when heap alloc fail. 2. retry happens if ion_shrink succeed, if no process could be kill, ion_alloc will return fail without retry. For example, if all process have adj=0, then ion_shrink will not be called.
+/*
- ion_shrink
- kill all tasks referd the buffer by selected task
- */
+static int ion_shrink(struct ion_heap *heap, int kill_adj) +{
struct rb_node *n;struct ion_client *client = NULL;struct ion_device *dev = heap->dev;struct task_struct *selected = NULL;int selected_size = 0;int selected_oom_score_adj = 0;for (n = rb_first(&dev->clients); n; n = rb_next(n)) {size_t size;struct task_struct *p;client = rb_entry(n, struct ion_client, node);if (!client->task)continue;p = client->task;if ((p->signal->oom_score_adj <= kill_adj) ||(p->signal->oom_score_adj < selected_oom_score_adj))continue;size = ion_debug_heap_total(client, heap->type);if (!size)continue;if (size < selected_size)continue;selected = p;selected_size = size;selected_oom_score_adj = p->signal->oom_score_adj;}if (selected) {/* kill all proeces refer buffer shared with this client */mutex_lock(&client->lock);for (n = rb_first(&client->handles); n; n = rb_next(n)) {struct rb_node *r;struct ion_client *c;struct ion_handle *handle = rb_entry(n,struct ion_handle,node);for (r = rb_first(&dev->clients); r; r = rb_next(r)) {struct ion_handle *h;c = rb_entry(r, struct ion_client, node);h = ion_handle_lookup(c, handle->buffer);if (!IS_ERR_OR_NULL(h)) {send_sig(SIGKILL, c->task, 0);pr_info("SIGKILL pid: %u\n",c->task->pid);}}}mutex_unlock(&client->lock);send_sig(SIGKILL, selected, 0);set_tsk_thread_flag(selected, TIF_MEMDIE);pr_info("SIGKILL pid: %u size: %u adj: %u\n",selected->pid, selected_size,selected_oom_score_adj);msleep(20);The msleep() might not be the right way to do it. Signalling from client_destroy/buffer_free may be better.
msleep is let SIGKILL and schedule take effective as soon as possible. The release function of the killed process will be called immediately.
On Tue, Sep 4, 2012 at 3:24 PM, zhangfei gao zhangfei.gao@gmail.com wrote:
+retry: mutex_lock(&dev->lock); for (n = rb_first(&dev->heaps); n != NULL; n = rb_next(n)) {
struct ion_heap *heap = rb_entry(n, struct ion_heap, node);
heap = rb_entry(n, struct ion_heap, node); /* if the client doesn't support this heap type */ if (!((1 << heap->type) & client->heap_mask)) continue;@@ -404,6 +410,11 @@ struct ion_handle *ion_alloc(struct ion_client *client, size_t len, } mutex_unlock(&dev->lock);
if (buffer == ERR_PTR(-ENOMEM)) {if (!ion_shrink(heap, 0))goto retry;}The heap which is attempted for shrink in this patch would be the last heap registered by platform.
The heap is searched out according to id and type.
It should be the last heap user mentioned to try as per heap_mask. Else, it could go into infinite loop also. The last cma/carveout heap attempted for ion_buffer_create() would be a better candidate for ion_shrink(). Alos, exiting retry after a few iterations and returning allocation failure is a safe choice.
- ion_shrink happens only buffer == ERR_PTR(-ENOMEM), which happens
when heap alloc fail.
Take the case of heap ids 0-3 were registered with ion device and user asks for heap2. Heap 2 will be tried for ion_buffer_create() and assume it failed. The rbtree loop will iterate heap3 also but will not be attempted for ion_buffer_create() because of the heap_mask check. ion_shrink() will try shrinking heap3 instead of heap2.
- retry happens if ion_shrink succeed, if no process could be kill,
ion_alloc will return fail without retry. For example, if all process have adj=0, then ion_shrink will not be called.
Got it.
+/*
- ion_shrink
- kill all tasks referd the buffer by selected task
- */
+static int ion_shrink(struct ion_heap *heap, int kill_adj) +{
struct rb_node *n;struct ion_client *client = NULL;struct ion_device *dev = heap->dev;struct task_struct *selected = NULL;int selected_size = 0;int selected_oom_score_adj = 0;for (n = rb_first(&dev->clients); n; n = rb_next(n)) {size_t size;struct task_struct *p;client = rb_entry(n, struct ion_client, node);if (!client->task)continue;p = client->task;if ((p->signal->oom_score_adj <= kill_adj) ||(p->signal->oom_score_adj < selected_oom_score_adj))continue;size = ion_debug_heap_total(client, heap->type);if (!size)continue;if (size < selected_size)continue;selected = p;selected_size = size;selected_oom_score_adj = p->signal->oom_score_adj;}if (selected) {/* kill all proeces refer buffer shared with this client */mutex_lock(&client->lock);for (n = rb_first(&client->handles); n; n = rb_next(n)) {struct rb_node *r;struct ion_client *c;struct ion_handle *handle = rb_entry(n,struct ion_handle,node);for (r = rb_first(&dev->clients); r; r = rb_next(r)) {struct ion_handle *h;c = rb_entry(r, struct ion_client, node);h = ion_handle_lookup(c, handle->buffer);if (!IS_ERR_OR_NULL(h)) {send_sig(SIGKILL, c->task, 0);pr_info("SIGKILL pid: %u\n",c->task->pid);}}}mutex_unlock(&client->lock);send_sig(SIGKILL, selected, 0);set_tsk_thread_flag(selected, TIF_MEMDIE);pr_info("SIGKILL pid: %u size: %u adj: %u\n",selected->pid, selected_size,selected_oom_score_adj);msleep(20);The msleep() might not be the right way to do it. Signalling from client_destroy/buffer_free may be better.
msleep is let SIGKILL and schedule take effective as soon as possible. The release function of the killed process will be called immediately.
Correct me if wrong. It can't be assumed that release function of the killed process will be called immediately or within 20ms if system load is high. I am using msleep() as of now because of simplicity and ease of implementation. Safer option is to do a wait_event instead of msleep() with wake_up happening when buffers get released. It is a bit harder to do. Before send_sig() in ion_shrink(), the process going to wait should add itself to a waitque list. When the client gets released (ideally when buffer gets released), the wake_up should be sent to the waitqueues. This is what I was thinking.
On Tue, Sep 4, 2012 at 6:47 PM, Nishanth Peethambaran nishanth.peethu@gmail.com wrote:
On Tue, Sep 4, 2012 at 3:24 PM, zhangfei gao zhangfei.gao@gmail.com wrote:
+retry: mutex_lock(&dev->lock); for (n = rb_first(&dev->heaps); n != NULL; n = rb_next(n)) {
struct ion_heap *heap = rb_entry(n, struct ion_heap, node);
heap = rb_entry(n, struct ion_heap, node); /* if the client doesn't support this heap type */ if (!((1 << heap->type) & client->heap_mask)) continue;@@ -404,6 +410,11 @@ struct ion_handle *ion_alloc(struct ion_client *client, size_t len, } mutex_unlock(&dev->lock);
if (buffer == ERR_PTR(-ENOMEM)) {if (!ion_shrink(heap, 0))goto retry;}The heap which is attempted for shrink in this patch would be the last heap registered by platform.
The heap is searched out according to id and type.
It should be the last heap user mentioned to try as per heap_mask. Else, it could go into infinite loop also. The last cma/carveout heap attempted for ion_buffer_create() would be a better candidate for ion_shrink(). Alos, exiting retry after a few iterations and returning allocation failure is a safe choice.
- ion_shrink happens only buffer == ERR_PTR(-ENOMEM), which happens
when heap alloc fail.
Take the case of heap ids 0-3 were registered with ion device and user asks for heap2. Heap 2 will be tried for ion_buffer_create() and assume it failed. The rbtree loop will iterate heap3 also but will not be attempted for ion_buffer_create() because of the heap_mask check. ion_shrink() will try shrinking heap3 instead of heap2.
Thanks Nishanth
There really have issue if several id with same type, which not considered before. Since ion_debug_heap_total takes care of type instead of id. It can be easily solved by change ion_debug_heap_total(type) to -> ion_debug_heap_total(id). Then ion_debug_heap_total return 0 for other id.
I see you already submit one patch to replace heap->type with heap->id. This patch could based on that one, or your patch base on this one :)
Besides, another mistake is heap_found should be used.
- retry happens if ion_shrink succeed, if no process could be kill,
ion_alloc will return fail without retry. For example, if all process have adj=0, then ion_shrink will not be called.
Got it.
+/*
- ion_shrink
- kill all tasks referd the buffer by selected task
- */
+static int ion_shrink(struct ion_heap *heap, int kill_adj) +{
struct rb_node *n;struct ion_client *client = NULL;struct ion_device *dev = heap->dev;struct task_struct *selected = NULL;int selected_size = 0;int selected_oom_score_adj = 0;for (n = rb_first(&dev->clients); n; n = rb_next(n)) {size_t size;struct task_struct *p;client = rb_entry(n, struct ion_client, node);if (!client->task)continue;p = client->task;if ((p->signal->oom_score_adj <= kill_adj) ||(p->signal->oom_score_adj < selected_oom_score_adj))continue;size = ion_debug_heap_total(client, heap->type);if (!size)continue;if (size < selected_size)continue;selected = p;selected_size = size;selected_oom_score_adj = p->signal->oom_score_adj;}if (selected) {/* kill all proeces refer buffer shared with this client */mutex_lock(&client->lock);for (n = rb_first(&client->handles); n; n = rb_next(n)) {struct rb_node *r;struct ion_client *c;struct ion_handle *handle = rb_entry(n,struct ion_handle,node);for (r = rb_first(&dev->clients); r; r = rb_next(r)) {struct ion_handle *h;c = rb_entry(r, struct ion_client, node);h = ion_handle_lookup(c, handle->buffer);if (!IS_ERR_OR_NULL(h)) {send_sig(SIGKILL, c->task, 0);pr_info("SIGKILL pid: %u\n",c->task->pid);}}}mutex_unlock(&client->lock);send_sig(SIGKILL, selected, 0);set_tsk_thread_flag(selected, TIF_MEMDIE);pr_info("SIGKILL pid: %u size: %u adj: %u\n",selected->pid, selected_size,selected_oom_score_adj);msleep(20);The msleep() might not be the right way to do it. Signalling from client_destroy/buffer_free may be better.
msleep is let SIGKILL and schedule take effective as soon as possible. The release function of the killed process will be called immediately.
Correct me if wrong. It can't be assumed that release function of the killed process will be called immediately or within 20ms if system load is high. I am using msleep() as of now because of simplicity and ease of implementation. Safer option is to do a wait_event instead of msleep() with wake_up happening when buffers get released. It is a bit harder to do. Before send_sig() in ion_shrink(), the process going to wait should add itself to a waitque list. When the client gets released (ideally when buffer gets released), the wake_up should be sent to the waitqueues. This is what I was thinking.
I don't think this is good suggestion. msleep is used to release cpu and let other task to run. If system load is high, no difference between msleep and wait_event. The killed process and alloc process are different, why they wait for each other. Also no wait in lowmemorykiller.c And the chance for calling ion_shrink suppose limited.
Thanks
On Wed, Sep 5, 2012 at 10:04 AM, zhangfei gao zhangfei.gao@gmail.com wrote:
On Tue, Sep 4, 2012 at 6:47 PM, Nishanth Peethambaran nishanth.peethu@gmail.com wrote:
On Tue, Sep 4, 2012 at 3:24 PM, zhangfei gao zhangfei.gao@gmail.com wrote:
+retry: mutex_lock(&dev->lock); for (n = rb_first(&dev->heaps); n != NULL; n = rb_next(n)) {
struct ion_heap *heap = rb_entry(n, struct ion_heap, node);
heap = rb_entry(n, struct ion_heap, node); /* if the client doesn't support this heap type */ if (!((1 << heap->type) & client->heap_mask)) continue;@@ -404,6 +410,11 @@ struct ion_handle *ion_alloc(struct ion_client *client, size_t len, } mutex_unlock(&dev->lock);
if (buffer == ERR_PTR(-ENOMEM)) {if (!ion_shrink(heap, 0))goto retry;}The heap which is attempted for shrink in this patch would be the last heap registered by platform.
The heap is searched out according to id and type.
It should be the last heap user mentioned to try as per heap_mask. Else, it could go into infinite loop also. The last cma/carveout heap attempted for ion_buffer_create() would be a better candidate for ion_shrink(). Alos, exiting retry after a few iterations and returning allocation failure is a safe choice.
- ion_shrink happens only buffer == ERR_PTR(-ENOMEM), which happens
when heap alloc fail.
Take the case of heap ids 0-3 were registered with ion device and user asks for heap2. Heap 2 will be tried for ion_buffer_create() and assume it failed. The rbtree loop will iterate heap3 also but will not be attempted for ion_buffer_create() because of the heap_mask check. ion_shrink() will try shrinking heap3 instead of heap2.
Thanks Nishanth
There really have issue if several id with same type, which not considered before. Since ion_debug_heap_total takes care of type instead of id. It can be easily solved by change ion_debug_heap_total(type) to -> ion_debug_heap_total(id). Then ion_debug_heap_total return 0 for other id.
I see you already submit one patch to replace heap->type with heap->id. This patch could based on that one, or your patch base on this one :)
Besides, another mistake is heap_found should be used.
- retry happens if ion_shrink succeed, if no process could be kill,
ion_alloc will return fail without retry. For example, if all process have adj=0, then ion_shrink will not be called.
Got it.
+/*
- ion_shrink
- kill all tasks referd the buffer by selected task
- */
+static int ion_shrink(struct ion_heap *heap, int kill_adj) +{
struct rb_node *n;struct ion_client *client = NULL;struct ion_device *dev = heap->dev;struct task_struct *selected = NULL;int selected_size = 0;int selected_oom_score_adj = 0;for (n = rb_first(&dev->clients); n; n = rb_next(n)) {size_t size;struct task_struct *p;client = rb_entry(n, struct ion_client, node);if (!client->task)continue;p = client->task;if ((p->signal->oom_score_adj <= kill_adj) ||(p->signal->oom_score_adj < selected_oom_score_adj))continue;size = ion_debug_heap_total(client, heap->type);if (!size)continue;if (size < selected_size)continue;selected = p;selected_size = size;selected_oom_score_adj = p->signal->oom_score_adj;}if (selected) {/* kill all proeces refer buffer shared with this client */mutex_lock(&client->lock);for (n = rb_first(&client->handles); n; n = rb_next(n)) {struct rb_node *r;struct ion_client *c;struct ion_handle *handle = rb_entry(n,struct ion_handle,node);for (r = rb_first(&dev->clients); r; r = rb_next(r)) {struct ion_handle *h;c = rb_entry(r, struct ion_client, node);h = ion_handle_lookup(c, handle->buffer);if (!IS_ERR_OR_NULL(h)) {send_sig(SIGKILL, c->task, 0);pr_info("SIGKILL pid: %u\n",c->task->pid);}}}mutex_unlock(&client->lock);send_sig(SIGKILL, selected, 0);set_tsk_thread_flag(selected, TIF_MEMDIE);pr_info("SIGKILL pid: %u size: %u adj: %u\n",selected->pid, selected_size,selected_oom_score_adj);msleep(20);The msleep() might not be the right way to do it. Signalling from client_destroy/buffer_free may be better.
msleep is let SIGKILL and schedule take effective as soon as possible. The release function of the killed process will be called immediately.
Correct me if wrong. It can't be assumed that release function of the killed process will be called immediately or within 20ms if system load is high. I am using msleep() as of now because of simplicity and ease of implementation. Safer option is to do a wait_event instead of msleep() with wake_up happening when buffers get released. It is a bit harder to do. Before send_sig() in ion_shrink(), the process going to wait should add itself to a waitque list. When the client gets released (ideally when buffer gets released), the wake_up should be sent to the waitqueues. This is what I was thinking.
I don't think this is good suggestion. msleep is used to release cpu and let other task to run. If system load is high, no difference between msleep and wait_event. The killed process and alloc process are different, why they wait for each other. Also no wait in lowmemorykiller.c And the chance for calling ion_shrink suppose limited.
Thanks
Usage of wait_event is to optimize the sleep time inside of a magic 20ms. If I get time, I shall implement it and post a patch.
linaro-mm-sig@lists.linaro.org
-
Nishanth Peethambaran -
zhangfei gao -
Zhangfei Gao