Skip to contentSkip to conversation

binder: print warnings when detecting oneway spamming.

PATCH Superseded Martijn Coenen 2 participants 3 messages Aug 20, 2020 Android drivers

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 binder: print warnings when detecting oneway spamming.

The most common cause of the binder transaction buffer filling up is a client rapidly firing oneway transactions into a process, before it has a chance to handle them. Yet the root cause of this is often hard to debug, because either the system or the app will stop, and by that time binder debug information we dump in bugreports is no longer relevant.

This change warns as soon as a process dips below 80% of its oneway space (less than 100kB available in the configuration), when any one process is responsible for either more than 50 transactions, or more than 50% of the oneway space.

---
 drivers/android/binder.c       |  2 +-
 drivers/android/binder_alloc.c | 49 +++++++++++++++++++++++++++++++---
 drivers/android/binder_alloc.h |  5 +++-
 3 files changed, 51 insertions(+), 5 deletions(-)
drivers/android/binder.c +1 -1
index f936530a19b0..946332bc871a 100644
@@ -3136,7 +3136,7 @@ static void binder_transaction(struct binder_proc *proc,
 
 	t->buffer = binder_alloc_new_buf(&target_proc->alloc, tr->data_size,
 		tr->offsets_size, extra_buffers_size,
-		!reply && (t->flags & TF_ONE_WAY));
+		!reply && (t->flags & TF_ONE_WAY), current->tgid);
 	if (IS_ERR(t->buffer)) {
 		/*
 		 * -ESRCH indicates VMA cleared. The target is dying.
drivers/android/binder_alloc.c +46 -3

88 lines not on this page · raw message

drivers/android/binder_alloc.h +4 -1
index db9c1b984695..55d8b4106766 100644
@@ -32,6 +32,7 @@ struct binder_transaction;
  * @offsets_size:       size of array of offsets
  * @extra_buffers_size: size of space for other objects (like sg lists)
  * @user_data:          user pointer to base of buffer space
+ * @pid:                pid to attribute the buffer to (caller)
  *
  * Bookkeeping structure for binder transaction buffers
  */
@@ -51,6 +52,7 @@ struct binder_buffer {
 	size_t offsets_size;
 	size_t extra_buffers_size;
 	void __user *user_data;
+	int    pid;
 };
 
 /**
@@ -117,7 +119,8 @@ extern struct binder_buffer *binder_alloc_new_buf(struct binder_alloc *alloc,
 						  size_t data_size,
 						  size_t offsets_size,
 						  size_t extra_buffers_size,
-						  int is_async);
+						  int is_async,
+						  int pid);
 extern void binder_alloc_init(struct binder_alloc *alloc);
 extern int binder_alloc_shrinker_init(void);
 extern void binder_alloc_vma_close(struct binder_alloc *alloc);
Signature · 1 line
2.28.0.220.ged08abb693-goog
#1 lore raw
I love your patch! Yet something to improve:

Hi Martijn,

I love your patch! Yet something to improve:

[auto build test ERROR on staging/staging-testing]
[also build test ERROR on v5.9-rc1 next-20200820]
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Martijn-Coenen/binder-print-warnings-when-detecting-oneway-spamming/20200820-155358
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git bc752d2f345bf55d71b3422a6a24890ea03168dc
config: s390-randconfig-r002-20200818 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 4deda57106f7c9b982a49cb907c33e3966c8de7f)

reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# install s390 cross compiling tool for clang build
# apt-get install binutils-s390x-linux-gnu
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=s390

If you fix the issue, kindly add following tag as appropriate

All errors (new ones prefixed by >>):

>> drivers/android/binder_alloc_selftest.c:122:61: error: too few arguments to function call, expected 6, have 5
                   buffers[i] = binder_alloc_new_buf(alloc, sizes[i], 0, 0, 0);
                                ~~~~~~~~~~~~~~~~~~~~                         ^
   drivers/android/binder_alloc.h:118:30: note: 'binder_alloc_new_buf' declared here
   extern struct binder_buffer *binder_alloc_new_buf(struct binder_alloc *alloc,
                                ^

1 error generated.

# https://github.com/0day-ci/linux/commit/9d0b269f4468d6793f6fd76a410fdde39dbf6ac2
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Martijn-Coenen/binder-print-warnings-when-detecting-oneway-spamming/20200820-155358 git checkout 9d0b269f4468d6793f6fd76a410fdde39dbf6ac2
vim +122 drivers/android/binder_alloc_selftest.c

4175e2b46fd4b9 Sherry Yang 2017-08-23  114  
4175e2b46fd4b9 Sherry Yang 2017-08-23  115  static void binder_selftest_alloc_buf(struct binder_alloc *alloc,
4175e2b46fd4b9 Sherry Yang 2017-08-23  116  				      struct binder_buffer *buffers[],
4175e2b46fd4b9 Sherry Yang 2017-08-23  117  				      size_t *sizes, int *seq)
4175e2b46fd4b9 Sherry Yang 2017-08-23  118  {
4175e2b46fd4b9 Sherry Yang 2017-08-23  119  	int i;
4175e2b46fd4b9 Sherry Yang 2017-08-23  120  
4175e2b46fd4b9 Sherry Yang 2017-08-23  121  	for (i = 0; i < BUFFER_NUM; i++) {
4175e2b46fd4b9 Sherry Yang 2017-08-23 @122  		buffers[i] = binder_alloc_new_buf(alloc, sizes[i], 0, 0, 0);
4175e2b46fd4b9 Sherry Yang 2017-08-23  123  		if (IS_ERR(buffers[i]) ||
4175e2b46fd4b9 Sherry Yang 2017-08-23  124  		    !check_buffer_pages_allocated(alloc, buffers[i],
4175e2b46fd4b9 Sherry Yang 2017-08-23  125  						  sizes[i])) {
4175e2b46fd4b9 Sherry Yang 2017-08-23  126  			pr_err_size_seq(sizes, seq);
4175e2b46fd4b9 Sherry Yang 2017-08-23  127  			binder_selftest_failures++;
4175e2b46fd4b9 Sherry Yang 2017-08-23  128  		}
4175e2b46fd4b9 Sherry Yang 2017-08-23  129  	}
4175e2b46fd4b9 Sherry Yang 2017-08-23  130  }

4175e2b46fd4b9 Sherry Yang 2017-08-23 131

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]

#2 lore raw
Quoted 23 lines · kernel test robot

Hi Martijn,

I love your patch! Yet something to improve:

[auto build test ERROR on staging/staging-testing]
[also build test ERROR on v5.9-rc1 next-20200820]
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Martijn-Coenen/binder-print-warnings-when-detecting-oneway-spamming/20200820-155358
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git bc752d2f345bf55d71b3422a6a24890ea03168dc
config: s390-randconfig-r002-20200818 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 4deda57106f7c9b982a49cb907c33e3966c8de7f)

reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# install s390 cross compiling tool for clang build
# apt-get install binutils-s390x-linux-gnu
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=s390

Quoted 7 lines

If you fix the issue, kindly add following tag as appropriate

All errors (new ones prefixed by >>):

Quoted 1 line
Quoted 1 line

drivers/android/binder_alloc_selftest.c:122:61: error: too few arguments to function call, expected 6, have 5

                   buffers[i] = binder_alloc_new_buf(alloc, sizes[i], 0, 0, 0);

missed this call-site, will send v2.

Martijn

Quoted 34 lines
                                ~~~~~~~~~~~~~~~~~~~~                         ^
   drivers/android/binder_alloc.h:118:30: note: 'binder_alloc_new_buf' declared here
   extern struct binder_buffer *binder_alloc_new_buf(struct binder_alloc *alloc,
                                ^

1 error generated.

# https://github.com/0day-ci/linux/commit/9d0b269f4468d6793f6fd76a410fdde39dbf6ac2
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Martijn-Coenen/binder-print-warnings-when-detecting-oneway-spamming/20200820-155358 git checkout 9d0b269f4468d6793f6fd76a410fdde39dbf6ac2
vim +122 drivers/android/binder_alloc_selftest.c

4175e2b46fd4b9 Sherry Yang 2017-08-23  114
4175e2b46fd4b9 Sherry Yang 2017-08-23  115  static void binder_selftest_alloc_buf(struct binder_alloc *alloc,
4175e2b46fd4b9 Sherry Yang 2017-08-23  116                                    struct binder_buffer *buffers[],
4175e2b46fd4b9 Sherry Yang 2017-08-23  117                                    size_t *sizes, int *seq)
4175e2b46fd4b9 Sherry Yang 2017-08-23  118  {
4175e2b46fd4b9 Sherry Yang 2017-08-23  119      int i;
4175e2b46fd4b9 Sherry Yang 2017-08-23  120
4175e2b46fd4b9 Sherry Yang 2017-08-23  121      for (i = 0; i < BUFFER_NUM; i++) {
4175e2b46fd4b9 Sherry Yang 2017-08-23 @122              buffers[i] = binder_alloc_new_buf(alloc, sizes[i], 0, 0, 0);
4175e2b46fd4b9 Sherry Yang 2017-08-23  123              if (IS_ERR(buffers[i]) ||
4175e2b46fd4b9 Sherry Yang 2017-08-23  124                  !check_buffer_pages_allocated(alloc, buffers[i],
4175e2b46fd4b9 Sherry Yang 2017-08-23  125                                                sizes[i])) {
4175e2b46fd4b9 Sherry Yang 2017-08-23  126                      pr_err_size_seq(sizes, seq);
4175e2b46fd4b9 Sherry Yang 2017-08-23  127                      binder_selftest_failures++;
4175e2b46fd4b9 Sherry Yang 2017-08-23  128              }
4175e2b46fd4b9 Sherry Yang 2017-08-23  129      }
4175e2b46fd4b9 Sherry Yang 2017-08-23  130  }
4175e2b46fd4b9 Sherry Yang 2017-08-23  131

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org