Re: [PATCH] android: binder: use VM_ALLOC to get vm area.

[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 <opensource.ganesh@xxxxxxxxx>
> ---
>  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