Re: [PATCH 2/5] android: binder: Add allocator selftest

[Sherry Yang]

Hi Geert,

This selftest can't run until userspace has called mmap. Since it is
hooked into ioctl, it's probably not worth the effort of making it a
module, since ioctl will have to check at runtime whether the selftest
module has been loaded, which would add overhead even when the test is
not enabled.

Thanks,
Sherry

On Sun, Sep 10, 2017 at 7:48 AM, Geert Uytterhoeven
 wrote:
> Hi Sherry,
>
> On Wed, Aug 16, 2017 at 2:25 AM, Sherry Yang  wrote:
>> binder_alloc_selftest tests that alloc_new_buf handles page allocation and
>> deallocation properly when allocate and free buffers. The test allocates 5
>> buffers of various sizes to cover all possible page alignment cases, and
>> frees the buffers using a list of exhaustive freeing order.
>>
>> Test: boot the device with ANDROID_BINDER_IPC_SELFTEST config option
>> enabled. Allocator selftest passes.
>>
>> Change-Id: I9064f60c85b1e0389c88e927e2b147ec92cae0d1
>> Signed-off-by: Sherry Yang 
>> ---
>>  drivers/android/Kconfig |  10 ++
>>  drivers/android/Makefile|   1 +
>>  drivers/android/binder.c|   2 +
>>  drivers/android/binder_alloc.h  |   5 +
>>  drivers/android/binder_alloc_selftest.c | 271 
>> 
>>  5 files changed, 289 insertions(+)
>>  create mode 100644 drivers/android/binder_alloc_selftest.c
>>
>> diff --git a/drivers/android/Kconfig b/drivers/android/Kconfig
>> index 832e885349b1..0f295704abd4 100644
>> --- a/drivers/android/Kconfig
>> +++ b/drivers/android/Kconfig
>> @@ -44,6 +44,16 @@ config ANDROID_BINDER_IPC_32BIT
>>
>>   Note that enabling this will break newer Android user-space.
>>
>> +config ANDROID_BINDER_IPC_SELFTEST
>> +   bool "Android Binder IPC Driver Selftest"
>
> What about making this tristate...
>
>> +   depends on ANDROID_BINDER_IPC
>> +   ---help---
>> + This feature allows binder selftest to run.
>> +
>> + Binder selftest checks the allocation and free of binder buffers
>> + exhaustively with combinations of various buffer sizes and
>> + alignments.
>> +
>>  endif # if ANDROID
>>
>>  endmenu
>> diff --git a/drivers/android/Makefile b/drivers/android/Makefile
>> index 4b7c726bb560..a01254c43ee3 100644
>> --- a/drivers/android/Makefile
>> +++ b/drivers/android/Makefile
>> @@ -1,3 +1,4 @@
>>  ccflags-y += -I$(src)  # needed for trace events
>>
>>  obj-$(CONFIG_ANDROID_BINDER_IPC)   += binder.o binder_alloc.o
>> +obj-$(CONFIG_ANDROID_BINDER_IPC_SELFTEST) += binder_alloc_selftest.o
>> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
>> index 9f95d7093f32..b31e64c6f666 100644
>> --- a/drivers/android/binder.c
>> +++ b/drivers/android/binder.c
>> @@ -4225,6 +4225,8 @@ static long binder_ioctl(struct file *filp, unsigned 
>> int cmd, unsigned long arg)
>> /*pr_info("binder_ioctl: %d:%d %x %lx\n",
>> proc->pid, current->pid, cmd, arg);*/
>>
>> +   binder_selftest_alloc(&proc->alloc);
>
> ... and calling the selftest function from module_init() in
> drivers/android/binder_alloc_selftest.c?
>
> That way the test can either run builtin during starup, or during module load,
> like most other selftests?
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- 
> ge...@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like 
> that.
> -- Linus Torvalds
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2] android: binder: Drop lru lock in isolate callback

[Sherry Yang]

Drop the global lru lock in isolate callback
before calling zap_page_range which calls
cond_resched, and re-acquire the global lru
lock before returning. Also change return
code to LRU_REMOVED_RETRY.

Use mmput_async when fail to acquire mmap sem
in an atomic context.

Fix "BUG: sleeping function called from invalid context"
errors when CONFIG_DEBUG_ATOMIC_SLEEP is enabled.

Also restore mmput_async, which was initially introduced in
ec8d7c14e ("mm, oom_reaper: do not mmput synchronously from
the oom reaper context"), and was removed in 212925802
("mm: oom: let oom_reap_task and exit_mmap run concurrently").

Fixes: f2517eb76f1f2 ("android: binder: Add global lru shrinker to binder")
Reported-by: Kyle Yan 
Acked-by: Arve Hjønnevåg 
Signed-off-by: Sherry Yang 
---
Changes in v2:
Combined '[PATCH] android: binder: Drop lru lock in isolate callback'
with '[PATCH] mm: Restore mmput_async'.

 drivers/android/binder_alloc.c | 18 --
 include/linux/sched/mm.h   |  6 ++
 kernel/fork.c  | 18 ++
 3 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index 8fe165844e47..064f5e31ec55 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -913,6 +913,7 @@ enum lru_status binder_alloc_free_page(struct list_head 
*item,
struct binder_alloc *alloc;
uintptr_t page_addr;
size_t index;
+   struct vm_area_struct *vma;
 
alloc = page->alloc;
if (!mutex_trylock(&alloc->mutex))
@@ -923,16 +924,22 @@ enum lru_status binder_alloc_free_page(struct list_head 
*item,
 
index = page - alloc->pages;
page_addr = (uintptr_t)alloc->buffer + index * PAGE_SIZE;
-   if (alloc->vma) {
+   vma = alloc->vma;
+   if (vma) {
mm = get_task_mm(alloc->tsk);
if (!mm)
goto err_get_task_mm_failed;
if (!down_write_trylock(&mm->mmap_sem))
goto err_down_write_mmap_sem_failed;
+   }
+
+   list_lru_isolate(lru, item);
+   spin_unlock(lock);
 
+   if (vma) {
trace_binder_unmap_user_start(alloc, index);
 
-   zap_page_range(alloc->vma,
+   zap_page_range(vma,
   page_addr + alloc->user_buffer_offset,
   PAGE_SIZE);
 
@@ -950,13 +957,12 @@ enum lru_status binder_alloc_free_page(struct list_head 
*item,
 
trace_binder_unmap_kernel_end(alloc, index);
 
-   list_lru_isolate(lru, item);
-
+   spin_lock(lock);
mutex_unlock(&alloc->mutex);
-   return LRU_REMOVED;
+   return LRU_REMOVED_RETRY;
 
 err_down_write_mmap_sem_failed:
-   mmput(mm);
+   mmput_async(mm);
 err_get_task_mm_failed:
 err_page_already_freed:
mutex_unlock(&alloc->mutex);
diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h
index 3a19c253bdb1..ae53e413fb13 100644
--- a/include/linux/sched/mm.h
+++ b/include/linux/sched/mm.h
@@ -84,6 +84,12 @@ static inline bool mmget_not_zero(struct mm_struct *mm)
 
 /* mmput gets rid of the mappings and all user-space */
 extern void mmput(struct mm_struct *);
+#ifdef CONFIG_MMU
+/* same as above but performs the slow path from the async context. Can
+ * be called from the atomic context as well
+ */
+void mmput_async(struct mm_struct *);
+#endif
 
 /* Grab a reference to a task's mm, if it is not already going away */
 extern struct mm_struct *get_task_mm(struct task_struct *task);
diff --git a/kernel/fork.c b/kernel/fork.c
index 10646182440f..e702cb9ffbd8 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -946,6 +946,24 @@ void mmput(struct mm_struct *mm)
 }
 EXPORT_SYMBOL_GPL(mmput);
 
+#ifdef CONFIG_MMU
+static void mmput_async_fn(struct work_struct *work)
+{
+   struct mm_struct *mm = container_of(work, struct mm_struct,
+   async_put_work);
+
+   __mmput(mm);
+}
+
+void mmput_async(struct mm_struct *mm)
+{
+   if (atomic_dec_and_test(&mm->mm_users)) {
+   INIT_WORK(&mm->async_put_work, mmput_async_fn);
+   schedule_work(&mm->async_put_work);
+   }
+}
+#endif
+
 /**
  * set_mm_exe_file - change a reference to the mm's executable file
  *
-- 
2.11.0 (Apple Git-81)

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 2/2] android: binder: Don't get mm from task

[Sherry Yang]

Use binder_alloc struct's mm_struct rather than getting
a reference to the mm struct through get_task_mm to
avoid a potential deadlock between lru lock, task lock and
dentry lock, since a thread can be holding the task lock
and the dentry lock while trying to acquire the lru lock.

Acked-by: Arve Hjønnevåg 
Signed-off-by: Sherry Yang 
---
 drivers/android/binder_alloc.c | 22 +-
 drivers/android/binder_alloc.h |  1 -
 2 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index b87ecf77f9d1..e283670695db 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -215,17 +215,12 @@ static int binder_update_page_range(struct binder_alloc 
*alloc, int allocate,
}
}
 
-   if (need_mm)
-   mm = get_task_mm(alloc->tsk);
+   if (need_mm && mmget_not_zero(alloc->vma_vm_mm))
+   mm = alloc->vma_vm_mm;
 
if (mm) {
down_write(&mm->mmap_sem);
vma = alloc->vma;
-   if (vma && mm != alloc->vma_vm_mm) {
-   pr_err("%d: vma mm and task mm mismatch\n",
-   alloc->pid);
-   vma = NULL;
-   }
}
 
if (!vma && need_mm) {
@@ -718,6 +713,7 @@ int binder_alloc_mmap_handler(struct binder_alloc *alloc,
barrier();
alloc->vma = vma;
alloc->vma_vm_mm = vma->vm_mm;
+   mmgrab(alloc->vma_vm_mm);
 
return 0;
 
@@ -793,6 +789,8 @@ void binder_alloc_deferred_release(struct binder_alloc 
*alloc)
vfree(alloc->buffer);
}
mutex_unlock(&alloc->mutex);
+   if (alloc->vma_vm_mm)
+   mmdrop(alloc->vma_vm_mm);
 
binder_alloc_debug(BINDER_DEBUG_OPEN_CLOSE,
 "%s: %d buffers %d, pages %d\n",
@@ -887,7 +885,6 @@ int binder_alloc_get_allocated_count(struct binder_alloc 
*alloc)
 void binder_alloc_vma_close(struct binder_alloc *alloc)
 {
WRITE_ONCE(alloc->vma, NULL);
-   WRITE_ONCE(alloc->vma_vm_mm, NULL);
 }
 
 /**
@@ -924,9 +921,9 @@ enum lru_status binder_alloc_free_page(struct list_head 
*item,
page_addr = (uintptr_t)alloc->buffer + index * PAGE_SIZE;
vma = alloc->vma;
if (vma) {
-   mm = get_task_mm(alloc->tsk);
-   if (!mm)
-   goto err_get_task_mm_failed;
+   if (!mmget_not_zero(alloc->vma_vm_mm))
+   goto err_mmget;
+   mm = alloc->vma_vm_mm;
if (!down_write_trylock(&mm->mmap_sem))
goto err_down_write_mmap_sem_failed;
}
@@ -961,7 +958,7 @@ enum lru_status binder_alloc_free_page(struct list_head 
*item,
 
 err_down_write_mmap_sem_failed:
mmput_async(mm);
-err_get_task_mm_failed:
+err_mmget:
 err_page_already_freed:
mutex_unlock(&alloc->mutex);
 err_get_alloc_mutex_failed:
@@ -1000,7 +997,6 @@ struct shrinker binder_shrinker = {
  */
 void binder_alloc_init(struct binder_alloc *alloc)
 {
-   alloc->tsk = current->group_leader;
alloc->pid = current->group_leader->pid;
mutex_init(&alloc->mutex);
INIT_LIST_HEAD(&alloc->buffers);
diff --git a/drivers/android/binder_alloc.h b/drivers/android/binder_alloc.h
index a3a3602c689c..2dd33b6df104 100644
--- a/drivers/android/binder_alloc.h
+++ b/drivers/android/binder_alloc.h
@@ -100,7 +100,6 @@ struct binder_lru_page {
  */
 struct binder_alloc {
struct mutex mutex;
-   struct task_struct *tsk;
struct vm_area_struct *vma;
struct mm_struct *vma_vm_mm;
void *buffer;
-- 
2.11.0 (Apple Git-81)

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 1/2] android: binder: Remove unused vma argument

[Sherry Yang]

The vma argument in update_page_range is no longer
used after 74310e06 ("android: binder: Move buffer
out of area shared with user space"), since mmap_handler
no longer calls update_page_range with a vma.

Acked-by: Arve Hjønnevåg 
Signed-off-by: Sherry Yang 
---
 drivers/android/binder_alloc.c | 16 +++-
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index 064f5e31ec55..b87ecf77f9d1 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -186,12 +186,12 @@ struct binder_buffer *binder_alloc_prepare_to_free(struct 
binder_alloc *alloc,
 }
 
 static int binder_update_page_range(struct binder_alloc *alloc, int allocate,
-   void *start, void *end,
-   struct vm_area_struct *vma)
+   void *start, void *end)
 {
void *page_addr;
unsigned long user_page_addr;
struct binder_lru_page *page;
+   struct vm_area_struct *vma = NULL;
struct mm_struct *mm = NULL;
bool need_mm = false;
 
@@ -215,7 +215,7 @@ static int binder_update_page_range(struct binder_alloc 
*alloc, int allocate,
}
}
 
-   if (!vma && need_mm)
+   if (need_mm)
mm = get_task_mm(alloc->tsk);
 
if (mm) {
@@ -442,7 +442,7 @@ struct binder_buffer *binder_alloc_new_buf_locked(struct 
binder_alloc *alloc,
if (end_page_addr > has_page_addr)
end_page_addr = has_page_addr;
ret = binder_update_page_range(alloc, 1,
-   (void *)PAGE_ALIGN((uintptr_t)buffer->data), end_page_addr, NULL);
+   (void *)PAGE_ALIGN((uintptr_t)buffer->data), end_page_addr);
if (ret)
return ERR_PTR(ret);
 
@@ -483,7 +483,7 @@ struct binder_buffer *binder_alloc_new_buf_locked(struct 
binder_alloc *alloc,
 err_alloc_buf_struct_failed:
binder_update_page_range(alloc, 0,
 (void *)PAGE_ALIGN((uintptr_t)buffer->data),
-end_page_addr, NULL);
+end_page_addr);
return ERR_PTR(-ENOMEM);
 }
 
@@ -567,8 +567,7 @@ static void binder_delete_free_buffer(struct binder_alloc 
*alloc,
   alloc->pid, buffer->data,
   prev->data, next->data);
binder_update_page_range(alloc, 0, buffer_start_page(buffer),
-buffer_start_page(buffer) + PAGE_SIZE,
-NULL);
+buffer_start_page(buffer) + PAGE_SIZE);
}
list_del(&buffer->entry);
kfree(buffer);
@@ -605,8 +604,7 @@ static void binder_free_buf_locked(struct binder_alloc 
*alloc,
 
binder_update_page_range(alloc, 0,
(void *)PAGE_ALIGN((uintptr_t)buffer->data),
-   (void *)(((uintptr_t)buffer->data + buffer_size) & PAGE_MASK),
-   NULL);
+   (void *)(((uintptr_t)buffer->data + buffer_size) & PAGE_MASK));
 
rb_erase(&buffer->rb_node, &alloc->allocated_buffers);
buffer->free = 1;
-- 
2.11.0 (Apple Git-81)

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 2/2] android: binder: Fix null ptr dereference in debug msg

[Sherry Yang]

Don't access next->data in kernel debug message when the
next buffer is null.

Acked-by: Arve Hjønnevåg 
Signed-off-by: Sherry Yang 
---
 drivers/android/binder_alloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index 47e2c032ad3d..6f6f745605af 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -560,7 +560,7 @@ static void binder_delete_free_buffer(struct binder_alloc 
*alloc,
binder_alloc_debug(BINDER_DEBUG_BUFFER_ALLOC,
   "%d: merge free, buffer %pK do not share 
page with %pK or %pK\n",
   alloc->pid, buffer->data,
-  prev->data, next->data);
+  prev->data, next ? next->data : NULL);
binder_update_page_range(alloc, 0, buffer_start_page(buffer),
 buffer_start_page(buffer) + PAGE_SIZE);
}
-- 
2.11.0 (Apple Git-81)

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 1/2] android: binder: Change binder_shrinker to static

[Sherry Yang]

binder_shrinker struct is not used anywhere outside of
binder_alloc.c and should be static.

Acked-by: Arve Hjønnevåg 
Signed-off-by: Sherry Yang 
---
 drivers/android/binder_alloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index e283670695db..47e2c032ad3d 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -982,7 +982,7 @@ binder_shrink_scan(struct shrinker *shrink, struct 
shrink_control *sc)
return ret;
 }
 
-struct shrinker binder_shrinker = {
+static struct shrinker binder_shrinker = {
.count_objects = binder_shrink_count,
.scan_objects = binder_shrink_scan,
.seeks = DEFAULT_SEEKS,
-- 
2.11.0 (Apple Git-81)

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 3/4] android: binder: Remove unused vma argument

[Sherry Yang]

The vma argument in update_page_range is no longer
used after 74310e06 ("android: binder: Move buffer
out of area shared with user space"), since mmap_handler
no longer calls update_page_range with a vma.

Acked-by: Arve Hjønnevåg 
Signed-off-by: Sherry Yang 
---
 drivers/android/binder_alloc.c | 16 +++-
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index c2819a3d58a6..36bcea1fc47c 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -186,12 +186,12 @@ struct binder_buffer *binder_alloc_prepare_to_free(struct 
binder_alloc *alloc,
 }
 
 static int binder_update_page_range(struct binder_alloc *alloc, int allocate,
-   void *start, void *end,
-   struct vm_area_struct *vma)
+   void *start, void *end)
 {
void *page_addr;
unsigned long user_page_addr;
struct binder_lru_page *page;
+   struct vm_area_struct *vma = NULL;
struct mm_struct *mm = NULL;
bool need_mm = false;
 
@@ -215,7 +215,7 @@ static int binder_update_page_range(struct binder_alloc 
*alloc, int allocate,
}
}
 
-   if (!vma && need_mm && mmget_not_zero(alloc->vma_vm_mm))
+   if (need_mm && mmget_not_zero(alloc->vma_vm_mm))
mm = alloc->vma_vm_mm;
 
if (mm) {
@@ -437,7 +437,7 @@ struct binder_buffer *binder_alloc_new_buf_locked(struct 
binder_alloc *alloc,
if (end_page_addr > has_page_addr)
end_page_addr = has_page_addr;
ret = binder_update_page_range(alloc, 1,
-   (void *)PAGE_ALIGN((uintptr_t)buffer->data), end_page_addr, NULL);
+   (void *)PAGE_ALIGN((uintptr_t)buffer->data), end_page_addr);
if (ret)
return ERR_PTR(ret);
 
@@ -478,7 +478,7 @@ struct binder_buffer *binder_alloc_new_buf_locked(struct 
binder_alloc *alloc,
 err_alloc_buf_struct_failed:
binder_update_page_range(alloc, 0,
 (void *)PAGE_ALIGN((uintptr_t)buffer->data),
-end_page_addr, NULL);
+end_page_addr);
return ERR_PTR(-ENOMEM);
 }
 
@@ -562,8 +562,7 @@ static void binder_delete_free_buffer(struct binder_alloc 
*alloc,
   alloc->pid, buffer->data,
   prev->data, next ? next->data : NULL);
binder_update_page_range(alloc, 0, buffer_start_page(buffer),
-buffer_start_page(buffer) + PAGE_SIZE,
-NULL);
+buffer_start_page(buffer) + PAGE_SIZE);
}
list_del(&buffer->entry);
kfree(buffer);
@@ -600,8 +599,7 @@ static void binder_free_buf_locked(struct binder_alloc 
*alloc,
 
binder_update_page_range(alloc, 0,
(void *)PAGE_ALIGN((uintptr_t)buffer->data),
-   (void *)(((uintptr_t)buffer->data + buffer_size) & PAGE_MASK),
-   NULL);
+   (void *)(((uintptr_t)buffer->data + buffer_size) & PAGE_MASK));
 
rb_erase(&buffer->rb_node, &alloc->allocated_buffers);
buffer->free = 1;
-- 
2.11.0 (Apple Git-81)

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 4/4] android: binder: Change binder_shrinker to static

