Hi,
I ran into series of following binder mmap failures with v4.0.0-rc1:
------------[ cut here ]------------
WARNING: CPU: 0 PID: 1971 at mm/vmalloc.c:130
vmap_page_range_noflush+0x119/0x144()
CPU: 0 PID: 1971 Comm: healthd Not tainted 4.0.0-rc1-00399-g7da3fdc-dirty #157
Hardware name: ARM-Versatile Express
[<c001246d>] (unwind_backtrace) from [<c000f7f9>] (show_stack+0x11/0x14)
[<c000f7f9>] (show_stack) from [<c049a221>] (dump_stack+0x59/0x7c)
[<c049a221>] (dump_stack) from [<c001cf21>] (warn_slowpath_common+0x55/0x84)
[<c001cf21>] (warn_slowpath_common) from [<c001cfe3>]
(warn_slowpath_null+0x17/0x1c)
[<c001cfe3>] (warn_slowpath_null) from [<c00c66c5>]
(vmap_page_range_noflush+0x119/0x144)
[<c00c66c5>] (vmap_page_range_noflush) from [<c00c716b>] (map_vm_area+0x27/0x48)
[<c00c716b>] (map_vm_area) from [<c038ddaf>]
(binder_update_page_range+0x12f/0x27c)
[<c038ddaf>] (binder_update_page_range) from [<c038e857>]
(binder_mmap+0xbf/0x1ac)
[<c038e857>] (binder_mmap) from [<c00c2dc7>] (mmap_region+0x2eb/0x4d4)
[<c00c2dc7>] (mmap_region) from [<c00c3197>] (do_mmap_pgoff+0x1e7/0x250)
[<c00c3197>] (do_mmap_pgoff) from [<c00b35b5>] (vm_mmap_pgoff+0x45/0x60)
[<c00b35b5>] (vm_mmap_pgoff) from [<c00c1f39>] (SyS_mmap_pgoff+0x5d/0x80)
[<c00c1f39>] (SyS_mmap_pgoff) from [<c000ce81>] (ret_fast_syscall+0x1/0x5c)
---[ end trace 48c2c4b9a1349e54 ]---
binder: 1982: binder_alloc_buf failed to map page at f0e00000 in kernel
binder: binder_mmap: 1982 b6bde000-b6cdc000 alloc small buf failed -12
Turned out that the following commit tripped off binder:
----------8<----------
commit 71394fe50146202f2c8d92cf50f5ebc761acf254
Author: Andrey Ryabinin <[email protected]>
Date: Fri Feb 13 14:40:03 2015 -0800
mm: vmalloc: add flag preventing guard hole allocation
---------->8----------
Explicitly disabling the vmalloc no guard (VM_NO_GUARD) flag in binder
worked fine for me. So does a fix like this look reasonable enough to
submit?
----------8<----------
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -601,6 +601,7 @@ static int binder_update_page_range(struct
binder_proc *proc, int allocate,
goto err_alloc_page_failed;
}
tmp_area.addr = page_addr;
+ tmp_area.flags &= ~VM_NO_GUARD;
tmp_area.size = PAGE_SIZE + PAGE_SIZE /* guard page? */;
ret = map_vm_area(&tmp_area, PAGE_KERNEL, page);
if (ret) {
---------->8----------
Regards,
Amit Pundir
On Fri, Feb 27, 2015 at 03:34:32AM +0530, Amit Pundir wrote:
> Hi,
>
> I ran into series of following binder mmap failures with v4.0.0-rc1:
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 1971 at mm/vmalloc.c:130
> vmap_page_range_noflush+0x119/0x144()
> CPU: 0 PID: 1971 Comm: healthd Not tainted 4.0.0-rc1-00399-g7da3fdc-dirty #157
> Hardware name: ARM-Versatile Express
> [<c001246d>] (unwind_backtrace) from [<c000f7f9>] (show_stack+0x11/0x14)
> [<c000f7f9>] (show_stack) from [<c049a221>] (dump_stack+0x59/0x7c)
> [<c049a221>] (dump_stack) from [<c001cf21>] (warn_slowpath_common+0x55/0x84)
> [<c001cf21>] (warn_slowpath_common) from [<c001cfe3>]
> (warn_slowpath_null+0x17/0x1c)
> [<c001cfe3>] (warn_slowpath_null) from [<c00c66c5>]
> (vmap_page_range_noflush+0x119/0x144)
> [<c00c66c5>] (vmap_page_range_noflush) from [<c00c716b>] (map_vm_area+0x27/0x48)
> [<c00c716b>] (map_vm_area) from [<c038ddaf>]
> (binder_update_page_range+0x12f/0x27c)
> [<c038ddaf>] (binder_update_page_range) from [<c038e857>]
> (binder_mmap+0xbf/0x1ac)
> [<c038e857>] (binder_mmap) from [<c00c2dc7>] (mmap_region+0x2eb/0x4d4)
> [<c00c2dc7>] (mmap_region) from [<c00c3197>] (do_mmap_pgoff+0x1e7/0x250)
> [<c00c3197>] (do_mmap_pgoff) from [<c00b35b5>] (vm_mmap_pgoff+0x45/0x60)
> [<c00b35b5>] (vm_mmap_pgoff) from [<c00c1f39>] (SyS_mmap_pgoff+0x5d/0x80)
> [<c00c1f39>] (SyS_mmap_pgoff) from [<c000ce81>] (ret_fast_syscall+0x1/0x5c)
> ---[ end trace 48c2c4b9a1349e54 ]---
> binder: 1982: binder_alloc_buf failed to map page at f0e00000 in kernel
> binder: binder_mmap: 1982 b6bde000-b6cdc000 alloc small buf failed -12
>
>
> Turned out that the following commit tripped off binder:
> ----------8<----------
> commit 71394fe50146202f2c8d92cf50f5ebc761acf254
> Author: Andrey Ryabinin <[email protected]>
> Date: Fri Feb 13 14:40:03 2015 -0800
>
> mm: vmalloc: add flag preventing guard hole allocation
> ---------->8----------
>
>
> Explicitly disabling the vmalloc no guard (VM_NO_GUARD) flag in binder
> worked fine for me. So does a fix like this look reasonable enough to
> submit?
> ----------8<----------
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -601,6 +601,7 @@ static int binder_update_page_range(struct
> binder_proc *proc, int allocate,
> goto err_alloc_page_failed;
> }
> tmp_area.addr = page_addr;
> + tmp_area.flags &= ~VM_NO_GUARD;
> tmp_area.size = PAGE_SIZE + PAGE_SIZE /* guard page? */;
> ret = map_vm_area(&tmp_area, PAGE_KERNEL, page);
> if (ret) {
> ---------->8----------
This looks like a valid fix, please send it in.
thanks,
greg k-h
On Thu, Feb 26, 2015 at 2:04 PM, Amit Pundir <[email protected]> wrote:
> Hi,
>
> I ran into series of following binder mmap failures with v4.0.0-rc1:
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 1971 at mm/vmalloc.c:130
> vmap_page_range_noflush+0x119/0x144()
> CPU: 0 PID: 1971 Comm: healthd Not tainted 4.0.0-rc1-00399-g7da3fdc-dirty #157
> Hardware name: ARM-Versatile Express
> [<c001246d>] (unwind_backtrace) from [<c000f7f9>] (show_stack+0x11/0x14)
> [<c000f7f9>] (show_stack) from [<c049a221>] (dump_stack+0x59/0x7c)
> [<c049a221>] (dump_stack) from [<c001cf21>] (warn_slowpath_common+0x55/0x84)
> [<c001cf21>] (warn_slowpath_common) from [<c001cfe3>]
> (warn_slowpath_null+0x17/0x1c)
> [<c001cfe3>] (warn_slowpath_null) from [<c00c66c5>]
> (vmap_page_range_noflush+0x119/0x144)
> [<c00c66c5>] (vmap_page_range_noflush) from [<c00c716b>] (map_vm_area+0x27/0x48)
> [<c00c716b>] (map_vm_area) from [<c038ddaf>]
> (binder_update_page_range+0x12f/0x27c)
> [<c038ddaf>] (binder_update_page_range) from [<c038e857>]
> (binder_mmap+0xbf/0x1ac)
> [<c038e857>] (binder_mmap) from [<c00c2dc7>] (mmap_region+0x2eb/0x4d4)
> [<c00c2dc7>] (mmap_region) from [<c00c3197>] (do_mmap_pgoff+0x1e7/0x250)
> [<c00c3197>] (do_mmap_pgoff) from [<c00b35b5>] (vm_mmap_pgoff+0x45/0x60)
> [<c00b35b5>] (vm_mmap_pgoff) from [<c00c1f39>] (SyS_mmap_pgoff+0x5d/0x80)
> [<c00c1f39>] (SyS_mmap_pgoff) from [<c000ce81>] (ret_fast_syscall+0x1/0x5c)
> ---[ end trace 48c2c4b9a1349e54 ]---
> binder: 1982: binder_alloc_buf failed to map page at f0e00000 in kernel
> binder: binder_mmap: 1982 b6bde000-b6cdc000 alloc small buf failed -12
>
>
> Turned out that the following commit tripped off binder:
> ----------8<----------
> commit 71394fe50146202f2c8d92cf50f5ebc761acf254
> Author: Andrey Ryabinin <[email protected]>
> Date: Fri Feb 13 14:40:03 2015 -0800
>
> mm: vmalloc: add flag preventing guard hole allocation
> ---------->8----------
>
>
> Explicitly disabling the vmalloc no guard (VM_NO_GUARD) flag in binder
> worked fine for me. So does a fix like this look reasonable enough to
> submit?
> ----------8<----------
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -601,6 +601,7 @@ static int binder_update_page_range(struct
> binder_proc *proc, int allocate,
> goto err_alloc_page_failed;
> }
> tmp_area.addr = page_addr;
> + tmp_area.flags &= ~VM_NO_GUARD;
This variable is not initialized, so I would expect this to add a
warning. Setting it to VM_NO_GUARD and removing, " + PAGE_SIZE /*
guard page? */" fromt he next line would be better. However, the "new"
map_kernel_range_noflush api seems like a better api to use for this,
since it removes the need to create a dummy vm_struct at all.
> tmp_area.size = PAGE_SIZE + PAGE_SIZE /* guard page? */;
> ret = map_vm_area(&tmp_area, PAGE_KERNEL, page);
> if (ret) {
> ---------->8----------
>
>
> Regards,
> Amit Pundir
--
Arve Hjønnevåg
On Thu, 26 Feb 2015, Arve Hjønnevåg wrote:
> > ----------8<----------
> > --- a/drivers/android/binder.c
> > +++ b/drivers/android/binder.c
> > @@ -601,6 +601,7 @@ static int binder_update_page_range(struct
> > binder_proc *proc, int allocate,
> > goto err_alloc_page_failed;
> > }
> > tmp_area.addr = page_addr;
> > + tmp_area.flags &= ~VM_NO_GUARD;
>
> This variable is not initialized, so I would expect this to add a
> warning. Setting it to VM_NO_GUARD and removing, " + PAGE_SIZE /*
> guard page? */" fromt he next line would be better. However, the "new"
> map_kernel_range_noflush api seems like a better api to use for this,
> since it removes the need to create a dummy vm_struct at all.
>
Yeah, this is what you want to do and it's a bug in commit 71394fe50146
("mm: vmalloc: add flag preventing guard hole allocation") that started to
look at tmp_area.size without fixing up any of the callers when
tmp_area.addr and tmp_area.size were only important in the past. It
shouldn't take much work to make this use map_kernel_range_noflush().
> > tmp_area.size = PAGE_SIZE + PAGE_SIZE /* guard page? */;
> > ret = map_vm_area(&tmp_area, PAGE_KERNEL, page);
> > if (ret) {
> > ---------->8----------
binder_update_page_range() initializes only addr and size
fields in 'struct vm_struct tmp_area;' and passes it to
map_vm_area().
Before 71394fe50146 ("mm: vmalloc: add flag preventing guard hole allocation")
this was because map_vm_area() didn't use any other fields
in vm_struct except addr and size.
Now get_vm_area_size() (used in map_vm_area()) reads vm_struct's
flags to determine whether vm area has guard hole or not.
binder_update_page_range() don't initialize flags field, so
this causes following binder mmap failures:
-----------[ cut here ]------------
WARNING: CPU: 0 PID: 1971 at mm/vmalloc.c:130
vmap_page_range_noflush+0x119/0x144()
CPU: 0 PID: 1971 Comm: healthd Not tainted 4.0.0-rc1-00399-g7da3fdc-dirty #157
Hardware name: ARM-Versatile Express
[<c001246d>] (unwind_backtrace) from [<c000f7f9>] (show_stack+0x11/0x14)
[<c000f7f9>] (show_stack) from [<c049a221>] (dump_stack+0x59/0x7c)
[<c049a221>] (dump_stack) from [<c001cf21>] (warn_slowpath_common+0x55/0x84)
[<c001cf21>] (warn_slowpath_common) from [<c001cfe3>]
(warn_slowpath_null+0x17/0x1c)
[<c001cfe3>] (warn_slowpath_null) from [<c00c66c5>]
(vmap_page_range_noflush+0x119/0x144)
[<c00c66c5>] (vmap_page_range_noflush) from [<c00c716b>] (map_vm_area+0x27/0x48)
[<c00c716b>] (map_vm_area) from [<c038ddaf>]
(binder_update_page_range+0x12f/0x27c)
[<c038ddaf>] (binder_update_page_range) from [<c038e857>]
(binder_mmap+0xbf/0x1ac)
[<c038e857>] (binder_mmap) from [<c00c2dc7>] (mmap_region+0x2eb/0x4d4)
[<c00c2dc7>] (mmap_region) from [<c00c3197>] (do_mmap_pgoff+0x1e7/0x250)
[<c00c3197>] (do_mmap_pgoff) from [<c00b35b5>] (vm_mmap_pgoff+0x45/0x60)
[<c00b35b5>] (vm_mmap_pgoff) from [<c00c1f39>] (SyS_mmap_pgoff+0x5d/0x80)
[<c00c1f39>] (SyS_mmap_pgoff) from [<c000ce81>] (ret_fast_syscall+0x1/0x5c)
---[ end trace 48c2c4b9a1349e54 ]---
binder: 1982: binder_alloc_buf failed to map page at f0e00000 in kernel
binder: binder_mmap: 1982 b6bde000-b6cdc000 alloc small buf failed -12
Use map_kernel_range_noflush() instead of map_vm_area() as this is better
API for binder's purposes and it allows to get rid of 'vm_struct tmp_area' at all.
Fixes: 71394fe50146 ("mm: vmalloc: add flag preventing guard hole allocation")
Signed-off-by: Andrey Ryabinin <[email protected]>
Reported-by: Amit Pundir <[email protected]>
---
drivers/android/binder.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 33b09b6..a984fbb 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -551,7 +551,6 @@ static int binder_update_page_range(struct binder_proc *proc, int allocate,
{
void *page_addr;
unsigned long user_page_addr;
- struct vm_struct tmp_area;
struct page **page;
struct mm_struct *mm;
@@ -600,9 +599,10 @@ static int binder_update_page_range(struct binder_proc *proc, int allocate,
proc->pid, page_addr);
goto err_alloc_page_failed;
}
- tmp_area.addr = page_addr;
- tmp_area.size = PAGE_SIZE + PAGE_SIZE /* guard page? */;
- ret = map_vm_area(&tmp_area, PAGE_KERNEL, page);
+ ret = map_kernel_range_noflush((unsigned long)page_addr,
+ PAGE_SIZE, PAGE_KERNEL, page);
+ flush_cache_vmap((unsigned long)page_addr,
+ (unsigned long)page_addr + PAGE_SIZE);
if (ret) {
pr_err("%d: binder_alloc_buf failed to map page at %p in kernel\n",
proc->pid, page_addr);
--
2.3.0
On Fri, Feb 27, 2015 at 8:30 AM, Andrey Ryabinin <[email protected]> wrote:
> binder_update_page_range() initializes only addr and size
> fields in 'struct vm_struct tmp_area;' and passes it to
> map_vm_area().
>
> Before 71394fe50146 ("mm: vmalloc: add flag preventing guard hole allocation")
> this was because map_vm_area() didn't use any other fields
> in vm_struct except addr and size.
>
> Now get_vm_area_size() (used in map_vm_area()) reads vm_struct's
> flags to determine whether vm area has guard hole or not.
>
> binder_update_page_range() don't initialize flags field, so
> this causes following binder mmap failures:
> -----------[ cut here ]------------
> WARNING: CPU: 0 PID: 1971 at mm/vmalloc.c:130
> vmap_page_range_noflush+0x119/0x144()
> CPU: 0 PID: 1971 Comm: healthd Not tainted 4.0.0-rc1-00399-g7da3fdc-dirty #157
> Hardware name: ARM-Versatile Express
> [<c001246d>] (unwind_backtrace) from [<c000f7f9>] (show_stack+0x11/0x14)
> [<c000f7f9>] (show_stack) from [<c049a221>] (dump_stack+0x59/0x7c)
> [<c049a221>] (dump_stack) from [<c001cf21>] (warn_slowpath_common+0x55/0x84)
> [<c001cf21>] (warn_slowpath_common) from [<c001cfe3>]
> (warn_slowpath_null+0x17/0x1c)
> [<c001cfe3>] (warn_slowpath_null) from [<c00c66c5>]
> (vmap_page_range_noflush+0x119/0x144)
> [<c00c66c5>] (vmap_page_range_noflush) from [<c00c716b>] (map_vm_area+0x27/0x48)
> [<c00c716b>] (map_vm_area) from [<c038ddaf>]
> (binder_update_page_range+0x12f/0x27c)
> [<c038ddaf>] (binder_update_page_range) from [<c038e857>]
> (binder_mmap+0xbf/0x1ac)
> [<c038e857>] (binder_mmap) from [<c00c2dc7>] (mmap_region+0x2eb/0x4d4)
> [<c00c2dc7>] (mmap_region) from [<c00c3197>] (do_mmap_pgoff+0x1e7/0x250)
> [<c00c3197>] (do_mmap_pgoff) from [<c00b35b5>] (vm_mmap_pgoff+0x45/0x60)
> [<c00b35b5>] (vm_mmap_pgoff) from [<c00c1f39>] (SyS_mmap_pgoff+0x5d/0x80)
> [<c00c1f39>] (SyS_mmap_pgoff) from [<c000ce81>] (ret_fast_syscall+0x1/0x5c)
> ---[ end trace 48c2c4b9a1349e54 ]---
> binder: 1982: binder_alloc_buf failed to map page at f0e00000 in kernel
> binder: binder_mmap: 1982 b6bde000-b6cdc000 alloc small buf failed -12
>
> Use map_kernel_range_noflush() instead of map_vm_area() as this is better
> API for binder's purposes and it allows to get rid of 'vm_struct tmp_area' at all.
>
> Fixes: 71394fe50146 ("mm: vmalloc: add flag preventing guard hole allocation")
> Signed-off-by: Andrey Ryabinin <[email protected]>
> Reported-by: Amit Pundir <[email protected]>
> ---
> drivers/android/binder.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index 33b09b6..a984fbb 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -551,7 +551,6 @@ static int binder_update_page_range(struct binder_proc *proc, int allocate,
> {
> void *page_addr;
> unsigned long user_page_addr;
> - struct vm_struct tmp_area;
> struct page **page;
> struct mm_struct *mm;
>
> @@ -600,9 +599,10 @@ static int binder_update_page_range(struct binder_proc *proc, int allocate,
> proc->pid, page_addr);
> goto err_alloc_page_failed;
> }
> - tmp_area.addr = page_addr;
> - tmp_area.size = PAGE_SIZE + PAGE_SIZE /* guard page? */;
> - ret = map_vm_area(&tmp_area, PAGE_KERNEL, page);
> + ret = map_kernel_range_noflush((unsigned long)page_addr,
> + PAGE_SIZE, PAGE_KERNEL, page);
> + flush_cache_vmap((unsigned long)page_addr,
> + (unsigned long)page_addr + PAGE_SIZE);
> if (ret) {
> pr_err("%d: binder_alloc_buf failed to map page at %p in kernel\n",
> proc->pid, page_addr);
So with this patch I don't see the warnings, but I'm still seeing:
[ 11.154283] binder: 1956: binder_alloc_buf failed to map page at
f0340000 in kernel
[ 11.154916] binder: binder_mmap: 1956 b6ce0000-b6d00000 alloc small
buf failed -12
over and over. So I don't think this patch is quite right.
thanks
-john
On 02/27/2015 08:26 PM, John Stultz wrote:
> On Fri, Feb 27, 2015 at 8:30 AM, Andrey Ryabinin <[email protected]> wrote:
>> binder_update_page_range() initializes only addr and size
>> fields in 'struct vm_struct tmp_area;' and passes it to
>> map_vm_area().
>>
>> Before 71394fe50146 ("mm: vmalloc: add flag preventing guard hole allocation")
>> this was because map_vm_area() didn't use any other fields
>> in vm_struct except addr and size.
>>
>> Now get_vm_area_size() (used in map_vm_area()) reads vm_struct's
>> flags to determine whether vm area has guard hole or not.
>>
>> binder_update_page_range() don't initialize flags field, so
>> this causes following binder mmap failures:
>> -----------[ cut here ]------------
>> WARNING: CPU: 0 PID: 1971 at mm/vmalloc.c:130
>> vmap_page_range_noflush+0x119/0x144()
>> CPU: 0 PID: 1971 Comm: healthd Not tainted 4.0.0-rc1-00399-g7da3fdc-dirty #157
>> Hardware name: ARM-Versatile Express
>> [<c001246d>] (unwind_backtrace) from [<c000f7f9>] (show_stack+0x11/0x14)
>> [<c000f7f9>] (show_stack) from [<c049a221>] (dump_stack+0x59/0x7c)
>> [<c049a221>] (dump_stack) from [<c001cf21>] (warn_slowpath_common+0x55/0x84)
>> [<c001cf21>] (warn_slowpath_common) from [<c001cfe3>]
>> (warn_slowpath_null+0x17/0x1c)
>> [<c001cfe3>] (warn_slowpath_null) from [<c00c66c5>]
>> (vmap_page_range_noflush+0x119/0x144)
>> [<c00c66c5>] (vmap_page_range_noflush) from [<c00c716b>] (map_vm_area+0x27/0x48)
>> [<c00c716b>] (map_vm_area) from [<c038ddaf>]
>> (binder_update_page_range+0x12f/0x27c)
>> [<c038ddaf>] (binder_update_page_range) from [<c038e857>]
>> (binder_mmap+0xbf/0x1ac)
>> [<c038e857>] (binder_mmap) from [<c00c2dc7>] (mmap_region+0x2eb/0x4d4)
>> [<c00c2dc7>] (mmap_region) from [<c00c3197>] (do_mmap_pgoff+0x1e7/0x250)
>> [<c00c3197>] (do_mmap_pgoff) from [<c00b35b5>] (vm_mmap_pgoff+0x45/0x60)
>> [<c00b35b5>] (vm_mmap_pgoff) from [<c00c1f39>] (SyS_mmap_pgoff+0x5d/0x80)
>> [<c00c1f39>] (SyS_mmap_pgoff) from [<c000ce81>] (ret_fast_syscall+0x1/0x5c)
>> ---[ end trace 48c2c4b9a1349e54 ]---
>> binder: 1982: binder_alloc_buf failed to map page at f0e00000 in kernel
>> binder: binder_mmap: 1982 b6bde000-b6cdc000 alloc small buf failed -12
>>
>> Use map_kernel_range_noflush() instead of map_vm_area() as this is better
>> API for binder's purposes and it allows to get rid of 'vm_struct tmp_area' at all.
>>
>> Fixes: 71394fe50146 ("mm: vmalloc: add flag preventing guard hole allocation")
>> Signed-off-by: Andrey Ryabinin <[email protected]>
>> Reported-by: Amit Pundir <[email protected]>
>> ---
>> drivers/android/binder.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
>> index 33b09b6..a984fbb 100644
>> --- a/drivers/android/binder.c
>> +++ b/drivers/android/binder.c
>> @@ -551,7 +551,6 @@ static int binder_update_page_range(struct binder_proc *proc, int allocate,
>> {
>> void *page_addr;
>> unsigned long user_page_addr;
>> - struct vm_struct tmp_area;
>> struct page **page;
>> struct mm_struct *mm;
>>
>> @@ -600,9 +599,10 @@ static int binder_update_page_range(struct binder_proc *proc, int allocate,
>> proc->pid, page_addr);
>> goto err_alloc_page_failed;
>> }
>> - tmp_area.addr = page_addr;
>> - tmp_area.size = PAGE_SIZE + PAGE_SIZE /* guard page? */;
>> - ret = map_vm_area(&tmp_area, PAGE_KERNEL, page);
>> + ret = map_kernel_range_noflush((unsigned long)page_addr,
>> + PAGE_SIZE, PAGE_KERNEL, page);
>> + flush_cache_vmap((unsigned long)page_addr,
>> + (unsigned long)page_addr + PAGE_SIZE);
>> if (ret) {
>> pr_err("%d: binder_alloc_buf failed to map page at %p in kernel\n",
>> proc->pid, page_addr);
>
> So with this patch I don't see the warnings, but I'm still seeing:
> [ 11.154283] binder: 1956: binder_alloc_buf failed to map page at
> f0340000 in kernel
> [ 11.154916] binder: binder_mmap: 1956 b6ce0000-b6d00000 alloc small
> buf failed -12
>
> over and over. So I don't think this patch is quite right.
>
Thanks.
Seems that error check is wrong now.
map_vm_area() returns 0 on success, map_kernel_range_noflush() number of mapped pages.
> thanks
> -john
>
binder_update_page_range() initializes only addr and size
fields in 'struct vm_struct tmp_area;' and passes it to
map_vm_area().
Before 71394fe50146 ("mm: vmalloc: add flag preventing guard hole allocation")
this was because map_vm_area() didn't use any other fields
in vm_struct except addr and size.
Now get_vm_area_size() (used in map_vm_area()) reads vm_struct's
flags to determine whether vm area has guard hole or not.
binder_update_page_range() don't initialize flags field, so
this causes following binder mmap failures:
-----------[ cut here ]------------
WARNING: CPU: 0 PID: 1971 at mm/vmalloc.c:130
vmap_page_range_noflush+0x119/0x144()
CPU: 0 PID: 1971 Comm: healthd Not tainted 4.0.0-rc1-00399-g7da3fdc-dirty #157
Hardware name: ARM-Versatile Express
[<c001246d>] (unwind_backtrace) from [<c000f7f9>] (show_stack+0x11/0x14)
[<c000f7f9>] (show_stack) from [<c049a221>] (dump_stack+0x59/0x7c)
[<c049a221>] (dump_stack) from [<c001cf21>] (warn_slowpath_common+0x55/0x84)
[<c001cf21>] (warn_slowpath_common) from [<c001cfe3>]
(warn_slowpath_null+0x17/0x1c)
[<c001cfe3>] (warn_slowpath_null) from [<c00c66c5>]
(vmap_page_range_noflush+0x119/0x144)
[<c00c66c5>] (vmap_page_range_noflush) from [<c00c716b>] (map_vm_area+0x27/0x48)
[<c00c716b>] (map_vm_area) from [<c038ddaf>]
(binder_update_page_range+0x12f/0x27c)
[<c038ddaf>] (binder_update_page_range) from [<c038e857>]
(binder_mmap+0xbf/0x1ac)
[<c038e857>] (binder_mmap) from [<c00c2dc7>] (mmap_region+0x2eb/0x4d4)
[<c00c2dc7>] (mmap_region) from [<c00c3197>] (do_mmap_pgoff+0x1e7/0x250)
[<c00c3197>] (do_mmap_pgoff) from [<c00b35b5>] (vm_mmap_pgoff+0x45/0x60)
[<c00b35b5>] (vm_mmap_pgoff) from [<c00c1f39>] (SyS_mmap_pgoff+0x5d/0x80)
[<c00c1f39>] (SyS_mmap_pgoff) from [<c000ce81>] (ret_fast_syscall+0x1/0x5c)
---[ end trace 48c2c4b9a1349e54 ]---
binder: 1982: binder_alloc_buf failed to map page at f0e00000 in kernel
binder: binder_mmap: 1982 b6bde000-b6cdc000 alloc small buf failed -12
Use map_kernel_range_noflush() instead of map_vm_area() as this is better
API for binder's purposes and it allows to get rid of 'vm_struct tmp_area' at all.
Fixes: 71394fe50146 ("mm: vmalloc: add flag preventing guard hole allocation")
Signed-off-by: Andrey Ryabinin <[email protected]>
Reported-by: Amit Pundir <[email protected]>
---
Changes since v1:
- fixed ret check after map_kernel_ranges_noflush().
drivers/android/binder.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 33b09b6..6607f3c 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -551,7 +551,6 @@ static int binder_update_page_range(struct binder_proc *proc, int allocate,
{
void *page_addr;
unsigned long user_page_addr;
- struct vm_struct tmp_area;
struct page **page;
struct mm_struct *mm;
@@ -600,10 +599,11 @@ static int binder_update_page_range(struct binder_proc *proc, int allocate,
proc->pid, page_addr);
goto err_alloc_page_failed;
}
- tmp_area.addr = page_addr;
- tmp_area.size = PAGE_SIZE + PAGE_SIZE /* guard page? */;
- ret = map_vm_area(&tmp_area, PAGE_KERNEL, page);
- if (ret) {
+ ret = map_kernel_range_noflush((unsigned long)page_addr,
+ PAGE_SIZE, PAGE_KERNEL, page);
+ flush_cache_vmap((unsigned long)page_addr,
+ (unsigned long)page_addr + PAGE_SIZE);
+ if (ret != 1) {
pr_err("%d: binder_alloc_buf failed to map page at %p in kernel\n",
proc->pid, page_addr);
goto err_map_kernel_failed;
--
2.3.0
On Fri, Feb 27, 2015 at 9:44 AM, Andrey Ryabinin <[email protected]> wrote:
> binder_update_page_range() initializes only addr and size
> fields in 'struct vm_struct tmp_area;' and passes it to
> map_vm_area().
>
> Before 71394fe50146 ("mm: vmalloc: add flag preventing guard hole allocation")
> this was because map_vm_area() didn't use any other fields
> in vm_struct except addr and size.
>
> Now get_vm_area_size() (used in map_vm_area()) reads vm_struct's
> flags to determine whether vm area has guard hole or not.
>
> binder_update_page_range() don't initialize flags field, so
> this causes following binder mmap failures:
> -----------[ cut here ]------------
> WARNING: CPU: 0 PID: 1971 at mm/vmalloc.c:130
> vmap_page_range_noflush+0x119/0x144()
> CPU: 0 PID: 1971 Comm: healthd Not tainted 4.0.0-rc1-00399-g7da3fdc-dirty #157
> Hardware name: ARM-Versatile Express
> [<c001246d>] (unwind_backtrace) from [<c000f7f9>] (show_stack+0x11/0x14)
> [<c000f7f9>] (show_stack) from [<c049a221>] (dump_stack+0x59/0x7c)
> [<c049a221>] (dump_stack) from [<c001cf21>] (warn_slowpath_common+0x55/0x84)
> [<c001cf21>] (warn_slowpath_common) from [<c001cfe3>]
> (warn_slowpath_null+0x17/0x1c)
> [<c001cfe3>] (warn_slowpath_null) from [<c00c66c5>]
> (vmap_page_range_noflush+0x119/0x144)
> [<c00c66c5>] (vmap_page_range_noflush) from [<c00c716b>] (map_vm_area+0x27/0x48)
> [<c00c716b>] (map_vm_area) from [<c038ddaf>]
> (binder_update_page_range+0x12f/0x27c)
> [<c038ddaf>] (binder_update_page_range) from [<c038e857>]
> (binder_mmap+0xbf/0x1ac)
> [<c038e857>] (binder_mmap) from [<c00c2dc7>] (mmap_region+0x2eb/0x4d4)
> [<c00c2dc7>] (mmap_region) from [<c00c3197>] (do_mmap_pgoff+0x1e7/0x250)
> [<c00c3197>] (do_mmap_pgoff) from [<c00b35b5>] (vm_mmap_pgoff+0x45/0x60)
> [<c00b35b5>] (vm_mmap_pgoff) from [<c00c1f39>] (SyS_mmap_pgoff+0x5d/0x80)
> [<c00c1f39>] (SyS_mmap_pgoff) from [<c000ce81>] (ret_fast_syscall+0x1/0x5c)
> ---[ end trace 48c2c4b9a1349e54 ]---
> binder: 1982: binder_alloc_buf failed to map page at f0e00000 in kernel
> binder: binder_mmap: 1982 b6bde000-b6cdc000 alloc small buf failed -12
>
> Use map_kernel_range_noflush() instead of map_vm_area() as this is better
> API for binder's purposes and it allows to get rid of 'vm_struct tmp_area' at all.
>
> Fixes: 71394fe50146 ("mm: vmalloc: add flag preventing guard hole allocation")
> Signed-off-by: Andrey Ryabinin <[email protected]>
> Reported-by: Amit Pundir <[email protected]>
> ---
> Changes since v1:
> - fixed ret check after map_kernel_ranges_noflush().
>
> drivers/android/binder.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index 33b09b6..6607f3c 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -551,7 +551,6 @@ static int binder_update_page_range(struct binder_proc *proc, int allocate,
> {
> void *page_addr;
> unsigned long user_page_addr;
> - struct vm_struct tmp_area;
> struct page **page;
> struct mm_struct *mm;
>
> @@ -600,10 +599,11 @@ static int binder_update_page_range(struct binder_proc *proc, int allocate,
> proc->pid, page_addr);
> goto err_alloc_page_failed;
> }
> - tmp_area.addr = page_addr;
> - tmp_area.size = PAGE_SIZE + PAGE_SIZE /* guard page? */;
> - ret = map_vm_area(&tmp_area, PAGE_KERNEL, page);
> - if (ret) {
> + ret = map_kernel_range_noflush((unsigned long)page_addr,
> + PAGE_SIZE, PAGE_KERNEL, page);
> + flush_cache_vmap((unsigned long)page_addr,
> + (unsigned long)page_addr + PAGE_SIZE);
> + if (ret != 1) {
> pr_err("%d: binder_alloc_buf failed to map page at %p in kernel\n",
> proc->pid, page_addr);
> goto err_map_kernel_failed;
This seems to work better. Thanks!
Tested-by: John Stultz <[email protected]>
-john
On Fri, 27 Feb 2015, Andrey Ryabinin wrote:
> binder_update_page_range() initializes only addr and size
> fields in 'struct vm_struct tmp_area;' and passes it to
> map_vm_area().
>
> Before 71394fe50146 ("mm: vmalloc: add flag preventing guard hole allocation")
> this was because map_vm_area() didn't use any other fields
> in vm_struct except addr and size.
>
> Now get_vm_area_size() (used in map_vm_area()) reads vm_struct's
> flags to determine whether vm area has guard hole or not.
>
> binder_update_page_range() don't initialize flags field, so
> this causes following binder mmap failures:
> -----------[ cut here ]------------
> WARNING: CPU: 0 PID: 1971 at mm/vmalloc.c:130
> vmap_page_range_noflush+0x119/0x144()
> CPU: 0 PID: 1971 Comm: healthd Not tainted 4.0.0-rc1-00399-g7da3fdc-dirty #157
> Hardware name: ARM-Versatile Express
> [<c001246d>] (unwind_backtrace) from [<c000f7f9>] (show_stack+0x11/0x14)
> [<c000f7f9>] (show_stack) from [<c049a221>] (dump_stack+0x59/0x7c)
> [<c049a221>] (dump_stack) from [<c001cf21>] (warn_slowpath_common+0x55/0x84)
> [<c001cf21>] (warn_slowpath_common) from [<c001cfe3>]
> (warn_slowpath_null+0x17/0x1c)
> [<c001cfe3>] (warn_slowpath_null) from [<c00c66c5>]
> (vmap_page_range_noflush+0x119/0x144)
> [<c00c66c5>] (vmap_page_range_noflush) from [<c00c716b>] (map_vm_area+0x27/0x48)
> [<c00c716b>] (map_vm_area) from [<c038ddaf>]
> (binder_update_page_range+0x12f/0x27c)
> [<c038ddaf>] (binder_update_page_range) from [<c038e857>]
> (binder_mmap+0xbf/0x1ac)
> [<c038e857>] (binder_mmap) from [<c00c2dc7>] (mmap_region+0x2eb/0x4d4)
> [<c00c2dc7>] (mmap_region) from [<c00c3197>] (do_mmap_pgoff+0x1e7/0x250)
> [<c00c3197>] (do_mmap_pgoff) from [<c00b35b5>] (vm_mmap_pgoff+0x45/0x60)
> [<c00b35b5>] (vm_mmap_pgoff) from [<c00c1f39>] (SyS_mmap_pgoff+0x5d/0x80)
> [<c00c1f39>] (SyS_mmap_pgoff) from [<c000ce81>] (ret_fast_syscall+0x1/0x5c)
> ---[ end trace 48c2c4b9a1349e54 ]---
> binder: 1982: binder_alloc_buf failed to map page at f0e00000 in kernel
> binder: binder_mmap: 1982 b6bde000-b6cdc000 alloc small buf failed -12
>
> Use map_kernel_range_noflush() instead of map_vm_area() as this is better
> API for binder's purposes and it allows to get rid of 'vm_struct tmp_area' at all.
>
> Fixes: 71394fe50146 ("mm: vmalloc: add flag preventing guard hole allocation")
> Signed-off-by: Andrey Ryabinin <[email protected]>
> Reported-by: Amit Pundir <[email protected]>
Acked-by: David Rientjes <[email protected]>