| Message ID | CAMi1Hd0frW_BqLwPQ7M_CKbA8-SeYhykwWYFZ-_iSre5r3zK7Q@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Feb 27, 2015 at 8:30 AM, Andrey Ryabinin <a.ryabinin@samsung.com> 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 <a.ryabinin@samsung.com> > Reported-by: Amit Pundir <amit.pundir@linaro.org> > --- > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On Fri, Feb 27, 2015 at 9:44 AM, Andrey Ryabinin <a.ryabinin@samsung.com> 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 <a.ryabinin@samsung.com> > Reported-by: Amit Pundir <amit.pundir@linaro.org> > --- > 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 <john.stultz@linaro.org> -john -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On 27 February 2015 at 23:14, Andrey Ryabinin <a.ryabinin@samsung.com> 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 <a.ryabinin@samsung.com> > Reported-by: Amit Pundir <amit.pundir@linaro.org> > --- > 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; > -- Works for me. Thanks. Tested-by: Amit Pundir <amit.pundir@linaro.org> > 2.3.0 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
--- 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);
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 <a.ryabinin@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<---------- if (ret) { ---------->8---------- Regards, Amit Pundir -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/