[Sherry Yang]

binder_shrinker struct is not used anywhere outside of
binder_alloc.c and should be static.

Acked-by: Arve Hjønnevåg 
Signed-off-by: Sherry Yang 
---
 drivers/android/binder_alloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index 36bcea1fc47c..6f6f745605af 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -982,7 +982,7 @@ binder_shrink_scan(struct shrinker *shrink, struct 
shrink_control *sc)
return ret;
 }
 
-struct shrinker binder_shrinker = {
+static struct shrinker binder_shrinker = {
.count_objects = binder_shrink_count,
.scan_objects = binder_shrink_scan,
.seeks = DEFAULT_SEEKS,
-- 
2.11.0 (Apple Git-81)

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 1/4] android: binder: Don't get mm from task

[Sherry Yang]

Use binder_alloc struct's mm_struct rather than getting
a reference to the mm struct through get_task_mm to
avoid a potential deadlock between lru lock, task lock and
dentry lock, since a thread can be holding the task lock
and the dentry lock while trying to acquire the lru lock.

Acked-by: Arve Hjønnevåg 
Signed-off-by: Sherry Yang 
---
 drivers/android/binder_alloc.c | 22 +-
 drivers/android/binder_alloc.h |  1 -
 2 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index 064f5e31ec55..e12072b1d507 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -215,17 +215,12 @@ static int binder_update_page_range(struct binder_alloc 
*alloc, int allocate,
}
}
 
-   if (!vma && need_mm)
-   mm = get_task_mm(alloc->tsk);
+   if (!vma && need_mm && mmget_not_zero(alloc->vma_vm_mm))
+   mm = alloc->vma_vm_mm;
 
if (mm) {
down_write(&mm->mmap_sem);
vma = alloc->vma;
-   if (vma && mm != alloc->vma_vm_mm) {
-   pr_err("%d: vma mm and task mm mismatch\n",
-   alloc->pid);
-   vma = NULL;
-   }
}
 
