Re: [V2] android: binder: use VM_ALLOC to get vm area
A gentle ping.
Thanks.
2016-11-15 21:18 GMT+08:00 Ganesh Mahendran :
> Hi, Greg
>
> 2016-11-15 18:18 GMT+08:00 Greg KH :
>> On Tue, Nov 15, 2016 at 05:55:39PM +0800, Ganesh Mahendran wrote:
>>> VM_IOREMAP is used to access hardware through a mechanism called
>>> I/O mapped memory. Android binder is a IPC machanism which will
>>> not access I/O memory.
>>>
>>> Also VM_IOREMAP has alignment requiement which may not needed in
>>> binder.
>>> __get_vm_area_node()
>>> {
>>> ...
>>> if (flags & VM_IOREMAP)
>>> align = 1ul << clamp_t(int, fls_long(size),
>>>PAGE_SHIFT, IOREMAP_MAX_ORDER);
>>> ...
>>> }
>>>
>>> This patch use VM_ALLOC to get vm area.
>>>
>>> Below is the throughput test result:
>>>
>>> # ./binderThroughputTest -w 100
>>> I run this command 10 times:
>>>beforeafter
>>> average iterations per sec: 11199.9 11886.9
>>>
>>> No performance regression found throgh binder test.
>>>
>>> Signed-off-by: Ganesh Mahendran
>>> ---
>>> drivers/android/binder.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> What changed from v1?
>
> Sorry for missing the change information.
>
> In V2, I run the binder test. And there is no side effect with this
> patch.
>
>>
>> Always list that below the --- line.
>
> Thanks for reminder.
>
>>
>> thanks,
>>
>> greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [V2] android: binder: use VM_ALLOC to get vm area
Hi, Greg: 2017-02-09 18:17 GMT+08:00 Greg KH : > On Thu, Feb 09, 2017 at 05:54:03PM +0800, Ganesh Mahendran wrote: >> A gentle ping. > > I don't see a patch here that can be accepted, what are you asking for > a response from? I sent a patch before: https://patchwork.kernel.org/patch/9429257/ Please help to review. Thanks. > > confused, > > greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/3] staging: lowmemorykiller: change lowmem_adj to lowmem_score_adj
om_adj is deprecated, and in lowmemorykiller module, we use score adj
to do the comparing.
---
oom_score_adj = p->signal->oom_score_adj;
if (oom_score_adj < min_score_adj) {
task_unlock(p);
continue;
}
---
This patch makes the variable name consistent with the usage.
Signed-off-by: Ganesh Mahendran
---
drivers/staging/android/lowmemorykiller.c | 29 +++--
1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/android/lowmemorykiller.c
b/drivers/staging/android/lowmemorykiller.c
index 24d2745..6da9260 100644
--- a/drivers/staging/android/lowmemorykiller.c
+++ b/drivers/staging/android/lowmemorykiller.c
@@ -7,9 +7,9 @@
* /sys/module/lowmemorykiller/parameters/minfree. Both files take a comma
* separated list of numbers in ascending order.
*
- * For example, write "0,8" to /sys/module/lowmemorykiller/parameters/adj and
- * "1024,4096" to /sys/module/lowmemorykiller/parameters/minfree to kill
- * processes with a oom_score_adj value of 8 or higher when the free memory
+ * For example, write "0,470" to
/sys/module/lowmemorykiller/parameters/score_adj
+ * and "1024,4096" to /sys/module/lowmemorykiller/parameters/minfree to kill
+ * processes with a oom_score_adj value of 470 or higher when the free memory
* drops below 4096 pages and kill processes with a oom_score_adj value of 0 or
* higher when the free memory drops below 1024 pages.
*
@@ -44,14 +44,15 @@
#include
static u32 lowmem_debug_level = 1;
-static short lowmem_adj[6] = {
+
+static short lowmem_score_adj[6] = {
0,
- 1,
- 6,
- 12,
+ 58,
+ 352,
+ 705,
};
-static int lowmem_adj_size = 4;
+static int lowmem_score_adj_size = 4;
static int lowmem_minfree[6] = {
3 * 512,/* 6MB */
2 * 1024, /* 8MB */
@@ -89,20 +90,20 @@ static unsigned long lowmem_scan(struct shrinker *s, struct
shrink_control *sc)
int minfree = 0;
int selected_tasksize = 0;
short selected_oom_score_adj;
- int array_size = ARRAY_SIZE(lowmem_adj);
+ int array_size = ARRAY_SIZE(lowmem_score_adj);
int other_free = global_page_state(NR_FREE_PAGES) - totalreserve_pages;
int other_file = global_page_state(NR_FILE_PAGES) -
global_page_state(NR_SHMEM) -
total_swapcache_pages();
- if (lowmem_adj_size < array_size)
- array_size = lowmem_adj_size;
+ if (lowmem_score_adj_size < array_size)
+ array_size = lowmem_score_adj_size;
if (lowmem_minfree_size < array_size)
array_size = lowmem_minfree_size;
for (i = 0; i < array_size; i++) {
minfree = lowmem_minfree[i];
if (other_free < minfree && other_file < minfree) {
- min_score_adj = lowmem_adj[i];
+ min_score_adj = lowmem_score_adj[i];
break;
}
}
@@ -165,7 +166,7 @@ static unsigned long lowmem_scan(struct shrinker *s, struct
shrink_control *sc)
if (selected->mm)
task_set_lmk_waiting(selected);
task_unlock(selected);
- lowmem_print(1, "Killing '%s' (%d), adj %hd,\n"
+ lowmem_print(1, "Killing '%s' (%d), score adj %hd,\n"
" to free %ldkB on behalf of '%s' (%d)
because\n"
" cache %ldkB is below limit %ldkB for
oom_score_adj %hd\n"
" Free memory is %ldkB above reserved\n",
@@ -205,7 +206,7 @@ device_initcall(lowmem_init);
* bootargs behaviour is to continue using module_param here.
*/
module_param_named(cost, lowmem_shrinker.seeks, int, S_IRUGO | S_IWUSR);
-module_param_array_named(adj, lowmem_adj, short, &lowmem_adj_size,
+module_param_array_named(score_adj, lowmem_score_adj, short,
&lowmem_score_adj_size,
S_IRUGO | S_IWUSR);
module_param_array_named(minfree, lowmem_minfree, uint, &lowmem_minfree_size,
S_IRUGO | S_IWUSR);
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/3] staging: lowmemorykiller: count anon pages only when we have swap devices
lowmem_count() should only count anon pages when we have swap device.
Signed-off-by: Ganesh Mahendran
---
drivers/staging/android/lowmemorykiller.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/android/lowmemorykiller.c
b/drivers/staging/android/lowmemorykiller.c
index 6da9260..1d8de47 100644
--- a/drivers/staging/android/lowmemorykiller.c
+++ b/drivers/staging/android/lowmemorykiller.c
@@ -73,10 +73,14 @@ static unsigned long lowmem_deathpending_timeout;
static unsigned long lowmem_count(struct shrinker *s,
struct shrink_control *sc)
{
- return global_page_state(NR_ACTIVE_ANON) +
- global_page_state(NR_ACTIVE_FILE) +
- global_page_state(NR_INACTIVE_ANON) +
- global_page_state(NR_INACTIVE_FILE);
+ unsigned long freeable = global_page_state(NR_ACTIVE_FILE) +
+ global_page_state(NR_INACTIVE_FILE);
+
+ if (get_nr_swap_pages() > 0)
+ freeable += global_page_state(NR_ACTIVE_ANON) +
+ global_page_state(NR_INACTIVE_ANON);
+
+ return freeable;
}
static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control *sc)
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/3] staging: lowmemorykiller: select the task with maximum rss to kill
Current task selecting logic in LMK does not fully aware of the memory
pressure. It may select the task with maximum score adj, but with
least tasksize.
For example, if min_score_adj is 200, and there are 2 tasks in system:
task a: score adj 500, tasksize 200M
task b: score adj 1000, tasksize 1M
Current LMK logic will select *task b*. But now the system already have
much memory pressure.
We should select the task with maximum task from all the tasks which
score adj >= min_score_adj.
Signed-off-by: Ganesh Mahendran
---
drivers/staging/android/lowmemorykiller.c | 25 -
1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/android/lowmemorykiller.c
b/drivers/staging/android/lowmemorykiller.c
index 1d8de47..5fcfcfe 100644
--- a/drivers/staging/android/lowmemorykiller.c
+++ b/drivers/staging/android/lowmemorykiller.c
@@ -122,8 +122,6 @@ static unsigned long lowmem_scan(struct shrinker *s, struct
shrink_control *sc)
return 0;
}
- selected_oom_score_adj = min_score_adj;
-
rcu_read_lock();
for_each_process(tsk) {
struct task_struct *p;
@@ -151,18 +149,19 @@ static unsigned long lowmem_scan(struct shrinker *s,
struct shrink_control *sc)
task_unlock(p);
if (tasksize <= 0)
continue;
- if (selected) {
- if (oom_score_adj < selected_oom_score_adj)
- continue;
- if (oom_score_adj == selected_oom_score_adj &&
- tasksize <= selected_tasksize)
- continue;
+
+ /*
+* From the processes which score adj >= min_score_adj,
+* we select the one with the maximum tasksize.
+*/
+ if (selected_tasksize < tasksize) {
+ selected = p;
+ selected_tasksize = tasksize;
+ selected_oom_score_adj = oom_score_adj;
+
+ lowmem_print(2, "select '%s' (%d), adj %hd, size %d, to
kill\n",
+p->comm, p->pid, oom_score_adj,
tasksize);
}
- selected = p;
- selected_tasksize = tasksize;
- selected_oom_score_adj = oom_score_adj;
- lowmem_print(2, "select '%s' (%d), adj %hd, size %d, to kill\n",
-p->comm, p->pid, oom_score_adj, tasksize);
}
if (selected) {
task_lock(selected);
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/3] staging: lowmemorykiller: change lowmem_adj to lowmem_score_adj
Hi, David:
On Tue, Jun 21, 2016 at 01:27:40PM -0700, David Rientjes wrote:
> On Tue, 21 Jun 2016, Ganesh Mahendran wrote:
>
> > om_adj is deprecated, and in lowmemorykiller module, we use score adj
> > to do the comparing.
> > ---
> > oom_score_adj = p->signal->oom_score_adj;
> > if (oom_score_adj < min_score_adj) {
> > task_unlock(p);
> > continue;
> > }
> > ---
> >
> > This patch makes the variable name consistent with the usage.
> >
>
> Umm, I don't think you can just remove a parameter to a module and replace
> it with something that has a different unit and not think that userspace
> will break as a result.
You are right, this change will break android AMS which will set the LMK
watermark via /sys/module/lowmemorykiller/parameters/adj.
Please help to review below change. Only make the varialbe name consistent
with the variable usage.
--
>From 394872fc1993a04ae471b10d7f971d4544812ec4 Mon Sep 17 00:00:00 2001
From: Ganesh Mahendran
Date: Wed, 22 Jun 2016 10:53:13 +0800
Subject: [PATCH v2] staging: lowmemorykiller: make variable name consistent with
the varialbe usage
LMK use oom_score_adj to do the comparing. But the variable name is
*_adj. This patch makes thevarialbe name consistent with the varialb
usage to avoid ambiguity.
*_adj -> *_score_adj
Signed-off-by: Ganesh Mahendran
---
v2:
do not change user API - David
---
drivers/staging/android/lowmemorykiller.c | 23 ---
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/android/lowmemorykiller.c
b/drivers/staging/android/lowmemorykiller.c
index 24d2745..6568bbf 100644
--- a/drivers/staging/android/lowmemorykiller.c
+++ b/drivers/staging/android/lowmemorykiller.c
@@ -44,14 +44,15 @@
#include
static u32 lowmem_debug_level = 1;
-static short lowmem_adj[6] = {
+
+static short lowmem_score_adj[6] = {
0,
- 1,
- 6,
- 12,
+ 58,
+ 352,
+ 705,
};
-static int lowmem_adj_size = 4;
+static int lowmem_score_adj_size = 4;
static int lowmem_minfree[6] = {
3 * 512,/* 6MB */
2 * 1024, /* 8MB */
@@ -89,20 +90,20 @@ static unsigned long lowmem_scan(struct shrinker *s, struct
shrink_control *sc)
int minfree = 0;
int selected_tasksize = 0;
short selected_oom_score_adj;
- int array_size = ARRAY_SIZE(lowmem_adj);
+ int array_size = ARRAY_SIZE(lowmem_score_adj);
int other_free = global_page_state(NR_FREE_PAGES) - totalreserve_pages;
int other_file = global_page_state(NR_FILE_PAGES) -
global_page_state(NR_SHMEM) -
total_swapcache_pages();
- if (lowmem_adj_size < array_size)
- array_size = lowmem_adj_size;
+ if (lowmem_score_adj_size < array_size)
+ array_size = lowmem_score_adj_size;
if (lowmem_minfree_size < array_size)
array_size = lowmem_minfree_size;
for (i = 0; i < array_size; i++) {
minfree = lowmem_minfree[i];
if (other_free < minfree && other_file < minfree) {
- min_score_adj = lowmem_adj[i];
+ min_score_adj = lowmem_score_adj[i];
break;
}
}
@@ -165,7 +166,7 @@ static unsigned long lowmem_scan(struct shrinker *s, struct
shrink_control *sc)
if (selected->mm)
task_set_lmk_waiting(selected);
task_unlock(selected);
- lowmem_print(1, "Killing '%s' (%d), adj %hd,\n"
+ lowmem_print(1, "Killing '%s' (%d), score adj %hd,\n"
" to free %ldkB on behalf of '%s' (%d)
because\n"
" cache %ldkB is below limit %ldkB for
oom_score_adj %hd\n"
" Free memory is %ldkB above reserved\n",
@@ -205,7 +206,7 @@ device_initcall(lowmem_init);
* bootargs behaviour is to continue using module_param here.
*/
module_param_named(cost, lowmem_shrinker.seeks, int, S_IRUGO | S_IWUSR);
-module_param_array_named(adj, lowmem_adj, short, &lowmem_adj_size,
+module_param_array_named(adj, lowmem_score_adj, short, &lowmem_score_adj_size,
S_IRUGO | S_IWUSR);
module_param_array_named(minfree, lowmem_minfree, uint, &lowmem_minfree_size,
S_IRUGO | S_IWUSR);
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/3] staging: lowmemorykiller: count anon pages only when we have swap devices
Hi, David:
On Tue, Jun 21, 2016 at 01:22:00PM -0700, David Rientjes wrote:
> On Tue, 21 Jun 2016, Ganesh Mahendran wrote:
>
> > lowmem_count() should only count anon pages when we have swap device.
> >
>
> Why?
I make a mistake. I thought lowmem_count will return the shrinkalbe page
of a process.
>
> > Signed-off-by: Ganesh Mahendran
> > ---
> > drivers/staging/android/lowmemorykiller.c | 12
> > 1 file changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/staging/android/lowmemorykiller.c
> > b/drivers/staging/android/lowmemorykiller.c
> > index 6da9260..1d8de47 100644
> > --- a/drivers/staging/android/lowmemorykiller.c
> > +++ b/drivers/staging/android/lowmemorykiller.c
> > @@ -73,10 +73,14 @@ static unsigned long lowmem_deathpending_timeout;
> > static unsigned long lowmem_count(struct shrinker *s,
> > struct shrink_control *sc)
> > {
> > - return global_page_state(NR_ACTIVE_ANON) +
> > - global_page_state(NR_ACTIVE_FILE) +
> > - global_page_state(NR_INACTIVE_ANON) +
> > - global_page_state(NR_INACTIVE_FILE);
> > + unsigned long freeable = global_page_state(NR_ACTIVE_FILE) +
> > + global_page_state(NR_INACTIVE_FILE);
> > +
> > + if (get_nr_swap_pages() > 0)
> > + freeable += global_page_state(NR_ACTIVE_ANON) +
> > + global_page_state(NR_INACTIVE_ANON);
> > +
> > + return freeable;
> > }
> >
> > static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control
> > *sc)
>
> Shouldn't this be advertising the amount of memory that is freeable by
> killing the process with the highest priority oom_score_adj? It's not
> legitimate to say it can free all anon and file memory if nothing is oom
> killable, so this function is wrong both originally and with your patched
> version.
Yes, so should we just simply return 1 to make do_shrink_slab() go ahead?
Then lowmem_scan() will do the real job to scan all the process.
Thanks.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/3] staging: lowmemorykiller: select the task with maximum rss to kill
Hi, David: On Tue, Jun 21, 2016 at 01:14:36PM -0700, David Rientjes wrote: > On Tue, 21 Jun 2016, Ganesh Mahendran wrote: > > > Current task selecting logic in LMK does not fully aware of the memory > > pressure. It may select the task with maximum score adj, but with > > least tasksize. > > > > For example, if min_score_adj is 200, and there are 2 tasks in system: > >task a: score adj 500, tasksize 200M > >task b: score adj 1000, tasksize 1M > > Current LMK logic will select *task b*. But now the system already have > > much memory pressure. > > > > We should select the task with maximum task from all the tasks which > > score adj >= min_score_adj. > > > > Unfortunately, I'm not sure that we can get away with this although I > agree that it is a better result (kill a large process, avoid lowmem or > oom for longer). Yes, from our testing with this patch applied, system works more smoothly, and user have better experience. > > It changes the kill order for systems that have already fine-tuned their > oom_score_adj settings and can regress because of this change. If systems > really want task b to be killed above, this breaks and they have no > immediate way of fixing it. I think the processes with score_adj >= min_score_adj are all acceptable if we kill them. In android products, LMK does the main job to free memory when system is hard to shrink file/anon pages. If LMK does not free enough memory, the system will be very slow before OOM is triggered. During this period, user will have bad experience. So LMK need to work effectivly to let system running smoothly, as user experience is very important for android system. Thanks. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/3] staging: lowmemorykiller: count anon pages only when we have swap devices
2016-06-23 16:42 GMT+08:00 Sergey Senozhatsky
:
> On (06/22/16 11:27), Ganesh Mahendran wrote:
> [..]
>> > > Signed-off-by: Ganesh Mahendran
>> > > ---
>> > > drivers/staging/android/lowmemorykiller.c | 12
>> > > 1 file changed, 8 insertions(+), 4 deletions(-)
>> > >
>> > > diff --git a/drivers/staging/android/lowmemorykiller.c
>> > > b/drivers/staging/android/lowmemorykiller.c
>> > > index 6da9260..1d8de47 100644
>> > > --- a/drivers/staging/android/lowmemorykiller.c
>> > > +++ b/drivers/staging/android/lowmemorykiller.c
>> > > @@ -73,10 +73,14 @@ static unsigned long lowmem_deathpending_timeout;
>> > > static unsigned long lowmem_count(struct shrinker *s,
>> > > struct shrink_control *sc)
>> > > {
>> > > - return global_page_state(NR_ACTIVE_ANON) +
>> > > - global_page_state(NR_ACTIVE_FILE) +
>> > > - global_page_state(NR_INACTIVE_ANON) +
>> > > - global_page_state(NR_INACTIVE_FILE);
>> > > + unsigned long freeable = global_page_state(NR_ACTIVE_FILE) +
>> > > + global_page_state(NR_INACTIVE_FILE);
>> > > +
>> > > + if (get_nr_swap_pages() > 0)
>> > > + freeable += global_page_state(NR_ACTIVE_ANON) +
>> > > + global_page_state(NR_INACTIVE_ANON);
>> > > +
>> > > + return freeable;
>> > > }
>> > >
>> > > static unsigned long lowmem_scan(struct shrinker *s, struct
>> > > shrink_control *sc)
>> >
>> > Shouldn't this be advertising the amount of memory that is freeable by
>> > killing the process with the highest priority oom_score_adj? It's not
>> > legitimate to say it can free all anon and file memory if nothing is oom
>> > killable, so this function is wrong both originally and with your patched
>> > version.
>>
>> Yes, so should we just simply return 1 to make do_shrink_slab() go ahead?
>> Then lowmem_scan() will do the real job to scan all the process.
>
> hm, looking at ->scan (lowmem_scan) shouldn't it return SHRINK_STOP
> when it has nothing to free instead of 0?
Yes, you are right. It should return SHRINK_STOP.
>
> -ss
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] android: binder: use VM_ALLOC to get vm area.
VM_IOREMAP is used to access hardware through a mechanism called
I/O mapped memory. Android binder is a IPC machanism which will
not access I/O memory.
Also VM_IOREMAP has alignment requiement which may not needed in
binder.
__get_vm_area_node()
{
...
if (flags & VM_IOREMAP)
align = 1ul << clamp_t(int, fls_long(size),
PAGE_SHIFT, IOREMAP_MAX_ORDER);
...
}
This patch use VM_ALLOC to get vm area.
Signed-off-by: Ganesh Mahendran
---
drivers/android/binder.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 16288e7..3511d5c 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -2885,7 +2885,7 @@ static int binder_mmap(struct file *filp, struct
vm_area_struct *vma)
goto err_already_mapped;
}
- area = get_vm_area(vma->vm_end - vma->vm_start, VM_IOREMAP);
+ area = get_vm_area(vma->vm_end - vma->vm_start, VM_ALLOC);
if (area == NULL) {
ret = -ENOMEM;
failure_string = "get_vm_area";
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] android: binder: use VM_ALLOC to get vm area.
Hi, Greg:
2016-09-02 3:02 GMT+08:00 Greg KH :
> On Thu, Sep 01, 2016 at 02:41:04PM +0800, Ganesh Mahendran wrote:
>> VM_IOREMAP is used to access hardware through a mechanism called
>> I/O mapped memory. Android binder is a IPC machanism which will
>> not access I/O memory.
>>
>> Also VM_IOREMAP has alignment requiement which may not needed in
>> binder.
>>
>> __get_vm_area_node()
>> {
>> ...
>> if (flags & VM_IOREMAP)
>> align = 1ul << clamp_t(int, fls_long(size),
>>PAGE_SHIFT, IOREMAP_MAX_ORDER);
>> ...
>> }
>>
>>
>> This patch use VM_ALLOC to get vm area.
>>
>> Signed-off-by: Ganesh Mahendran
>> ---
>> drivers/android/binder.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
>> index 16288e7..3511d5c 100644
>> --- a/drivers/android/binder.c
>> +++ b/drivers/android/binder.c
>> @@ -2885,7 +2885,7 @@ static int binder_mmap(struct file *filp, struct
>> vm_area_struct *vma)
>> goto err_already_mapped;
>> }
>>
>> - area = get_vm_area(vma->vm_end - vma->vm_start, VM_IOREMAP);
>> + area = get_vm_area(vma->vm_end - vma->vm_start, VM_ALLOC);
>> if (area == NULL) {
>> ret = -ENOMEM;
>> failure_string = "get_vm_area";
>
> What change have you noticed with this patch? Have you tested it?
> Found that previously reserved iomemory is now free for binder to use
> where it wasn't? What kind of change does the system now run as because
> of this?
I did some sanity test in Android system. I will do more test using:
https://android.googlesource.com/platform/frameworks/native/+/master/libs/binder/tests/
>
> And are you _sure_ the alignment requirement isn't needed for binder?
> Have you verified this with the userspace binder library?
I will do the binder test.
Thanks.
>
> This is messy, tricky, stuff, I'm loath to change it without loads of
> testing having happened...
>
> thanks,
>
> greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] android: binder: use VM_ALLOC to get vm area.
2016-09-02 3:59 GMT+08:00 Arve Hjønnevåg :
> On Thu, Sep 1, 2016 at 12:02 PM, Greg KH wrote:
>> On Thu, Sep 01, 2016 at 02:41:04PM +0800, Ganesh Mahendran wrote:
>>> VM_IOREMAP is used to access hardware through a mechanism called
>>> I/O mapped memory. Android binder is a IPC machanism which will
>>> not access I/O memory.
>>>
>>> Also VM_IOREMAP has alignment requiement which may not needed in
>>> binder.
>>>
>>> __get_vm_area_node()
>>> {
>>> ...
>>> if (flags & VM_IOREMAP)
>>> align = 1ul << clamp_t(int, fls_long(size),
>>> PAGE_SHIFT, IOREMAP_MAX_ORDER);
>>> ...
>>> }
>>>
>>>
>>> This patch use VM_ALLOC to get vm area.
>>>
>>> Signed-off-by: Ganesh Mahendran
>>> ---
>>> drivers/android/binder.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
>>> index 16288e7..3511d5c 100644
>>> --- a/drivers/android/binder.c
>>> +++ b/drivers/android/binder.c
>>> @@ -2885,7 +2885,7 @@ static int binder_mmap(struct file *filp, struct
>>> vm_area_struct *vma)
>>> goto err_already_mapped;
>>> }
>>>
>>> - area = get_vm_area(vma->vm_end - vma->vm_start, VM_IOREMAP);
>>> + area = get_vm_area(vma->vm_end - vma->vm_start, VM_ALLOC);
>>> if (area == NULL) {
>>> ret = -ENOMEM;
>>> failure_string = "get_vm_area";
>>
>> What change have you noticed with this patch? Have you tested it?
>> Found that previously reserved iomemory is now free for binder to use
>> where it wasn't? What kind of change does the system now run as because
>> of this?
>>
>> And are you _sure_ the alignment requirement isn't needed for binder?
>> Have you verified this with the userspace binder library?
>>
>> This is messy, tricky, stuff, I'm loath to change it without loads of
>> testing having happened...
>>
>> thanks,
>>
>> greg k-h
>
> There is no alignment requirement on this area unless
> cache_is_vipt_aliasing returns true. In that case the area needs to be
> aligned with vma->vm_start which is done manually in the code right
> after this allocation. If there are no other side effects of changing
> this flag this change should be safe, but please run all the tests at
> https://android.googlesource.com/platform/frameworks/native/+/master/libs/binder/tests/
> to test it.
Thanks for your suggestion.
I only did some android app performance test. I will do more binder test.
Thanks.
>
> --
> Arve Hjønnevåg
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[V2] android: binder: use VM_ALLOC to get vm area
VM_IOREMAP is used to access hardware through a mechanism called
I/O mapped memory. Android binder is a IPC machanism which will
not access I/O memory.
Also VM_IOREMAP has alignment requiement which may not needed in
binder.
__get_vm_area_node()
{
...
if (flags & VM_IOREMAP)
align = 1ul << clamp_t(int, fls_long(size),
PAGE_SHIFT, IOREMAP_MAX_ORDER);
...
}
This patch use VM_ALLOC to get vm area.
Below is the throughput test result:
# ./binderThroughputTest -w 100
I run this command 10 times:
beforeafter
average iterations per sec: 11199.9 11886.9
No performance regression found throgh binder test.
Signed-off-by: Ganesh Mahendran
---
drivers/android/binder.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 3c71b98..b5908ec 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -2901,7 +2901,7 @@ static int binder_mmap(struct file *filp, struct
vm_area_struct *vma)
goto err_already_mapped;
}
- area = get_vm_area(vma->vm_end - vma->vm_start, VM_IOREMAP);
+ area = get_vm_area(vma->vm_end - vma->vm_start, VM_ALLOC);
if (area == NULL) {
ret = -ENOMEM;
failure_string = "get_vm_area";
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [V2] android: binder: use VM_ALLOC to get vm area
Hi, Greg
2016-11-15 18:18 GMT+08:00 Greg KH :
> On Tue, Nov 15, 2016 at 05:55:39PM +0800, Ganesh Mahendran wrote:
>> VM_IOREMAP is used to access hardware through a mechanism called
>> I/O mapped memory. Android binder is a IPC machanism which will
>> not access I/O memory.
>>
>> Also VM_IOREMAP has alignment requiement which may not needed in
>> binder.
>> __get_vm_area_node()
>> {
>> ...
>> if (flags & VM_IOREMAP)
>> align = 1ul << clamp_t(int, fls_long(size),
>>PAGE_SHIFT, IOREMAP_MAX_ORDER);
>> ...
>> }
>>
>> This patch use VM_ALLOC to get vm area.
>>
>> Below is the throughput test result:
>>
>> # ./binderThroughputTest -w 100
>> I run this command 10 times:
>>before after
>> average iterations per sec: 11199.9 11886.9
>>
>> No performance regression found throgh binder test.
>>
>> Signed-off-by: Ganesh Mahendran
>> ---
>> drivers/android/binder.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> What changed from v1?
Sorry for missing the change information.
In V2, I run the binder test. And there is no side effect with this
patch.
>
> Always list that below the --- line.
Thanks for reminder.
>
> thanks,
>
> greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
