2016-09-02 3:59 GMT+08:00 Arve Hjønnevåg <arve@xxxxxxxxxxx>:
> On Thu, Sep 1, 2016 at 12:02 PM, Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> 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 <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
>
> 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
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
