Re: Regression in v4.0.0-rc1 with Android Binder

2015-02-26 Thread David Rientjes
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(_area, PAGE_KERNEL, page);
> > if (ret) {
> > -->8--

Re: Regression in v4.0.0-rc1 with Android Binder

2015-02-26 Thread Arve Hjønnevåg
On Thu, Feb 26, 2015 at 2:04 PM, 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
> [] (unwind_backtrace) from [] (show_stack+0x11/0x14)
> [] (show_stack) from [] (dump_stack+0x59/0x7c)
> [] (dump_stack) from [] (warn_slowpath_common+0x55/0x84)
> [] (warn_slowpath_common) from []
> (warn_slowpath_null+0x17/0x1c)
> [] (warn_slowpath_null) from []
> (vmap_page_range_noflush+0x119/0x144)
> [] (vmap_page_range_noflush) from [] 
> (map_vm_area+0x27/0x48)
> [] (map_vm_area) from []
> (binder_update_page_range+0x12f/0x27c)
> [] (binder_update_page_range) from []
> (binder_mmap+0xbf/0x1ac)
> [] (binder_mmap) from [] (mmap_region+0x2eb/0x4d4)
> [] (mmap_region) from [] (do_mmap_pgoff+0x1e7/0x250)
> [] (do_mmap_pgoff) from [] (vm_mmap_pgoff+0x45/0x60)
> [] (vm_mmap_pgoff) from [] (SyS_mmap_pgoff+0x5d/0x80)
> [] (SyS_mmap_pgoff) from [] (ret_fast_syscall+0x1/0x5c)
> ---[ end trace 48c2c4b9a1349e54 ]---
> binder: 1982: binder_alloc_buf failed to map page at f0e0 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 
> 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(_area, PAGE_KERNEL, page);
> if (ret) {
> -->8--
>
>
> Regards,
> Amit Pundir

-- 
Arve Hjønnevåg
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Regression in v4.0.0-rc1 with Android Binder

2015-02-26 Thread Greg KH
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
> [] (unwind_backtrace) from [] (show_stack+0x11/0x14)
> [] (show_stack) from [] (dump_stack+0x59/0x7c)
> [] (dump_stack) from [] (warn_slowpath_common+0x55/0x84)
> [] (warn_slowpath_common) from []
> (warn_slowpath_null+0x17/0x1c)
> [] (warn_slowpath_null) from []
> (vmap_page_range_noflush+0x119/0x144)
> [] (vmap_page_range_noflush) from [] 
> (map_vm_area+0x27/0x48)
> [] (map_vm_area) from []
> (binder_update_page_range+0x12f/0x27c)
> [] (binder_update_page_range) from []
> (binder_mmap+0xbf/0x1ac)
> [] (binder_mmap) from [] (mmap_region+0x2eb/0x4d4)
> [] (mmap_region) from [] (do_mmap_pgoff+0x1e7/0x250)
> [] (do_mmap_pgoff) from [] (vm_mmap_pgoff+0x45/0x60)
> [] (vm_mmap_pgoff) from [] (SyS_mmap_pgoff+0x5d/0x80)
> [] (SyS_mmap_pgoff) from [] (ret_fast_syscall+0x1/0x5c)
> ---[ end trace 48c2c4b9a1349e54 ]---
> binder: 1982: binder_alloc_buf failed to map page at f0e0 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 
> 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(_area, PAGE_KERNEL, page);
> if (ret) {
> -->8--

This looks like a valid fix, please send it in.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Regression in v4.0.0-rc1 with Android Binder

2015-02-26 Thread Amit Pundir
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
[] (unwind_backtrace) from [] (show_stack+0x11/0x14)
[] (show_stack) from [] (dump_stack+0x59/0x7c)
[] (dump_stack) from [] (warn_slowpath_common+0x55/0x84)
[] (warn_slowpath_common) from []
(warn_slowpath_null+0x17/0x1c)
[] (warn_slowpath_null) from []
(vmap_page_range_noflush+0x119/0x144)
[] (vmap_page_range_noflush) from [] (map_vm_area+0x27/0x48)
[] (map_vm_area) from []
(binder_update_page_range+0x12f/0x27c)
[] (binder_update_page_range) from []
(binder_mmap+0xbf/0x1ac)
[] (binder_mmap) from [] (mmap_region+0x2eb/0x4d4)
[] (mmap_region) from [] (do_mmap_pgoff+0x1e7/0x250)
[] (do_mmap_pgoff) from [] (vm_mmap_pgoff+0x45/0x60)
[] (vm_mmap_pgoff) from [] (SyS_mmap_pgoff+0x5d/0x80)
[] (SyS_mmap_pgoff) from [] (ret_fast_syscall+0x1/0x5c)
---[ end trace 48c2c4b9a1349e54 ]---
binder: 1982: binder_alloc_buf failed to map page at f0e0 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 
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(_area, PAGE_KERNEL, page);
if (ret) {
-->8--


Regards,
Amit Pundir
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Regression in v4.0.0-rc1 with Android Binder

2015-02-26 Thread Amit Pundir
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 f0e0 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 a.ryabi...@samsung.com
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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Regression in v4.0.0-rc1 with Android Binder

2015-02-26 Thread Greg KH
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 f0e0 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 a.ryabi...@samsung.com
 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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Regression in v4.0.0-rc1 with Android Binder

2015-02-26 Thread Arve Hjønnevåg
On Thu, Feb 26, 2015 at 2:04 PM, Amit Pundir amit.pun...@linaro.org 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 f0e0 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 a.ryabi...@samsung.com
 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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Regression in v4.0.0-rc1 with Android Binder

2015-02-26 Thread David Rientjes
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--