We should better handle killing Binder_ processes in OOM |
|||||||||||||||||||||||||||
Issue descriptionThis is described pretty well here: https://chromium-review.googlesource.com/c/463866/3//COMMIT_MSG Basically when we OOM kill we end up choosing a "Binder_XXX" process as a victim, then we fail to kill it. This isn't as fatal as it once was due to the fixes in bug #702707 , but it's still not a great situation to be in. Our OOM kill path is very slow so we really want to avoid OOM kills when possible. Trying to kill lots of processes that are just not going to actually die (and free their memory) is also a bit dumb. Let's fix Binder so it handles being killed more gracefully.
,
May 4 2017
Note also that while I was poking at kevin today I saw at least one instance where I got a bunch of "refused to die" even after fixing Binder. In that case it appeared to be that all the tasks were sitting there waiting on RCU. I'll keep trying to dig, but possibly that's a separate (but similar) bug.
,
May 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/b1f939bcc931bbce22c54a5a190c8050cd7fc5a3 commit b1f939bcc931bbce22c54a5a190c8050cd7fc5a3 Author: Dmitry Torokhov <dtor@chromium.org> Date: Wed May 10 03:36:27 2017 CHROMIUM: android: binder: get rid of BINDER_LOOPER_STATE_WAITING Instead of keeping track of threads that are in "wait for" state, let's wake up all of them when flushing deferred actions. Threads that are already woken up will see no difference and threads that are sleeping will be woken up. I believe this also fixes a bug: threads in binder_poll() will also get woken up now. BUG= chromium:718637 TEST=Boot on Caroline, play with Android apps Change-Id: Ic64d46c17bcef46f193587e29acbc32cce57ece4 Signed-off-by: Dmitry Torokhov <dtor@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/495767 Reviewed-by: Douglas Anderson <dianders@chromium.org> Reviewed-by: Sonny Rao <sonnyrao@chromium.org> [modify] https://crrev.com/b1f939bcc931bbce22c54a5a190c8050cd7fc5a3/drivers/staging/android/binder.c
,
May 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/b4a91eae0934b89dca9d0e8b0ad61ed7e6511edf commit b4a91eae0934b89dca9d0e8b0ad61ed7e6511edf Author: Dmitry Torokhov <dtor@chromium.org> Date: Wed May 10 03:36:28 2017 CHROMIUM: android: binder: reset BINDER_LOOPER_STATE_NEED_RETURN earlier Instead of resetting this flag after returning from ioctl, let's reset it right before we try to wait for event. This should help us drop the binder lock (in a subsequent patch) when thread is forcibly woken up. I believe it also fixes issue with binder_poll() immediately signalling data ready on a newly registered thread (because new threads set BINDER_LOOPER_STATE_NEED_RETURN when we create their objects). BUG= chromium:718637 TEST=Boot on Caroline, play with Android apps Change-Id: I50d10a4d23da3ead3fcc7f7032c459ab9bf21fb2 Signed-off-by: Dmitry Torokhov <dtor@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/495768 Reviewed-by: Douglas Anderson <dianders@chromium.org> [modify] https://crrev.com/b4a91eae0934b89dca9d0e8b0ad61ed7e6511edf/drivers/staging/android/binder.c
,
May 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/6580a647d51b5382c01d6da4b7ae62f27eb16122 commit 6580a647d51b5382c01d6da4b7ae62f27eb16122 Author: Dmitry Torokhov <dtor@chromium.org> Date: Wed May 10 03:36:29 2017 CHROMIUM: android: binder: push binder_lock down into ioctl ops Instead of taking binder lock and locating process thread data, even when neither is needed, let's push this down into individual handlers. BUG= chromium:718637 TEST=Boot on Caroline, play with Android apps Change-Id: I0aadd59968fa674225eb2fee58de43ec15a94bdb Signed-off-by: Dmitry Torokhov <dtor@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/495769 Reviewed-by: Douglas Anderson <dianders@chromium.org> Reviewed-by: Sonny Rao <sonnyrao@chromium.org> [modify] https://crrev.com/6580a647d51b5382c01d6da4b7ae62f27eb16122/drivers/staging/android/binder.c
,
May 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/4c572220e433609bb950367155448d72b2561b59 commit 4c572220e433609bb950367155448d72b2561b59 Author: Dmitry Torokhov <dtor@chromium.org> Date: Wed May 10 03:36:31 2017 CHROMIUM: android: binder: do not take binder lock when thread forcibly woken up When thread is forcibly woken up, it tries to acquire global binder lock to adjust some statistics. This causes trouble when binder lock is contended, for example during OOM kill process, where we can't kill threads because they wait on this lock. BUG= chromium:718637 TEST=Boot on Caroline, play with Android apps Change-Id: Ic26f20bef4a0db34e3a767a6026f685628b45421 Signed-off-by: Dmitry Torokhov <dtor@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/495770 Reviewed-by: Douglas Anderson <dianders@chromium.org> [modify] https://crrev.com/4c572220e433609bb950367155448d72b2561b59/drivers/staging/android/binder.c
,
May 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/a5af8008287cbe5ed1fe0ee5ab60ad018c4adcdf commit a5af8008287cbe5ed1fe0ee5ab60ad018c4adcdf Author: Douglas Anderson <dianders@chromium.org> Date: Wed May 10 03:36:32 2017 CHROMIUM: android: binder: introduce binder_lock_interruptible When flushing/killing threads there is slight chance that thread is already waiting on binder lock in binder_thread_read(). Let's create binder_lock_interruptible() and use it there, so we can kick waiting thread and it will return a bit earlier, lessening ocntention on the binder lock. BUG= chromium:718637 TEST=Boot on Caroline, play with Android apps Change-Id: Ia8429bf5b07d133a0cb43e1cd17737e996740000 Signed-off-by: Douglas Anderson <dianders@chromium.org> Signed-off-by: Dmitry Torokhov <dtor@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/498008 [modify] https://crrev.com/a5af8008287cbe5ed1fe0ee5ab60ad018c4adcdf/drivers/staging/android/binder.c
,
May 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/77bbf31384b62831d6f4a6158d5c885c71c5be59 commit 77bbf31384b62831d6f4a6158d5c885c71c5be59 Author: Dmitry Torokhov <dtor@chromium.org> Date: Wed May 10 03:36:33 2017 CHROMIUM: android: binder: do not create thread when trying to kill it Add an option to binder_get_thread() to forego creating a new thread and use it when processing request to kill thread. BUG= chromium:718637 TEST=Boot on Caroline, play with Android apps Change-Id: I9a8a426dcfab39b72e546c70e87e5dc1f1c2309c Signed-off-by: Dmitry Torokhov <dtor@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/498010 Reviewed-by: Douglas Anderson <dianders@chromium.org> [modify] https://crrev.com/77bbf31384b62831d6f4a6158d5c885c71c5be59/drivers/staging/android/binder.c
,
May 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/e1df4a833f6b37131f2fe64212000fcb0e9fa2ff commit e1df4a833f6b37131f2fe64212000fcb0e9fa2ff Author: Dmitry Torokhov <dmitry.torokhov@gmail.com> Date: Wed May 10 03:36:26 2017 FROMLIST: android: binder: check result of binder_get_thread() in binder_poll() If binder_get_thread() fails to give us a thread data, we should avoid dereferencing a NULL pointer and return POLLERR instead. Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> BUG= chromium:718637 TEST=Boot on Caroline, play with Android apps (am from https://patchwork.kernel.org/patch/9716673/) Signed-off-by: Dmitry Torokhov <dtor@chromium.org> Change-Id: I4aa5b9dd0d247921c3093cb3bfca4f148fed1b2c Reviewed-on: https://chromium-review.googlesource.com/498634 Reviewed-by: Douglas Anderson <dianders@chromium.org> [modify] https://crrev.com/e1df4a833f6b37131f2fe64212000fcb0e9fa2ff/drivers/staging/android/binder.c
,
May 10 2017
Dmitry's changes landed, but only in 3.18. We probably want similar changes in 3.14 and 4.4. Dmitry: will you post those? --- I'm going to call this M-60. I'd be a bit hesitant to merge these back to M-59 since I'd personally like to see the maximum possible bake time to these binder changes. A few notes about the changes: * They could possibly affect binder even in the non-OOM case. Although we've reviewed the changes as best we can and don't know of any problems, it is theoretically possible that we could regress binder behavior in general somehow. * They shouldn't affect non-binder. I believe that means that anyone who hasn't enabled ARC++ would be safe. -- Note that we could possibly think about merging to M-59, but we need to be sure the changes are getting enough testing...
,
May 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/6787a6a271c2b833f73807ed230f0f750a941f1d commit 6787a6a271c2b833f73807ed230f0f750a941f1d Author: Dmitry Torokhov <dmitry.torokhov@gmail.com> Date: Fri May 12 05:27:24 2017 FROMLIST: android: binder: check result of binder_get_thread() in binder_poll() If binder_get_thread() fails to give us a thread data, we should avoid dereferencing a NULL pointer and return POLLERR instead. Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> BUG= chromium:718637 TEST=Boot on Caroline, play with Android apps (am from https://patchwork.kernel.org/patch/9716673/) Signed-off-by: Dmitry Torokhov <dtor@chromium.org> Change-Id: I4aa5b9dd0d247921c3093cb3bfca4f148fed1b2c Reviewed-on: https://chromium-review.googlesource.com/498634 Reviewed-by: Douglas Anderson <dianders@chromium.org> (cherry-picked from commit e1df4a833f6b37131f2fe64212000fcb0e9fa2ff) Reviewed-on: https://chromium-review.googlesource.com/503459 [modify] https://crrev.com/6787a6a271c2b833f73807ed230f0f750a941f1d/drivers/android/binder.c
,
May 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/0328ce2cf3d502c86dc4fd34996a4ecb7c8e91a1 commit 0328ce2cf3d502c86dc4fd34996a4ecb7c8e91a1 Author: Dmitry Torokhov <dtor@chromium.org> Date: Fri May 12 05:27:25 2017 CHROMIUM: android: binder: get rid of BINDER_LOOPER_STATE_WAITING Instead of keeping track of threads that are in "wait for" state, let's wake up all of them when flushing deferred actions. Threads that are already woken up will see no difference and threads that are sleeping will be woken up. I believe this also fixes a bug: threads in binder_poll() will also get woken up now. BUG= chromium:718637 TEST=Boot on Caroline, play with Android apps Change-Id: Ic64d46c17bcef46f193587e29acbc32cce57ece4 Signed-off-by: Dmitry Torokhov <dtor@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/495767 Reviewed-by: Douglas Anderson <dianders@chromium.org> Reviewed-by: Sonny Rao <sonnyrao@chromium.org> Signed-off-by: Dmitry Torokhov <dtor@chromium.org> (cherry-picked from commit b1f939bcc931bbce22c54a5a190c8050cd7fc5a3) Reviewed-on: https://chromium-review.googlesource.com/503460 [modify] https://crrev.com/0328ce2cf3d502c86dc4fd34996a4ecb7c8e91a1/drivers/android/binder.c
,
May 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/1adb74a6d0ee3018b41dd0b70fd81ba9761478ad commit 1adb74a6d0ee3018b41dd0b70fd81ba9761478ad Author: Dmitry Torokhov <dtor@chromium.org> Date: Fri May 12 05:27:26 2017 CHROMIUM: android: binder: reset BINDER_LOOPER_STATE_NEED_RETURN earlier Instead of resetting this flag after returning from ioctl, let's reset it right before we try to wait for event. This should help us drop the binder lock (in a subsequent patch) when thread is forcibly woken up. I believe it also fixes issue with binder_poll() immediately signalling data ready on a newly registered thread (because new threads set BINDER_LOOPER_STATE_NEED_RETURN when we create their objects). BUG= chromium:718637 TEST=Boot on Caroline, play with Android apps Change-Id: I50d10a4d23da3ead3fcc7f7032c459ab9bf21fb2 Signed-off-by: Dmitry Torokhov <dtor@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/495768 Reviewed-by: Douglas Anderson <dianders@chromium.org> (cherry picked from commit b4a91eae0934b89dca9d0e8b0ad61ed7e6511edf) Reviewed-on: https://chromium-review.googlesource.com/503461 [modify] https://crrev.com/1adb74a6d0ee3018b41dd0b70fd81ba9761478ad/drivers/android/binder.c
,
May 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/b3e87f79562691c78cc06c3dd551d017d5bf9d15 commit b3e87f79562691c78cc06c3dd551d017d5bf9d15 Author: Dmitry Torokhov <dtor@chromium.org> Date: Fri May 12 05:27:28 2017 CHROMIUM: android: binder: push binder_lock down into ioctl ops Instead of taking binder lock and locating process thread data, even when neither is needed, let's push this down into individual handlers. BUG= chromium:718637 TEST=Boot on Caroline, play with Android apps Change-Id: I0aadd59968fa674225eb2fee58de43ec15a94bdb Signed-off-by: Dmitry Torokhov <dtor@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/495769 Reviewed-by: Douglas Anderson <dianders@chromium.org> Reviewed-by: Sonny Rao <sonnyrao@chromium.org> (cherry picked from commit 6580a647d51b5382c01d6da4b7ae62f27eb16122) Reviewed-on: https://chromium-review.googlesource.com/503462 [modify] https://crrev.com/b3e87f79562691c78cc06c3dd551d017d5bf9d15/drivers/android/binder.c
,
May 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/b9593078bf763d08bd6f799ef5cf50d775923d3e commit b9593078bf763d08bd6f799ef5cf50d775923d3e Author: Douglas Anderson <dianders@chromium.org> Date: Fri May 12 05:27:30 2017 CHROMIUM: android: binder: introduce binder_lock_interruptible When flushing/killing threads there is slight chance that thread is already waiting on binder lock in binder_thread_read(). Let's create binder_lock_interruptible() and use it there, so we can kick waiting thread and it will return a bit earlier, lessening ocntention on the binder lock. BUG= chromium:718637 TEST=Boot on Caroline, play with Android apps Change-Id: Ia8429bf5b07d133a0cb43e1cd17737e996740000 Signed-off-by: Douglas Anderson <dianders@chromium.org> Signed-off-by: Dmitry Torokhov <dtor@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/498008 (cherry picked from commit a5af8008287cbe5ed1fe0ee5ab60ad018c4adcdf) Reviewed-on: https://chromium-review.googlesource.com/503464 [modify] https://crrev.com/b9593078bf763d08bd6f799ef5cf50d775923d3e/drivers/android/binder.c
,
May 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/1fee77f30d4bbfd3cb766d23f758bc3e82ab032b commit 1fee77f30d4bbfd3cb766d23f758bc3e82ab032b Author: Dmitry Torokhov <dtor@chromium.org> Date: Fri May 12 05:27:29 2017 CHROMIUM: android: binder: do not take binder lock when thread forcibly woken up When thread is forcibly woken up, it tries to acquire global binder lock to adjust some statistics. This causes trouble when binder lock is contended, for example during OOM kill process, where we can't kill threads because they wait on this lock. BUG= chromium:718637 TEST=Boot on Caroline, play with Android apps Change-Id: Ic26f20bef4a0db34e3a767a6026f685628b45421 Signed-off-by: Dmitry Torokhov <dtor@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/495770 Reviewed-by: Douglas Anderson <dianders@chromium.org> (cherry picked from commit 4c572220e433609bb950367155448d72b2561b59) Reviewed-on: https://chromium-review.googlesource.com/503463 [modify] https://crrev.com/1fee77f30d4bbfd3cb766d23f758bc3e82ab032b/drivers/android/binder.c
,
May 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/8947c6d9853ca41008003d48634636bfbc57de1b commit 8947c6d9853ca41008003d48634636bfbc57de1b Author: Dmitry Torokhov <dtor@chromium.org> Date: Fri May 12 05:27:31 2017 CHROMIUM: android: binder: do not create thread when trying to kill it Add an option to binder_get_thread() to forego creating a new thread and use it when processing request to kill thread. BUG= chromium:718637 TEST=Boot on Caroline, play with Android apps Change-Id: I9a8a426dcfab39b72e546c70e87e5dc1f1c2309c Signed-off-by: Dmitry Torokhov <dtor@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/498010 Reviewed-by: Douglas Anderson <dianders@chromium.org> (cherry picked from commit 77bbf31384b62831d6f4a6158d5c885c71c5be59) Reviewed-on: https://chromium-review.googlesource.com/503465 [modify] https://crrev.com/8947c6d9853ca41008003d48634636bfbc57de1b/drivers/android/binder.c
,
May 12 2017
Sounds like we're not currently planning on doing this on 3.14 unless we find some evidence that it's needed. I took a quick look and didn't see that, though perhaps we should look again once other crashes (like the weird CPU errata) calm down... Anyway, marking Fixed for now. Thanks dtor@
,
May 18 2017
,
May 18 2017
,
May 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/8742a55a65d9919079b9250a0a9858bb2aa32f05 commit 8742a55a65d9919079b9250a0a9858bb2aa32f05 Author: Dmitry Torokhov <dmitry.torokhov@gmail.com> Date: Fri May 19 00:57:03 2017 FROMLIST: android: binder: check result of binder_get_thread() in binder_poll() If binder_get_thread() fails to give us a thread data, we should avoid dereferencing a NULL pointer and return POLLERR instead. Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> BUG= chromium:718637 TEST=Boot on Caroline, play with Android apps (am from https://patchwork.kernel.org/patch/9716673/) Signed-off-by: Dmitry Torokhov <dtor@chromium.org> Change-Id: I4aa5b9dd0d247921c3093cb3bfca4f148fed1b2c Reviewed-on: https://chromium-review.googlesource.com/498634 Reviewed-by: Douglas Anderson <dianders@chromium.org> (cherry-picked from commit e1df4a833f6b37131f2fe64212000fcb0e9fa2ff) Reviewed-on: https://chromium-review.googlesource.com/503459 (cherry picked from commit 6787a6a271c2b833f73807ed230f0f750a941f1d) Reviewed-on: https://chromium-review.googlesource.com/508484 [modify] https://crrev.com/8742a55a65d9919079b9250a0a9858bb2aa32f05/drivers/android/binder.c
,
May 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/e3f6e80dfa234ba6632be5f3076872ff9f247ed1 commit e3f6e80dfa234ba6632be5f3076872ff9f247ed1 Author: Dmitry Torokhov <dtor@chromium.org> Date: Fri May 19 00:57:06 2017 CHROMIUM: android: binder: get rid of BINDER_LOOPER_STATE_WAITING Instead of keeping track of threads that are in "wait for" state, let's wake up all of them when flushing deferred actions. Threads that are already woken up will see no difference and threads that are sleeping will be woken up. I believe this also fixes a bug: threads in binder_poll() will also get woken up now. BUG= chromium:718637 TEST=Boot on Caroline, play with Android apps Change-Id: Ic64d46c17bcef46f193587e29acbc32cce57ece4 Signed-off-by: Dmitry Torokhov <dtor@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/495767 Reviewed-by: Douglas Anderson <dianders@chromium.org> Reviewed-by: Sonny Rao <sonnyrao@chromium.org> Signed-off-by: Dmitry Torokhov <dtor@chromium.org> (cherry-picked from commit b1f939bcc931bbce22c54a5a190c8050cd7fc5a3) Reviewed-on: https://chromium-review.googlesource.com/503460 (cherry picked from commit 0328ce2cf3d502c86dc4fd34996a4ecb7c8e91a1) Reviewed-on: https://chromium-review.googlesource.com/508485 [modify] https://crrev.com/e3f6e80dfa234ba6632be5f3076872ff9f247ed1/drivers/android/binder.c
,
May 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/453bb1a36dbacefbbccaebd7181a03504fe2c5ab commit 453bb1a36dbacefbbccaebd7181a03504fe2c5ab Author: Dmitry Torokhov <dmitry.torokhov@gmail.com> Date: Fri May 19 00:57:10 2017 FROMLIST: android: binder: check result of binder_get_thread() in binder_poll() If binder_get_thread() fails to give us a thread data, we should avoid dereferencing a NULL pointer and return POLLERR instead. Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> BUG= chromium:718637 TEST=Boot on Caroline, play with Android apps (am from https://patchwork.kernel.org/patch/9716673/) Signed-off-by: Dmitry Torokhov <dtor@chromium.org> Change-Id: I4aa5b9dd0d247921c3093cb3bfca4f148fed1b2c Reviewed-on: https://chromium-review.googlesource.com/498634 Reviewed-by: Douglas Anderson <dianders@chromium.org> (cherry picked from commit e1df4a833f6b37131f2fe64212000fcb0e9fa2ff) Reviewed-on: https://chromium-review.googlesource.com/508856 [modify] https://crrev.com/453bb1a36dbacefbbccaebd7181a03504fe2c5ab/drivers/staging/android/binder.c
,
May 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/6d01d7d10b68d5974189def771ecae764ce5fddf commit 6d01d7d10b68d5974189def771ecae764ce5fddf Author: Dmitry Torokhov <dtor@chromium.org> Date: Fri May 19 00:57:18 2017 CHROMIUM: android: binder: get rid of BINDER_LOOPER_STATE_WAITING Instead of keeping track of threads that are in "wait for" state, let's wake up all of them when flushing deferred actions. Threads that are already woken up will see no difference and threads that are sleeping will be woken up. I believe this also fixes a bug: threads in binder_poll() will also get woken up now. BUG= chromium:718637 TEST=Boot on Caroline, play with Android apps Change-Id: Ic64d46c17bcef46f193587e29acbc32cce57ece4 Signed-off-by: Dmitry Torokhov <dtor@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/495767 Reviewed-by: Douglas Anderson <dianders@chromium.org> Reviewed-by: Sonny Rao <sonnyrao@chromium.org> (cherry picked from commit b1f939bcc931bbce22c54a5a190c8050cd7fc5a3) Reviewed-on: https://chromium-review.googlesource.com/508857 [modify] https://crrev.com/6d01d7d10b68d5974189def771ecae764ce5fddf/drivers/staging/android/binder.c
,
May 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/719b7b300f517a14e5d54877cd4c04ef17ad61ec commit 719b7b300f517a14e5d54877cd4c04ef17ad61ec Author: Dmitry Torokhov <dtor@chromium.org> Date: Fri May 19 00:57:21 2017 CHROMIUM: android: binder: reset BINDER_LOOPER_STATE_NEED_RETURN earlier Instead of resetting this flag after returning from ioctl, let's reset it right before we try to wait for event. This should help us drop the binder lock (in a subsequent patch) when thread is forcibly woken up. I believe it also fixes issue with binder_poll() immediately signalling data ready on a newly registered thread (because new threads set BINDER_LOOPER_STATE_NEED_RETURN when we create their objects). BUG= chromium:718637 TEST=Boot on Caroline, play with Android apps Change-Id: I50d10a4d23da3ead3fcc7f7032c459ab9bf21fb2 Signed-off-by: Dmitry Torokhov <dtor@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/495768 Reviewed-by: Douglas Anderson <dianders@chromium.org> (cherry picked from commit b4a91eae0934b89dca9d0e8b0ad61ed7e6511edf) Reviewed-on: https://chromium-review.googlesource.com/503461 (cherry picked from commit 1adb74a6d0ee3018b41dd0b70fd81ba9761478ad) Reviewed-on: https://chromium-review.googlesource.com/508486 [modify] https://crrev.com/719b7b300f517a14e5d54877cd4c04ef17ad61ec/drivers/android/binder.c
,
May 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/98056b5b884ec0ea5fda264d7fed7f87791b8cfc commit 98056b5b884ec0ea5fda264d7fed7f87791b8cfc Author: Dmitry Torokhov <dtor@chromium.org> Date: Fri May 19 00:57:25 2017 CHROMIUM: android: binder: push binder_lock down into ioctl ops Instead of taking binder lock and locating process thread data, even when neither is needed, let's push this down into individual handlers. BUG= chromium:718637 TEST=Boot on Caroline, play with Android apps Change-Id: I0aadd59968fa674225eb2fee58de43ec15a94bdb Signed-off-by: Dmitry Torokhov <dtor@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/495769 Reviewed-by: Douglas Anderson <dianders@chromium.org> Reviewed-by: Sonny Rao <sonnyrao@chromium.org> (cherry picked from commit 6580a647d51b5382c01d6da4b7ae62f27eb16122) Reviewed-on: https://chromium-review.googlesource.com/503462 (cherry picked from commit b3e87f79562691c78cc06c3dd551d017d5bf9d15) Reviewed-on: https://chromium-review.googlesource.com/508487 [modify] https://crrev.com/98056b5b884ec0ea5fda264d7fed7f87791b8cfc/drivers/android/binder.c
,
May 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/e3a7e0c2ae5ae381dff0bf9a58c60b84d647edaa commit e3a7e0c2ae5ae381dff0bf9a58c60b84d647edaa Author: Dmitry Torokhov <dtor@chromium.org> Date: Fri May 19 00:57:28 2017 CHROMIUM: android: binder: do not take binder lock when thread forcibly woken up When thread is forcibly woken up, it tries to acquire global binder lock to adjust some statistics. This causes trouble when binder lock is contended, for example during OOM kill process, where we can't kill threads because they wait on this lock. BUG= chromium:718637 TEST=Boot on Caroline, play with Android apps Change-Id: Ic26f20bef4a0db34e3a767a6026f685628b45421 Signed-off-by: Dmitry Torokhov <dtor@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/495770 Reviewed-by: Douglas Anderson <dianders@chromium.org> (cherry picked from commit 4c572220e433609bb950367155448d72b2561b59) Reviewed-on: https://chromium-review.googlesource.com/503463 (cherry picked from commit 1fee77f30d4bbfd3cb766d23f758bc3e82ab032b) Reviewed-on: https://chromium-review.googlesource.com/508948 [modify] https://crrev.com/e3a7e0c2ae5ae381dff0bf9a58c60b84d647edaa/drivers/android/binder.c
,
May 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/7428f4564766306256c5b077ea33a0984b4af578 commit 7428f4564766306256c5b077ea33a0984b4af578 Author: Dmitry Torokhov <dtor@chromium.org> Date: Fri May 19 00:57:31 2017 CHROMIUM: android: binder: reset BINDER_LOOPER_STATE_NEED_RETURN earlier Instead of resetting this flag after returning from ioctl, let's reset it right before we try to wait for event. This should help us drop the binder lock (in a subsequent patch) when thread is forcibly woken up. I believe it also fixes issue with binder_poll() immediately signalling data ready on a newly registered thread (because new threads set BINDER_LOOPER_STATE_NEED_RETURN when we create their objects). BUG= chromium:718637 TEST=Boot on Caroline, play with Android apps Change-Id: I50d10a4d23da3ead3fcc7f7032c459ab9bf21fb2 Signed-off-by: Dmitry Torokhov <dtor@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/495768 Reviewed-by: Douglas Anderson <dianders@chromium.org> (cherry picked from commit b4a91eae0934b89dca9d0e8b0ad61ed7e6511edf) Reviewed-on: https://chromium-review.googlesource.com/508858 [modify] https://crrev.com/7428f4564766306256c5b077ea33a0984b4af578/drivers/staging/android/binder.c
,
May 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/6aa71335998b7d39c180657933b0aa7bb3199417 commit 6aa71335998b7d39c180657933b0aa7bb3199417 Author: Dmitry Torokhov <dtor@chromium.org> Date: Fri May 19 00:57:40 2017 CHROMIUM: android: binder: push binder_lock down into ioctl ops Instead of taking binder lock and locating process thread data, even when neither is needed, let's push this down into individual handlers. BUG= chromium:718637 TEST=Boot on Caroline, play with Android apps Change-Id: I0aadd59968fa674225eb2fee58de43ec15a94bdb Signed-off-by: Dmitry Torokhov <dtor@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/495769 Reviewed-by: Douglas Anderson <dianders@chromium.org> Reviewed-by: Sonny Rao <sonnyrao@chromium.org> (cherry picked from commit 6580a647d51b5382c01d6da4b7ae62f27eb16122) Reviewed-on: https://chromium-review.googlesource.com/508859 [modify] https://crrev.com/6aa71335998b7d39c180657933b0aa7bb3199417/drivers/staging/android/binder.c
,
May 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/dac34f0bbdea9096b396a1839fcf681d13f35f35 commit dac34f0bbdea9096b396a1839fcf681d13f35f35 Author: Douglas Anderson <dianders@chromium.org> Date: Fri May 19 00:57:50 2017 CHROMIUM: android: binder: introduce binder_lock_interruptible When flushing/killing threads there is slight chance that thread is already waiting on binder lock in binder_thread_read(). Let's create binder_lock_interruptible() and use it there, so we can kick waiting thread and it will return a bit earlier, lessening ocntention on the binder lock. BUG= chromium:718637 TEST=Boot on Caroline, play with Android apps Change-Id: Ia8429bf5b07d133a0cb43e1cd17737e996740000 Signed-off-by: Douglas Anderson <dianders@chromium.org> Signed-off-by: Dmitry Torokhov <dtor@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/498008 (cherry picked from commit a5af8008287cbe5ed1fe0ee5ab60ad018c4adcdf) Reviewed-on: https://chromium-review.googlesource.com/503464 (cherry picked from commit b9593078bf763d08bd6f799ef5cf50d775923d3e) Reviewed-on: https://chromium-review.googlesource.com/508949 [modify] https://crrev.com/dac34f0bbdea9096b396a1839fcf681d13f35f35/drivers/android/binder.c
,
May 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/6edeb1c463c3ed622c3250368199fccf93c8e85c commit 6edeb1c463c3ed622c3250368199fccf93c8e85c Author: Dmitry Torokhov <dtor@chromium.org> Date: Fri May 19 00:57:54 2017 CHROMIUM: android: binder: do not create thread when trying to kill it Add an option to binder_get_thread() to forego creating a new thread and use it when processing request to kill thread. BUG= chromium:718637 TEST=Boot on Caroline, play with Android apps Change-Id: I9a8a426dcfab39b72e546c70e87e5dc1f1c2309c Signed-off-by: Dmitry Torokhov <dtor@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/498010 Reviewed-by: Douglas Anderson <dianders@chromium.org> (cherry picked from commit 77bbf31384b62831d6f4a6158d5c885c71c5be59) Reviewed-on: https://chromium-review.googlesource.com/503465 (cherry picked from commit 8947c6d9853ca41008003d48634636bfbc57de1b) Reviewed-on: https://chromium-review.googlesource.com/508950 [modify] https://crrev.com/6edeb1c463c3ed622c3250368199fccf93c8e85c/drivers/android/binder.c
,
May 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/5254a5eedc6c48a35f0ce5fdc3b74ceabb97c3e6 commit 5254a5eedc6c48a35f0ce5fdc3b74ceabb97c3e6 Author: Dmitry Torokhov <dtor@chromium.org> Date: Fri May 19 00:57:57 2017 CHROMIUM: android: binder: do not take binder lock when thread forcibly woken up When thread is forcibly woken up, it tries to acquire global binder lock to adjust some statistics. This causes trouble when binder lock is contended, for example during OOM kill process, where we can't kill threads because they wait on this lock. BUG= chromium:718637 TEST=Boot on Caroline, play with Android apps Change-Id: Ic26f20bef4a0db34e3a767a6026f685628b45421 Signed-off-by: Dmitry Torokhov <dtor@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/495770 Reviewed-by: Douglas Anderson <dianders@chromium.org> (cherry picked from commit 4c572220e433609bb950367155448d72b2561b59) Reviewed-on: https://chromium-review.googlesource.com/508860 [modify] https://crrev.com/5254a5eedc6c48a35f0ce5fdc3b74ceabb97c3e6/drivers/staging/android/binder.c
,
May 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/e79716411d01f03d4e72421054fca8b792a01b45 commit e79716411d01f03d4e72421054fca8b792a01b45 Author: Douglas Anderson <dianders@chromium.org> Date: Fri May 19 00:58:00 2017 CHROMIUM: android: binder: introduce binder_lock_interruptible When flushing/killing threads there is slight chance that thread is already waiting on binder lock in binder_thread_read(). Let's create binder_lock_interruptible() and use it there, so we can kick waiting thread and it will return a bit earlier, lessening ocntention on the binder lock. BUG= chromium:718637 TEST=Boot on Caroline, play with Android apps Change-Id: Ia8429bf5b07d133a0cb43e1cd17737e996740000 Signed-off-by: Douglas Anderson <dianders@chromium.org> Signed-off-by: Dmitry Torokhov <dtor@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/498008 (cherry picked from commit a5af8008287cbe5ed1fe0ee5ab60ad018c4adcdf) Reviewed-on: https://chromium-review.googlesource.com/508861 [modify] https://crrev.com/e79716411d01f03d4e72421054fca8b792a01b45/drivers/staging/android/binder.c
,
May 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/aff44a464772a7f37609400729e7eed9c8c86d7d commit aff44a464772a7f37609400729e7eed9c8c86d7d Author: Dmitry Torokhov <dtor@chromium.org> Date: Fri May 19 00:58:03 2017 CHROMIUM: android: binder: do not create thread when trying to kill it Add an option to binder_get_thread() to forego creating a new thread and use it when processing request to kill thread. BUG= chromium:718637 TEST=Boot on Caroline, play with Android apps Change-Id: I9a8a426dcfab39b72e546c70e87e5dc1f1c2309c Signed-off-by: Dmitry Torokhov <dtor@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/498010 Reviewed-by: Douglas Anderson <dianders@chromium.org> (cherry picked from commit 77bbf31384b62831d6f4a6158d5c885c71c5be59) Reviewed-on: https://chromium-review.googlesource.com/508862 [modify] https://crrev.com/aff44a464772a7f37609400729e7eed9c8c86d7d/drivers/staging/android/binder.c
,
May 19 2017
,
Jun 2 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/e28e7eb40fad680445cb4dc207fb9e604765f695 commit e28e7eb40fad680445cb4dc207fb9e604765f695 Author: Dmitry Torokhov <dtor@chromium.org> Date: Fri Jun 02 19:25:46 2017 CHROMIUM: android: binder: another use of binder_lock_interruptible When flushing/killing threads there is chance that we just fetched data from userspace and now waiting on the binder lock to be able to select a new thread and start submitting request. Convert it to binder_lock_interruptible(), so if the thread is selected by the OOM killer it will die quickly. BUG= chromium:718637 TEST=Boot on Caroline, play with Android apps Change-Id: Iff6126fea31f8db17255f368b95e1902ba90aaa9 Signed-off-by: Dmitry Torokhov <dtor@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/522233 Reviewed-by: Douglas Anderson <dianders@chromium.org> [modify] https://crrev.com/e28e7eb40fad680445cb4dc207fb9e604765f695/drivers/staging/android/binder.c
,
Jun 2 2017
Request merge of the above to at least R60. Wouldn't hurt in R59, but also not expected to be a panacea too so maybe not worth it? Pushing 4.4 at <https://chromium-review.googlesource.com/c/522928/>
,
Jun 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/e3f6010d84a00090a50605e4d0417c02b4a491c7 commit e3f6010d84a00090a50605e4d0417c02b4a491c7 Author: Dmitry Torokhov <dtor@chromium.org> Date: Sat Jun 03 01:39:42 2017 CHROMIUM: android: binder: another use of binder_lock_interruptible When flushing/killing threads there is chance that we just fetched data from userspace and now waiting on the binder lock to be able to select a new thread and start submitting request. Convert it to binder_lock_interruptible(), so if the thread is selected by the OOM killer it will die quickly. BUG= chromium:718637 TEST=Boot on Caroline, play with Android apps Change-Id: Iff6126fea31f8db17255f368b95e1902ba90aaa9 Signed-off-by: Dmitry Torokhov <dtor@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/522233 Reviewed-by: Douglas Anderson <dianders@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/522928 Commit-Ready: Douglas Anderson <dianders@chromium.org> Tested-by: Douglas Anderson <dianders@chromium.org> [modify] https://crrev.com/e3f6010d84a00090a50605e4d0417c02b4a491c7/drivers/android/binder.c
,
Jun 3 2017
Your change meets the bar and is auto-approved for M60. Please go ahead and merge the CL to branch 3112 manually. Please contact milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), josafat@(ChromeOS), bustamante@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jun 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/08dd8fe6bc74d2398144932031dba0d7820ca2c7 commit 08dd8fe6bc74d2398144932031dba0d7820ca2c7 Author: Dmitry Torokhov <dtor@chromium.org> Date: Mon Jun 05 15:30:32 2017 CHROMIUM: android: binder: another use of binder_lock_interruptible When flushing/killing threads there is chance that we just fetched data from userspace and now waiting on the binder lock to be able to select a new thread and start submitting request. Convert it to binder_lock_interruptible(), so if the thread is selected by the OOM killer it will die quickly. BUG= chromium:718637 TEST=Boot on Caroline, play with Android apps Change-Id: Iff6126fea31f8db17255f368b95e1902ba90aaa9 Signed-off-by: Dmitry Torokhov <dtor@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/522233 Reviewed-by: Douglas Anderson <dianders@chromium.org> (cherry picked from commit e28e7eb40fad680445cb4dc207fb9e604765f695) Reviewed-on: https://chromium-review.googlesource.com/524264 Tested-by: Douglas Anderson <dianders@chromium.org> [modify] https://crrev.com/08dd8fe6bc74d2398144932031dba0d7820ca2c7/drivers/staging/android/binder.c
,
Jun 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/9bac92997203b33533db584b1adc929745c7ed6f commit 9bac92997203b33533db584b1adc929745c7ed6f Author: Dmitry Torokhov <dtor@chromium.org> Date: Mon Jun 05 15:30:34 2017 CHROMIUM: android: binder: another use of binder_lock_interruptible When flushing/killing threads there is chance that we just fetched data from userspace and now waiting on the binder lock to be able to select a new thread and start submitting request. Convert it to binder_lock_interruptible(), so if the thread is selected by the OOM killer it will die quickly. BUG= chromium:718637 TEST=Boot on Caroline, play with Android apps Change-Id: Iff6126fea31f8db17255f368b95e1902ba90aaa9 Signed-off-by: Dmitry Torokhov <dtor@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/522233 Reviewed-by: Douglas Anderson <dianders@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/522928 Commit-Ready: Douglas Anderson <dianders@chromium.org> Tested-by: Douglas Anderson <dianders@chromium.org> (cherry picked from commit e3f6010d84a00090a50605e4d0417c02b4a491c7) Reviewed-on: https://chromium-review.googlesource.com/524263 [modify] https://crrev.com/9bac92997203b33533db584b1adc929745c7ed6f/drivers/android/binder.c
,
Jun 5 2017
Landed in R60. Up to dtor@ if he wants to request R59. - Pretty late for R59. ~ Probably won't help tons in R59, will just make one case of "refused to die" better. + Will significantly reduce the number of "Binder refused to die", which might help avoid confusion when analyzing crashes.
,
Jun 5 2017
Yes, I do want it on 59: the "meat" of the change is already exercised well enough on the exiting of the function in question, we are just extending it to cover entering as well.
,
Jun 5 2017
This bug requires manual review: We are only 0 days from stable. Please contact the milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jun 7 2017
Grace to confirm for M59
,
Jun 8 2017
,
Jun 9 2017
59 just went stable. Unless there's an urgent reason for this merge, please target M60.
,
Jun 21 2017
,
Jun 24 2017
,
Aug 3
|
|||||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||||
Comment 1 by semenzato@chromium.org
, May 4 2017