if (!vma && need_mm) {
@@ -720,6 +715,7 @@ int binder_alloc_mmap_handler(struct binder_alloc *alloc,
barrier();
alloc->vma = vma;
alloc->vma_vm_mm = vma->vm_mm;
+   mmgrab(alloc->vma_vm_mm);
 
return 0;
 
@@ -795,6 +791,8 @@ void binder_alloc_deferred_release(struct binder_alloc 
*alloc)
vfree(alloc->buffer);
}
mutex_unlock(&alloc->mutex);
+   if (alloc->vma_vm_mm)
+   mmdrop(alloc->vma_vm_mm);
 
binder_alloc_debug(BINDER_DEBUG_OPEN_CLOSE,
 "%s: %d buffers %d, pages %d\n",
@@ -889,7 +887,6 @@ int binder_alloc_get_allocated_count(struct binder_alloc 
*alloc)
 void binder_alloc_vma_close(struct binder_alloc *alloc)
 {
WRITE_ONCE(alloc->vma, NULL);
-   WRITE_ONCE(alloc->vma_vm_mm, NULL);
 }
 
 /**
@@ -926,9 +923,9 @@ enum lru_status binder_alloc_free_page(struct list_head 
*item,
page_addr = (uintptr_t)alloc->buffer + index * PAGE_SIZE;
vma = alloc->vma;
if (vma) {
-   mm = get_task_mm(alloc->tsk);
-   if (!mm)
-   goto err_get_task_mm_failed;
+   if (!mmget_not_zero(alloc->vma_vm_mm))
+   goto err_mmget;
+   mm = alloc->vma_vm_mm;
if (!down_write_trylock(&mm->mmap_sem))
goto err_down_write_mmap_sem_failed;
}
@@ -963,7 +960,7 @@ enum lru_status binder_alloc_free_page(struct list_head 
*item,
 
 err_down_write_mmap_sem_failed:
mmput_async(mm);
-err_get_task_mm_failed:
+err_mmget:
 err_page_already_freed:
mutex_unlock(&alloc->mutex);
 err_get_alloc_mutex_failed:
@@ -1002,7 +999,6 @@ struct shrinker binder_shrinker = {
  */
 void binder_alloc_init(struct binder_alloc *alloc)
 {
-   alloc->tsk = current->group_leader;
alloc->pid = current->group_leader->pid;
mutex_init(&alloc->mutex);
INIT_LIST_HEAD(&alloc->buffers);
diff --git a/drivers/android/binder_alloc.h b/drivers/android/binder_alloc.h
index a3a3602c689c..2dd33b6df104 100644
--- a/drivers/android/binder_alloc.h
+++ b/drivers/android/binder_alloc.h
@@ -100,7 +100,6 @@ struct binder_lru_page {
  */
 struct binder_alloc {
struct mutex mutex;
-   struct task_struct *tsk;
struct vm_area_struct *vma;
struct mm_struct *vma_vm_mm;
void *buffer;
-- 
2.11.0 (Apple Git-81)

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 2/4] android: binder: Fix null ptr dereference in debug msg

[Sherry Yang]

Don't access next->data in kernel debug message when the
next buffer is null.

Acked-by: Arve Hjønnevåg 
Signed-off-by: Sherry Yang 
---
 drivers/android/binder_alloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index e12072b1d507..c2819a3d58a6 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -560,7 +560,7 @@ static void binder_delete_free_buffer(struct binder_alloc 
*alloc,
binder_alloc_debug(BINDER_DEBUG_BUFFER_ALLOC,
   "%d: merge free, buffer %pK do not share 
page with %pK or %pK\n",
   alloc->pid, buffer->data,
-  prev->data, next->data);
+  prev->data, next ? next->data : NULL);
binder_update_page_range(alloc, 0, buffer_start_page(buffer),
 buffer_start_page(buffer) + PAGE_SIZE,
 NULL);
-- 
2.11.0 (Apple Git-81)

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] android: binder: Check for errors in binder_alloc_shrinker_init().

[Sherry Yang]

Hi Tetsuo,

It looks like this patch was not submitted to LKML. Perhaps you want
to send it to the mailing list and add the correct set of recipients
using scripts/get_maintainer.pl as suggested here
https://www.kernel.org/doc/html/v4.12/process/submitting-patches.html.

-Sherry

On Wed, Nov 29, 2017 at 8:29 AM, Tetsuo Handa
 wrote:
> Both list_lru_init() and register_shrinker() might return an error.
>
> Signed-off-by: Tetsuo Handa 
> Cc: Sherry Yang 
> Cc: Greg Kroah-Hartman 
> Cc: Michal Hocko 
> ---
>  drivers/android/binder.c   |  4 +++-
>  drivers/android/binder_alloc.c | 12 +---
>  drivers/android/binder_alloc.h |  2 +-
>  3 files changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index 85b0bb4..a54a0f1 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -5569,7 +5569,9 @@ static int __init binder_init(void)
> struct binder_device *device;
> struct hlist_node *tmp;
>
> -   binder_alloc_shrinker_init();
> +   ret = binder_alloc_shrinker_init();
> +   if (ret)
> +   return ret;
>
> atomic_set(&binder_transaction_log.cur, ~0U);
> atomic_set(&binder_transaction_log_failed.cur, ~0U);
> diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
> index 0dba2308..fdf9d9f 100644
> --- a/drivers/android/binder_alloc.c
> +++ b/drivers/android/binder_alloc.c
> @@ -1006,8 +1006,14 @@ void binder_alloc_init(struct binder_alloc *alloc)
> INIT_LIST_HEAD(&alloc->buffers);
>  }
>
> -void binder_alloc_shrinker_init(void)
> +int binder_alloc_shrinker_init(void)
>  {
> -   list_lru_init(&binder_alloc_lru);
> -   register_shrinker(&binder_shrinker);
> +   int ret = list_lru_init(&binder_alloc_lru);
> +
> +   if (ret == 0) {
> +   ret = register_shrinker(&binder_shrinker);
> +   if (ret)
> +   list_lru_destroy(&binder_alloc_lru);
> +   }
> +   return ret;
>  }
> diff --git a/drivers/android/binder_alloc.h b/drivers/android/binder_alloc.h
> index 0b14530..9ef64e5 100644
> --- a/drivers/android/binder_alloc.h
> +++ b/drivers/android/binder_alloc.h
> @@ -130,7 +130,7 @@ extern struct binder_buffer *binder_alloc_new_buf(struct 
> binder_alloc *alloc,
>   size_t extra_buffers_size,
>   int is_async);
>  extern void binder_alloc_init(struct binder_alloc *alloc);
> -void binder_alloc_shrinker_init(void);
> +extern int binder_alloc_shrinker_init(void);
>  extern void binder_alloc_vma_close(struct binder_alloc *alloc);
>  extern struct binder_buffer *
>  binder_alloc_prepare_to_free(struct binder_alloc *alloc,
> --
> 1.8.3.1
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 2/2] android: binder: Rate-limit debug and userspace triggered err msgs

[Sherry Yang]

Use rate-limited debug messages where userspace can trigger
excessive log spams.

Acked-by: Arve Hjønnevåg 
Signed-off-by: Sherry Yang 
---
 drivers/android/binder.c   |  5 +++--
 drivers/android/binder_alloc.c | 41 +-
 2 files changed, 29 insertions(+), 17 deletions(-)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 95283f3bb51c..78cc1190282c 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -71,6 +71,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include "binder_alloc.h"
@@ -161,13 +162,13 @@ module_param_call(stop_on_user_error, 
binder_set_stop_on_user_error,
 #define binder_debug(mask, x...) \
do { \
if (binder_debug_mask & mask) \
-   pr_info(x); \
+   pr_info_ratelimited(x); \
} while (0)
 
 #define binder_user_error(x...) \
do { \
if (binder_debug_mask & BINDER_DEBUG_USER_ERROR) \
-   pr_info(x); \
+   pr_info_ratelimited(x); \
if (binder_stop_on_user_error) \
binder_stop_on_user_error = 2; \
} while (0)
diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index 2628806c64a2..e16116e9ad1f 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -28,6 +28,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "binder_alloc.h"
 #include "binder_trace.h"
 
@@ -36,11 +37,12 @@ struct list_lru binder_alloc_lru;
 static DEFINE_MUTEX(binder_alloc_mmap_lock);
 
 enum {
+   BINDER_DEBUG_USER_ERROR = 1U << 0,
BINDER_DEBUG_OPEN_CLOSE = 1U << 1,
BINDER_DEBUG_BUFFER_ALLOC   = 1U << 2,
BINDER_DEBUG_BUFFER_ALLOC_ASYNC = 1U << 3,
 };
-static uint32_t binder_alloc_debug_mask;
+static uint32_t binder_alloc_debug_mask = BINDER_DEBUG_USER_ERROR;
 
 module_param_named(debug_mask, binder_alloc_debug_mask,
   uint, 0644);
@@ -48,7 +50,7 @@ module_param_named(debug_mask, binder_alloc_debug_mask,
 #define binder_alloc_debug(mask, x...) \
do { \
if (binder_alloc_debug_mask & mask) \
-   pr_info(x); \
+   pr_info_ratelimited(x); \
} while (0)
 
 static struct binder_buffer *binder_buffer_next(struct binder_buffer *buffer)
@@ -152,8 +154,10 @@ static struct binder_buffer 
*binder_alloc_prepare_to_free_locked(
 * free the buffer twice
 */
if (buffer->free_in_progress) {
-   pr_err("%d:%d FREE_BUFFER u%016llx user freed 
buffer twice\n",
-  alloc->pid, current->pid, (u64)user_ptr);
+   binder_alloc_debug(BINDER_DEBUG_USER_ERROR,
+  "%d:%d FREE_BUFFER u%016llx 
user freed buffer twice\n",
+  alloc->pid, current->pid,
+  (u64)user_ptr);
return NULL;
}
buffer->free_in_progress = 1;
@@ -224,8 +228,9 @@ static int binder_update_page_range(struct binder_alloc 
*alloc, int allocate,
}
 
if (!vma && need_mm) {
-   pr_err("%d: binder_alloc_buf failed to map pages in userspace, 
no vma\n",
-   alloc->pid);
+   binder_alloc_debug(BINDER_DEBUG_USER_ERROR,
+  "%d: binder_alloc_buf failed to map pages in 
userspace, no vma\n",
+  alloc->pid);
goto err_no_vma;
}
 
@@ -344,8 +349,9 @@ static struct binder_buffer *binder_alloc_new_buf_locked(
int ret;
 
if (alloc->vma == NULL) {
-   pr_err("%d: binder_alloc_buf, no vma\n",
-  alloc->pid);
+   binder_alloc_debug(BINDER_DEBUG_USER_ERROR,
+  "%d: binder_alloc_buf, no vma\n",
+  alloc->pid);
return ERR_PTR(-ESRCH);
}
 
@@ -417,11 +423,14 @@ static struct binder_buffer *binder_alloc_new_buf_locked(
if (buffer_size > largest_free_size)
largest_free_size = buffer_size;
}
-   pr_err("%d: binder_alloc_buf size %zd failed, no address 
space\n",
-   alloc->pid, size);
-   pr_err("allocated: %zd (num: %zd largest: %zd), free: %zd (num: 
%zd largest: %zd)\n",
-  total_alloc_size, allocated_buffers, largest_alloc_size,
-

[PATCH 1/2] android: binder: Show extra_buffers_size in trace

[Sherry Yang]

Add extra_buffers_size to the binder_transaction_alloc_buf tracepoint.

Acked-by: Arve Hjønnevåg 
Signed-off-by: Sherry Yang 
---
 drivers/android/binder_trace.h | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/android/binder_trace.h b/drivers/android/binder_trace.h
index 76e3b9c8a8a2..588eb3ec3507 100644
--- a/drivers/android/binder_trace.h
+++ b/drivers/android/binder_trace.h
@@ -248,14 +248,17 @@ DECLARE_EVENT_CLASS(binder_buffer_class,
__field(int, debug_id)
__field(size_t, data_size)
__field(size_t, offsets_size)
+   __field(size_t, extra_buffers_size)
),
TP_fast_assign(
__entry->debug_id = buf->debug_id;
__entry->data_size = buf->data_size;
__entry->offsets_size = buf->offsets_size;
+   __entry->extra_buffers_size = buf->extra_buffers_size;
),
-   TP_printk("transaction=%d data_size=%zd offsets_size=%zd",
- __entry->debug_id, __entry->data_size, __entry->offsets_size)
+   TP_printk("transaction=%d data_size=%zd offsets_size=%zd 
extra_buffers_size=%zd",
+ __entry->debug_id, __entry->data_size, __entry->offsets_size,
+ __entry->extra_buffers_size)
 );
 
 DEFINE_EVENT(binder_buffer_class, binder_transaction_alloc_buf,
-- 
2.18.0.345.g5c9ce644c3-goog

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2] android: binder: Rate-limit debug and userspace triggered err msgs

[Sherry Yang]

Use rate-limited debug messages where userspace can trigger
excessive log spams.

Acked-by: Arve Hjønnevåg 
Signed-off-by: Sherry Yang 
---
v2: rebase onto char-misc-next to resolve include order
difference from master

 drivers/android/binder.c   |  5 +++--
 drivers/android/binder_alloc.c | 41 +-
 2 files changed, 29 insertions(+), 17 deletions(-)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 1cc2fa16af8b..d58763b6b009 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -70,6 +70,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
@@ -163,13 +164,13 @@ module_param_call(stop_on_user_error, 
binder_set_stop_on_user_error,
 #define binder_debug(mask, x...) \
do { \
if (binder_debug_mask & mask) \
-   pr_info(x); \
+   pr_info_ratelimited(x); \
} while (0)
 
 #define binder_user_error(x...) \
do { \
if (binder_debug_mask & BINDER_DEBUG_USER_ERROR) \
-   pr_info(x); \
+   pr_info_ratelimited(x); \
if (binder_stop_on_user_error) \
binder_stop_on_user_error = 2; \
} while (0)
diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index 2c258dcf9d72..3f3b7b253445 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -27,6 +27,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include "binder_alloc.h"
 #include "binder_trace.h"
@@ -36,11 +37,12 @@ struct list_lru binder_alloc_lru;
 static DEFINE_MUTEX(binder_alloc_mmap_lock);
 
 enum {
+   BINDER_DEBUG_USER_ERROR = 1U << 0,
BINDER_DEBUG_OPEN_CLOSE = 1U << 1,
BINDER_DEBUG_BUFFER_ALLOC   = 1U << 2,
BINDER_DEBUG_BUFFER_ALLOC_ASYNC = 1U << 3,
 };
-static uint32_t binder_alloc_debug_mask;
+static uint32_t binder_alloc_debug_mask = BINDER_DEBUG_USER_ERROR;
 
 module_param_named(debug_mask, binder_alloc_debug_mask,
   uint, 0644);
@@ -48,7 +50,7 @@ module_param_named(debug_mask, binder_alloc_debug_mask,
 #define binder_alloc_debug(mask, x...) \
do { \
if (binder_alloc_debug_mask & mask) \
-   pr_info(x); \
+   pr_info_ratelimited(x); \
} while (0)
 
 static struct binder_buffer *binder_buffer_next(struct binder_buffer *buffer)
@@ -152,8 +154,10 @@ static struct binder_buffer 
*binder_alloc_prepare_to_free_locked(
 * free the buffer twice
 */
if (buffer->free_in_progress) {
-   pr_err("%d:%d FREE_BUFFER u%016llx user freed 
buffer twice\n",
-  alloc->pid, current->pid, (u64)user_ptr);
+   binder_alloc_debug(BINDER_DEBUG_USER_ERROR,
+  "%d:%d FREE_BUFFER u%016llx 
user freed buffer twice\n",
+  alloc->pid, current->pid,
+  (u64)user_ptr);
return NULL;
}
buffer->free_in_progress = 1;
@@ -224,8 +228,9 @@ static int binder_update_page_range(struct binder_alloc 
*alloc, int allocate,
}
 
if (!vma && need_mm) {
-   pr_err("%d: binder_alloc_buf failed to map pages in userspace, 
no vma\n",
-   alloc->pid);
+   binder_alloc_debug(BINDER_DEBUG_USER_ERROR,
+  "%d: binder_alloc_buf failed to map pages in 
userspace, no vma\n",
+  alloc->pid);
goto err_no_vma;
}
 
@@ -344,8 +349,9 @@ static struct binder_buffer *binder_alloc_new_buf_locked(
int ret;
 
if (alloc->vma == NULL) {
-   pr_err("%d: binder_alloc_buf, no vma\n",
-  alloc->pid);
+   binder_alloc_debug(BINDER_DEBUG_USER_ERROR,
+  "%d: binder_alloc_buf, no vma\n",
+  alloc->pid);
return ERR_PTR(-ESRCH);
}
 
@@ -417,11 +423,14 @@ static struct binder_buffer *binder_alloc_new_buf_locked(
if (buffer_size > largest_free_size)
largest_free_size = buffer_size;
}
-   pr_err("%d: binder_alloc_buf size %zd failed, no address 
space\n",
-   alloc->pid, size);
-   pr_err("allocated: %zd (num: %zd largest: %zd), free: %zd (num: 
%zd largest: %zd)\n",
-  total_

[PATCH] android: binder: no outgoing transaction when thread todo has transaction

[Sherry Yang]

When a process dies, failed reply is sent to the sender of any transaction
queued on a dead thread's todo list. The sender asserts that the
received failed reply corresponds to the head of the transaction stack.
This assert can fail if the dead thread is allowed to send outgoing
transactions when there is already a transaction on its todo list,
because this new transaction can end up on the transaction stack of the
original sender. The following steps illustrate how this assertion can
fail.

1. Thread1 sends txn19 to Thread2
   (T1->transaction_stack=txn19, T2->todo+=txn19)
2. Without processing todo list, Thread2 sends txn20 to Thread1
   (T1->todo+=txn20, T2->transaction_stack=txn20)
3. T1 processes txn20 on its todo list
   (T1->transaction_stack=txn20->txn19, T1->todo=)
4. T2 dies, T2->todo cleanup attempts to send failed reply for txn19, but
   T1->transaction_stack points to txn20 -- assertion failes

Step 2. is the incorrect behavior. When there is a transaction on a
thread's todo list, this thread should not be able to send any outgoing
synchronous transactions. Only the head of the todo list needs to be
checked because only threads that are waiting for proc work can directly
receive work from another thread, and no work is allowed to be queued
on such a thread without waking up the thread. This patch also enforces
that a thread is not waiting for proc work when a work is directly
enqueued to its todo list.

Acked-by: Arve Hjønnevåg 
Signed-off-by: Sherry Yang 
---
 drivers/android/binder.c | 44 +---
 1 file changed, 32 insertions(+), 12 deletions(-)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index d58763b6b009..f2081934eb8b 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -822,6 +822,7 @@ static void
 binder_enqueue_deferred_thread_work_ilocked(struct binder_thread *thread,
struct binder_work *work)
 {
+   WARN_ON(!list_empty(&thread->waiting_thread_node));
binder_enqueue_work_ilocked(work, &thread->todo);
 }
 
@@ -839,6 +840,7 @@ static void
 binder_enqueue_thread_work_ilocked(struct binder_thread *thread,
   struct binder_work *work)
 {
+   WARN_ON(!list_empty(&thread->waiting_thread_node));
binder_enqueue_work_ilocked(work, &thread->todo);
thread->process_todo = true;
 }
@@ -1270,19 +1272,12 @@ static int binder_inc_node_nilocked(struct binder_node 
*node, int strong,
} else
node->local_strong_refs++;
if (!node->has_strong_ref && target_list) {
+   struct binder_thread *thread = container_of(target_list,
+   struct binder_thread, todo);
binder_dequeue_work_ilocked(&node->work);
-   /*
-* Note: this function is the only place where we queue
-* directly to a thread->todo without using the
-* corresponding binder_enqueue_thread_work() helper
-* functions; in this case it's ok to not set the
-* process_todo flag, since we know this node work will
-* always be followed by other work that starts queue
-* processing: in case of synchronous transactions, a
-* BR_REPLY or BR_ERROR; in case of oneway
-* transactions, a BR_TRANSACTION_COMPLETE.
-*/
-   binder_enqueue_work_ilocked(&node->work, target_list);
+   BUG_ON(&thread->todo != target_list);
+   binder_enqueue_deferred_thread_work_ilocked(thread,
+  &node->work);
}
} else {
if (!internal)
@@ -2723,6 +2718,7 @@ static void binder_transaction(struct binder_proc *proc,
 {
int ret;
struct binder_transaction *t;
+   struct binder_work *w;
struct binder_work *tcomplete;
binder_size_t *offp, *off_end, *off_start;
binder_size_t off_min;
@@ -2864,6 +2860,29 @@ static void binder_transaction(struct binder_proc *proc,
goto err_invalid_target_handle;
}
binder_inner_proc_lock(proc);
+
+   w = list_first_entry_or_null(&thread->todo,
+struct binder_work, entry);
+   if (!(tr->flags & TF_ONE_WAY) && w &&
+   w->type == BINDER_WORK_TRANSACTION) {
+   /*
+* Do not allow new outgoing transaction from a
+

[PATCH 2/5] android: binder: Add allocator selftest

[Sherry Yang]

binder_alloc_selftest tests that alloc_new_buf handles page allocation and
deallocation properly when allocate and free buffers. The test allocates 5
buffers of various sizes to cover all possible page alignment cases, and
frees the buffers using a list of exhaustive freeing order.

Test: boot the device with ANDROID_BINDER_IPC_SELFTEST config option
enabled. Allocator selftest passes.

Change-Id: I9064f60c85b1e0389c88e927e2b147ec92cae0d1
Signed-off-by: Sherry Yang 
---
 drivers/android/Kconfig |  10 ++
 drivers/android/Makefile|   1 +
 drivers/android/binder.c|   2 +
 drivers/android/binder_alloc.h  |   5 +
 drivers/android/binder_alloc_selftest.c | 271 
 5 files changed, 289 insertions(+)
 create mode 100644 drivers/android/binder_alloc_selftest.c

diff --git a/drivers/android/Kconfig b/drivers/android/Kconfig
index 832e885349b1..0f295704abd4 100644
--- a/drivers/android/Kconfig
+++ b/drivers/android/Kconfig
@@ -44,6 +44,16 @@ config ANDROID_BINDER_IPC_32BIT
 
  Note that enabling this will break newer Android user-space.
 
+config ANDROID_BINDER_IPC_SELFTEST
+   bool "Android Binder IPC Driver Selftest"
+   depends on ANDROID_BINDER_IPC
+   ---help---
+ This feature allows binder selftest to run.
+
+ Binder selftest checks the allocation and free of binder buffers
+ exhaustively with combinations of various buffer sizes and
+ alignments.
+
 endif # if ANDROID
 
 endmenu
diff --git a/drivers/android/Makefile b/drivers/android/Makefile
index 4b7c726bb560..a01254c43ee3 100644
--- a/drivers/android/Makefile
+++ b/drivers/android/Makefile
@@ -1,3 +1,4 @@
 ccflags-y += -I$(src)  # needed for trace events
 
 obj-$(CONFIG_ANDROID_BINDER_IPC)   += binder.o binder_alloc.o
+obj-$(CONFIG_ANDROID_BINDER_IPC_SELFTEST) += binder_alloc_selftest.o
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 9f95d7093f32..b31e64c6f666 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -4225,6 +4225,8 @@ static long binder_ioctl(struct file *filp, unsigned int 
cmd, unsigned long arg)
/*pr_info("binder_ioctl: %d:%d %x %lx\n",
proc->pid, current->pid, cmd, arg);*/
 
+   binder_selftest_alloc(&proc->alloc);
+
trace_binder_ioctl(cmd, arg);
 
ret = wait_event_interruptible(binder_user_error_wait, 
binder_stop_on_user_error < 2);
diff --git a/drivers/android/binder_alloc.h b/drivers/android/binder_alloc.h
index 088e4ffc6230..4f02cc084c15 100644
--- a/drivers/android/binder_alloc.h
+++ b/drivers/android/binder_alloc.h
@@ -102,6 +102,11 @@ struct binder_alloc {
int pid;
 };
 
+#ifdef CONFIG_ANDROID_BINDER_IPC_SELFTEST
+void binder_selftest_alloc(struct binder_alloc *alloc);
+#else
+static inline void binder_selftest_alloc(struct binder_alloc *alloc) {}
+#endif
 extern struct binder_buffer *binder_alloc_new_buf(struct binder_alloc *alloc,
  size_t data_size,
  size_t offsets_size,
diff --git a/drivers/android/binder_alloc_selftest.c 
b/drivers/android/binder_alloc_selftest.c
new file mode 100644
index ..cc00ab6ee29d
--- /dev/null
+++ b/drivers/android/binder_alloc_selftest.c
@@ -0,0 +1,271 @@
+/* binder_alloc_selftest.c
+ *
+ * Android IPC Subsystem
+ *
+ * Copyright (C) 2017 Google, Inc.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include 
+#include 
+#include "binder_alloc.h"
+
+#define BUFFER_NUM 5
+#define BUFFER_MIN_SIZE (PAGE_SIZE / 8)
+
+static bool binder_selftest_run = true;
+static int binder_selftest_failures;
+static DEFINE_MUTEX(binder_selftest_lock);
+
+/**
+ * enum buf_end_align_type - Page alignment of a buffer
+ * end with regard to the end of the previous buffer.
+ *
+ * In the pictures below, buf2 refers to the buffer we
+ * are aligning. buf1 refers to previous buffer by addr.
+ * Symbol [ means the start of a buffer, ] means the end
+ * of a buffer, and | means page boundaries.
+ */
+enum buf_end_align_type {
+   /**
+* @SAME_PAGE_UNALIGNED: The end of this buffer is on
+* the same page as the end of the previous buffer and
+* is not page aligned. Examples:
+* buf1 ][ buf2 ][ ...
+* buf1 ]|[ buf2 ][ ...
+*/
+   SAME_PAGE_UNALIGNED = 0,
+   /**
+* @SAME_PAGE_AL

[PATCH 4/5] android: binder: Add global lru shrinker to binder

[Sherry Yang]

Hold on to the pages allocated and mapped for transaction
buffers until the system is under memory pressure. When
that happens, use linux shrinker to free pages. Without
using shrinker, patch "android: binder: Move buffer out
of area shared with user space" will cause a significant
slow down for small transactions that fit into the first
page because free list buffer header used to be inlined
with buffer data.

In addition to prevent the performance regression for
small transactions, this patch improves the performance
for transactions that take up more than one page.

Modify alloc selftest to work with the shrinker change.

Test: Run memory intensive applications (Chrome and Camera)
to trigger shrinker callbacks. Binder frees memory as expected.
Test: Run binderThroughputTest with high memory pressure
option enabled.

Change-Id: I83409cc8b7d99684c99bc383880f3dfedaedca83
Signed-off-by: Sherry Yang 
---
 drivers/android/binder.c|   2 +
 drivers/android/binder_alloc.c  | 172 +++-
 drivers/android/binder_alloc.h  |  23 -
 drivers/android/binder_alloc_selftest.c |  68 ++---
 4 files changed, 225 insertions(+), 40 deletions(-)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index b31e64c6f666..fc5a4b9f3d97 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -5243,6 +5243,8 @@ static int __init binder_init(void)
struct binder_device *device;
struct hlist_node *tmp;
 
+   binder_alloc_shrinker_init();
+
atomic_set(&binder_transaction_log.cur, ~0U);
atomic_set(&binder_transaction_log_failed.cur, ~0U);
 
diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index e96659215f25..11a08bf72bcc 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -27,9 +27,12 @@
 #include 
 #include 
 #include 
+#include 
 #include "binder_alloc.h"
 #include "binder_trace.h"
 
+struct list_lru binder_alloc_lru;
+
 static DEFINE_MUTEX(binder_alloc_mmap_lock);
 
 enum {
@@ -188,8 +191,9 @@ static int binder_update_page_range(struct binder_alloc 
*alloc, int allocate,
 {
void *page_addr;
unsigned long user_page_addr;
-   struct page **page;
-   struct mm_struct *mm;
+   struct binder_lru_page *page;
+   struct mm_struct *mm = NULL;
+   bool need_mm = false;
 
binder_alloc_debug(BINDER_DEBUG_BUFFER_ALLOC,
 "%d: %s pages %pK-%pK\n", alloc->pid,
@@ -200,9 +204,18 @@ static int binder_update_page_range(struct binder_alloc 
*alloc, int allocate,
 
trace_binder_update_page_range(alloc, allocate, start, end);
 
-   if (vma)
-   mm = NULL;
-   else
+   if (allocate == 0)
+   goto free_range;
+
+   for (page_addr = start; page_addr < end; page_addr += PAGE_SIZE) {
+   page = &alloc->pages[(page_addr - alloc->buffer) / PAGE_SIZE];
+   if (!page->page_ptr) {
+   need_mm = true;
+   break;
+   }
+   }
+
+   if (!vma && need_mm)
mm = get_task_mm(alloc->tsk);
 
if (mm) {
@@ -215,10 +228,7 @@ static int binder_update_page_range(struct binder_alloc 
*alloc, int allocate,
}
}
 
-   if (allocate == 0)
-   goto free_range;
-
-   if (vma == NULL) {
+   if (!vma && need_mm) {
pr_err("%d: binder_alloc_buf failed to map pages in userspace, 
no vma\n",
alloc->pid);
goto err_no_vma;
@@ -226,18 +236,33 @@ static int binder_update_page_range(struct binder_alloc 
*alloc, int allocate,
 
for (page_addr = start; page_addr < end; page_addr += PAGE_SIZE) {
int ret;
+   bool on_lru;
 
page = &alloc->pages[(page_addr - alloc->buffer) / PAGE_SIZE];
 
-   BUG_ON(*page);
-   *page = alloc_page(GFP_KERNEL | __GFP_HIGHMEM | __GFP_ZERO);
-   if (*page == NULL) {
+   if (page->page_ptr) {
+   on_lru = list_lru_del(&binder_alloc_lru, &page->lru);
+   WARN_ON(!on_lru);
+   continue;
+   }
+
+   if (WARN_ON(!vma))
+   goto err_page_ptr_cleared;
+
+   page->page_ptr = alloc_page(GFP_KERNEL |
+   __GFP_HIGHMEM |
+   __GFP_ZERO);
+   if (!page->page_ptr) {
pr_err("%d: binder_alloc_buf failed for page at %pK\n",
alloc->pid, page_addr);
goto err_alloc_page_failed;
}
+   page->alloc = alloc;
+   INIT

[PATCH 5/5] android: binder: Add shrinker tracepoints

[Sherry Yang]

Add tracepoints in binder transaction allocator to
record lru hits and alloc/free page.

Change-Id: I7715f943c57d6172c35bdff8298d8c5aef24a51a
Signed-off-by: Sherry Yang 
---
 drivers/android/binder_alloc.c | 27 +++--
 drivers/android/binder_trace.h | 55 ++
 2 files changed, 80 insertions(+), 2 deletions(-)

diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index 11a08bf72bcc..78c42c0d62b9 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -237,18 +237,25 @@ static int binder_update_page_range(struct binder_alloc 
*alloc, int allocate,
for (page_addr = start; page_addr < end; page_addr += PAGE_SIZE) {
int ret;
bool on_lru;
+   size_t index;
 
-   page = &alloc->pages[(page_addr - alloc->buffer) / PAGE_SIZE];
+   index = (page_addr - alloc->buffer) / PAGE_SIZE;
+   page = &alloc->pages[index];
 
if (page->page_ptr) {
+   trace_binder_alloc_lru_start(alloc, index);
+
on_lru = list_lru_del(&binder_alloc_lru, &page->lru);
WARN_ON(!on_lru);
+
+   trace_binder_alloc_lru_end(alloc, index);
continue;
}
 
if (WARN_ON(!vma))
goto err_page_ptr_cleared;
 
+   trace_binder_alloc_page_start(alloc, index);
page->page_ptr = alloc_page(GFP_KERNEL |
__GFP_HIGHMEM |
__GFP_ZERO);
@@ -278,6 +285,8 @@ static int binder_update_page_range(struct binder_alloc 
*alloc, int allocate,
   alloc->pid, user_page_addr);
goto err_vm_insert_page_failed;
}
+
+   trace_binder_alloc_page_end(alloc, index);
/* vm_insert_page does not seem to increment the refcount */
}
if (mm) {
@@ -290,11 +299,17 @@ static int binder_update_page_range(struct binder_alloc 
*alloc, int allocate,
for (page_addr = end - PAGE_SIZE; page_addr >= start;
 page_addr -= PAGE_SIZE) {
bool ret;
+   size_t index;
 
-   page = &alloc->pages[(page_addr - alloc->buffer) / PAGE_SIZE];
+   index = (page_addr - alloc->buffer) / PAGE_SIZE;
+   page = &alloc->pages[index];
+
+   trace_binder_free_lru_start(alloc, index);
 
ret = list_lru_add(&binder_alloc_lru, &page->lru);
WARN_ON(!ret);
+
+   trace_binder_free_lru_end(alloc, index);
continue;
 
 err_vm_insert_page_failed:
@@ -888,18 +903,26 @@ enum lru_status binder_alloc_free_page(struct list_head 
*item,
if (!down_write_trylock(&mm->mmap_sem))
goto err_down_write_mmap_sem_failed;
 
+   trace_binder_unmap_user_start(alloc, index);
+
zap_page_range(alloc->vma,
   page_addr + alloc->user_buffer_offset,
   PAGE_SIZE);
 
+   trace_binder_unmap_user_end(alloc, index);
+
up_write(&mm->mmap_sem);
mmput(mm);
}
 
+   trace_binder_unmap_kernel_start(alloc, index);
+
unmap_kernel_range(page_addr, PAGE_SIZE);
__free_page(page->page_ptr);
page->page_ptr = NULL;
 
+   trace_binder_unmap_kernel_end(alloc, index);
+
list_lru_isolate(lru, item);
 
mutex_unlock(&alloc->mutex);
diff --git a/drivers/android/binder_trace.h b/drivers/android/binder_trace.h
index 7967db16ba5a..76e3b9c8a8a2 100644
--- a/drivers/android/binder_trace.h
+++ b/drivers/android/binder_trace.h
@@ -291,6 +291,61 @@ TRACE_EVENT(binder_update_page_range,
  __entry->offset, __entry->size)
 );
 
+DECLARE_EVENT_CLASS(binder_lru_page_class,
+   TP_PROTO(const struct binder_alloc *alloc, size_t page_index),
+   TP_ARGS(alloc, page_index),
+   TP_STRUCT__entry(
+   __field(int, proc)
+   __field(size_t, page_index)
+   ),
+   TP_fast_assign(
+   __entry->proc = alloc->pid;
+   __entry->page_index = page_index;
+   ),
+   TP_printk("proc=%d page_index=%zu",
+ __entry->proc, __entry->page_index)
+);
+
+DEFINE_EVENT(binder_lru_page_class, binder_alloc_lru_start,
+   TP_PROTO(const struct binder_alloc *alloc, size_t page_index),
+   TP_ARGS(alloc, page_index));
+
+DEFINE_EVENT(binder_lru_page_class, binder_alloc_lru_end,
+   TP_PROTO(const struct binder_alloc *alloc, size_t page_index),
+   TP_ARGS(alloc, page_index));
+
+DEFINE_EVENT(binder_lr

[PATCH 3/5] android: binder: Move buffer out of area shared with user space

[Sherry Yang]

Binder driver allocates buffer meta data in a region that is mapped
in user space. These meta data contain pointers in the kernel.

This patch allocates buffer meta data on the kernel heap that is
not mapped in user space, and uses a pointer to refer to the data mapped.

Change-Id: Ie19d2393a5015d9ee8bc26c41ce27e6eaa6e52fb
Signed-off-by: Sherry Yang 
---
 drivers/android/binder_alloc.c  | 144 +++-
 drivers/android/binder_alloc.h  |   2 +-
 drivers/android/binder_alloc_selftest.c |  11 ++-
 3 files changed, 90 insertions(+), 67 deletions(-)

diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index f15af2b55a62..e96659215f25 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -62,9 +62,9 @@ static size_t binder_alloc_buffer_size(struct binder_alloc 
*alloc,
   struct binder_buffer *buffer)
 {
if (list_is_last(&buffer->entry, &alloc->buffers))
-   return alloc->buffer +
-  alloc->buffer_size - (void *)buffer->data;
-   return (size_t)binder_buffer_next(buffer) - (size_t)buffer->data;
+   return (u8 *)alloc->buffer +
+   alloc->buffer_size - (u8 *)buffer->data;
+   return (u8 *)binder_buffer_next(buffer)->data - (u8 *)buffer->data;
 }
 
 static void binder_insert_free_buffer(struct binder_alloc *alloc,
@@ -114,9 +114,9 @@ static void binder_insert_allocated_buffer_locked(
buffer = rb_entry(parent, struct binder_buffer, rb_node);
BUG_ON(buffer->free);
 
-   if (new_buffer < buffer)
+   if (new_buffer->data < buffer->data)
p = &parent->rb_left;
-   else if (new_buffer > buffer)
+   else if (new_buffer->data > buffer->data)
p = &parent->rb_right;
else
BUG();
@@ -131,18 +131,17 @@ static struct binder_buffer 
*binder_alloc_prepare_to_free_locked(
 {
struct rb_node *n = alloc->allocated_buffers.rb_node;
struct binder_buffer *buffer;
-   struct binder_buffer *kern_ptr;
+   void *kern_ptr;
 
-   kern_ptr = (struct binder_buffer *)(user_ptr - alloc->user_buffer_offset
-   - offsetof(struct binder_buffer, data));
+   kern_ptr = (void *)(user_ptr - alloc->user_buffer_offset);
 
while (n) {
buffer = rb_entry(n, struct binder_buffer, rb_node);
BUG_ON(buffer->free);
 
-   if (kern_ptr < buffer)
+   if (kern_ptr < buffer->data)
n = n->rb_left;
-   else if (kern_ptr > buffer)
+   else if (kern_ptr > buffer->data)
n = n->rb_right;
else {
/*
@@ -330,6 +329,9 @@ struct binder_buffer *binder_alloc_new_buf_locked(struct 
binder_alloc *alloc,
return ERR_PTR(-ENOSPC);
}
 
+   /* Pad 0-size buffers so they get assigned unique addresses */
+   size = max(size, sizeof(void *));
+
while (n) {
buffer = rb_entry(n, struct binder_buffer, rb_node);
BUG_ON(!buffer->free);
@@ -389,14 +391,9 @@ struct binder_buffer *binder_alloc_new_buf_locked(struct 
binder_alloc *alloc,
 
has_page_addr =
(void *)(((uintptr_t)buffer->data + buffer_size) & PAGE_MASK);
-   if (n == NULL) {
-   if (size + sizeof(struct binder_buffer) + 4 >= buffer_size)
-   buffer_size = size; /* no room for other buffers */
-   else
-   buffer_size = size + sizeof(struct binder_buffer);
-   }
+   WARN_ON(n && buffer_size != size);
end_page_addr =
-   (void *)PAGE_ALIGN((uintptr_t)buffer->data + buffer_size);
+   (void *)PAGE_ALIGN((uintptr_t)buffer->data + size);
if (end_page_addr > has_page_addr)
end_page_addr = has_page_addr;
ret = binder_update_page_range(alloc, 1,
@@ -404,17 +401,25 @@ struct binder_buffer *binder_alloc_new_buf_locked(struct 
binder_alloc *alloc,
if (ret)
return ERR_PTR(ret);
 
-   rb_erase(best_fit, &alloc->free_buffers);
-   buffer->free = 0;
-   buffer->free_in_progress = 0;
-   binder_insert_allocated_buffer_locked(alloc, buffer);
if (buffer_size != size) {
-   struct binder_buffer *new_buffer = (void *)buffer->data + size;
+   struct binder_buffer *new_buffer;
 
+   new_buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
+   if (!new_buffer) {
+   pr_err("%s: %d failed to alloc new buffer struct\n",
+  __func__, a

[PATCH 1/5] android: binder: Refactor prev and next buffer into a helper function

[Sherry Yang]

Change-Id: Ie2a446ad9907f0d306fd1b8e6d79d87e48826ce2
Signed-off-by: Sherry Yang 
---
 drivers/android/binder_alloc.c | 24 +++-
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index 40f31df60580..f15af2b55a62 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -48,14 +48,23 @@ module_param_named(debug_mask, binder_alloc_debug_mask,
pr_info(x); \
} while (0)
 
+static struct binder_buffer *binder_buffer_next(struct binder_buffer *buffer)
+{
+   return list_entry(buffer->entry.next, struct binder_buffer, entry);
+}
+
+static struct binder_buffer *binder_buffer_prev(struct binder_buffer *buffer)
+{
+   return list_entry(buffer->entry.prev, struct binder_buffer, entry);
+}
+
 static size_t binder_alloc_buffer_size(struct binder_alloc *alloc,
   struct binder_buffer *buffer)
 {
if (list_is_last(&buffer->entry, &alloc->buffers))
return alloc->buffer +
   alloc->buffer_size - (void *)buffer->data;
-   return (size_t)list_entry(buffer->entry.next,
- struct binder_buffer, entry) - (size_t)buffer->data;
+   return (size_t)binder_buffer_next(buffer) - (size_t)buffer->data;
 }
 
 static void binder_insert_free_buffer(struct binder_alloc *alloc,
@@ -470,7 +479,7 @@ static void binder_delete_free_buffer(struct binder_alloc 
*alloc,
int free_page_start = 1;
 
BUG_ON(alloc->buffers.next == &buffer->entry);
-   prev = list_entry(buffer->entry.prev, struct binder_buffer, entry);
+   prev = binder_buffer_prev(buffer);
BUG_ON(!prev->free);
if (buffer_end_page(prev) == buffer_start_page(buffer)) {
free_page_start = 0;
@@ -482,8 +491,7 @@ static void binder_delete_free_buffer(struct binder_alloc 
*alloc,
}
 
if (!list_is_last(&buffer->entry, &alloc->buffers)) {
-   next = list_entry(buffer->entry.next,
- struct binder_buffer, entry);
+   next = binder_buffer_next(buffer);
if (buffer_start_page(next) == buffer_end_page(buffer)) {
free_page_end = 0;
if (buffer_start_page(next) ==
@@ -544,8 +552,7 @@ static void binder_free_buf_locked(struct binder_alloc 
*alloc,
rb_erase(&buffer->rb_node, &alloc->allocated_buffers);
buffer->free = 1;
if (!list_is_last(&buffer->entry, &alloc->buffers)) {
-   struct binder_buffer *next = list_entry(buffer->entry.next,
-   struct binder_buffer, entry);
+   struct binder_buffer *next = binder_buffer_next(buffer);
 
if (next->free) {
rb_erase(&next->rb_node, &alloc->free_buffers);
@@ -553,8 +560,7 @@ static void binder_free_buf_locked(struct binder_alloc 
*alloc,
}
}
if (alloc->buffers.next != &buffer->entry) {
-   struct binder_buffer *prev = list_entry(buffer->entry.prev,
-   struct binder_buffer, entry);
+   struct binder_buffer *prev = binder_buffer_prev(buffer);
 
if (prev->free) {
binder_delete_free_buffer(alloc, buffer);
-- 
2.14.1.480.gb18f417b89-goog

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 1/5] android: binder: Refactor prev and next buffer into a helper function

[Sherry Yang]

Use helper functions buffer_next and buffer_prev instead
of list_entry to get the next and previous buffers.

Signed-off-by: Sherry Yang 
---
 drivers/android/binder_alloc.c | 24 +++-
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index 40f31df60580..f15af2b55a62 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -48,14 +48,23 @@ module_param_named(debug_mask, binder_alloc_debug_mask,
pr_info(x); \
} while (0)
 
+static struct binder_buffer *binder_buffer_next(struct binder_buffer *buffer)
+{
+   return list_entry(buffer->entry.next, struct binder_buffer, entry);
+}
+
+static struct binder_buffer *binder_buffer_prev(struct binder_buffer *buffer)
+{
+   return list_entry(buffer->entry.prev, struct binder_buffer, entry);
+}
+
 static size_t binder_alloc_buffer_size(struct binder_alloc *alloc,
   struct binder_buffer *buffer)
 {
if (list_is_last(&buffer->entry, &alloc->buffers))
return alloc->buffer +
   alloc->buffer_size - (void *)buffer->data;
-   return (size_t)list_entry(buffer->entry.next,
- struct binder_buffer, entry) - (size_t)buffer->data;
+   return (size_t)binder_buffer_next(buffer) - (size_t)buffer->data;
 }
 
 static void binder_insert_free_buffer(struct binder_alloc *alloc,
@@ -470,7 +479,7 @@ static void binder_delete_free_buffer(struct binder_alloc 
*alloc,
int free_page_start = 1;
 
BUG_ON(alloc->buffers.next == &buffer->entry);
-   prev = list_entry(buffer->entry.prev, struct binder_buffer, entry);
+   prev = binder_buffer_prev(buffer);
BUG_ON(!prev->free);
if (buffer_end_page(prev) == buffer_start_page(buffer)) {
free_page_start = 0;
@@ -482,8 +491,7 @@ static void binder_delete_free_buffer(struct binder_alloc 
*alloc,
}
 
if (!list_is_last(&buffer->entry, &alloc->buffers)) {
-   next = list_entry(buffer->entry.next,
- struct binder_buffer, entry);
+   next = binder_buffer_next(buffer);
if (buffer_start_page(next) == buffer_end_page(buffer)) {
free_page_end = 0;
if (buffer_start_page(next) ==
@@ -544,8 +552,7 @@ static void binder_free_buf_locked(struct binder_alloc 
*alloc,
rb_erase(&buffer->rb_node, &alloc->allocated_buffers);
buffer->free = 1;
if (!list_is_last(&buffer->entry, &alloc->buffers)) {
-   struct binder_buffer *next = list_entry(buffer->entry.next,
-   struct binder_buffer, entry);
+   struct binder_buffer *next = binder_buffer_next(buffer);
 
if (next->free) {
rb_erase(&next->rb_node, &alloc->free_buffers);
@@ -553,8 +560,7 @@ static void binder_free_buf_locked(struct binder_alloc 
*alloc,
}
}
if (alloc->buffers.next != &buffer->entry) {
-   struct binder_buffer *prev = list_entry(buffer->entry.prev,
-   struct binder_buffer, entry);
+   struct binder_buffer *prev = binder_buffer_prev(buffer);
 
if (prev->free) {
binder_delete_free_buffer(alloc, buffer);
-- 
2.14.1.342.g6490525c54-goog

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 2/5] android: binder: Add allocator selftest

[Sherry Yang]

binder_alloc_selftest tests that alloc_new_buf handles page allocation and
deallocation properly when allocate and free buffers. The test allocates 5
buffers of various sizes to cover all possible page alignment cases, and
frees the buffers using a list of exhaustive freeing order.

Test: boot the device with ANDROID_BINDER_IPC_SELFTEST config option
enabled. Allocator selftest passes.

Signed-off-by: Sherry Yang 
---
 drivers/android/Kconfig |  10 ++
 drivers/android/Makefile|   1 +
 drivers/android/binder.c|   2 +
 drivers/android/binder_alloc.h  |   5 +
 drivers/android/binder_alloc_selftest.c | 271 
 5 files changed, 289 insertions(+)
 create mode 100644 drivers/android/binder_alloc_selftest.c

diff --git a/drivers/android/Kconfig b/drivers/android/Kconfig
index 832e885349b1..0f295704abd4 100644
--- a/drivers/android/Kconfig
+++ b/drivers/android/Kconfig
@@ -44,6 +44,16 @@ config ANDROID_BINDER_IPC_32BIT
 
  Note that enabling this will break newer Android user-space.
 
+config ANDROID_BINDER_IPC_SELFTEST
+   bool "Android Binder IPC Driver Selftest"
+   depends on ANDROID_BINDER_IPC
+   ---help---
+ This feature allows binder selftest to run.
+
+ Binder selftest checks the allocation and free of binder buffers
+ exhaustively with combinations of various buffer sizes and
+ alignments.
+
 endif # if ANDROID
 
 endmenu
diff --git a/drivers/android/Makefile b/drivers/android/Makefile
index 4b7c726bb560..a01254c43ee3 100644
--- a/drivers/android/Makefile
+++ b/drivers/android/Makefile
@@ -1,3 +1,4 @@
 ccflags-y += -I$(src)  # needed for trace events
 
 obj-$(CONFIG_ANDROID_BINDER_IPC)   += binder.o binder_alloc.o
+obj-$(CONFIG_ANDROID_BINDER_IPC_SELFTEST) += binder_alloc_selftest.o
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 9f95d7093f32..b31e64c6f666 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -4225,6 +4225,8 @@ static long binder_ioctl(struct file *filp, unsigned int 
cmd, unsigned long arg)
/*pr_info("binder_ioctl: %d:%d %x %lx\n",
proc->pid, current->pid, cmd, arg);*/
 
+   binder_selftest_alloc(&proc->alloc);
+
trace_binder_ioctl(cmd, arg);
 
ret = wait_event_interruptible(binder_user_error_wait, 
binder_stop_on_user_error < 2);
diff --git a/drivers/android/binder_alloc.h b/drivers/android/binder_alloc.h
index 088e4ffc6230..4f02cc084c15 100644
--- a/drivers/android/binder_alloc.h
+++ b/drivers/android/binder_alloc.h
@@ -102,6 +102,11 @@ struct binder_alloc {
int pid;
 };
 
+#ifdef CONFIG_ANDROID_BINDER_IPC_SELFTEST
+void binder_selftest_alloc(struct binder_alloc *alloc);
+#else
+static inline void binder_selftest_alloc(struct binder_alloc *alloc) {}
+#endif
 extern struct binder_buffer *binder_alloc_new_buf(struct binder_alloc *alloc,
  size_t data_size,
  size_t offsets_size,
diff --git a/drivers/android/binder_alloc_selftest.c 
b/drivers/android/binder_alloc_selftest.c
new file mode 100644
index ..cc00ab6ee29d
--- /dev/null
+++ b/drivers/android/binder_alloc_selftest.c
@@ -0,0 +1,271 @@
+/* binder_alloc_selftest.c
+ *
+ * Android IPC Subsystem
+ *
+ * Copyright (C) 2017 Google, Inc.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include 
+#include 
+#include "binder_alloc.h"
+
+#define BUFFER_NUM 5
+#define BUFFER_MIN_SIZE (PAGE_SIZE / 8)
+
+static bool binder_selftest_run = true;
+static int binder_selftest_failures;
+static DEFINE_MUTEX(binder_selftest_lock);
+
+/**
+ * enum buf_end_align_type - Page alignment of a buffer
+ * end with regard to the end of the previous buffer.
+ *
+ * In the pictures below, buf2 refers to the buffer we
+ * are aligning. buf1 refers to previous buffer by addr.
+ * Symbol [ means the start of a buffer, ] means the end
+ * of a buffer, and | means page boundaries.
+ */
+enum buf_end_align_type {
+   /**
+* @SAME_PAGE_UNALIGNED: The end of this buffer is on
+* the same page as the end of the previous buffer and
+* is not page aligned. Examples:
+* buf1 ][ buf2 ][ ...
+* buf1 ]|[ buf2 ][ ...
+*/
+   SAME_PAGE_UNALIGNED = 0,
+   /**
+* @SAME_PAGE_ALIGNED: When the end of the previous buffer
+   

[PATCH v2 3/5] android: binder: Move buffer out of area shared with user space

[Sherry Yang]

Binder driver allocates buffer meta data in a region that is mapped
in user space. These meta data contain pointers in the kernel.

This patch allocates buffer meta data on the kernel heap that is
not mapped in user space, and uses a pointer to refer to the data mapped.

Signed-off-by: Sherry Yang 
---
 drivers/android/binder_alloc.c  | 144 +++-
 drivers/android/binder_alloc.h  |   2 +-
 drivers/android/binder_alloc_selftest.c |  11 ++-
 3 files changed, 90 insertions(+), 67 deletions(-)

diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index f15af2b55a62..e96659215f25 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -62,9 +62,9 @@ static size_t binder_alloc_buffer_size(struct binder_alloc 
*alloc,
   struct binder_buffer *buffer)
 {
if (list_is_last(&buffer->entry, &alloc->buffers))
-   return alloc->buffer +
-  alloc->buffer_size - (void *)buffer->data;
-   return (size_t)binder_buffer_next(buffer) - (size_t)buffer->data;
+   return (u8 *)alloc->buffer +
+   alloc->buffer_size - (u8 *)buffer->data;
+   return (u8 *)binder_buffer_next(buffer)->data - (u8 *)buffer->data;
 }
 
 static void binder_insert_free_buffer(struct binder_alloc *alloc,
@@ -114,9 +114,9 @@ static void binder_insert_allocated_buffer_locked(
buffer = rb_entry(parent, struct binder_buffer, rb_node);
BUG_ON(buffer->free);
 
-   if (new_buffer < buffer)
+   if (new_buffer->data < buffer->data)
p = &parent->rb_left;
-   else if (new_buffer > buffer)
+   else if (new_buffer->data > buffer->data)
p = &parent->rb_right;
else
BUG();
@@ -131,18 +131,17 @@ static struct binder_buffer 
*binder_alloc_prepare_to_free_locked(
 {
struct rb_node *n = alloc->allocated_buffers.rb_node;
struct binder_buffer *buffer;
-   struct binder_buffer *kern_ptr;
+   void *kern_ptr;
 
-   kern_ptr = (struct binder_buffer *)(user_ptr - alloc->user_buffer_offset
-   - offsetof(struct binder_buffer, data));
+   kern_ptr = (void *)(user_ptr - alloc->user_buffer_offset);
 
while (n) {
buffer = rb_entry(n, struct binder_buffer, rb_node);
BUG_ON(buffer->free);
 
-   if (kern_ptr < buffer)
+   if (kern_ptr < buffer->data)
n = n->rb_left;
-   else if (kern_ptr > buffer)
+   else if (kern_ptr > buffer->data)
n = n->rb_right;
else {
/*
@@ -330,6 +329,9 @@ struct binder_buffer *binder_alloc_new_buf_locked(struct 
binder_alloc *alloc,
return ERR_PTR(-ENOSPC);
}
 
+   /* Pad 0-size buffers so they get assigned unique addresses */
+   size = max(size, sizeof(void *));
+
while (n) {
buffer = rb_entry(n, struct binder_buffer, rb_node);
BUG_ON(!buffer->free);
@@ -389,14 +391,9 @@ struct binder_buffer *binder_alloc_new_buf_locked(struct 
binder_alloc *alloc,
 
has_page_addr =
(void *)(((uintptr_t)buffer->data + buffer_size) & PAGE_MASK);
-   if (n == NULL) {
-   if (size + sizeof(struct binder_buffer) + 4 >= buffer_size)
-   buffer_size = size; /* no room for other buffers */
-   else
-   buffer_size = size + sizeof(struct binder_buffer);
-   }
+   WARN_ON(n && buffer_size != size);
end_page_addr =
-   (void *)PAGE_ALIGN((uintptr_t)buffer->data + buffer_size);
+   (void *)PAGE_ALIGN((uintptr_t)buffer->data + size);
if (end_page_addr > has_page_addr)
end_page_addr = has_page_addr;
ret = binder_update_page_range(alloc, 1,
@@ -404,17 +401,25 @@ struct binder_buffer *binder_alloc_new_buf_locked(struct 
binder_alloc *alloc,
if (ret)
return ERR_PTR(ret);
 
-   rb_erase(best_fit, &alloc->free_buffers);
-   buffer->free = 0;
-   buffer->free_in_progress = 0;
-   binder_insert_allocated_buffer_locked(alloc, buffer);
if (buffer_size != size) {
-   struct binder_buffer *new_buffer = (void *)buffer->data + size;
+   struct binder_buffer *new_buffer;
 
+   new_buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
+   if (!new_buffer) {
+   pr_err("%s: %d failed to alloc new buffer struct\n",
+  __func__, alloc->pid);
+   

[PATCH v2 5/5] android: binder: Add shrinker tracepoints

[Sherry Yang]

Add tracepoints in binder transaction allocator to
record lru hits and alloc/free page.

Signed-off-by: Sherry Yang 
---
 drivers/android/binder_alloc.c | 27 +++--
 drivers/android/binder_trace.h | 55 ++
 2 files changed, 80 insertions(+), 2 deletions(-)

diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index 11a08bf72bcc..78c42c0d62b9 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -237,18 +237,25 @@ static int binder_update_page_range(struct binder_alloc 
*alloc, int allocate,
for (page_addr = start; page_addr < end; page_addr += PAGE_SIZE) {
int ret;
bool on_lru;
+   size_t index;
 
-   page = &alloc->pages[(page_addr - alloc->buffer) / PAGE_SIZE];
+   index = (page_addr - alloc->buffer) / PAGE_SIZE;
+   page = &alloc->pages[index];
 
if (page->page_ptr) {
+   trace_binder_alloc_lru_start(alloc, index);
+
on_lru = list_lru_del(&binder_alloc_lru, &page->lru);
WARN_ON(!on_lru);
+
+   trace_binder_alloc_lru_end(alloc, index);
continue;
}
 
if (WARN_ON(!vma))
goto err_page_ptr_cleared;
 
+   trace_binder_alloc_page_start(alloc, index);
page->page_ptr = alloc_page(GFP_KERNEL |
__GFP_HIGHMEM |
__GFP_ZERO);
@@ -278,6 +285,8 @@ static int binder_update_page_range(struct binder_alloc 
*alloc, int allocate,
   alloc->pid, user_page_addr);
goto err_vm_insert_page_failed;
}
+
+   trace_binder_alloc_page_end(alloc, index);
/* vm_insert_page does not seem to increment the refcount */
}
if (mm) {
@@ -290,11 +299,17 @@ static int binder_update_page_range(struct binder_alloc 
*alloc, int allocate,
for (page_addr = end - PAGE_SIZE; page_addr >= start;
 page_addr -= PAGE_SIZE) {
bool ret;
+   size_t index;
 
-   page = &alloc->pages[(page_addr - alloc->buffer) / PAGE_SIZE];
+   index = (page_addr - alloc->buffer) / PAGE_SIZE;
+   page = &alloc->pages[index];
+
+   trace_binder_free_lru_start(alloc, index);
 
ret = list_lru_add(&binder_alloc_lru, &page->lru);
WARN_ON(!ret);
+
+   trace_binder_free_lru_end(alloc, index);
continue;
 
 err_vm_insert_page_failed:
@@ -888,18 +903,26 @@ enum lru_status binder_alloc_free_page(struct list_head 
*item,
if (!down_write_trylock(&mm->mmap_sem))
goto err_down_write_mmap_sem_failed;
 
+   trace_binder_unmap_user_start(alloc, index);
+
zap_page_range(alloc->vma,
   page_addr + alloc->user_buffer_offset,
   PAGE_SIZE);
 
+   trace_binder_unmap_user_end(alloc, index);
+
up_write(&mm->mmap_sem);
mmput(mm);
}
 
+   trace_binder_unmap_kernel_start(alloc, index);
+
unmap_kernel_range(page_addr, PAGE_SIZE);
__free_page(page->page_ptr);
page->page_ptr = NULL;
 
+   trace_binder_unmap_kernel_end(alloc, index);
+
list_lru_isolate(lru, item);
 
mutex_unlock(&alloc->mutex);
diff --git a/drivers/android/binder_trace.h b/drivers/android/binder_trace.h
index 7967db16ba5a..76e3b9c8a8a2 100644
--- a/drivers/android/binder_trace.h
+++ b/drivers/android/binder_trace.h
@@ -291,6 +291,61 @@ TRACE_EVENT(binder_update_page_range,
  __entry->offset, __entry->size)
 );
 
+DECLARE_EVENT_CLASS(binder_lru_page_class,
+   TP_PROTO(const struct binder_alloc *alloc, size_t page_index),
+   TP_ARGS(alloc, page_index),
+   TP_STRUCT__entry(
+   __field(int, proc)
+   __field(size_t, page_index)
+   ),
+   TP_fast_assign(
+   __entry->proc = alloc->pid;
+   __entry->page_index = page_index;
+   ),
+   TP_printk("proc=%d page_index=%zu",
+ __entry->proc, __entry->page_index)
+);
+
+DEFINE_EVENT(binder_lru_page_class, binder_alloc_lru_start,
+   TP_PROTO(const struct binder_alloc *alloc, size_t page_index),
+   TP_ARGS(alloc, page_index));
+
+DEFINE_EVENT(binder_lru_page_class, binder_alloc_lru_end,
+   TP_PROTO(const struct binder_alloc *alloc, size_t page_index),
+   TP_ARGS(alloc, page_index));
+
+DEFINE_EVENT(binder_lru_page_class, binder_free_lru_start,
+   

[PATCH v2 4/5] android: binder: Add global lru shrinker to binder

[Sherry Yang]

Hold on to the pages allocated and mapped for transaction
buffers until the system is under memory pressure. When
that happens, use linux shrinker to free pages. Without
using shrinker, patch "android: binder: Move buffer out
of area shared with user space" will cause a significant
slow down for small transactions that fit into the first
page because free list buffer header used to be inlined
with buffer data.

In addition to prevent the performance regression for
small transactions, this patch improves the performance
for transactions that take up more than one page.

Modify alloc selftest to work with the shrinker change.

Test: Run memory intensive applications (Chrome and Camera)
to trigger shrinker callbacks. Binder frees memory as expected.
Test: Run binderThroughputTest with high memory pressure
option enabled.

Signed-off-by: Sherry Yang 
---
 drivers/android/binder.c|   2 +
 drivers/android/binder_alloc.c  | 172 +++-
 drivers/android/binder_alloc.h  |  23 -
 drivers/android/binder_alloc_selftest.c |  68 ++---
 4 files changed, 225 insertions(+), 40 deletions(-)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index b31e64c6f666..fc5a4b9f3d97 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -5243,6 +5243,8 @@ static int __init binder_init(void)
struct binder_device *device;
struct hlist_node *tmp;
 
+   binder_alloc_shrinker_init();
+
atomic_set(&binder_transaction_log.cur, ~0U);
atomic_set(&binder_transaction_log_failed.cur, ~0U);
 
diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index e96659215f25..11a08bf72bcc 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -27,9 +27,12 @@
 #include 
 #include 
 #include 
+#include 
 #include "binder_alloc.h"
 #include "binder_trace.h"
 
+struct list_lru binder_alloc_lru;
+
 static DEFINE_MUTEX(binder_alloc_mmap_lock);
 
 enum {
@@ -188,8 +191,9 @@ static int binder_update_page_range(struct binder_alloc 
*alloc, int allocate,
 {
void *page_addr;
unsigned long user_page_addr;
-   struct page **page;
-   struct mm_struct *mm;
+   struct binder_lru_page *page;
+   struct mm_struct *mm = NULL;
+   bool need_mm = false;
 
binder_alloc_debug(BINDER_DEBUG_BUFFER_ALLOC,
 "%d: %s pages %pK-%pK\n", alloc->pid,
@@ -200,9 +204,18 @@ static int binder_update_page_range(struct binder_alloc 
*alloc, int allocate,
 
trace_binder_update_page_range(alloc, allocate, start, end);
 
-   if (vma)
-   mm = NULL;
-   else
+   if (allocate == 0)
+   goto free_range;
+
+   for (page_addr = start; page_addr < end; page_addr += PAGE_SIZE) {
+   page = &alloc->pages[(page_addr - alloc->buffer) / PAGE_SIZE];
+   if (!page->page_ptr) {
+   need_mm = true;
+   break;
+   }
+   }
+
+   if (!vma && need_mm)
mm = get_task_mm(alloc->tsk);
 
if (mm) {
@@ -215,10 +228,7 @@ static int binder_update_page_range(struct binder_alloc 
*alloc, int allocate,
}
}
 
-   if (allocate == 0)
-   goto free_range;
-
-   if (vma == NULL) {
+   if (!vma && need_mm) {
pr_err("%d: binder_alloc_buf failed to map pages in userspace, 
no vma\n",
alloc->pid);
goto err_no_vma;
@@ -226,18 +236,33 @@ static int binder_update_page_range(struct binder_alloc 
*alloc, int allocate,
 
for (page_addr = start; page_addr < end; page_addr += PAGE_SIZE) {
int ret;
+   bool on_lru;
 
page = &alloc->pages[(page_addr - alloc->buffer) / PAGE_SIZE];
 
-   BUG_ON(*page);
-   *page = alloc_page(GFP_KERNEL | __GFP_HIGHMEM | __GFP_ZERO);
-   if (*page == NULL) {
+   if (page->page_ptr) {
+   on_lru = list_lru_del(&binder_alloc_lru, &page->lru);
+   WARN_ON(!on_lru);
+   continue;
+   }
+
+   if (WARN_ON(!vma))
+   goto err_page_ptr_cleared;
+
+   page->page_ptr = alloc_page(GFP_KERNEL |
+   __GFP_HIGHMEM |
+   __GFP_ZERO);
+   if (!page->page_ptr) {
pr_err("%d: binder_alloc_buf failed for page at %pK\n",
alloc->pid, page_addr);
goto err_alloc_page_failed;
}
+   page->alloc = alloc;
+   INIT_LIST_HEAD(&pag

[PATCH v2 0/5] android: binder: move allocator metadata and add shrinker

[Sherry Yang]

This patch set moves internal kernel data in the binder driver
out of mmap regions that is readable by user space. A shrinker
is added to the driver to dynamically manage the memory used
by binder transactions and only free pages when the system is
under memory pressure. This patch set also adds tests and
refactoring in binder allocator.

  android: binder: Refactor prev and next buffer into a helper function
  android: binder: Add allocator selftest
  android: binder: Move buffer out of area shared with user space
  android: binder: Add global lru shrinker to binder
  android: binder: Add shrinker tracepoints

 drivers/android/Kconfig |  10 +
 drivers/android/Makefile|   1 +
 drivers/android/binder.c|   4 +
 drivers/android/binder_alloc.c  | 365 --
 drivers/android/binder_alloc.h  |  30 +-
 drivers/android/binder_alloc_selftest.c | 310 ++
 drivers/android/binder_trace.h  |  55 
 7 files changed, 678 insertions(+), 97 deletions(-)
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 2/6] android: binder: Add allocator selftest

[Sherry Yang]

binder_alloc_selftest tests that alloc_new_buf handles page allocation and
deallocation properly when allocate and free buffers. The test allocates 5
buffers of various sizes to cover all possible page alignment cases, and
frees the buffers using a list of exhaustive freeing order.

Test: boot the device with ANDROID_BINDER_IPC_SELFTEST config option
enabled. Allocator selftest passes.

Signed-off-by: Sherry Yang 
---
 drivers/android/Kconfig |  10 ++
 drivers/android/Makefile|   1 +
 drivers/android/binder.c|   2 +
 drivers/android/binder_alloc.h  |   5 +
 drivers/android/binder_alloc_selftest.c | 271 
 5 files changed, 289 insertions(+)
 create mode 100644 drivers/android/binder_alloc_selftest.c

diff --git a/drivers/android/Kconfig b/drivers/android/Kconfig
index 832e885349b1..0f295704abd4 100644
--- a/drivers/android/Kconfig
+++ b/drivers/android/Kconfig
@@ -44,6 +44,16 @@ config ANDROID_BINDER_IPC_32BIT
 
  Note that enabling this will break newer Android user-space.
 
+config ANDROID_BINDER_IPC_SELFTEST
+   bool "Android Binder IPC Driver Selftest"
+   depends on ANDROID_BINDER_IPC
+   ---help---
+ This feature allows binder selftest to run.
+
+ Binder selftest checks the allocation and free of binder buffers
+ exhaustively with combinations of various buffer sizes and
+ alignments.
+
 endif # if ANDROID
 
 endmenu
diff --git a/drivers/android/Makefile b/drivers/android/Makefile
index 4b7c726bb560..a01254c43ee3 100644
--- a/drivers/android/Makefile
+++ b/drivers/android/Makefile
@@ -1,3 +1,4 @@
 ccflags-y += -I$(src)  # needed for trace events
 
 obj-$(CONFIG_ANDROID_BINDER_IPC)   += binder.o binder_alloc.o
+obj-$(CONFIG_ANDROID_BINDER_IPC_SELFTEST) += binder_alloc_selftest.o
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 9f95d7093f32..b31e64c6f666 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -4225,6 +4225,8 @@ static long binder_ioctl(struct file *filp, unsigned int 
cmd, unsigned long arg)
/*pr_info("binder_ioctl: %d:%d %x %lx\n",
proc->pid, current->pid, cmd, arg);*/
 
+   binder_selftest_alloc(&proc->alloc);
+
trace_binder_ioctl(cmd, arg);
 
ret = wait_event_interruptible(binder_user_error_wait, 
binder_stop_on_user_error < 2);
diff --git a/drivers/android/binder_alloc.h b/drivers/android/binder_alloc.h
index 088e4ffc6230..4f02cc084c15 100644
--- a/drivers/android/binder_alloc.h
+++ b/drivers/android/binder_alloc.h
@@ -102,6 +102,11 @@ struct binder_alloc {
int pid;
 };
 
+#ifdef CONFIG_ANDROID_BINDER_IPC_SELFTEST
+void binder_selftest_alloc(struct binder_alloc *alloc);
+#else
+static inline void binder_selftest_alloc(struct binder_alloc *alloc) {}
+#endif
 extern struct binder_buffer *binder_alloc_new_buf(struct binder_alloc *alloc,
  size_t data_size,
  size_t offsets_size,
diff --git a/drivers/android/binder_alloc_selftest.c 
b/drivers/android/binder_alloc_selftest.c
new file mode 100644
index ..cc00ab6ee29d
--- /dev/null
+++ b/drivers/android/binder_alloc_selftest.c
@@ -0,0 +1,271 @@
+/* binder_alloc_selftest.c
+ *
+ * Android IPC Subsystem
+ *
+ * Copyright (C) 2017 Google, Inc.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include 
+#include 
+#include "binder_alloc.h"
+
+#define BUFFER_NUM 5
+#define BUFFER_MIN_SIZE (PAGE_SIZE / 8)
+
+static bool binder_selftest_run = true;
+static int binder_selftest_failures;
+static DEFINE_MUTEX(binder_selftest_lock);
+
+/**
+ * enum buf_end_align_type - Page alignment of a buffer
+ * end with regard to the end of the previous buffer.
+ *
+ * In the pictures below, buf2 refers to the buffer we
+ * are aligning. buf1 refers to previous buffer by addr.
+ * Symbol [ means the start of a buffer, ] means the end
+ * of a buffer, and | means page boundaries.
+ */
+enum buf_end_align_type {
+   /**
+* @SAME_PAGE_UNALIGNED: The end of this buffer is on
+* the same page as the end of the previous buffer and
+* is not page aligned. Examples:
+* buf1 ][ buf2 ][ ...
+* buf1 ]|[ buf2 ][ ...
+*/
+   SAME_PAGE_UNALIGNED = 0,
+   /**
+* @SAME_PAGE_ALIGNED: When the end of the previous buffer
+   

[PATCH v3 5/6] android: binder: Add shrinker tracepoints

[Sherry Yang]

Add tracepoints in binder transaction allocator to
record lru hits and alloc/free page.

Signed-off-by: Sherry Yang 
---
 drivers/android/binder_alloc.c | 27 +++--
 drivers/android/binder_trace.h | 55 ++
 2 files changed, 80 insertions(+), 2 deletions(-)

diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index 8eb7e9e9a21f..2624a502fcde 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -237,18 +237,25 @@ static int binder_update_page_range(struct binder_alloc 
*alloc, int allocate,
for (page_addr = start; page_addr < end; page_addr += PAGE_SIZE) {
int ret;
bool on_lru;
+   size_t index;
 
-   page = &alloc->pages[(page_addr - alloc->buffer) / PAGE_SIZE];
+   index = (page_addr - alloc->buffer) / PAGE_SIZE;
+   page = &alloc->pages[index];
 
if (page->page_ptr) {
+   trace_binder_alloc_lru_start(alloc, index);
+
on_lru = list_lru_del(&binder_alloc_lru, &page->lru);
WARN_ON(!on_lru);
+
+   trace_binder_alloc_lru_end(alloc, index);
continue;
}
 
if (WARN_ON(!vma))
goto err_page_ptr_cleared;
 
+   trace_binder_alloc_page_start(alloc, index);
page->page_ptr = alloc_page(GFP_KERNEL |
__GFP_HIGHMEM |
__GFP_ZERO);
@@ -278,6 +285,8 @@ static int binder_update_page_range(struct binder_alloc 
*alloc, int allocate,
   alloc->pid, user_page_addr);
goto err_vm_insert_page_failed;
}
+
+   trace_binder_alloc_page_end(alloc, index);
/* vm_insert_page does not seem to increment the refcount */
}
if (mm) {
@@ -290,11 +299,17 @@ static int binder_update_page_range(struct binder_alloc 
*alloc, int allocate,
for (page_addr = end - PAGE_SIZE; page_addr >= start;
 page_addr -= PAGE_SIZE) {
bool ret;
+   size_t index;
 
-   page = &alloc->pages[(page_addr - alloc->buffer) / PAGE_SIZE];
+   index = (page_addr - alloc->buffer) / PAGE_SIZE;
+   page = &alloc->pages[index];
+
+   trace_binder_free_lru_start(alloc, index);
 
ret = list_lru_add(&binder_alloc_lru, &page->lru);
WARN_ON(!ret);
+
+   trace_binder_free_lru_end(alloc, index);
continue;
 
 err_vm_insert_page_failed:
@@ -887,18 +902,26 @@ enum lru_status binder_alloc_free_page(struct list_head 
*item,
if (!down_write_trylock(&mm->mmap_sem))
goto err_down_write_mmap_sem_failed;
 
+   trace_binder_unmap_user_start(alloc, index);
+
zap_page_range(alloc->vma,
   page_addr + alloc->user_buffer_offset,
   PAGE_SIZE);
 
+   trace_binder_unmap_user_end(alloc, index);
+
up_write(&mm->mmap_sem);
mmput(mm);
}
 
+   trace_binder_unmap_kernel_start(alloc, index);
+
unmap_kernel_range(page_addr, PAGE_SIZE);
__free_page(page->page_ptr);
page->page_ptr = NULL;
 
+   trace_binder_unmap_kernel_end(alloc, index);
+
list_lru_isolate(lru, item);
 
mutex_unlock(&alloc->mutex);
diff --git a/drivers/android/binder_trace.h b/drivers/android/binder_trace.h
index 7967db16ba5a..76e3b9c8a8a2 100644
--- a/drivers/android/binder_trace.h
+++ b/drivers/android/binder_trace.h
@@ -291,6 +291,61 @@ TRACE_EVENT(binder_update_page_range,
  __entry->offset, __entry->size)
 );
 
+DECLARE_EVENT_CLASS(binder_lru_page_class,
+   TP_PROTO(const struct binder_alloc *alloc, size_t page_index),
+   TP_ARGS(alloc, page_index),
+   TP_STRUCT__entry(
+   __field(int, proc)
+   __field(size_t, page_index)
+   ),
+   TP_fast_assign(
+   __entry->proc = alloc->pid;
+   __entry->page_index = page_index;
+   ),
+   TP_printk("proc=%d page_index=%zu",
+ __entry->proc, __entry->page_index)
+);
+
+DEFINE_EVENT(binder_lru_page_class, binder_alloc_lru_start,
+   TP_PROTO(const struct binder_alloc *alloc, size_t page_index),
+   TP_ARGS(alloc, page_index));
+
+DEFINE_EVENT(binder_lru_page_class, binder_alloc_lru_end,
+   TP_PROTO(const struct binder_alloc *alloc, size_t page_index),
+   TP_ARGS(alloc, page_index));
+
+DEFINE_EVENT(binder_lru_page_class, binder_free_lru_start,
+   

[PATCH v3 1/6] android: binder: Refactor prev and next buffer into a helper function

[Sherry Yang]

Use helper functions buffer_next and buffer_prev instead
of list_entry to get the next and previous buffers.

Signed-off-by: Sherry Yang 
---
 drivers/android/binder_alloc.c | 24 +++-
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index 40f31df60580..f15af2b55a62 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -48,14 +48,23 @@ module_param_named(debug_mask, binder_alloc_debug_mask,
pr_info(x); \
} while (0)
 
+static struct binder_buffer *binder_buffer_next(struct binder_buffer *buffer)
+{
+   return list_entry(buffer->entry.next, struct binder_buffer, entry);
+}
+
+static struct binder_buffer *binder_buffer_prev(struct binder_buffer *buffer)
+{
+   return list_entry(buffer->entry.prev, struct binder_buffer, entry);
+}
+
 static size_t binder_alloc_buffer_size(struct binder_alloc *alloc,
   struct binder_buffer *buffer)
 {
if (list_is_last(&buffer->entry, &alloc->buffers))
return alloc->buffer +
   alloc->buffer_size - (void *)buffer->data;
-   return (size_t)list_entry(buffer->entry.next,
- struct binder_buffer, entry) - (size_t)buffer->data;
+   return (size_t)binder_buffer_next(buffer) - (size_t)buffer->data;
 }
 
 static void binder_insert_free_buffer(struct binder_alloc *alloc,
@@ -470,7 +479,7 @@ static void binder_delete_free_buffer(struct binder_alloc 
*alloc,
int free_page_start = 1;
 
BUG_ON(alloc->buffers.next == &buffer->entry);
-   prev = list_entry(buffer->entry.prev, struct binder_buffer, entry);
+   prev = binder_buffer_prev(buffer);
BUG_ON(!prev->free);
if (buffer_end_page(prev) == buffer_start_page(buffer)) {
free_page_start = 0;
@@ -482,8 +491,7 @@ static void binder_delete_free_buffer(struct binder_alloc 
*alloc,
}
 
if (!list_is_last(&buffer->entry, &alloc->buffers)) {
-   next = list_entry(buffer->entry.next,
- struct binder_buffer, entry);
+   next = binder_buffer_next(buffer);
if (buffer_start_page(next) == buffer_end_page(buffer)) {
free_page_end = 0;
if (buffer_start_page(next) ==
@@ -544,8 +552,7 @@ static void binder_free_buf_locked(struct binder_alloc 
*alloc,
rb_erase(&buffer->rb_node, &alloc->allocated_buffers);
buffer->free = 1;
if (!list_is_last(&buffer->entry, &alloc->buffers)) {
-   struct binder_buffer *next = list_entry(buffer->entry.next,
-   struct binder_buffer, entry);
+   struct binder_buffer *next = binder_buffer_next(buffer);
 
if (next->free) {
rb_erase(&next->rb_node, &alloc->free_buffers);
@@ -553,8 +560,7 @@ static void binder_free_buf_locked(struct binder_alloc 
*alloc,
}
}
if (alloc->buffers.next != &buffer->entry) {
-   struct binder_buffer *prev = list_entry(buffer->entry.prev,
-   struct binder_buffer, entry);
+   struct binder_buffer *prev = binder_buffer_prev(buffer);
 
if (prev->free) {
binder_delete_free_buffer(alloc, buffer);
-- 
2.14.1.342.g6490525c54-goog

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 6/6] android: binder: Add page usage in binder stats

[Sherry Yang]

Add the number of active, lru, and free pages for
each binder process in binder stats

Signed-off-by: Sherry Yang 
---
 drivers/android/binder.c   |  2 ++
 drivers/android/binder_alloc.c | 28 
 drivers/android/binder_alloc.h |  2 ++
 3 files changed, 32 insertions(+)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index fc5a4b9f3d97..7210e0dba3ef 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -5047,6 +5047,8 @@ static void print_binder_proc_stats(struct seq_file *m,
count = binder_alloc_get_allocated_count(&proc->alloc);
seq_printf(m, "  buffers: %d\n", count);
 
+   binder_alloc_print_pages(m, &proc->alloc);
+
count = 0;
binder_inner_proc_lock(proc);
list_for_each_entry(w, &proc->todo, entry) {
diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index 2624a502fcde..8fe165844e47 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -831,6 +831,34 @@ void binder_alloc_print_allocated(struct seq_file *m,
mutex_unlock(&alloc->mutex);
 }
 
+/**
+ * binder_alloc_print_pages() - print page usage
+ * @m: seq_file for output via seq_printf()
+ * @alloc: binder_alloc for this proc
+ */
+void binder_alloc_print_pages(struct seq_file *m,
+ struct binder_alloc *alloc)
+{
+   struct binder_lru_page *page;
+   int i;
+   int active = 0;
+   int lru = 0;
+   int free = 0;
+
+   mutex_lock(&alloc->mutex);
+   for (i = 0; i < alloc->buffer_size / PAGE_SIZE; i++) {
+   page = &alloc->pages[i];
+   if (!page->page_ptr)
+   free++;
+   else if (list_empty(&page->lru))
+   active++;
+   else
+   lru++;
+   }
+   mutex_unlock(&alloc->mutex);
+   seq_printf(m, "  pages: %d:%d:%d\n", active, lru, free);
+}
+
 /**
  * binder_alloc_get_allocated_count() - return count of buffers
  * @alloc: binder_alloc for this proc
diff --git a/drivers/android/binder_alloc.h b/drivers/android/binder_alloc.h
index fa707cc63393..a3a3602c689c 100644
--- a/drivers/android/binder_alloc.h
+++ b/drivers/android/binder_alloc.h
@@ -142,6 +142,8 @@ extern void binder_alloc_deferred_release(struct 
binder_alloc *alloc);
 extern int binder_alloc_get_allocated_count(struct binder_alloc *alloc);
 extern void binder_alloc_print_allocated(struct seq_file *m,
 struct binder_alloc *alloc);
+void binder_alloc_print_pages(struct seq_file *m,
+ struct binder_alloc *alloc);
 
 /**
  * binder_alloc_get_free_async_space() - get free space available for async
-- 
2.14.1.342.g6490525c54-goog

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3 4/6] android: binder: Add global lru shrinker to binder

[Sherry Yang]

Hold on to the pages allocated and mapped for transaction
buffers until the system is under memory pressure. When
that happens, use linux shrinker to free pages. Without
using shrinker, patch "android: binder: Move buffer out
of area shared with user space" will cause a significant
slow down for small transactions that fit into the first
page because free list buffer header used to be inlined
with buffer data.

In addition to prevent the performance regression for
small transactions, this patch improves the performance
for transactions that take up more than one page.

Modify alloc selftest to work with the shrinker change.

Test: Run memory intensive applications (Chrome and Camera)
to trigger shrinker callbacks. Binder frees memory as expected.
Test: Run binderThroughputTest with high memory pressure
option enabled.

Signed-off-by: Sherry Yang 
---
 drivers/android/binder.c|   2 +
 drivers/android/binder_alloc.c  | 172 +++-
 drivers/android/binder_alloc.h  |  23 -
 drivers/android/binder_alloc_selftest.c |  68 ++---
 4 files changed, 225 insertions(+), 40 deletions(-)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index b31e64c6f666..fc5a4b9f3d97 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -5243,6 +5243,8 @@ static int __init binder_init(void)
struct binder_device *device;
struct hlist_node *tmp;
 
+   binder_alloc_shrinker_init();
+
atomic_set(&binder_transaction_log.cur, ~0U);
atomic_set(&binder_transaction_log_failed.cur, ~0U);
 
diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index ddf5f1a379a4..8eb7e9e9a21f 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -27,9 +27,12 @@
 #include 
 #include 
 #include 
+#include 
 #include "binder_alloc.h"
 #include "binder_trace.h"
 
+struct list_lru binder_alloc_lru;
+
 static DEFINE_MUTEX(binder_alloc_mmap_lock);
 
 enum {
@@ -188,8 +191,9 @@ static int binder_update_page_range(struct binder_alloc 
*alloc, int allocate,
 {
void *page_addr;
unsigned long user_page_addr;
-   struct page **page;
-   struct mm_struct *mm;
+   struct binder_lru_page *page;
+   struct mm_struct *mm = NULL;
+   bool need_mm = false;
 
binder_alloc_debug(BINDER_DEBUG_BUFFER_ALLOC,
 "%d: %s pages %pK-%pK\n", alloc->pid,
@@ -200,9 +204,18 @@ static int binder_update_page_range(struct binder_alloc 
*alloc, int allocate,
 
trace_binder_update_page_range(alloc, allocate, start, end);
 
-   if (vma)
-   mm = NULL;
-   else
+   if (allocate == 0)
+   goto free_range;
+
+   for (page_addr = start; page_addr < end; page_addr += PAGE_SIZE) {
+   page = &alloc->pages[(page_addr - alloc->buffer) / PAGE_SIZE];
+   if (!page->page_ptr) {
+   need_mm = true;
+   break;
+   }
+   }
+
+   if (!vma && need_mm)
mm = get_task_mm(alloc->tsk);
 
if (mm) {
@@ -215,10 +228,7 @@ static int binder_update_page_range(struct binder_alloc 
*alloc, int allocate,
}
}
 
-   if (allocate == 0)
-   goto free_range;
-
-   if (vma == NULL) {
+   if (!vma && need_mm) {
pr_err("%d: binder_alloc_buf failed to map pages in userspace, 
no vma\n",
alloc->pid);
goto err_no_vma;
@@ -226,18 +236,33 @@ static int binder_update_page_range(struct binder_alloc 
*alloc, int allocate,
 
for (page_addr = start; page_addr < end; page_addr += PAGE_SIZE) {
int ret;
+   bool on_lru;
 
page = &alloc->pages[(page_addr - alloc->buffer) / PAGE_SIZE];
 
-   BUG_ON(*page);
-   *page = alloc_page(GFP_KERNEL | __GFP_HIGHMEM | __GFP_ZERO);
-   if (*page == NULL) {
+   if (page->page_ptr) {
+   on_lru = list_lru_del(&binder_alloc_lru, &page->lru);
+   WARN_ON(!on_lru);
+   continue;
+   }
+
+   if (WARN_ON(!vma))
+   goto err_page_ptr_cleared;
+
+   page->page_ptr = alloc_page(GFP_KERNEL |
+   __GFP_HIGHMEM |
+   __GFP_ZERO);
+   if (!page->page_ptr) {
pr_err("%d: binder_alloc_buf failed for page at %pK\n",
alloc->pid, page_addr);
goto err_alloc_page_failed;
}
+   page->alloc = alloc;
+   INIT_LIST_HEAD(&pag

[PATCH v3 3/6] android: binder: Move buffer out of area shared with user space

[Sherry Yang]

Binder driver allocates buffer meta data in a region that is mapped
in user space. These meta data contain pointers in the kernel.

This patch allocates buffer meta data on the kernel heap that is
not mapped in user space, and uses a pointer to refer to the data mapped.

Also move alloc->buffers initialization from mmap to init since it's
now used even when mmap failed or was not called.

Signed-off-by: Sherry Yang 
---
 drivers/android/binder_alloc.c  | 146 +++-
 drivers/android/binder_alloc.h  |   2 +-
 drivers/android/binder_alloc_selftest.c |  11 ++-
 3 files changed, 91 insertions(+), 68 deletions(-)

diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index f15af2b55a62..ddf5f1a379a4 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -62,9 +62,9 @@ static size_t binder_alloc_buffer_size(struct binder_alloc 
*alloc,
   struct binder_buffer *buffer)
 {
if (list_is_last(&buffer->entry, &alloc->buffers))
-   return alloc->buffer +
-  alloc->buffer_size - (void *)buffer->data;
-   return (size_t)binder_buffer_next(buffer) - (size_t)buffer->data;
+   return (u8 *)alloc->buffer +
+   alloc->buffer_size - (u8 *)buffer->data;
+   return (u8 *)binder_buffer_next(buffer)->data - (u8 *)buffer->data;
 }
 
 static void binder_insert_free_buffer(struct binder_alloc *alloc,
@@ -114,9 +114,9 @@ static void binder_insert_allocated_buffer_locked(
buffer = rb_entry(parent, struct binder_buffer, rb_node);
BUG_ON(buffer->free);
 
-   if (new_buffer < buffer)
+   if (new_buffer->data < buffer->data)
p = &parent->rb_left;
-   else if (new_buffer > buffer)
+   else if (new_buffer->data > buffer->data)
p = &parent->rb_right;
else
BUG();
@@ -131,18 +131,17 @@ static struct binder_buffer 
*binder_alloc_prepare_to_free_locked(
 {
struct rb_node *n = alloc->allocated_buffers.rb_node;
struct binder_buffer *buffer;
-   struct binder_buffer *kern_ptr;
+   void *kern_ptr;
 
-   kern_ptr = (struct binder_buffer *)(user_ptr - alloc->user_buffer_offset
-   - offsetof(struct binder_buffer, data));
+   kern_ptr = (void *)(user_ptr - alloc->user_buffer_offset);
 
while (n) {
buffer = rb_entry(n, struct binder_buffer, rb_node);
BUG_ON(buffer->free);
 
-   if (kern_ptr < buffer)
+   if (kern_ptr < buffer->data)
n = n->rb_left;
-   else if (kern_ptr > buffer)
+   else if (kern_ptr > buffer->data)
n = n->rb_right;
else {
/*
@@ -330,6 +329,9 @@ struct binder_buffer *binder_alloc_new_buf_locked(struct 
binder_alloc *alloc,
return ERR_PTR(-ENOSPC);
}
 
+   /* Pad 0-size buffers so they get assigned unique addresses */
+   size = max(size, sizeof(void *));
+
while (n) {
buffer = rb_entry(n, struct binder_buffer, rb_node);
BUG_ON(!buffer->free);
@@ -389,14 +391,9 @@ struct binder_buffer *binder_alloc_new_buf_locked(struct 
binder_alloc *alloc,
 
has_page_addr =
(void *)(((uintptr_t)buffer->data + buffer_size) & PAGE_MASK);
-   if (n == NULL) {
-   if (size + sizeof(struct binder_buffer) + 4 >= buffer_size)
-   buffer_size = size; /* no room for other buffers */
-   else
-   buffer_size = size + sizeof(struct binder_buffer);
-   }
+   WARN_ON(n && buffer_size != size);
end_page_addr =
-   (void *)PAGE_ALIGN((uintptr_t)buffer->data + buffer_size);
+   (void *)PAGE_ALIGN((uintptr_t)buffer->data + size);
if (end_page_addr > has_page_addr)
end_page_addr = has_page_addr;
ret = binder_update_page_range(alloc, 1,
@@ -404,17 +401,25 @@ struct binder_buffer *binder_alloc_new_buf_locked(struct 
binder_alloc *alloc,
if (ret)
return ERR_PTR(ret);
 
-   rb_erase(best_fit, &alloc->free_buffers);
-   buffer->free = 0;
-   buffer->free_in_progress = 0;
-   binder_insert_allocated_buffer_locked(alloc, buffer);
if (buffer_size != size) {
-   struct binder_buffer *new_buffer = (void *)buffer->data + size;
+   struct binder_buffer *new_buffer;
 
+   new_buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
+   if (!new_buffer) {
+  

Re: [PATCH v3 1/6] android: binder: Refactor prev and next buffer into a helper function

[Sherry Yang]

On Tue, Aug 29, 2017 at 11:07 PM, Greg Kroah-Hartman
 wrote:
> On Tue, Aug 29, 2017 at 05:46:57PM -0700, Sherry Yang wrote:
>> Use helper functions buffer_next and buffer_prev instead
>> of list_entry to get the next and previous buffers.
>>
>> Signed-off-by: Sherry Yang 
>> ---
>>  drivers/android/binder_alloc.c | 24 +++-
>>  1 file changed, 15 insertions(+), 9 deletions(-)
>
> What changed from the previous version?

This specific patch didn't change. The change is only in [patch v3
3/6] (crash fix) and in [patch v3 6/6] (new patch that add stats).

> Always put the changes below the --- line.  Like the documentation says
> to do so.

Do you mean I should use --annotate to git send-email to specify what
has changed across versions for each patch? I used --compose and put
what changed from v2 to v3 in the introductory message [patch 0/6].

> Also, don't I already have these patches in my tree?  Do you want me to
> revert the existing ones and use the new ones, or what about a fixup
> patch for any differences?

I got a message saying a test failed on [patch 3/5]. I resubmitted the
entire thread because I wasn't sure if you would drop the failing
patch set. If the entire failing patch set is dropped, then v3 can be
used as a replacement.

If you prefer a fixup patch, I can post another patch set (the crash
fix and the new patch) on top of what you already applied, but it
might be easier to do what's described in the previous paragraph (drop
v2 and apply v3).

Sherry Yang

> confused,
>
> greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] android: binder: fixup crash introduced by moving buffer hdr

[Sherry Yang]

Fix crash introduced by 74310e06be4d74dcf67cd108366710dee5c576d5
(android: binder: Move buffer out of area shared with user space)
when close is called after open without mmap in between.

Signed-off-by: Sherry Yang 
---
 drivers/android/binder_alloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index 78c42c0d62b9..2624a502fcde 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -713,7 +713,6 @@ int binder_alloc_mmap_handler(struct binder_alloc *alloc,
}
 
buffer->data = alloc->buffer;
-   INIT_LIST_HEAD(&alloc->buffers);
list_add(&buffer->entry, &alloc->buffers);
buffer->free = 1;
binder_insert_free_buffer(alloc, buffer);
@@ -972,6 +971,7 @@ void binder_alloc_init(struct binder_alloc *alloc)
alloc->tsk = current->group_leader;
alloc->pid = current->group_leader->pid;
mutex_init(&alloc->mutex);
+   INIT_LIST_HEAD(&alloc->buffers);
 }
 
 void binder_alloc_shrinker_init(void)
-- 
2.14.1.581.gf28d330327-goog

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 1/6] android: binder: Refactor prev and next buffer into a helper function

[Sherry Yang]

On Wed, Aug 30, 2017 at 9:28 PM, Greg Kroah-Hartman
 wrote:
> Ok, exactly what git commit ids should I revert from the tree?  But
> really, a fix would be easier at this point :)

I sent a fixup patch as a separate reply to this email. Please also
apply [patch v3 6/6] as it was not in v2, which you applied.

Thanks,
Sherry
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] android: binder: Add page usage in binder stats

[Sherry Yang]

Add the number of active, lru, and free pages for
each binder process in binder stats

Signed-off-by: Sherry Yang 
---
 drivers/android/binder.c   |  2 ++
 drivers/android/binder_alloc.c | 28 
 drivers/android/binder_alloc.h |  2 ++
 3 files changed, 32 insertions(+)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index ba9e613b42d6..56b380292cc5 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -5047,6 +5047,8 @@ static void print_binder_proc_stats(struct seq_file *m,
count = binder_alloc_get_allocated_count(&proc->alloc);
seq_printf(m, "  buffers: %d\n", count);
 
+   binder_alloc_print_pages(m, &proc->alloc);
+
count = 0;
binder_inner_proc_lock(proc);
list_for_each_entry(w, &proc->todo, entry) {
diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index 2624a502fcde..8fe165844e47 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -831,6 +831,34 @@ void binder_alloc_print_allocated(struct seq_file *m,
mutex_unlock(&alloc->mutex);
 }
 
+/**
+ * binder_alloc_print_pages() - print page usage
+ * @m: seq_file for output via seq_printf()
+ * @alloc: binder_alloc for this proc
+ */
+void binder_alloc_print_pages(struct seq_file *m,
+ struct binder_alloc *alloc)
+{
+   struct binder_lru_page *page;
+   int i;
+   int active = 0;
+   int lru = 0;
+   int free = 0;
+
+   mutex_lock(&alloc->mutex);
+   for (i = 0; i < alloc->buffer_size / PAGE_SIZE; i++) {
+   page = &alloc->pages[i];
+   if (!page->page_ptr)
+   free++;
+   else if (list_empty(&page->lru))
+   active++;
+   else
+   lru++;
+   }
+   mutex_unlock(&alloc->mutex);
+   seq_printf(m, "  pages: %d:%d:%d\n", active, lru, free);
+}
+
 /**
  * binder_alloc_get_allocated_count() - return count of buffers
  * @alloc: binder_alloc for this proc
diff --git a/drivers/android/binder_alloc.h b/drivers/android/binder_alloc.h
index fa707cc63393..a3a3602c689c 100644
--- a/drivers/android/binder_alloc.h
+++ b/drivers/android/binder_alloc.h
@@ -142,6 +142,8 @@ extern void binder_alloc_deferred_release(struct 
binder_alloc *alloc);
 extern int binder_alloc_get_allocated_count(struct binder_alloc *alloc);
 extern void binder_alloc_print_allocated(struct seq_file *m,
 struct binder_alloc *alloc);
+void binder_alloc_print_pages(struct seq_file *m,
+ struct binder_alloc *alloc);
 
 /**
  * binder_alloc_get_free_async_space() - get free space available for async
-- 
2.14.1.581.gf28d330327-goog

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] android: binder: Drop lru lock in isolate callback

[Sherry Yang]

Drop the global lru lock in isolate callback
before calling zap_page_range which calls
cond_resched, and re-acquire the global lru
lock before returning. Also change return
code to LRU_REMOVED_RETRY.

Use mmput_async when fail to acquire mmap sem
in an atomic context.

Fix "BUG: sleeping function called from invalid context"
errors when CONFIG_DEBUG_ATOMIC_SLEEP is enabled.

Fixes: f2517eb76f1f2 ("android: binder: Add global lru shrinker to binder")
Reported-by: Kyle Yan 
Acked-by: Arve Hjønnevåg 
Signed-off-by: Sherry Yang 
---
 drivers/android/binder_alloc.c | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index 8fe165844e47..064f5e31ec55 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -913,6 +913,7 @@ enum lru_status binder_alloc_free_page(struct list_head 
*item,
struct binder_alloc *alloc;
uintptr_t page_addr;
size_t index;
+   struct vm_area_struct *vma;
 
alloc = page->alloc;
if (!mutex_trylock(&alloc->mutex))
@@ -923,16 +924,22 @@ enum lru_status binder_alloc_free_page(struct list_head 
*item,
 
index = page - alloc->pages;
page_addr = (uintptr_t)alloc->buffer + index * PAGE_SIZE;
-   if (alloc->vma) {
+   vma = alloc->vma;
+   if (vma) {
mm = get_task_mm(alloc->tsk);
if (!mm)
goto err_get_task_mm_failed;
if (!down_write_trylock(&mm->mmap_sem))
goto err_down_write_mmap_sem_failed;
+   }
+
+   list_lru_isolate(lru, item);
+   spin_unlock(lock);
 
+   if (vma) {
trace_binder_unmap_user_start(alloc, index);
 
-   zap_page_range(alloc->vma,
+   zap_page_range(vma,
   page_addr + alloc->user_buffer_offset,
   PAGE_SIZE);
 
@@ -950,13 +957,12 @@ enum lru_status binder_alloc_free_page(struct list_head 
*item,
 
trace_binder_unmap_kernel_end(alloc, index);
 
-   list_lru_isolate(lru, item);
-
+   spin_lock(lock);
mutex_unlock(&alloc->mutex);
-   return LRU_REMOVED;
+   return LRU_REMOVED_RETRY;
 
 err_down_write_mmap_sem_failed:
-   mmput(mm);
+   mmput_async(mm);
 err_get_task_mm_failed:
 err_page_already_freed:
mutex_unlock(&alloc->mutex);
-- 
2.11.0 (Apple Git-81)

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel