* [PATCH 0/4] Add binder state and statistics to binderfs
@ 2019-08-27 20:41 Hridya Valsaraju
2019-08-27 20:41 ` [PATCH 1/4] binder: add a mount option to show global stats Hridya Valsaraju
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: Hridya Valsaraju @ 2019-08-27 20:41 UTC (permalink / raw)
To: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos,
Martijn Coenen, Joel Fernandes, Christian Brauner, devel,
linux-kernel
Cc: kernel-team, Hridya Valsaraju
Currently, the only way to access binder state and
statistics is through debugfs. We need a way to
access the same even when debugfs is not mounted.
These patches add a mount option to make this
information available in binderfs without affecting
its presence in debugfs. The following debugfs nodes
will be made available in a binderfs instance when
mounted with the mount option 'stats=global' or 'stats=local'.
/sys/kernel/debug/binder/failed_transaction_log
/sys/kernel/debug/binder/proc
/sys/kernel/debug/binder/state
/sys/kernel/debug/binder/stats
/sys/kernel/debug/binder/transaction_log
/sys/kernel/debug/binder/transactions
Hridya Valsaraju (4):
binder: add a mount option to show global stats
binder: Add stats, state and transactions files
binder: Make transaction_log available in binderfs
binder: Add binder_proc logging to binderfs
drivers/android/binder.c | 87 +++++-----
drivers/android/binder_internal.h | 84 ++++++++++
drivers/android/binderfs.c | 256 ++++++++++++++++++++++++++----
3 files changed, 355 insertions(+), 72 deletions(-)
--
2.23.0.187.g17f5b7556c-goog
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/4] binder: add a mount option to show global stats
2019-08-27 20:41 [PATCH 0/4] Add binder state and statistics to binderfs Hridya Valsaraju
@ 2019-08-27 20:41 ` Hridya Valsaraju
2019-08-28 9:22 ` Greg Kroah-Hartman
2019-08-27 20:41 ` [PATCH 2/4] binder: Add stats, state and transactions files Hridya Valsaraju
` (2 subsequent siblings)
3 siblings, 1 reply; 15+ messages in thread
From: Hridya Valsaraju @ 2019-08-27 20:41 UTC (permalink / raw)
To: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos,
Martijn Coenen, Joel Fernandes, Christian Brauner, devel,
linux-kernel
Cc: kernel-team, Hridya Valsaraju
Currently, all binder state and statistics live in debugfs.
We need this information even when debugfs is not mounted.
This patch adds the mount option 'stats' to enable a binderfs
instance to have binder debug information present in the same.
'stats=global' will enable the global binder statistics. In
the future, 'stats=local' will enable binder statistics local
to the binderfs instance. The two modes 'global' and 'local'
will be mutually exclusive. 'stats=global' option is only available
for a binderfs instance mounted in the initial user namespace.
An attempt to use the option to mount a binderfs instance in
another user namespace will return an EPERM error.
Signed-off-by: Hridya Valsaraju <hridya@google.com>
---
drivers/android/binderfs.c | 47 ++++++++++++++++++++++++++++++++++++--
1 file changed, 45 insertions(+), 2 deletions(-)
diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
index cc2e71576396..d95d179aec58 100644
--- a/drivers/android/binderfs.c
+++ b/drivers/android/binderfs.c
@@ -51,18 +51,27 @@ static DEFINE_IDA(binderfs_minors);
/**
* binderfs_mount_opts - mount options for binderfs
* @max: maximum number of allocatable binderfs binder devices
+ * @stats_mode: enable binder stats in binderfs.
*/
struct binderfs_mount_opts {
int max;
+ int stats_mode;
};
enum {
Opt_max,
+ Opt_stats_mode,
Opt_err
};
+enum binderfs_stats_mode {
+ STATS_NONE,
+ STATS_GLOBAL,
+};
+
static const match_table_t tokens = {
{ Opt_max, "max=%d" },
+ { Opt_stats_mode, "stats=%s" },
{ Opt_err, NULL }
};
@@ -290,8 +299,9 @@ static void binderfs_evict_inode(struct inode *inode)
static int binderfs_parse_mount_opts(char *data,
struct binderfs_mount_opts *opts)
{
- char *p;
+ char *p, *stats;
opts->max = BINDERFS_MAX_MINOR;
+ opts->stats_mode = STATS_NONE;
while ((p = strsep(&data, ",")) != NULL) {
substring_t args[MAX_OPT_ARGS];
@@ -311,6 +321,24 @@ static int binderfs_parse_mount_opts(char *data,
opts->max = max_devices;
break;
+ case Opt_stats_mode:
+ stats = match_strdup(&args[0]);
+ if (!stats)
+ return -ENOMEM;
+
+ if (strcmp(stats, "global") != 0) {
+ kfree(stats);
+ return -EINVAL;
+ }
+
+ if (!capable(CAP_SYS_ADMIN)) {
+ kfree(stats);
+ return -EINVAL;
+ }
+
+ opts->stats_mode = STATS_GLOBAL;
+ kfree(stats);
+ break;
default:
pr_err("Invalid mount options\n");
return -EINVAL;
@@ -322,8 +350,21 @@ static int binderfs_parse_mount_opts(char *data,
static int binderfs_remount(struct super_block *sb, int *flags, char *data)
{
+ int prev_stats_mode, ret;
struct binderfs_info *info = sb->s_fs_info;
- return binderfs_parse_mount_opts(data, &info->mount_opts);
+
+ prev_stats_mode = info->mount_opts.stats_mode;
+ ret = binderfs_parse_mount_opts(data, &info->mount_opts);
+ if (ret)
+ return ret;
+
+ if (prev_stats_mode != info->mount_opts.stats_mode) {
+ pr_info("Binderfs stats mode cannot be changed during a remount\n");
+ info->mount_opts.stats_mode = prev_stats_mode;
+ return -EINVAL;
+ }
+
+ return 0;
}
static int binderfs_show_mount_opts(struct seq_file *seq, struct dentry *root)
@@ -333,6 +374,8 @@ static int binderfs_show_mount_opts(struct seq_file *seq, struct dentry *root)
info = root->d_sb->s_fs_info;
if (info->mount_opts.max <= BINDERFS_MAX_MINOR)
seq_printf(seq, ",max=%d", info->mount_opts.max);
+ if (info->mount_opts.stats_mode == STATS_GLOBAL)
+ seq_printf(seq, ",stats=global");
return 0;
}
--
2.23.0.187.g17f5b7556c-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/4] binder: Add stats, state and transactions files
2019-08-27 20:41 [PATCH 0/4] Add binder state and statistics to binderfs Hridya Valsaraju
2019-08-27 20:41 ` [PATCH 1/4] binder: add a mount option to show global stats Hridya Valsaraju
@ 2019-08-27 20:41 ` Hridya Valsaraju
2019-08-28 12:58 ` Christian Brauner
2019-08-27 20:41 ` [PATCH 3/4] binder: Make transaction_log available in binderfs Hridya Valsaraju
2019-08-27 20:41 ` [PATCH 4/4] binder: Add binder_proc logging to binderfs Hridya Valsaraju
3 siblings, 1 reply; 15+ messages in thread
From: Hridya Valsaraju @ 2019-08-27 20:41 UTC (permalink / raw)
To: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos,
Martijn Coenen, Joel Fernandes, Christian Brauner, devel,
linux-kernel
Cc: kernel-team, Hridya Valsaraju
The following binder stat files currently live in debugfs.
/sys/kernel/debug/binder/state
/sys/kernel/debug/binder/stats
/sys/kernel/debug/binder/transactions
This patch makes these files available in a binderfs instance
mounted with the mount option 'stats=global'. For example, if a binderfs
instance is mounted at path /dev/binderfs, the above files will be
available at the following locations:
/dev/binderfs/binder_logs/state
/dev/binderfs/binder_logs/stats
/dev/binderfs/binder_logs/transactions
This provides a way to access them even when debugfs is not mounted.
Signed-off-by: Hridya Valsaraju <hridya@google.com>
---
drivers/android/binder.c | 15 ++--
drivers/android/binder_internal.h | 8 ++
drivers/android/binderfs.c | 137 +++++++++++++++++++++++++++++-
3 files changed, 150 insertions(+), 10 deletions(-)
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index ca6b21a53321..de795bd229c4 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -6055,7 +6055,7 @@ static void print_binder_proc_stats(struct seq_file *m,
}
-static int state_show(struct seq_file *m, void *unused)
+int binder_state_show(struct seq_file *m, void *unused)
{
struct binder_proc *proc;
struct binder_node *node;
@@ -6094,7 +6094,7 @@ static int state_show(struct seq_file *m, void *unused)
return 0;
}
-static int stats_show(struct seq_file *m, void *unused)
+int binder_stats_show(struct seq_file *m, void *unused)
{
struct binder_proc *proc;
@@ -6110,7 +6110,7 @@ static int stats_show(struct seq_file *m, void *unused)
return 0;
}
-static int transactions_show(struct seq_file *m, void *unused)
+int binder_transactions_show(struct seq_file *m, void *unused)
{
struct binder_proc *proc;
@@ -6198,9 +6198,6 @@ const struct file_operations binder_fops = {
.release = binder_release,
};
-DEFINE_SHOW_ATTRIBUTE(state);
-DEFINE_SHOW_ATTRIBUTE(stats);
-DEFINE_SHOW_ATTRIBUTE(transactions);
DEFINE_SHOW_ATTRIBUTE(transaction_log);
static int __init init_binder_device(const char *name)
@@ -6256,17 +6253,17 @@ static int __init binder_init(void)
0444,
binder_debugfs_dir_entry_root,
NULL,
- &state_fops);
+ &binder_state_fops);
debugfs_create_file("stats",
0444,
binder_debugfs_dir_entry_root,
NULL,
- &stats_fops);
+ &binder_stats_fops);
debugfs_create_file("transactions",
0444,
binder_debugfs_dir_entry_root,
NULL,
- &transactions_fops);
+ &binder_transactions_fops);
debugfs_create_file("transaction_log",
0444,
binder_debugfs_dir_entry_root,
diff --git a/drivers/android/binder_internal.h b/drivers/android/binder_internal.h
index fe8c745dc8e0..12ef96f256c6 100644
--- a/drivers/android/binder_internal.h
+++ b/drivers/android/binder_internal.h
@@ -57,4 +57,12 @@ static inline int __init init_binderfs(void)
}
#endif
+int binder_stats_show(struct seq_file *m, void *unused);
+DEFINE_SHOW_ATTRIBUTE(binder_stats);
+
+int binder_state_show(struct seq_file *m, void *unused);
+DEFINE_SHOW_ATTRIBUTE(binder_state);
+
+int binder_transactions_show(struct seq_file *m, void *unused);
+DEFINE_SHOW_ATTRIBUTE(binder_transactions);
#endif /* _LINUX_BINDER_INTERNAL_H */
diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
index d95d179aec58..d542f9b8d8ab 100644
--- a/drivers/android/binderfs.c
+++ b/drivers/android/binderfs.c
@@ -280,7 +280,7 @@ static void binderfs_evict_inode(struct inode *inode)
clear_inode(inode);
- if (!device)
+ if (!device || S_ISREG(inode->i_mode))
return;
mutex_lock(&binderfs_minors_mutex);
@@ -504,6 +504,138 @@ static const struct inode_operations binderfs_dir_inode_operations = {
.unlink = binderfs_unlink,
};
+static struct inode *binderfs_make_inode(struct super_block *sb, int mode)
+{
+ struct inode *ret;
+
+ ret = new_inode(sb);
+ if (ret) {
+ ret->i_ino = iunique(sb, BINDERFS_MAX_MINOR + INODE_OFFSET);
+ ret->i_mode = mode;
+ ret->i_atime = ret->i_mtime = ret->i_ctime = current_time(ret);
+ }
+ return ret;
+}
+
+static struct dentry *binderfs_create_dentry(struct dentry *dir,
+ const char *name)
+{
+ struct dentry *dentry;
+
+ dentry = lookup_one_len(name, dir, strlen(name));
+ if (IS_ERR(dentry))
+ return dentry;
+
+ /* Return error if the file/dir already exists. */
+ if (d_really_is_positive(dentry)) {
+ dput(dentry);
+ return ERR_PTR(-EEXIST);
+ }
+
+ return dentry;
+}
+
+static struct dentry *binderfs_create_file(struct dentry *dir, const char *name,
+ const struct file_operations *fops,
+ void *data)
+{
+ struct dentry *dentry;
+ struct inode *new_inode, *dir_inode;
+ struct super_block *sb;
+
+ dir_inode = dir->d_inode;
+ inode_lock(dir_inode);
+
+ dentry = binderfs_create_dentry(dir, name);
+ if (IS_ERR(dentry))
+ goto out;
+
+ sb = dir_inode->i_sb;
+ new_inode = binderfs_make_inode(sb, S_IFREG | 0444);
+ if (!new_inode) {
+ dput(dentry);
+ dentry = ERR_PTR(-ENOMEM);
+ goto out;
+ }
+
+ new_inode->i_fop = fops;
+ new_inode->i_private = data;
+ d_instantiate(dentry, new_inode);
+ fsnotify_create(dir_inode, dentry);
+
+out:
+ inode_unlock(dir_inode);
+ return dentry;
+}
+
+static struct dentry *binderfs_create_dir(struct dentry *parent,
+ const char *name)
+{
+ struct dentry *dentry;
+ struct inode *new_inode, *parent_inode;
+ struct super_block *sb;
+
+ parent_inode = d_inode(parent);
+ inode_lock(parent_inode);
+
+ dentry = binderfs_create_dentry(parent, name);
+ if (IS_ERR(dentry))
+ goto out;
+
+ sb = parent_inode->i_sb;
+ new_inode = binderfs_make_inode(sb, S_IFDIR | 0755);
+ if (!new_inode) {
+ dput(dentry);
+ dentry = ERR_PTR(-ENOMEM);
+ goto out;
+ }
+
+ new_inode->i_fop = &simple_dir_operations;
+ new_inode->i_op = &simple_dir_inode_operations;
+
+ inc_nlink(new_inode);
+ d_instantiate(dentry, new_inode);
+ inc_nlink(parent_inode);
+ fsnotify_mkdir(parent_inode, dentry);
+out:
+ inode_unlock(parent_inode);
+ return dentry;
+}
+
+static int init_binder_logs(struct super_block *sb)
+{
+ struct dentry *binder_logs_root_dir, *file_dentry;
+ int ret = 0;
+
+ binder_logs_root_dir = binderfs_create_dir(sb->s_root,
+ "binder_logs");
+ if (IS_ERR(binder_logs_root_dir)) {
+ ret = PTR_ERR(binder_logs_root_dir);
+ goto out;
+ }
+
+ file_dentry = binderfs_create_file(binder_logs_root_dir, "stats",
+ &binder_stats_fops, NULL);
+ if (IS_ERR(file_dentry)) {
+ ret = PTR_ERR(file_dentry);
+ goto out;
+ }
+
+ file_dentry = binderfs_create_file(binder_logs_root_dir, "state",
+ &binder_state_fops, NULL);
+ if (IS_ERR(file_dentry)) {
+ ret = PTR_ERR(file_dentry);
+ goto out;
+ }
+
+ file_dentry = binderfs_create_file(binder_logs_root_dir, "transactions",
+ &binder_transactions_fops, NULL);
+ if (IS_ERR(file_dentry))
+ ret = PTR_ERR(file_dentry);
+out:
+ return ret;
+}
+
static int binderfs_fill_super(struct super_block *sb, void *data, int silent)
{
int ret;
@@ -582,6 +714,9 @@ static int binderfs_fill_super(struct super_block *sb, void *data, int silent)
}
+ if (info->mount_opts.stats_mode == STATS_GLOBAL)
+ return init_binder_logs(sb);
+
return 0;
}
--
2.23.0.187.g17f5b7556c-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/4] binder: Make transaction_log available in binderfs
2019-08-27 20:41 [PATCH 0/4] Add binder state and statistics to binderfs Hridya Valsaraju
2019-08-27 20:41 ` [PATCH 1/4] binder: add a mount option to show global stats Hridya Valsaraju
2019-08-27 20:41 ` [PATCH 2/4] binder: Add stats, state and transactions files Hridya Valsaraju
@ 2019-08-27 20:41 ` Hridya Valsaraju
2019-08-28 12:59 ` Christian Brauner
2019-08-27 20:41 ` [PATCH 4/4] binder: Add binder_proc logging to binderfs Hridya Valsaraju
3 siblings, 1 reply; 15+ messages in thread
From: Hridya Valsaraju @ 2019-08-27 20:41 UTC (permalink / raw)
To: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos,
Martijn Coenen, Joel Fernandes, Christian Brauner, devel,
linux-kernel
Cc: kernel-team, Hridya Valsaraju
Currently, the binder transaction log files 'transaction_log'
and 'failed_transaction_log' live in debugfs at the following locations:
/sys/kernel/debug/binder/failed_transaction_log
/sys/kernel/debug/binder/transaction_log
This patch makes these files also available in a binderfs instance
mounted with the mount option "stats=global".
It does not affect the presence of these files in debugfs.
If a binderfs instance is mounted at path /dev/binderfs, the location of
these files will be as follows:
/dev/binderfs/binder_logs/failed_transaction_log
/dev/binderfs/binder_logs/transaction_log
This change provides an alternate option to access these files when
debugfs is not mounted.
Signed-off-by: Hridya Valsaraju <hridya@google.com>
---
drivers/android/binder.c | 34 +++++--------------------------
drivers/android/binder_internal.h | 30 +++++++++++++++++++++++++++
drivers/android/binderfs.c | 19 +++++++++++++++++
3 files changed, 54 insertions(+), 29 deletions(-)
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index de795bd229c4..bed217310197 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -197,30 +197,8 @@ static inline void binder_stats_created(enum binder_stat_types type)
atomic_inc(&binder_stats.obj_created[type]);
}
-struct binder_transaction_log_entry {
- int debug_id;
- int debug_id_done;
- int call_type;
- int from_proc;
- int from_thread;
- int target_handle;
- int to_proc;
- int to_thread;
- int to_node;
- int data_size;
- int offsets_size;
- int return_error_line;
- uint32_t return_error;
- uint32_t return_error_param;
- const char *context_name;
-};
-struct binder_transaction_log {
- atomic_t cur;
- bool full;
- struct binder_transaction_log_entry entry[32];
-};
-static struct binder_transaction_log binder_transaction_log;
-static struct binder_transaction_log binder_transaction_log_failed;
+struct binder_transaction_log binder_transaction_log;
+struct binder_transaction_log binder_transaction_log_failed;
static struct binder_transaction_log_entry *binder_transaction_log_add(
struct binder_transaction_log *log)
@@ -6166,7 +6144,7 @@ static void print_binder_transaction_log_entry(struct seq_file *m,
"\n" : " (incomplete)\n");
}
-static int transaction_log_show(struct seq_file *m, void *unused)
+int binder_transaction_log_show(struct seq_file *m, void *unused)
{
struct binder_transaction_log *log = m->private;
unsigned int log_cur = atomic_read(&log->cur);
@@ -6198,8 +6176,6 @@ const struct file_operations binder_fops = {
.release = binder_release,
};
-DEFINE_SHOW_ATTRIBUTE(transaction_log);
-
static int __init init_binder_device(const char *name)
{
int ret;
@@ -6268,12 +6244,12 @@ static int __init binder_init(void)
0444,
binder_debugfs_dir_entry_root,
&binder_transaction_log,
- &transaction_log_fops);
+ &binder_transaction_log_fops);
debugfs_create_file("failed_transaction_log",
0444,
binder_debugfs_dir_entry_root,
&binder_transaction_log_failed,
- &transaction_log_fops);
+ &binder_transaction_log_fops);
}
if (!IS_ENABLED(CONFIG_ANDROID_BINDERFS) &&
diff --git a/drivers/android/binder_internal.h b/drivers/android/binder_internal.h
index 12ef96f256c6..b9be42d9464c 100644
--- a/drivers/android/binder_internal.h
+++ b/drivers/android/binder_internal.h
@@ -65,4 +65,34 @@ DEFINE_SHOW_ATTRIBUTE(binder_state);
int binder_transactions_show(struct seq_file *m, void *unused);
DEFINE_SHOW_ATTRIBUTE(binder_transactions);
+
+int binder_transaction_log_show(struct seq_file *m, void *unused);
+DEFINE_SHOW_ATTRIBUTE(binder_transaction_log);
+
+struct binder_transaction_log_entry {
+ int debug_id;
+ int debug_id_done;
+ int call_type;
+ int from_proc;
+ int from_thread;
+ int target_handle;
+ int to_proc;
+ int to_thread;
+ int to_node;
+ int data_size;
+ int offsets_size;
+ int return_error_line;
+ uint32_t return_error;
+ uint32_t return_error_param;
+ const char *context_name;
+};
+
+struct binder_transaction_log {
+ atomic_t cur;
+ bool full;
+ struct binder_transaction_log_entry entry[32];
+};
+
+extern struct binder_transaction_log binder_transaction_log;
+extern struct binder_transaction_log binder_transaction_log_failed;
#endif /* _LINUX_BINDER_INTERNAL_H */
diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
index d542f9b8d8ab..dc25a7d759c9 100644
--- a/drivers/android/binderfs.c
+++ b/drivers/android/binderfs.c
@@ -630,8 +630,27 @@ static int init_binder_logs(struct super_block *sb)
file_dentry = binderfs_create_file(binder_logs_root_dir, "transactions",
&binder_transactions_fops, NULL);
+ if (IS_ERR(file_dentry)) {
+ ret = PTR_ERR(file_dentry);
+ goto out;
+ }
+
+ file_dentry = binderfs_create_file(binder_logs_root_dir,
+ "transaction_log",
+ &binder_transaction_log_fops,
+ &binder_transaction_log);
+ if (IS_ERR(file_dentry)) {
+ ret = PTR_ERR(file_dentry);
+ goto out;
+ }
+
+ file_dentry = binderfs_create_file(binder_logs_root_dir,
+ "failed_transaction_log",
+ &binder_transaction_log_fops,
+ &binder_transaction_log_failed);
if (IS_ERR(file_dentry))
ret = PTR_ERR(file_dentry);
+
out:
return ret;
}
--
2.23.0.187.g17f5b7556c-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 4/4] binder: Add binder_proc logging to binderfs
2019-08-27 20:41 [PATCH 0/4] Add binder state and statistics to binderfs Hridya Valsaraju
` (2 preceding siblings ...)
2019-08-27 20:41 ` [PATCH 3/4] binder: Make transaction_log available in binderfs Hridya Valsaraju
@ 2019-08-27 20:41 ` Hridya Valsaraju
2019-08-28 13:08 ` Christian Brauner
3 siblings, 1 reply; 15+ messages in thread
From: Hridya Valsaraju @ 2019-08-27 20:41 UTC (permalink / raw)
To: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos,
Martijn Coenen, Joel Fernandes, Christian Brauner, devel,
linux-kernel
Cc: kernel-team, Hridya Valsaraju
Currently /sys/kernel/debug/binder/proc contains
the debug data for every binder_proc instance.
This patch makes this information also available
in a binderfs instance mounted with a mount option
"stats=global" in addition to debugfs. The patch does
not affect the presence of the file in debugfs.
If a binderfs instance is mounted at path /dev/binderfs,
this file would be present at /dev/binderfs/binder_logs/proc.
This change provides an alternate way to access this file when debugfs
is not mounted.
Signed-off-by: Hridya Valsaraju <hridya@google.com>
---
drivers/android/binder.c | 38 ++++++++++++++++++-
drivers/android/binder_internal.h | 46 ++++++++++++++++++++++
drivers/android/binderfs.c | 63 ++++++++++++++-----------------
3 files changed, 111 insertions(+), 36 deletions(-)
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index bed217310197..37189838f32a 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -481,6 +481,7 @@ struct binder_priority {
* @inner_lock: can nest under outer_lock and/or node lock
* @outer_lock: no nesting under innor or node lock
* Lock order: 1) outer, 2) node, 3) inner
+ * @binderfs_entry: process-specific binderfs log file
*
* Bookkeeping structure for binder processes
*/
@@ -510,6 +511,7 @@ struct binder_proc {
struct binder_context *context;
spinlock_t inner_lock;
spinlock_t outer_lock;
+ struct dentry *binderfs_entry;
};
enum {
@@ -5347,6 +5349,8 @@ static int binder_open(struct inode *nodp, struct file *filp)
{
struct binder_proc *proc;
struct binder_device *binder_dev;
+ struct binderfs_info *info;
+ struct dentry *binder_binderfs_dir_entry_proc = NULL;
binder_debug(BINDER_DEBUG_OPEN_CLOSE, "%s: %d:%d\n", __func__,
current->group_leader->pid, current->pid);
@@ -5368,11 +5372,14 @@ static int binder_open(struct inode *nodp, struct file *filp)
}
/* binderfs stashes devices in i_private */
- if (is_binderfs_device(nodp))
+ if (is_binderfs_device(nodp)) {
binder_dev = nodp->i_private;
- else
+ info = nodp->i_sb->s_fs_info;
+ binder_binderfs_dir_entry_proc = info->proc_log_dir;
+ } else {
binder_dev = container_of(filp->private_data,
struct binder_device, miscdev);
+ }
proc->context = &binder_dev->context;
binder_alloc_init(&proc->alloc);
@@ -5403,6 +5410,27 @@ static int binder_open(struct inode *nodp, struct file *filp)
&proc_fops);
}
+ if (binder_binderfs_dir_entry_proc) {
+ char strbuf[11];
+ struct dentry *binderfs_entry;
+
+ snprintf(strbuf, sizeof(strbuf), "%u", proc->pid);
+ /*
+ * Similar to debugfs, the process specific log file is shared
+ * between contexts. If the file has already been created for a
+ * process, the following binderfs_create_file() call will
+ * fail if another context of the same process invoked
+ * binder_open(). This is ok since same as debugfs,
+ * the log file will contain information on all contexts of a
+ * given PID.
+ */
+ binderfs_entry = binderfs_create_file(binder_binderfs_dir_entry_proc,
+ strbuf, &proc_fops, (void *)(unsigned long)proc->pid);
+ if (!IS_ERR(binderfs_entry))
+ proc->binderfs_entry = binderfs_entry;
+
+ }
+
return 0;
}
@@ -5442,6 +5470,12 @@ static int binder_release(struct inode *nodp, struct file *filp)
struct binder_proc *proc = filp->private_data;
debugfs_remove(proc->debugfs_entry);
+
+ if (proc->binderfs_entry) {
+ binderfs_remove_file(proc->binderfs_entry);
+ proc->binderfs_entry = NULL;
+ }
+
binder_defer_work(proc, BINDER_DEFERRED_RELEASE);
return 0;
diff --git a/drivers/android/binder_internal.h b/drivers/android/binder_internal.h
index b9be42d9464c..bd47f7f72075 100644
--- a/drivers/android/binder_internal.h
+++ b/drivers/android/binder_internal.h
@@ -35,17 +35,63 @@ struct binder_device {
struct inode *binderfs_inode;
};
+/**
+ * binderfs_mount_opts - mount options for binderfs
+ * @max: maximum number of allocatable binderfs binder devices
+ * @stats_mode: enable binder stats in binderfs.
+ */
+struct binderfs_mount_opts {
+ int max;
+ int stats_mode;
+};
+
+/**
+ * binderfs_info - information about a binderfs mount
+ * @ipc_ns: The ipc namespace the binderfs mount belongs to.
+ * @control_dentry: This records the dentry of this binderfs mount
+ * binder-control device.
+ * @root_uid: uid that needs to be used when a new binder device is
+ * created.
+ * @root_gid: gid that needs to be used when a new binder device is
+ * created.
+ * @mount_opts: The mount options in use.
+ * @device_count: The current number of allocated binder devices.
+ * @proc_log_dir: Pointer to the directory dentry containing process-specific
+ * logs.
+ */
+struct binderfs_info {
+ struct ipc_namespace *ipc_ns;
+ struct dentry *control_dentry;
+ kuid_t root_uid;
+ kgid_t root_gid;
+ struct binderfs_mount_opts mount_opts;
+ int device_count;
+ struct dentry *proc_log_dir;
+};
+
extern const struct file_operations binder_fops;
extern char *binder_devices_param;
#ifdef CONFIG_ANDROID_BINDERFS
extern bool is_binderfs_device(const struct inode *inode);
+extern struct dentry *binderfs_create_file(struct dentry *dir, const char *name,
+ const struct file_operations *fops,
+ void *data);
+extern void binderfs_remove_file(struct dentry *dentry);
#else
static inline bool is_binderfs_device(const struct inode *inode)
{
return false;
}
+static inline struct dentry *binderfs_create_file(struct dentry *dir,
+ const char *name,
+ const struct file_operations *fops,
+ void *data)
+{
+ return NULL;
+}
+static inline void binderfs_remove_file(struct dentry *dentry) {}
#endif
#ifdef CONFIG_ANDROID_BINDERFS
diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
index dc25a7d759c9..c386a3738290 100644
--- a/drivers/android/binderfs.c
+++ b/drivers/android/binderfs.c
@@ -48,16 +48,6 @@ static dev_t binderfs_dev;
static DEFINE_MUTEX(binderfs_minors_mutex);
static DEFINE_IDA(binderfs_minors);
-/**
- * binderfs_mount_opts - mount options for binderfs
- * @max: maximum number of allocatable binderfs binder devices
- * @stats_mode: enable binder stats in binderfs.
- */
-struct binderfs_mount_opts {
- int max;
- int stats_mode;
-};
-
enum {
Opt_max,
Opt_stats_mode,
@@ -75,27 +65,6 @@ static const match_table_t tokens = {
{ Opt_err, NULL }
};
-/**
- * binderfs_info - information about a binderfs mount
- * @ipc_ns: The ipc namespace the binderfs mount belongs to.
- * @control_dentry: This records the dentry of this binderfs mount
- * binder-control device.
- * @root_uid: uid that needs to be used when a new binder device is
- * created.
- * @root_gid: gid that needs to be used when a new binder device is
- * created.
- * @mount_opts: The mount options in use.
- * @device_count: The current number of allocated binder devices.
- */
-struct binderfs_info {
- struct ipc_namespace *ipc_ns;
- struct dentry *control_dentry;
- kuid_t root_uid;
- kgid_t root_gid;
- struct binderfs_mount_opts mount_opts;
- int device_count;
-};
-
static inline struct binderfs_info *BINDERFS_I(const struct inode *inode)
{
return inode->i_sb->s_fs_info;
@@ -535,7 +504,22 @@ static struct dentry *binderfs_create_dentry(struct dentry *dir,
return dentry;
}
-static struct dentry *binderfs_create_file(struct dentry *dir, const char *name,
+void binderfs_remove_file(struct dentry *dentry)
+{
+ struct inode *dir;
+
+ dir = d_inode(dentry->d_parent);
+ inode_lock(dir);
+ if (simple_positive(dentry)) {
+ dget(dentry);
+ simple_unlink(dir, dentry);
+ d_delete(dentry);
+ dput(dentry);
+ }
+ inode_unlock(dir);
+}
+
+struct dentry *binderfs_create_file(struct dentry *dir, const char *name,
const struct file_operations *fops,
void *data)
{
@@ -604,7 +588,8 @@ static struct dentry *binderfs_create_dir(struct dentry *parent,
static int init_binder_logs(struct super_block *sb)
{
- struct dentry *binder_logs_root_dir, *file_dentry;
+ struct dentry *binder_logs_root_dir, *file_dentry, *proc_log_dir;
+ struct binderfs_info *info;
int ret = 0;
binder_logs_root_dir = binderfs_create_dir(sb->s_root,
@@ -648,8 +633,18 @@ static int init_binder_logs(struct super_block *sb)
"failed_transaction_log",
&binder_transaction_log_fops,
&binder_transaction_log_failed);
- if (IS_ERR(file_dentry))
+ if (IS_ERR(file_dentry)) {
ret = PTR_ERR(file_dentry);
+ goto out;
+ }
+
+ proc_log_dir = binderfs_create_dir(binder_logs_root_dir, "proc");
+ if (IS_ERR(proc_log_dir)) {
+ ret = PTR_ERR(proc_log_dir);
+ goto out;
+ }
+ info = sb->s_fs_info;
+ info->proc_log_dir = proc_log_dir;
out:
return ret;
--
2.23.0.187.g17f5b7556c-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] binder: add a mount option to show global stats
2019-08-27 20:41 ` [PATCH 1/4] binder: add a mount option to show global stats Hridya Valsaraju
@ 2019-08-28 9:22 ` Greg Kroah-Hartman
2019-08-28 12:39 ` Christian Brauner
0 siblings, 1 reply; 15+ messages in thread
From: Greg Kroah-Hartman @ 2019-08-28 9:22 UTC (permalink / raw)
To: Hridya Valsaraju
Cc: Arve Hjønnevåg, Todd Kjos, Martijn Coenen,
Joel Fernandes, Christian Brauner, devel, linux-kernel,
kernel-team
On Tue, Aug 27, 2019 at 01:41:49PM -0700, Hridya Valsaraju wrote:
> Currently, all binder state and statistics live in debugfs.
> We need this information even when debugfs is not mounted.
> This patch adds the mount option 'stats' to enable a binderfs
> instance to have binder debug information present in the same.
> 'stats=global' will enable the global binder statistics. In
> the future, 'stats=local' will enable binder statistics local
> to the binderfs instance. The two modes 'global' and 'local'
> will be mutually exclusive. 'stats=global' option is only available
> for a binderfs instance mounted in the initial user namespace.
> An attempt to use the option to mount a binderfs instance in
> another user namespace will return an EPERM error.
>
> Signed-off-by: Hridya Valsaraju <hridya@google.com>
> ---
> drivers/android/binderfs.c | 47 ++++++++++++++++++++++++++++++++++++--
> 1 file changed, 45 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
> index cc2e71576396..d95d179aec58 100644
> --- a/drivers/android/binderfs.c
> +++ b/drivers/android/binderfs.c
> @@ -51,18 +51,27 @@ static DEFINE_IDA(binderfs_minors);
> /**
> * binderfs_mount_opts - mount options for binderfs
> * @max: maximum number of allocatable binderfs binder devices
> + * @stats_mode: enable binder stats in binderfs.
> */
> struct binderfs_mount_opts {
> int max;
> + int stats_mode;
> };
>
> enum {
> Opt_max,
> + Opt_stats_mode,
> Opt_err
> };
>
> +enum binderfs_stats_mode {
> + STATS_NONE,
> + STATS_GLOBAL,
> +};
> +
> static const match_table_t tokens = {
> { Opt_max, "max=%d" },
> + { Opt_stats_mode, "stats=%s" },
> { Opt_err, NULL }
> };
>
> @@ -290,8 +299,9 @@ static void binderfs_evict_inode(struct inode *inode)
> static int binderfs_parse_mount_opts(char *data,
> struct binderfs_mount_opts *opts)
> {
> - char *p;
> + char *p, *stats;
> opts->max = BINDERFS_MAX_MINOR;
> + opts->stats_mode = STATS_NONE;
>
> while ((p = strsep(&data, ",")) != NULL) {
> substring_t args[MAX_OPT_ARGS];
> @@ -311,6 +321,24 @@ static int binderfs_parse_mount_opts(char *data,
>
> opts->max = max_devices;
> break;
> + case Opt_stats_mode:
> + stats = match_strdup(&args[0]);
> + if (!stats)
> + return -ENOMEM;
> +
> + if (strcmp(stats, "global") != 0) {
> + kfree(stats);
> + return -EINVAL;
> + }
> +
> + if (!capable(CAP_SYS_ADMIN)) {
> + kfree(stats);
> + return -EINVAL;
Can a non-CAP_SYS_ADMIN task even call this function? Anyway, if it
can, put the check at the top of the case, and just return early before
doing any extra work like checking values or allocating memory.
> + }
> +
> + opts->stats_mode = STATS_GLOBAL;
> + kfree(stats);
> + break;
> default:
> pr_err("Invalid mount options\n");
> return -EINVAL;
> @@ -322,8 +350,21 @@ static int binderfs_parse_mount_opts(char *data,
>
> static int binderfs_remount(struct super_block *sb, int *flags, char *data)
> {
> + int prev_stats_mode, ret;
> struct binderfs_info *info = sb->s_fs_info;
> - return binderfs_parse_mount_opts(data, &info->mount_opts);
> +
> + prev_stats_mode = info->mount_opts.stats_mode;
> + ret = binderfs_parse_mount_opts(data, &info->mount_opts);
> + if (ret)
> + return ret;
> +
> + if (prev_stats_mode != info->mount_opts.stats_mode) {
> + pr_info("Binderfs stats mode cannot be changed during a remount\n");
pr_err()?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] binder: add a mount option to show global stats
2019-08-28 9:22 ` Greg Kroah-Hartman
@ 2019-08-28 12:39 ` Christian Brauner
2019-08-28 17:21 ` Hridya Valsaraju
0 siblings, 1 reply; 15+ messages in thread
From: Christian Brauner @ 2019-08-28 12:39 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Hridya Valsaraju, Arve Hjønnevåg, Todd Kjos,
Martijn Coenen, Joel Fernandes, devel, linux-kernel, kernel-team
On Wed, Aug 28, 2019 at 11:22:37AM +0200, Greg Kroah-Hartman wrote:
> On Tue, Aug 27, 2019 at 01:41:49PM -0700, Hridya Valsaraju wrote:
> > Currently, all binder state and statistics live in debugfs.
> > We need this information even when debugfs is not mounted.
> > This patch adds the mount option 'stats' to enable a binderfs
> > instance to have binder debug information present in the same.
> > 'stats=global' will enable the global binder statistics. In
> > the future, 'stats=local' will enable binder statistics local
> > to the binderfs instance. The two modes 'global' and 'local'
> > will be mutually exclusive. 'stats=global' option is only available
> > for a binderfs instance mounted in the initial user namespace.
> > An attempt to use the option to mount a binderfs instance in
> > another user namespace will return an EPERM error.
> >
> > Signed-off-by: Hridya Valsaraju <hridya@google.com>
> > ---
> > drivers/android/binderfs.c | 47 ++++++++++++++++++++++++++++++++++++--
> > 1 file changed, 45 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
> > index cc2e71576396..d95d179aec58 100644
> > --- a/drivers/android/binderfs.c
> > +++ b/drivers/android/binderfs.c
> > @@ -51,18 +51,27 @@ static DEFINE_IDA(binderfs_minors);
> > /**
> > * binderfs_mount_opts - mount options for binderfs
> > * @max: maximum number of allocatable binderfs binder devices
> > + * @stats_mode: enable binder stats in binderfs.
> > */
> > struct binderfs_mount_opts {
> > int max;
> > + int stats_mode;
> > };
> >
> > enum {
> > Opt_max,
> > + Opt_stats_mode,
> > Opt_err
> > };
> >
> > +enum binderfs_stats_mode {
> > + STATS_NONE,
> > + STATS_GLOBAL,
> > +};
> > +
> > static const match_table_t tokens = {
> > { Opt_max, "max=%d" },
> > + { Opt_stats_mode, "stats=%s" },
> > { Opt_err, NULL }
> > };
> >
> > @@ -290,8 +299,9 @@ static void binderfs_evict_inode(struct inode *inode)
> > static int binderfs_parse_mount_opts(char *data,
> > struct binderfs_mount_opts *opts)
> > {
> > - char *p;
> > + char *p, *stats;
> > opts->max = BINDERFS_MAX_MINOR;
> > + opts->stats_mode = STATS_NONE;
> >
> > while ((p = strsep(&data, ",")) != NULL) {
> > substring_t args[MAX_OPT_ARGS];
> > @@ -311,6 +321,24 @@ static int binderfs_parse_mount_opts(char *data,
> >
> > opts->max = max_devices;
> > break;
> > + case Opt_stats_mode:
> > + stats = match_strdup(&args[0]);
> > + if (!stats)
> > + return -ENOMEM;
> > +
> > + if (strcmp(stats, "global") != 0) {
> > + kfree(stats);
> > + return -EINVAL;
> > + }
> > +
> > + if (!capable(CAP_SYS_ADMIN)) {
> > + kfree(stats);
> > + return -EINVAL;
>
> Can a non-CAP_SYS_ADMIN task even call this function? Anyway, if it
It can. A task that has CAP_SYS_ADMIN in the userns the corresponding
binderfs mount has been created in can change the max=<nr> mount option.
Only stats=global currently requires capable(CAP_SYS_ADMIN) aka
CAP_SYS_ADMIN in the initial userns to prevent non-initial userns from
snooping at global statistics.
> can, put the check at the top of the case, and just return early before
> doing any extra work like checking values or allocating memory.
>
> > + }
> > +
> > + opts->stats_mode = STATS_GLOBAL;
> > + kfree(stats);
> > + break;
> > default:
> > pr_err("Invalid mount options\n");
> > return -EINVAL;
> > @@ -322,8 +350,21 @@ static int binderfs_parse_mount_opts(char *data,
> >
> > static int binderfs_remount(struct super_block *sb, int *flags, char *data)
> > {
> > + int prev_stats_mode, ret;
> > struct binderfs_info *info = sb->s_fs_info;
> > - return binderfs_parse_mount_opts(data, &info->mount_opts);
> > +
> > + prev_stats_mode = info->mount_opts.stats_mode;
> > + ret = binderfs_parse_mount_opts(data, &info->mount_opts);
> > + if (ret)
> > + return ret;
> > +
> > + if (prev_stats_mode != info->mount_opts.stats_mode) {
> > + pr_info("Binderfs stats mode cannot be changed during a remount\n");
>
> pr_err()?
>
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/4] binder: Add stats, state and transactions files
2019-08-27 20:41 ` [PATCH 2/4] binder: Add stats, state and transactions files Hridya Valsaraju
@ 2019-08-28 12:58 ` Christian Brauner
2019-08-28 20:19 ` Hridya Valsaraju
0 siblings, 1 reply; 15+ messages in thread
From: Christian Brauner @ 2019-08-28 12:58 UTC (permalink / raw)
To: Hridya Valsaraju
Cc: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos,
Martijn Coenen, Joel Fernandes, devel, linux-kernel, kernel-team
On Tue, Aug 27, 2019 at 01:41:50PM -0700, Hridya Valsaraju wrote:
> The following binder stat files currently live in debugfs.
>
> /sys/kernel/debug/binder/state
> /sys/kernel/debug/binder/stats
> /sys/kernel/debug/binder/transactions
>
> This patch makes these files available in a binderfs instance
> mounted with the mount option 'stats=global'. For example, if a binderfs
> instance is mounted at path /dev/binderfs, the above files will be
> available at the following locations:
>
> /dev/binderfs/binder_logs/state
> /dev/binderfs/binder_logs/stats
> /dev/binderfs/binder_logs/transactions
>
> This provides a way to access them even when debugfs is not mounted.
>
> Signed-off-by: Hridya Valsaraju <hridya@google.com>
> ---
> drivers/android/binder.c | 15 ++--
> drivers/android/binder_internal.h | 8 ++
> drivers/android/binderfs.c | 137 +++++++++++++++++++++++++++++-
> 3 files changed, 150 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index ca6b21a53321..de795bd229c4 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -6055,7 +6055,7 @@ static void print_binder_proc_stats(struct seq_file *m,
> }
>
>
> -static int state_show(struct seq_file *m, void *unused)
> +int binder_state_show(struct seq_file *m, void *unused)
> {
> struct binder_proc *proc;
> struct binder_node *node;
> @@ -6094,7 +6094,7 @@ static int state_show(struct seq_file *m, void *unused)
> return 0;
> }
>
> -static int stats_show(struct seq_file *m, void *unused)
> +int binder_stats_show(struct seq_file *m, void *unused)
> {
> struct binder_proc *proc;
>
> @@ -6110,7 +6110,7 @@ static int stats_show(struct seq_file *m, void *unused)
> return 0;
> }
>
> -static int transactions_show(struct seq_file *m, void *unused)
> +int binder_transactions_show(struct seq_file *m, void *unused)
> {
> struct binder_proc *proc;
>
> @@ -6198,9 +6198,6 @@ const struct file_operations binder_fops = {
> .release = binder_release,
> };
>
> -DEFINE_SHOW_ATTRIBUTE(state);
> -DEFINE_SHOW_ATTRIBUTE(stats);
> -DEFINE_SHOW_ATTRIBUTE(transactions);
> DEFINE_SHOW_ATTRIBUTE(transaction_log);
>
> static int __init init_binder_device(const char *name)
> @@ -6256,17 +6253,17 @@ static int __init binder_init(void)
> 0444,
> binder_debugfs_dir_entry_root,
> NULL,
> - &state_fops);
> + &binder_state_fops);
> debugfs_create_file("stats",
> 0444,
> binder_debugfs_dir_entry_root,
> NULL,
> - &stats_fops);
> + &binder_stats_fops);
> debugfs_create_file("transactions",
> 0444,
> binder_debugfs_dir_entry_root,
> NULL,
> - &transactions_fops);
> + &binder_transactions_fops);
> debugfs_create_file("transaction_log",
> 0444,
> binder_debugfs_dir_entry_root,
> diff --git a/drivers/android/binder_internal.h b/drivers/android/binder_internal.h
> index fe8c745dc8e0..12ef96f256c6 100644
> --- a/drivers/android/binder_internal.h
> +++ b/drivers/android/binder_internal.h
> @@ -57,4 +57,12 @@ static inline int __init init_binderfs(void)
> }
> #endif
>
> +int binder_stats_show(struct seq_file *m, void *unused);
> +DEFINE_SHOW_ATTRIBUTE(binder_stats);
> +
> +int binder_state_show(struct seq_file *m, void *unused);
> +DEFINE_SHOW_ATTRIBUTE(binder_state);
> +
> +int binder_transactions_show(struct seq_file *m, void *unused);
> +DEFINE_SHOW_ATTRIBUTE(binder_transactions);
> #endif /* _LINUX_BINDER_INTERNAL_H */
> diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
> index d95d179aec58..d542f9b8d8ab 100644
> --- a/drivers/android/binderfs.c
> +++ b/drivers/android/binderfs.c
> @@ -280,7 +280,7 @@ static void binderfs_evict_inode(struct inode *inode)
>
> clear_inode(inode);
>
> - if (!device)
> + if (!device || S_ISREG(inode->i_mode))
Hm, remind me why we need the S_ISREG again?
Also we probably should do:
if (S_ISREG(inode->i_mode) || !device)
should this maybe be:
if (!S_ISCHR(inode->i_mode) || !device)
?
> return;
>
> mutex_lock(&binderfs_minors_mutex);
> @@ -504,6 +504,138 @@ static const struct inode_operations binderfs_dir_inode_operations = {
> .unlink = binderfs_unlink,
> };
>
> +static struct inode *binderfs_make_inode(struct super_block *sb, int mode)
> +{
> + struct inode *ret;
> +
> + ret = new_inode(sb);
> + if (ret) {
> + ret->i_ino = iunique(sb, BINDERFS_MAX_MINOR + INODE_OFFSET);
> + ret->i_mode = mode;
> + ret->i_atime = ret->i_mtime = ret->i_ctime = current_time(ret);
> + }
> + return ret;
> +}
> +
> +static struct dentry *binderfs_create_dentry(struct dentry *dir,
> + const char *name)
> +{
> + struct dentry *dentry;
> +
> + dentry = lookup_one_len(name, dir, strlen(name));
> + if (IS_ERR(dentry))
> + return dentry;
> +
> + /* Return error if the file/dir already exists. */
> + if (d_really_is_positive(dentry)) {
> + dput(dentry);
> + return ERR_PTR(-EEXIST);
> + }
> +
> + return dentry;
> +}
> +
> +static struct dentry *binderfs_create_file(struct dentry *dir, const char *name,
> + const struct file_operations *fops,
> + void *data)
> +{
> + struct dentry *dentry;
> + struct inode *new_inode, *dir_inode;
> + struct super_block *sb;
> +
> + dir_inode = dir->d_inode;
> + inode_lock(dir_inode);
> +
> + dentry = binderfs_create_dentry(dir, name);
> + if (IS_ERR(dentry))
> + goto out;
> +
> + sb = dir_inode->i_sb;
> + new_inode = binderfs_make_inode(sb, S_IFREG | 0444);
> + if (!new_inode) {
> + dput(dentry);
> + dentry = ERR_PTR(-ENOMEM);
> + goto out;
> + }
> +
> + new_inode->i_fop = fops;
> + new_inode->i_private = data;
> + d_instantiate(dentry, new_inode);
> + fsnotify_create(dir_inode, dentry);
> +
> +out:
> + inode_unlock(dir_inode);
> + return dentry;
> +}
> +
> +static struct dentry *binderfs_create_dir(struct dentry *parent,
> + const char *name)
> +{
> + struct dentry *dentry;
> + struct inode *new_inode, *parent_inode;
> + struct super_block *sb;
> +
> + parent_inode = d_inode(parent);
For consistency, could you use the same variable name for the directory
in which you create a new dentry? I don't care if its "dir_inode" like
above or "parent_inode".
> + inode_lock(parent_inode);
> +
> + dentry = binderfs_create_dentry(parent, name);
> + if (IS_ERR(dentry))
> + goto out;
> +
> + sb = parent_inode->i_sb;
> + new_inode = binderfs_make_inode(sb, S_IFDIR | 0755);
> + if (!new_inode) {
> + dput(dentry);
> + dentry = ERR_PTR(-ENOMEM);
> + goto out;
> + }
> +
> + new_inode->i_fop = &simple_dir_operations;
> + new_inode->i_op = &simple_dir_inode_operations;
> +
> + inc_nlink(new_inode);
> + d_instantiate(dentry, new_inode);
> + inc_nlink(parent_inode);
> + fsnotify_mkdir(parent_inode, dentry);
> +out:
For consistency please leave a \n after fsnotify_mkdir and the goto
label like you did in the function above.
> + inode_unlock(parent_inode);
> + return dentry;
> +}
> +
> +static int init_binder_logs(struct super_block *sb)
> +{
> + struct dentry *binder_logs_root_dir, *file_dentry;
Why "file_dentry" and not just simply "dentry" like everywhere else?
> + int ret = 0;
> +
> + binder_logs_root_dir = binderfs_create_dir(sb->s_root,
> + "binder_logs");
> + if (IS_ERR(binder_logs_root_dir)) {
> + ret = PTR_ERR(binder_logs_root_dir);
> + goto out;
> + }
> +
> + file_dentry = binderfs_create_file(binder_logs_root_dir, "stats",
> + &binder_stats_fops, NULL);
> + if (IS_ERR(file_dentry)) {
> + ret = PTR_ERR(file_dentry);
> + goto out;
> + }
> +
> + file_dentry = binderfs_create_file(binder_logs_root_dir, "state",
> + &binder_state_fops, NULL);
> + if (IS_ERR(file_dentry)) {
> + ret = PTR_ERR(file_dentry);
> + goto out;
> + }
> +
> + file_dentry = binderfs_create_file(binder_logs_root_dir, "transactions",
> + &binder_transactions_fops, NULL);
> + if (IS_ERR(file_dentry))
> + ret = PTR_ERR(file_dentry);
> +out:
> + return ret;
> +}
> +
> static int binderfs_fill_super(struct super_block *sb, void *data, int silent)
> {
> int ret;
> @@ -582,6 +714,9 @@ static int binderfs_fill_super(struct super_block *sb, void *data, int silent)
>
> }
>
> + if (info->mount_opts.stats_mode == STATS_GLOBAL)
> + return init_binder_logs(sb);
> +
> return 0;
> }
>
> --
> 2.23.0.187.g17f5b7556c-goog
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/4] binder: Make transaction_log available in binderfs
2019-08-27 20:41 ` [PATCH 3/4] binder: Make transaction_log available in binderfs Hridya Valsaraju
@ 2019-08-28 12:59 ` Christian Brauner
2019-08-28 16:05 ` Todd Kjos
0 siblings, 1 reply; 15+ messages in thread
From: Christian Brauner @ 2019-08-28 12:59 UTC (permalink / raw)
To: Hridya Valsaraju
Cc: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos,
Martijn Coenen, Joel Fernandes, Christian Brauner, devel,
linux-kernel, kernel-team
On Tue, Aug 27, 2019 at 01:41:51PM -0700, Hridya Valsaraju wrote:
> Currently, the binder transaction log files 'transaction_log'
> and 'failed_transaction_log' live in debugfs at the following locations:
>
> /sys/kernel/debug/binder/failed_transaction_log
> /sys/kernel/debug/binder/transaction_log
>
> This patch makes these files also available in a binderfs instance
> mounted with the mount option "stats=global".
> It does not affect the presence of these files in debugfs.
> If a binderfs instance is mounted at path /dev/binderfs, the location of
> these files will be as follows:
>
> /dev/binderfs/binder_logs/failed_transaction_log
> /dev/binderfs/binder_logs/transaction_log
>
> This change provides an alternate option to access these files when
> debugfs is not mounted.
>
> Signed-off-by: Hridya Valsaraju <hridya@google.com>
Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
> ---
> drivers/android/binder.c | 34 +++++--------------------------
> drivers/android/binder_internal.h | 30 +++++++++++++++++++++++++++
> drivers/android/binderfs.c | 19 +++++++++++++++++
> 3 files changed, 54 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index de795bd229c4..bed217310197 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -197,30 +197,8 @@ static inline void binder_stats_created(enum binder_stat_types type)
> atomic_inc(&binder_stats.obj_created[type]);
> }
>
> -struct binder_transaction_log_entry {
> - int debug_id;
> - int debug_id_done;
> - int call_type;
> - int from_proc;
> - int from_thread;
> - int target_handle;
> - int to_proc;
> - int to_thread;
> - int to_node;
> - int data_size;
> - int offsets_size;
> - int return_error_line;
> - uint32_t return_error;
> - uint32_t return_error_param;
> - const char *context_name;
> -};
> -struct binder_transaction_log {
> - atomic_t cur;
> - bool full;
> - struct binder_transaction_log_entry entry[32];
> -};
> -static struct binder_transaction_log binder_transaction_log;
> -static struct binder_transaction_log binder_transaction_log_failed;
> +struct binder_transaction_log binder_transaction_log;
> +struct binder_transaction_log binder_transaction_log_failed;
>
> static struct binder_transaction_log_entry *binder_transaction_log_add(
> struct binder_transaction_log *log)
> @@ -6166,7 +6144,7 @@ static void print_binder_transaction_log_entry(struct seq_file *m,
> "\n" : " (incomplete)\n");
> }
>
> -static int transaction_log_show(struct seq_file *m, void *unused)
> +int binder_transaction_log_show(struct seq_file *m, void *unused)
> {
> struct binder_transaction_log *log = m->private;
> unsigned int log_cur = atomic_read(&log->cur);
> @@ -6198,8 +6176,6 @@ const struct file_operations binder_fops = {
> .release = binder_release,
> };
>
> -DEFINE_SHOW_ATTRIBUTE(transaction_log);
> -
> static int __init init_binder_device(const char *name)
> {
> int ret;
> @@ -6268,12 +6244,12 @@ static int __init binder_init(void)
> 0444,
> binder_debugfs_dir_entry_root,
> &binder_transaction_log,
> - &transaction_log_fops);
> + &binder_transaction_log_fops);
> debugfs_create_file("failed_transaction_log",
> 0444,
> binder_debugfs_dir_entry_root,
> &binder_transaction_log_failed,
> - &transaction_log_fops);
> + &binder_transaction_log_fops);
> }
>
> if (!IS_ENABLED(CONFIG_ANDROID_BINDERFS) &&
> diff --git a/drivers/android/binder_internal.h b/drivers/android/binder_internal.h
> index 12ef96f256c6..b9be42d9464c 100644
> --- a/drivers/android/binder_internal.h
> +++ b/drivers/android/binder_internal.h
> @@ -65,4 +65,34 @@ DEFINE_SHOW_ATTRIBUTE(binder_state);
>
> int binder_transactions_show(struct seq_file *m, void *unused);
> DEFINE_SHOW_ATTRIBUTE(binder_transactions);
> +
> +int binder_transaction_log_show(struct seq_file *m, void *unused);
> +DEFINE_SHOW_ATTRIBUTE(binder_transaction_log);
> +
> +struct binder_transaction_log_entry {
> + int debug_id;
> + int debug_id_done;
> + int call_type;
> + int from_proc;
> + int from_thread;
> + int target_handle;
> + int to_proc;
> + int to_thread;
> + int to_node;
> + int data_size;
> + int offsets_size;
> + int return_error_line;
> + uint32_t return_error;
> + uint32_t return_error_param;
> + const char *context_name;
> +};
> +
> +struct binder_transaction_log {
> + atomic_t cur;
> + bool full;
> + struct binder_transaction_log_entry entry[32];
> +};
> +
> +extern struct binder_transaction_log binder_transaction_log;
> +extern struct binder_transaction_log binder_transaction_log_failed;
> #endif /* _LINUX_BINDER_INTERNAL_H */
> diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
> index d542f9b8d8ab..dc25a7d759c9 100644
> --- a/drivers/android/binderfs.c
> +++ b/drivers/android/binderfs.c
> @@ -630,8 +630,27 @@ static int init_binder_logs(struct super_block *sb)
>
> file_dentry = binderfs_create_file(binder_logs_root_dir, "transactions",
> &binder_transactions_fops, NULL);
> + if (IS_ERR(file_dentry)) {
> + ret = PTR_ERR(file_dentry);
> + goto out;
> + }
> +
> + file_dentry = binderfs_create_file(binder_logs_root_dir,
> + "transaction_log",
> + &binder_transaction_log_fops,
> + &binder_transaction_log);
> + if (IS_ERR(file_dentry)) {
> + ret = PTR_ERR(file_dentry);
> + goto out;
> + }
> +
> + file_dentry = binderfs_create_file(binder_logs_root_dir,
> + "failed_transaction_log",
> + &binder_transaction_log_fops,
> + &binder_transaction_log_failed);
> if (IS_ERR(file_dentry))
> ret = PTR_ERR(file_dentry);
> +
> out:
> return ret;
> }
> --
> 2.23.0.187.g17f5b7556c-goog
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/4] binder: Add binder_proc logging to binderfs
2019-08-27 20:41 ` [PATCH 4/4] binder: Add binder_proc logging to binderfs Hridya Valsaraju
@ 2019-08-28 13:08 ` Christian Brauner
2019-08-28 16:21 ` Todd Kjos
0 siblings, 1 reply; 15+ messages in thread
From: Christian Brauner @ 2019-08-28 13:08 UTC (permalink / raw)
To: Hridya Valsaraju
Cc: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos,
Martijn Coenen, Joel Fernandes, devel, linux-kernel, kernel-team
On Tue, Aug 27, 2019 at 01:41:52PM -0700, Hridya Valsaraju wrote:
> Currently /sys/kernel/debug/binder/proc contains
> the debug data for every binder_proc instance.
> This patch makes this information also available
> in a binderfs instance mounted with a mount option
> "stats=global" in addition to debugfs. The patch does
> not affect the presence of the file in debugfs.
>
> If a binderfs instance is mounted at path /dev/binderfs,
> this file would be present at /dev/binderfs/binder_logs/proc.
> This change provides an alternate way to access this file when debugfs
> is not mounted.
>
> Signed-off-by: Hridya Valsaraju <hridya@google.com>
I'm still wondering whether there's a nicer way to create those debuf
files per-process without doing it in binder_open() but it has worked
fine for a long time with debugfs.
Also, one minor question below. Otherwise
Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
> ---
> drivers/android/binder.c | 38 ++++++++++++++++++-
> drivers/android/binder_internal.h | 46 ++++++++++++++++++++++
> drivers/android/binderfs.c | 63 ++++++++++++++-----------------
> 3 files changed, 111 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index bed217310197..37189838f32a 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -481,6 +481,7 @@ struct binder_priority {
> * @inner_lock: can nest under outer_lock and/or node lock
> * @outer_lock: no nesting under innor or node lock
> * Lock order: 1) outer, 2) node, 3) inner
> + * @binderfs_entry: process-specific binderfs log file
> *
> * Bookkeeping structure for binder processes
> */
> @@ -510,6 +511,7 @@ struct binder_proc {
> struct binder_context *context;
> spinlock_t inner_lock;
> spinlock_t outer_lock;
> + struct dentry *binderfs_entry;
> };
>
> enum {
> @@ -5347,6 +5349,8 @@ static int binder_open(struct inode *nodp, struct file *filp)
> {
> struct binder_proc *proc;
> struct binder_device *binder_dev;
> + struct binderfs_info *info;
> + struct dentry *binder_binderfs_dir_entry_proc = NULL;
>
> binder_debug(BINDER_DEBUG_OPEN_CLOSE, "%s: %d:%d\n", __func__,
> current->group_leader->pid, current->pid);
> @@ -5368,11 +5372,14 @@ static int binder_open(struct inode *nodp, struct file *filp)
> }
>
> /* binderfs stashes devices in i_private */
> - if (is_binderfs_device(nodp))
> + if (is_binderfs_device(nodp)) {
> binder_dev = nodp->i_private;
> - else
> + info = nodp->i_sb->s_fs_info;
> + binder_binderfs_dir_entry_proc = info->proc_log_dir;
> + } else {
> binder_dev = container_of(filp->private_data,
> struct binder_device, miscdev);
> + }
> proc->context = &binder_dev->context;
> binder_alloc_init(&proc->alloc);
>
> @@ -5403,6 +5410,27 @@ static int binder_open(struct inode *nodp, struct file *filp)
> &proc_fops);
> }
>
> + if (binder_binderfs_dir_entry_proc) {
> + char strbuf[11];
> + struct dentry *binderfs_entry;
> +
> + snprintf(strbuf, sizeof(strbuf), "%u", proc->pid);
> + /*
> + * Similar to debugfs, the process specific log file is shared
> + * between contexts. If the file has already been created for a
> + * process, the following binderfs_create_file() call will
> + * fail if another context of the same process invoked
> + * binder_open(). This is ok since same as debugfs,
> + * the log file will contain information on all contexts of a
> + * given PID.
> + */
> + binderfs_entry = binderfs_create_file(binder_binderfs_dir_entry_proc,
> + strbuf, &proc_fops, (void *)(unsigned long)proc->pid);
> + if (!IS_ERR(binderfs_entry))
> + proc->binderfs_entry = binderfs_entry;
You are sure that you don't want to fail the open, when the debug file
cannot be created in the binderfs instance? I'm not objecting at all, I
just want to make sure that this is the semantics you want because it
would be easy to differentiate between the non-fail-debugfs and the new
binderfs case.
> +
> + }
> +
> return 0;
> }
>
> @@ -5442,6 +5470,12 @@ static int binder_release(struct inode *nodp, struct file *filp)
> struct binder_proc *proc = filp->private_data;
>
> debugfs_remove(proc->debugfs_entry);
> +
> + if (proc->binderfs_entry) {
> + binderfs_remove_file(proc->binderfs_entry);
> + proc->binderfs_entry = NULL;
> + }
> +
> binder_defer_work(proc, BINDER_DEFERRED_RELEASE);
>
> return 0;
> diff --git a/drivers/android/binder_internal.h b/drivers/android/binder_internal.h
> index b9be42d9464c..bd47f7f72075 100644
> --- a/drivers/android/binder_internal.h
> +++ b/drivers/android/binder_internal.h
> @@ -35,17 +35,63 @@ struct binder_device {
> struct inode *binderfs_inode;
> };
>
> +/**
> + * binderfs_mount_opts - mount options for binderfs
> + * @max: maximum number of allocatable binderfs binder devices
> + * @stats_mode: enable binder stats in binderfs.
> + */
> +struct binderfs_mount_opts {
> + int max;
> + int stats_mode;
> +};
> +
> +/**
> + * binderfs_info - information about a binderfs mount
> + * @ipc_ns: The ipc namespace the binderfs mount belongs to.
> + * @control_dentry: This records the dentry of this binderfs mount
> + * binder-control device.
> + * @root_uid: uid that needs to be used when a new binder device is
> + * created.
> + * @root_gid: gid that needs to be used when a new binder device is
> + * created.
> + * @mount_opts: The mount options in use.
> + * @device_count: The current number of allocated binder devices.
> + * @proc_log_dir: Pointer to the directory dentry containing process-specific
> + * logs.
> + */
> +struct binderfs_info {
> + struct ipc_namespace *ipc_ns;
> + struct dentry *control_dentry;
> + kuid_t root_uid;
> + kgid_t root_gid;
> + struct binderfs_mount_opts mount_opts;
> + int device_count;
> + struct dentry *proc_log_dir;
> +};
> +
> extern const struct file_operations binder_fops;
>
> extern char *binder_devices_param;
>
> #ifdef CONFIG_ANDROID_BINDERFS
> extern bool is_binderfs_device(const struct inode *inode);
> +extern struct dentry *binderfs_create_file(struct dentry *dir, const char *name,
> + const struct file_operations *fops,
> + void *data);
> +extern void binderfs_remove_file(struct dentry *dentry);
> #else
> static inline bool is_binderfs_device(const struct inode *inode)
> {
> return false;
> }
> +static inline struct dentry *binderfs_create_file(struct dentry *dir,
> + const char *name,
> + const struct file_operations *fops,
> + void *data)
> +{
> + return NULL;
> +}
> +static inline void binderfs_remove_file(struct dentry *dentry) {}
> #endif
>
> #ifdef CONFIG_ANDROID_BINDERFS
> diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
> index dc25a7d759c9..c386a3738290 100644
> --- a/drivers/android/binderfs.c
> +++ b/drivers/android/binderfs.c
> @@ -48,16 +48,6 @@ static dev_t binderfs_dev;
> static DEFINE_MUTEX(binderfs_minors_mutex);
> static DEFINE_IDA(binderfs_minors);
>
> -/**
> - * binderfs_mount_opts - mount options for binderfs
> - * @max: maximum number of allocatable binderfs binder devices
> - * @stats_mode: enable binder stats in binderfs.
> - */
> -struct binderfs_mount_opts {
> - int max;
> - int stats_mode;
> -};
> -
> enum {
> Opt_max,
> Opt_stats_mode,
> @@ -75,27 +65,6 @@ static const match_table_t tokens = {
> { Opt_err, NULL }
> };
>
> -/**
> - * binderfs_info - information about a binderfs mount
> - * @ipc_ns: The ipc namespace the binderfs mount belongs to.
> - * @control_dentry: This records the dentry of this binderfs mount
> - * binder-control device.
> - * @root_uid: uid that needs to be used when a new binder device is
> - * created.
> - * @root_gid: gid that needs to be used when a new binder device is
> - * created.
> - * @mount_opts: The mount options in use.
> - * @device_count: The current number of allocated binder devices.
> - */
> -struct binderfs_info {
> - struct ipc_namespace *ipc_ns;
> - struct dentry *control_dentry;
> - kuid_t root_uid;
> - kgid_t root_gid;
> - struct binderfs_mount_opts mount_opts;
> - int device_count;
> -};
> -
> static inline struct binderfs_info *BINDERFS_I(const struct inode *inode)
> {
> return inode->i_sb->s_fs_info;
> @@ -535,7 +504,22 @@ static struct dentry *binderfs_create_dentry(struct dentry *dir,
> return dentry;
> }
>
> -static struct dentry *binderfs_create_file(struct dentry *dir, const char *name,
> +void binderfs_remove_file(struct dentry *dentry)
> +{
> + struct inode *dir;
> +
> + dir = d_inode(dentry->d_parent);
> + inode_lock(dir);
> + if (simple_positive(dentry)) {
> + dget(dentry);
> + simple_unlink(dir, dentry);
> + d_delete(dentry);
> + dput(dentry);
> + }
> + inode_unlock(dir);
> +}
> +
> +struct dentry *binderfs_create_file(struct dentry *dir, const char *name,
> const struct file_operations *fops,
> void *data)
> {
> @@ -604,7 +588,8 @@ static struct dentry *binderfs_create_dir(struct dentry *parent,
>
> static int init_binder_logs(struct super_block *sb)
> {
> - struct dentry *binder_logs_root_dir, *file_dentry;
> + struct dentry *binder_logs_root_dir, *file_dentry, *proc_log_dir;
> + struct binderfs_info *info;
> int ret = 0;
>
> binder_logs_root_dir = binderfs_create_dir(sb->s_root,
> @@ -648,8 +633,18 @@ static int init_binder_logs(struct super_block *sb)
> "failed_transaction_log",
> &binder_transaction_log_fops,
> &binder_transaction_log_failed);
> - if (IS_ERR(file_dentry))
> + if (IS_ERR(file_dentry)) {
> ret = PTR_ERR(file_dentry);
> + goto out;
> + }
> +
> + proc_log_dir = binderfs_create_dir(binder_logs_root_dir, "proc");
> + if (IS_ERR(proc_log_dir)) {
> + ret = PTR_ERR(proc_log_dir);
> + goto out;
> + }
> + info = sb->s_fs_info;
> + info->proc_log_dir = proc_log_dir;
>
> out:
> return ret;
> --
> 2.23.0.187.g17f5b7556c-goog
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/4] binder: Make transaction_log available in binderfs
2019-08-28 12:59 ` Christian Brauner
@ 2019-08-28 16:05 ` Todd Kjos
0 siblings, 0 replies; 15+ messages in thread
From: Todd Kjos @ 2019-08-28 16:05 UTC (permalink / raw)
To: Christian Brauner
Cc: Hridya Valsaraju, Greg Kroah-Hartman, Arve Hjønnevåg,
Todd Kjos, Martijn Coenen, Joel Fernandes, Christian Brauner,
open list:ANDROID DRIVERS, LKML, Android Kernel Team
On Wed, Aug 28, 2019 at 5:59 AM Christian Brauner
<christian.brauner@ubuntu.com> wrote:
>
> On Tue, Aug 27, 2019 at 01:41:51PM -0700, Hridya Valsaraju wrote:
> > Currently, the binder transaction log files 'transaction_log'
> > and 'failed_transaction_log' live in debugfs at the following locations:
> >
> > /sys/kernel/debug/binder/failed_transaction_log
> > /sys/kernel/debug/binder/transaction_log
> >
> > This patch makes these files also available in a binderfs instance
> > mounted with the mount option "stats=global".
> > It does not affect the presence of these files in debugfs.
> > If a binderfs instance is mounted at path /dev/binderfs, the location of
> > these files will be as follows:
> >
> > /dev/binderfs/binder_logs/failed_transaction_log
> > /dev/binderfs/binder_logs/transaction_log
> >
> > This change provides an alternate option to access these files when
> > debugfs is not mounted.
> >
> > Signed-off-by: Hridya Valsaraju <hridya@google.com>
>
> Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
Acked-by: Todd Kjos <tkjos@google.com>
>
> > ---
> > drivers/android/binder.c | 34 +++++--------------------------
> > drivers/android/binder_internal.h | 30 +++++++++++++++++++++++++++
> > drivers/android/binderfs.c | 19 +++++++++++++++++
> > 3 files changed, 54 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> > index de795bd229c4..bed217310197 100644
> > --- a/drivers/android/binder.c
> > +++ b/drivers/android/binder.c
> > @@ -197,30 +197,8 @@ static inline void binder_stats_created(enum binder_stat_types type)
> > atomic_inc(&binder_stats.obj_created[type]);
> > }
> >
> > -struct binder_transaction_log_entry {
> > - int debug_id;
> > - int debug_id_done;
> > - int call_type;
> > - int from_proc;
> > - int from_thread;
> > - int target_handle;
> > - int to_proc;
> > - int to_thread;
> > - int to_node;
> > - int data_size;
> > - int offsets_size;
> > - int return_error_line;
> > - uint32_t return_error;
> > - uint32_t return_error_param;
> > - const char *context_name;
> > -};
> > -struct binder_transaction_log {
> > - atomic_t cur;
> > - bool full;
> > - struct binder_transaction_log_entry entry[32];
> > -};
> > -static struct binder_transaction_log binder_transaction_log;
> > -static struct binder_transaction_log binder_transaction_log_failed;
> > +struct binder_transaction_log binder_transaction_log;
> > +struct binder_transaction_log binder_transaction_log_failed;
> >
> > static struct binder_transaction_log_entry *binder_transaction_log_add(
> > struct binder_transaction_log *log)
> > @@ -6166,7 +6144,7 @@ static void print_binder_transaction_log_entry(struct seq_file *m,
> > "\n" : " (incomplete)\n");
> > }
> >
> > -static int transaction_log_show(struct seq_file *m, void *unused)
> > +int binder_transaction_log_show(struct seq_file *m, void *unused)
> > {
> > struct binder_transaction_log *log = m->private;
> > unsigned int log_cur = atomic_read(&log->cur);
> > @@ -6198,8 +6176,6 @@ const struct file_operations binder_fops = {
> > .release = binder_release,
> > };
> >
> > -DEFINE_SHOW_ATTRIBUTE(transaction_log);
> > -
> > static int __init init_binder_device(const char *name)
> > {
> > int ret;
> > @@ -6268,12 +6244,12 @@ static int __init binder_init(void)
> > 0444,
> > binder_debugfs_dir_entry_root,
> > &binder_transaction_log,
> > - &transaction_log_fops);
> > + &binder_transaction_log_fops);
> > debugfs_create_file("failed_transaction_log",
> > 0444,
> > binder_debugfs_dir_entry_root,
> > &binder_transaction_log_failed,
> > - &transaction_log_fops);
> > + &binder_transaction_log_fops);
> > }
> >
> > if (!IS_ENABLED(CONFIG_ANDROID_BINDERFS) &&
> > diff --git a/drivers/android/binder_internal.h b/drivers/android/binder_internal.h
> > index 12ef96f256c6..b9be42d9464c 100644
> > --- a/drivers/android/binder_internal.h
> > +++ b/drivers/android/binder_internal.h
> > @@ -65,4 +65,34 @@ DEFINE_SHOW_ATTRIBUTE(binder_state);
> >
> > int binder_transactions_show(struct seq_file *m, void *unused);
> > DEFINE_SHOW_ATTRIBUTE(binder_transactions);
> > +
> > +int binder_transaction_log_show(struct seq_file *m, void *unused);
> > +DEFINE_SHOW_ATTRIBUTE(binder_transaction_log);
> > +
> > +struct binder_transaction_log_entry {
> > + int debug_id;
> > + int debug_id_done;
> > + int call_type;
> > + int from_proc;
> > + int from_thread;
> > + int target_handle;
> > + int to_proc;
> > + int to_thread;
> > + int to_node;
> > + int data_size;
> > + int offsets_size;
> > + int return_error_line;
> > + uint32_t return_error;
> > + uint32_t return_error_param;
> > + const char *context_name;
> > +};
> > +
> > +struct binder_transaction_log {
> > + atomic_t cur;
> > + bool full;
> > + struct binder_transaction_log_entry entry[32];
> > +};
> > +
> > +extern struct binder_transaction_log binder_transaction_log;
> > +extern struct binder_transaction_log binder_transaction_log_failed;
> > #endif /* _LINUX_BINDER_INTERNAL_H */
> > diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
> > index d542f9b8d8ab..dc25a7d759c9 100644
> > --- a/drivers/android/binderfs.c
> > +++ b/drivers/android/binderfs.c
> > @@ -630,8 +630,27 @@ static int init_binder_logs(struct super_block *sb)
> >
> > file_dentry = binderfs_create_file(binder_logs_root_dir, "transactions",
> > &binder_transactions_fops, NULL);
> > + if (IS_ERR(file_dentry)) {
> > + ret = PTR_ERR(file_dentry);
> > + goto out;
> > + }
> > +
> > + file_dentry = binderfs_create_file(binder_logs_root_dir,
> > + "transaction_log",
> > + &binder_transaction_log_fops,
> > + &binder_transaction_log);
> > + if (IS_ERR(file_dentry)) {
> > + ret = PTR_ERR(file_dentry);
> > + goto out;
> > + }
> > +
> > + file_dentry = binderfs_create_file(binder_logs_root_dir,
> > + "failed_transaction_log",
> > + &binder_transaction_log_fops,
> > + &binder_transaction_log_failed);
> > if (IS_ERR(file_dentry))
> > ret = PTR_ERR(file_dentry);
> > +
> > out:
> > return ret;
> > }
> > --
> > 2.23.0.187.g17f5b7556c-goog
> >
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/4] binder: Add binder_proc logging to binderfs
2019-08-28 13:08 ` Christian Brauner
@ 2019-08-28 16:21 ` Todd Kjos
2019-08-28 20:42 ` Hridya Valsaraju
0 siblings, 1 reply; 15+ messages in thread
From: Todd Kjos @ 2019-08-28 16:21 UTC (permalink / raw)
To: Christian Brauner
Cc: Hridya Valsaraju, Greg Kroah-Hartman, Arve Hjønnevåg,
Todd Kjos, Martijn Coenen, Joel Fernandes,
open list:ANDROID DRIVERS, LKML, Android Kernel Team
On Wed, Aug 28, 2019 at 6:08 AM Christian Brauner
<christian.brauner@ubuntu.com> wrote:
>
> On Tue, Aug 27, 2019 at 01:41:52PM -0700, Hridya Valsaraju wrote:
> > Currently /sys/kernel/debug/binder/proc contains
> > the debug data for every binder_proc instance.
> > This patch makes this information also available
> > in a binderfs instance mounted with a mount option
> > "stats=global" in addition to debugfs. The patch does
> > not affect the presence of the file in debugfs.
> >
> > If a binderfs instance is mounted at path /dev/binderfs,
> > this file would be present at /dev/binderfs/binder_logs/proc.
> > This change provides an alternate way to access this file when debugfs
> > is not mounted.
> >
> > Signed-off-by: Hridya Valsaraju <hridya@google.com>
>
> I'm still wondering whether there's a nicer way to create those debuf
> files per-process without doing it in binder_open() but it has worked
> fine for a long time with debugfs.
>
> Also, one minor question below. Otherwise
>
> Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
Acked-by: Todd Kjos <tkjos@google.com>
>
> > ---
> > drivers/android/binder.c | 38 ++++++++++++++++++-
> > drivers/android/binder_internal.h | 46 ++++++++++++++++++++++
> > drivers/android/binderfs.c | 63 ++++++++++++++-----------------
> > 3 files changed, 111 insertions(+), 36 deletions(-)
> >
> > diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> > index bed217310197..37189838f32a 100644
> > --- a/drivers/android/binder.c
> > +++ b/drivers/android/binder.c
> > @@ -481,6 +481,7 @@ struct binder_priority {
> > * @inner_lock: can nest under outer_lock and/or node lock
> > * @outer_lock: no nesting under innor or node lock
> > * Lock order: 1) outer, 2) node, 3) inner
> > + * @binderfs_entry: process-specific binderfs log file
> > *
> > * Bookkeeping structure for binder processes
> > */
> > @@ -510,6 +511,7 @@ struct binder_proc {
> > struct binder_context *context;
> > spinlock_t inner_lock;
> > spinlock_t outer_lock;
> > + struct dentry *binderfs_entry;
> > };
> >
> > enum {
> > @@ -5347,6 +5349,8 @@ static int binder_open(struct inode *nodp, struct file *filp)
> > {
> > struct binder_proc *proc;
> > struct binder_device *binder_dev;
> > + struct binderfs_info *info;
> > + struct dentry *binder_binderfs_dir_entry_proc = NULL;
> >
> > binder_debug(BINDER_DEBUG_OPEN_CLOSE, "%s: %d:%d\n", __func__,
> > current->group_leader->pid, current->pid);
> > @@ -5368,11 +5372,14 @@ static int binder_open(struct inode *nodp, struct file *filp)
> > }
> >
> > /* binderfs stashes devices in i_private */
> > - if (is_binderfs_device(nodp))
> > + if (is_binderfs_device(nodp)) {
> > binder_dev = nodp->i_private;
> > - else
> > + info = nodp->i_sb->s_fs_info;
> > + binder_binderfs_dir_entry_proc = info->proc_log_dir;
> > + } else {
> > binder_dev = container_of(filp->private_data,
> > struct binder_device, miscdev);
> > + }
> > proc->context = &binder_dev->context;
> > binder_alloc_init(&proc->alloc);
> >
> > @@ -5403,6 +5410,27 @@ static int binder_open(struct inode *nodp, struct file *filp)
> > &proc_fops);
> > }
> >
> > + if (binder_binderfs_dir_entry_proc) {
> > + char strbuf[11];
> > + struct dentry *binderfs_entry;
> > +
> > + snprintf(strbuf, sizeof(strbuf), "%u", proc->pid);
> > + /*
> > + * Similar to debugfs, the process specific log file is shared
> > + * between contexts. If the file has already been created for a
> > + * process, the following binderfs_create_file() call will
> > + * fail if another context of the same process invoked
> > + * binder_open(). This is ok since same as debugfs,
> > + * the log file will contain information on all contexts of a
> > + * given PID.
> > + */
> > + binderfs_entry = binderfs_create_file(binder_binderfs_dir_entry_proc,
> > + strbuf, &proc_fops, (void *)(unsigned long)proc->pid);
> > + if (!IS_ERR(binderfs_entry))
> > + proc->binderfs_entry = binderfs_entry;
>
> You are sure that you don't want to fail the open, when the debug file
> cannot be created in the binderfs instance? I'm not objecting at all, I
> just want to make sure that this is the semantics you want because it
> would be easy to differentiate between the non-fail-debugfs and the new
> binderfs case.
I don't think we should fail the open, but it would be nice to have
some indication that it failed -- maybe a pr_warn() ?
>
> > +
> > + }
> > +
> > return 0;
> > }
> >
> > @@ -5442,6 +5470,12 @@ static int binder_release(struct inode *nodp, struct file *filp)
> > struct binder_proc *proc = filp->private_data;
> >
> > debugfs_remove(proc->debugfs_entry);
> > +
> > + if (proc->binderfs_entry) {
> > + binderfs_remove_file(proc->binderfs_entry);
> > + proc->binderfs_entry = NULL;
> > + }
> > +
> > binder_defer_work(proc, BINDER_DEFERRED_RELEASE);
> >
> > return 0;
> > diff --git a/drivers/android/binder_internal.h b/drivers/android/binder_internal.h
> > index b9be42d9464c..bd47f7f72075 100644
> > --- a/drivers/android/binder_internal.h
> > +++ b/drivers/android/binder_internal.h
> > @@ -35,17 +35,63 @@ struct binder_device {
> > struct inode *binderfs_inode;
> > };
> >
> > +/**
> > + * binderfs_mount_opts - mount options for binderfs
> > + * @max: maximum number of allocatable binderfs binder devices
> > + * @stats_mode: enable binder stats in binderfs.
> > + */
> > +struct binderfs_mount_opts {
> > + int max;
> > + int stats_mode;
> > +};
> > +
> > +/**
> > + * binderfs_info - information about a binderfs mount
> > + * @ipc_ns: The ipc namespace the binderfs mount belongs to.
> > + * @control_dentry: This records the dentry of this binderfs mount
> > + * binder-control device.
> > + * @root_uid: uid that needs to be used when a new binder device is
> > + * created.
> > + * @root_gid: gid that needs to be used when a new binder device is
> > + * created.
> > + * @mount_opts: The mount options in use.
> > + * @device_count: The current number of allocated binder devices.
> > + * @proc_log_dir: Pointer to the directory dentry containing process-specific
> > + * logs.
> > + */
> > +struct binderfs_info {
> > + struct ipc_namespace *ipc_ns;
> > + struct dentry *control_dentry;
> > + kuid_t root_uid;
> > + kgid_t root_gid;
> > + struct binderfs_mount_opts mount_opts;
> > + int device_count;
> > + struct dentry *proc_log_dir;
> > +};
> > +
> > extern const struct file_operations binder_fops;
> >
> > extern char *binder_devices_param;
> >
> > #ifdef CONFIG_ANDROID_BINDERFS
> > extern bool is_binderfs_device(const struct inode *inode);
> > +extern struct dentry *binderfs_create_file(struct dentry *dir, const char *name,
> > + const struct file_operations *fops,
> > + void *data);
> > +extern void binderfs_remove_file(struct dentry *dentry);
> > #else
> > static inline bool is_binderfs_device(const struct inode *inode)
> > {
> > return false;
> > }
> > +static inline struct dentry *binderfs_create_file(struct dentry *dir,
> > + const char *name,
> > + const struct file_operations *fops,
> > + void *data)
> > +{
> > + return NULL;
> > +}
> > +static inline void binderfs_remove_file(struct dentry *dentry) {}
> > #endif
> >
> > #ifdef CONFIG_ANDROID_BINDERFS
> > diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
> > index dc25a7d759c9..c386a3738290 100644
> > --- a/drivers/android/binderfs.c
> > +++ b/drivers/android/binderfs.c
> > @@ -48,16 +48,6 @@ static dev_t binderfs_dev;
> > static DEFINE_MUTEX(binderfs_minors_mutex);
> > static DEFINE_IDA(binderfs_minors);
> >
> > -/**
> > - * binderfs_mount_opts - mount options for binderfs
> > - * @max: maximum number of allocatable binderfs binder devices
> > - * @stats_mode: enable binder stats in binderfs.
> > - */
> > -struct binderfs_mount_opts {
> > - int max;
> > - int stats_mode;
> > -};
> > -
> > enum {
> > Opt_max,
> > Opt_stats_mode,
> > @@ -75,27 +65,6 @@ static const match_table_t tokens = {
> > { Opt_err, NULL }
> > };
> >
> > -/**
> > - * binderfs_info - information about a binderfs mount
> > - * @ipc_ns: The ipc namespace the binderfs mount belongs to.
> > - * @control_dentry: This records the dentry of this binderfs mount
> > - * binder-control device.
> > - * @root_uid: uid that needs to be used when a new binder device is
> > - * created.
> > - * @root_gid: gid that needs to be used when a new binder device is
> > - * created.
> > - * @mount_opts: The mount options in use.
> > - * @device_count: The current number of allocated binder devices.
> > - */
> > -struct binderfs_info {
> > - struct ipc_namespace *ipc_ns;
> > - struct dentry *control_dentry;
> > - kuid_t root_uid;
> > - kgid_t root_gid;
> > - struct binderfs_mount_opts mount_opts;
> > - int device_count;
> > -};
> > -
> > static inline struct binderfs_info *BINDERFS_I(const struct inode *inode)
> > {
> > return inode->i_sb->s_fs_info;
> > @@ -535,7 +504,22 @@ static struct dentry *binderfs_create_dentry(struct dentry *dir,
> > return dentry;
> > }
> >
> > -static struct dentry *binderfs_create_file(struct dentry *dir, const char *name,
> > +void binderfs_remove_file(struct dentry *dentry)
> > +{
> > + struct inode *dir;
> > +
> > + dir = d_inode(dentry->d_parent);
> > + inode_lock(dir);
> > + if (simple_positive(dentry)) {
> > + dget(dentry);
> > + simple_unlink(dir, dentry);
> > + d_delete(dentry);
> > + dput(dentry);
> > + }
> > + inode_unlock(dir);
> > +}
> > +
> > +struct dentry *binderfs_create_file(struct dentry *dir, const char *name,
> > const struct file_operations *fops,
> > void *data)
> > {
> > @@ -604,7 +588,8 @@ static struct dentry *binderfs_create_dir(struct dentry *parent,
> >
> > static int init_binder_logs(struct super_block *sb)
> > {
> > - struct dentry *binder_logs_root_dir, *file_dentry;
> > + struct dentry *binder_logs_root_dir, *file_dentry, *proc_log_dir;
> > + struct binderfs_info *info;
> > int ret = 0;
> >
> > binder_logs_root_dir = binderfs_create_dir(sb->s_root,
> > @@ -648,8 +633,18 @@ static int init_binder_logs(struct super_block *sb)
> > "failed_transaction_log",
> > &binder_transaction_log_fops,
> > &binder_transaction_log_failed);
> > - if (IS_ERR(file_dentry))
> > + if (IS_ERR(file_dentry)) {
> > ret = PTR_ERR(file_dentry);
> > + goto out;
> > + }
> > +
> > + proc_log_dir = binderfs_create_dir(binder_logs_root_dir, "proc");
> > + if (IS_ERR(proc_log_dir)) {
> > + ret = PTR_ERR(proc_log_dir);
> > + goto out;
> > + }
> > + info = sb->s_fs_info;
> > + info->proc_log_dir = proc_log_dir;
> >
> > out:
> > return ret;
> > --
> > 2.23.0.187.g17f5b7556c-goog
> >
>
> --
> To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@android.com.
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] binder: add a mount option to show global stats
2019-08-28 12:39 ` Christian Brauner
@ 2019-08-28 17:21 ` Hridya Valsaraju
0 siblings, 0 replies; 15+ messages in thread
From: Hridya Valsaraju @ 2019-08-28 17:21 UTC (permalink / raw)
To: Christian Brauner
Cc: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos,
Martijn Coenen, Joel Fernandes, devel, linux-kernel, kernel-team
On Wed, Aug 28, 2019 at 5:39 AM Christian Brauner
<christian.brauner@ubuntu.com> wrote:
>
> On Wed, Aug 28, 2019 at 11:22:37AM +0200, Greg Kroah-Hartman wrote:
> > On Tue, Aug 27, 2019 at 01:41:49PM -0700, Hridya Valsaraju wrote:
> > > Currently, all binder state and statistics live in debugfs.
> > > We need this information even when debugfs is not mounted.
> > > This patch adds the mount option 'stats' to enable a binderfs
> > > instance to have binder debug information present in the same.
> > > 'stats=global' will enable the global binder statistics. In
> > > the future, 'stats=local' will enable binder statistics local
> > > to the binderfs instance. The two modes 'global' and 'local'
> > > will be mutually exclusive. 'stats=global' option is only available
> > > for a binderfs instance mounted in the initial user namespace.
> > > An attempt to use the option to mount a binderfs instance in
> > > another user namespace will return an EPERM error.
> > >
> > > Signed-off-by: Hridya Valsaraju <hridya@google.com>
> > > ---
> > > drivers/android/binderfs.c | 47 ++++++++++++++++++++++++++++++++++++--
> > > 1 file changed, 45 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
> > > index cc2e71576396..d95d179aec58 100644
> > > --- a/drivers/android/binderfs.c
> > > +++ b/drivers/android/binderfs.c
> > > @@ -51,18 +51,27 @@ static DEFINE_IDA(binderfs_minors);
> > > /**
> > > * binderfs_mount_opts - mount options for binderfs
> > > * @max: maximum number of allocatable binderfs binder devices
> > > + * @stats_mode: enable binder stats in binderfs.
> > > */
> > > struct binderfs_mount_opts {
> > > int max;
> > > + int stats_mode;
> > > };
> > >
> > > enum {
> > > Opt_max,
> > > + Opt_stats_mode,
> > > Opt_err
> > > };
> > >
> > > +enum binderfs_stats_mode {
> > > + STATS_NONE,
> > > + STATS_GLOBAL,
> > > +};
> > > +
> > > static const match_table_t tokens = {
> > > { Opt_max, "max=%d" },
> > > + { Opt_stats_mode, "stats=%s" },
> > > { Opt_err, NULL }
> > > };
> > >
> > > @@ -290,8 +299,9 @@ static void binderfs_evict_inode(struct inode *inode)
> > > static int binderfs_parse_mount_opts(char *data,
> > > struct binderfs_mount_opts *opts)
> > > {
> > > - char *p;
> > > + char *p, *stats;
> > > opts->max = BINDERFS_MAX_MINOR;
> > > + opts->stats_mode = STATS_NONE;
> > >
> > > while ((p = strsep(&data, ",")) != NULL) {
> > > substring_t args[MAX_OPT_ARGS];
> > > @@ -311,6 +321,24 @@ static int binderfs_parse_mount_opts(char *data,
> > >
> > > opts->max = max_devices;
> > > break;
> > > + case Opt_stats_mode:
> > > + stats = match_strdup(&args[0]);
> > > + if (!stats)
> > > + return -ENOMEM;
> > > +
> > > + if (strcmp(stats, "global") != 0) {
> > > + kfree(stats);
> > > + return -EINVAL;
> > > + }
> > > +
> > > + if (!capable(CAP_SYS_ADMIN)) {
> > > + kfree(stats);
> > > + return -EINVAL;
> >
> > Can a non-CAP_SYS_ADMIN task even call this function? Anyway, if it
>
> It can. A task that has CAP_SYS_ADMIN in the userns the corresponding
> binderfs mount has been created in can change the max=<nr> mount option.
> Only stats=global currently requires capable(CAP_SYS_ADMIN) aka
> CAP_SYS_ADMIN in the initial userns to prevent non-initial userns from
> snooping at global statistics.
>
> > can, put the check at the top of the case, and just return early before
> > doing any extra work like checking values or allocating memory.
Thank you Greg and Christian for reviewing the patch! That makes
sense, I will make the change and send out v2 soon.
> >
> > > + }
> > > +
> > > + opts->stats_mode = STATS_GLOBAL;
> > > + kfree(stats);
> > > + break;
> > > default:
> > > pr_err("Invalid mount options\n");
> > > return -EINVAL;
> > > @@ -322,8 +350,21 @@ static int binderfs_parse_mount_opts(char *data,
> > >
> > > static int binderfs_remount(struct super_block *sb, int *flags, char *data)
> > > {
> > > + int prev_stats_mode, ret;
> > > struct binderfs_info *info = sb->s_fs_info;
> > > - return binderfs_parse_mount_opts(data, &info->mount_opts);
> > > +
> > > + prev_stats_mode = info->mount_opts.stats_mode;
> > > + ret = binderfs_parse_mount_opts(data, &info->mount_opts);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + if (prev_stats_mode != info->mount_opts.stats_mode) {
> > > + pr_info("Binderfs stats mode cannot be changed during a remount\n");
> >
> > pr_err()?
> >
> > thanks,
> >
> > greg k-h
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/4] binder: Add stats, state and transactions files
2019-08-28 12:58 ` Christian Brauner
@ 2019-08-28 20:19 ` Hridya Valsaraju
0 siblings, 0 replies; 15+ messages in thread
From: Hridya Valsaraju @ 2019-08-28 20:19 UTC (permalink / raw)
To: Christian Brauner
Cc: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos,
Martijn Coenen, Joel Fernandes, devel, linux-kernel, kernel-team
On Wed, Aug 28, 2019 at 5:58 AM Christian Brauner
<christian.brauner@ubuntu.com> wrote:
>
> On Tue, Aug 27, 2019 at 01:41:50PM -0700, Hridya Valsaraju wrote:
> > The following binder stat files currently live in debugfs.
> >
> > /sys/kernel/debug/binder/state
> > /sys/kernel/debug/binder/stats
> > /sys/kernel/debug/binder/transactions
> >
> > This patch makes these files available in a binderfs instance
> > mounted with the mount option 'stats=global'. For example, if a binderfs
> > instance is mounted at path /dev/binderfs, the above files will be
> > available at the following locations:
> >
> > /dev/binderfs/binder_logs/state
> > /dev/binderfs/binder_logs/stats
> > /dev/binderfs/binder_logs/transactions
> >
> > This provides a way to access them even when debugfs is not mounted.
> >
> > Signed-off-by: Hridya Valsaraju <hridya@google.com>
> > ---
> > drivers/android/binder.c | 15 ++--
> > drivers/android/binder_internal.h | 8 ++
> > drivers/android/binderfs.c | 137 +++++++++++++++++++++++++++++-
> > 3 files changed, 150 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> > index ca6b21a53321..de795bd229c4 100644
> > --- a/drivers/android/binder.c
> > +++ b/drivers/android/binder.c
> > @@ -6055,7 +6055,7 @@ static void print_binder_proc_stats(struct seq_file *m,
> > }
> >
> >
> > -static int state_show(struct seq_file *m, void *unused)
> > +int binder_state_show(struct seq_file *m, void *unused)
> > {
> > struct binder_proc *proc;
> > struct binder_node *node;
> > @@ -6094,7 +6094,7 @@ static int state_show(struct seq_file *m, void *unused)
> > return 0;
> > }
> >
> > -static int stats_show(struct seq_file *m, void *unused)
> > +int binder_stats_show(struct seq_file *m, void *unused)
> > {
> > struct binder_proc *proc;
> >
> > @@ -6110,7 +6110,7 @@ static int stats_show(struct seq_file *m, void *unused)
> > return 0;
> > }
> >
> > -static int transactions_show(struct seq_file *m, void *unused)
> > +int binder_transactions_show(struct seq_file *m, void *unused)
> > {
> > struct binder_proc *proc;
> >
> > @@ -6198,9 +6198,6 @@ const struct file_operations binder_fops = {
> > .release = binder_release,
> > };
> >
> > -DEFINE_SHOW_ATTRIBUTE(state);
> > -DEFINE_SHOW_ATTRIBUTE(stats);
> > -DEFINE_SHOW_ATTRIBUTE(transactions);
> > DEFINE_SHOW_ATTRIBUTE(transaction_log);
> >
> > static int __init init_binder_device(const char *name)
> > @@ -6256,17 +6253,17 @@ static int __init binder_init(void)
> > 0444,
> > binder_debugfs_dir_entry_root,
> > NULL,
> > - &state_fops);
> > + &binder_state_fops);
> > debugfs_create_file("stats",
> > 0444,
> > binder_debugfs_dir_entry_root,
> > NULL,
> > - &stats_fops);
> > + &binder_stats_fops);
> > debugfs_create_file("transactions",
> > 0444,
> > binder_debugfs_dir_entry_root,
> > NULL,
> > - &transactions_fops);
> > + &binder_transactions_fops);
> > debugfs_create_file("transaction_log",
> > 0444,
> > binder_debugfs_dir_entry_root,
> > diff --git a/drivers/android/binder_internal.h b/drivers/android/binder_internal.h
> > index fe8c745dc8e0..12ef96f256c6 100644
> > --- a/drivers/android/binder_internal.h
> > +++ b/drivers/android/binder_internal.h
> > @@ -57,4 +57,12 @@ static inline int __init init_binderfs(void)
> > }
> > #endif
> >
> > +int binder_stats_show(struct seq_file *m, void *unused);
> > +DEFINE_SHOW_ATTRIBUTE(binder_stats);
> > +
> > +int binder_state_show(struct seq_file *m, void *unused);
> > +DEFINE_SHOW_ATTRIBUTE(binder_state);
> > +
> > +int binder_transactions_show(struct seq_file *m, void *unused);
> > +DEFINE_SHOW_ATTRIBUTE(binder_transactions);
> > #endif /* _LINUX_BINDER_INTERNAL_H */
> > diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
> > index d95d179aec58..d542f9b8d8ab 100644
> > --- a/drivers/android/binderfs.c
> > +++ b/drivers/android/binderfs.c
> > @@ -280,7 +280,7 @@ static void binderfs_evict_inode(struct inode *inode)
> >
> > clear_inode(inode);
> >
> > - if (!device)
> > + if (!device || S_ISREG(inode->i_mode))
>
> Hm, remind me why we need the S_ISREG again?
Thanks for taking a look Christian! We need the additional check
because for some of the stat files(for example, the transaction log
file in patch 3/5), the binder driver uses the i_private field of its
inode to stash some data that will be used for content generation when
the file is opened.
> Also we probably should do:
>
> if (S_ISREG(inode->i_mode) || !device)
>
> should this maybe be:
>
> if (!S_ISCHR(inode->i_mode) || !device)
>
> ?
Sounds good to me, will make the change in v2!
>
> > return;
> >
> > mutex_lock(&binderfs_minors_mutex);
> > @@ -504,6 +504,138 @@ static const struct inode_operations binderfs_dir_inode_operations = {
> > .unlink = binderfs_unlink,
> > };
> >
> > +static struct inode *binderfs_make_inode(struct super_block *sb, int mode)
> > +{
> > + struct inode *ret;
> > +
> > + ret = new_inode(sb);
> > + if (ret) {
> > + ret->i_ino = iunique(sb, BINDERFS_MAX_MINOR + INODE_OFFSET);
> > + ret->i_mode = mode;
> > + ret->i_atime = ret->i_mtime = ret->i_ctime = current_time(ret);
> > + }
> > + return ret;
> > +}
> > +
> > +static struct dentry *binderfs_create_dentry(struct dentry *dir,
> > + const char *name)
> > +{
> > + struct dentry *dentry;
> > +
> > + dentry = lookup_one_len(name, dir, strlen(name));
> > + if (IS_ERR(dentry))
> > + return dentry;
> > +
> > + /* Return error if the file/dir already exists. */
> > + if (d_really_is_positive(dentry)) {
> > + dput(dentry);
> > + return ERR_PTR(-EEXIST);
> > + }
> > +
> > + return dentry;
> > +}
> > +
> > +static struct dentry *binderfs_create_file(struct dentry *dir, const char *name,
> > + const struct file_operations *fops,
> > + void *data)
> > +{
> > + struct dentry *dentry;
> > + struct inode *new_inode, *dir_inode;
> > + struct super_block *sb;
> > +
> > + dir_inode = dir->d_inode;
> > + inode_lock(dir_inode);
> > +
> > + dentry = binderfs_create_dentry(dir, name);
> > + if (IS_ERR(dentry))
> > + goto out;
> > +
> > + sb = dir_inode->i_sb;
> > + new_inode = binderfs_make_inode(sb, S_IFREG | 0444);
> > + if (!new_inode) {
> > + dput(dentry);
> > + dentry = ERR_PTR(-ENOMEM);
> > + goto out;
> > + }
> > +
> > + new_inode->i_fop = fops;
> > + new_inode->i_private = data;
> > + d_instantiate(dentry, new_inode);
> > + fsnotify_create(dir_inode, dentry);
> > +
> > +out:
> > + inode_unlock(dir_inode);
> > + return dentry;
> > +}
> > +
> > +static struct dentry *binderfs_create_dir(struct dentry *parent,
> > + const char *name)
> > +{
> > + struct dentry *dentry;
> > + struct inode *new_inode, *parent_inode;
> > + struct super_block *sb;
> > +
> > + parent_inode = d_inode(parent);
>
> For consistency, could you use the same variable name for the directory
> in which you create a new dentry? I don't care if its "dir_inode" like
> above or "parent_inode".
Makes sense, will make the change in v2.
>
> > + inode_lock(parent_inode);
> > +
> > + dentry = binderfs_create_dentry(parent, name);
> > + if (IS_ERR(dentry))
> > + goto out;
> > +
> > + sb = parent_inode->i_sb;
> > + new_inode = binderfs_make_inode(sb, S_IFDIR | 0755);
> > + if (!new_inode) {
> > + dput(dentry);
> > + dentry = ERR_PTR(-ENOMEM);
> > + goto out;
> > + }
> > +
> > + new_inode->i_fop = &simple_dir_operations;
> > + new_inode->i_op = &simple_dir_inode_operations;
> > +
> > + inc_nlink(new_inode);
> > + d_instantiate(dentry, new_inode);
> > + inc_nlink(parent_inode);
> > + fsnotify_mkdir(parent_inode, dentry);
> > +out:
>
> For consistency please leave a \n after fsnotify_mkdir and the goto
> label like you did in the function above.
Agreed, will fix in v2.
>
> > + inode_unlock(parent_inode);
> > + return dentry;
> > +}
> > +
> > +static int init_binder_logs(struct super_block *sb)
> > +{
> > + struct dentry *binder_logs_root_dir, *file_dentry;
>
> Why "file_dentry" and not just simply "dentry" like everywhere else?
I was trying to distinguish it from the directory dentries in the
function but I don't mind changing it to just 'dentry' for
consistency.
>
> > + int ret = 0;
> > +
> > + binder_logs_root_dir = binderfs_create_dir(sb->s_root,
> > + "binder_logs");
> > + if (IS_ERR(binder_logs_root_dir)) {
> > + ret = PTR_ERR(binder_logs_root_dir);
> > + goto out;
> > + }
> > +
> > + file_dentry = binderfs_create_file(binder_logs_root_dir, "stats",
> > + &binder_stats_fops, NULL);
> > + if (IS_ERR(file_dentry)) {
> > + ret = PTR_ERR(file_dentry);
> > + goto out;
> > + }
> > +
> > + file_dentry = binderfs_create_file(binder_logs_root_dir, "state",
> > + &binder_state_fops, NULL);
> > + if (IS_ERR(file_dentry)) {
> > + ret = PTR_ERR(file_dentry);
> > + goto out;
> > + }
> > +
> > + file_dentry = binderfs_create_file(binder_logs_root_dir, "transactions",
> > + &binder_transactions_fops, NULL);
> > + if (IS_ERR(file_dentry))
> > + ret = PTR_ERR(file_dentry);
> > +out:
> > + return ret;
> > +}
> > +
> > static int binderfs_fill_super(struct super_block *sb, void *data, int silent)
> > {
> > int ret;
> > @@ -582,6 +714,9 @@ static int binderfs_fill_super(struct super_block *sb, void *data, int silent)
> >
> > }
> >
> > + if (info->mount_opts.stats_mode == STATS_GLOBAL)
> > + return init_binder_logs(sb);
> > +
> > return 0;
> > }
> >
> > --
> > 2.23.0.187.g17f5b7556c-goog
> >
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/4] binder: Add binder_proc logging to binderfs
2019-08-28 16:21 ` Todd Kjos
@ 2019-08-28 20:42 ` Hridya Valsaraju
0 siblings, 0 replies; 15+ messages in thread
From: Hridya Valsaraju @ 2019-08-28 20:42 UTC (permalink / raw)
To: Todd Kjos
Cc: Christian Brauner, Greg Kroah-Hartman, Arve Hjønnevåg,
Todd Kjos, Martijn Coenen, Joel Fernandes,
open list:ANDROID DRIVERS, LKML, Android Kernel Team
On Wed, Aug 28, 2019 at 9:21 AM Todd Kjos <tkjos@google.com> wrote:
>
> On Wed, Aug 28, 2019 at 6:08 AM Christian Brauner
> <christian.brauner@ubuntu.com> wrote:
> >
> > On Tue, Aug 27, 2019 at 01:41:52PM -0700, Hridya Valsaraju wrote:
> > > Currently /sys/kernel/debug/binder/proc contains
> > > the debug data for every binder_proc instance.
> > > This patch makes this information also available
> > > in a binderfs instance mounted with a mount option
> > > "stats=global" in addition to debugfs. The patch does
> > > not affect the presence of the file in debugfs.
> > >
> > > If a binderfs instance is mounted at path /dev/binderfs,
> > > this file would be present at /dev/binderfs/binder_logs/proc.
> > > This change provides an alternate way to access this file when debugfs
> > > is not mounted.
> > >
> > > Signed-off-by: Hridya Valsaraju <hridya@google.com>
> >
> > I'm still wondering whether there's a nicer way to create those debuf
> > files per-process without doing it in binder_open() but it has worked
> > fine for a long time with debugfs.
> >
> > Also, one minor question below. Otherwise
> >
> > Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
>
> Acked-by: Todd Kjos <tkjos@google.com>
>
> >
> > > ---
> > > drivers/android/binder.c | 38 ++++++++++++++++++-
> > > drivers/android/binder_internal.h | 46 ++++++++++++++++++++++
> > > drivers/android/binderfs.c | 63 ++++++++++++++-----------------
> > > 3 files changed, 111 insertions(+), 36 deletions(-)
> > >
> > > diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> > > index bed217310197..37189838f32a 100644
> > > --- a/drivers/android/binder.c
> > > +++ b/drivers/android/binder.c
> > > @@ -481,6 +481,7 @@ struct binder_priority {
> > > * @inner_lock: can nest under outer_lock and/or node lock
> > > * @outer_lock: no nesting under innor or node lock
> > > * Lock order: 1) outer, 2) node, 3) inner
> > > + * @binderfs_entry: process-specific binderfs log file
> > > *
> > > * Bookkeeping structure for binder processes
> > > */
> > > @@ -510,6 +511,7 @@ struct binder_proc {
> > > struct binder_context *context;
> > > spinlock_t inner_lock;
> > > spinlock_t outer_lock;
> > > + struct dentry *binderfs_entry;
> > > };
> > >
> > > enum {
> > > @@ -5347,6 +5349,8 @@ static int binder_open(struct inode *nodp, struct file *filp)
> > > {
> > > struct binder_proc *proc;
> > > struct binder_device *binder_dev;
> > > + struct binderfs_info *info;
> > > + struct dentry *binder_binderfs_dir_entry_proc = NULL;
> > >
> > > binder_debug(BINDER_DEBUG_OPEN_CLOSE, "%s: %d:%d\n", __func__,
> > > current->group_leader->pid, current->pid);
> > > @@ -5368,11 +5372,14 @@ static int binder_open(struct inode *nodp, struct file *filp)
> > > }
> > >
> > > /* binderfs stashes devices in i_private */
> > > - if (is_binderfs_device(nodp))
> > > + if (is_binderfs_device(nodp)) {
> > > binder_dev = nodp->i_private;
> > > - else
> > > + info = nodp->i_sb->s_fs_info;
> > > + binder_binderfs_dir_entry_proc = info->proc_log_dir;
> > > + } else {
> > > binder_dev = container_of(filp->private_data,
> > > struct binder_device, miscdev);
> > > + }
> > > proc->context = &binder_dev->context;
> > > binder_alloc_init(&proc->alloc);
> > >
> > > @@ -5403,6 +5410,27 @@ static int binder_open(struct inode *nodp, struct file *filp)
> > > &proc_fops);
> > > }
> > >
> > > + if (binder_binderfs_dir_entry_proc) {
> > > + char strbuf[11];
> > > + struct dentry *binderfs_entry;
> > > +
> > > + snprintf(strbuf, sizeof(strbuf), "%u", proc->pid);
> > > + /*
> > > + * Similar to debugfs, the process specific log file is shared
> > > + * between contexts. If the file has already been created for a
> > > + * process, the following binderfs_create_file() call will
> > > + * fail if another context of the same process invoked
> > > + * binder_open(). This is ok since same as debugfs,
> > > + * the log file will contain information on all contexts of a
> > > + * given PID.
> > > + */
> > > + binderfs_entry = binderfs_create_file(binder_binderfs_dir_entry_proc,
> > > + strbuf, &proc_fops, (void *)(unsigned long)proc->pid);
> > > + if (!IS_ERR(binderfs_entry))
> > > + proc->binderfs_entry = binderfs_entry;
> >
> > You are sure that you don't want to fail the open, when the debug file
> > cannot be created in the binderfs instance? I'm not objecting at all, I
> > just want to make sure that this is the semantics you want because it
> > would be easy to differentiate between the non-fail-debugfs and the new
> > binderfs case.
>
> I don't think we should fail the open, but it would be nice to have
> some indication that it failed -- maybe a pr_warn() ?
Thanks for taking a look Christian and Todd! The reason I do not fail
binder_open() on an error here is because once the file has been
created for a process, if the same process invokes binder_open() again
from a different context, the binderfs_create_file() call would fail
with the EEXIST error code. This is expected behaviour since the log
file is shared between all contexts of a process(same as the behaviour
of the file in debugfs today). I can add a pr_warn() statement in v2
as Todd suggested so that the failure would be logged.
> >
> > > +
> > > + }
> > > +
> > > return 0;
> > > }
> > >
> > > @@ -5442,6 +5470,12 @@ static int binder_release(struct inode *nodp, struct file *filp)
> > > struct binder_proc *proc = filp->private_data;
> > >
> > > debugfs_remove(proc->debugfs_entry);
> > > +
> > > + if (proc->binderfs_entry) {
> > > + binderfs_remove_file(proc->binderfs_entry);
> > > + proc->binderfs_entry = NULL;
> > > + }
> > > +
> > > binder_defer_work(proc, BINDER_DEFERRED_RELEASE);
> > >
> > > return 0;
> > > diff --git a/drivers/android/binder_internal.h b/drivers/android/binder_internal.h
> > > index b9be42d9464c..bd47f7f72075 100644
> > > --- a/drivers/android/binder_internal.h
> > > +++ b/drivers/android/binder_internal.h
> > > @@ -35,17 +35,63 @@ struct binder_device {
> > > struct inode *binderfs_inode;
> > > };
> > >
> > > +/**
> > > + * binderfs_mount_opts - mount options for binderfs
> > > + * @max: maximum number of allocatable binderfs binder devices
> > > + * @stats_mode: enable binder stats in binderfs.
> > > + */
> > > +struct binderfs_mount_opts {
> > > + int max;
> > > + int stats_mode;
> > > +};
> > > +
> > > +/**
> > > + * binderfs_info - information about a binderfs mount
> > > + * @ipc_ns: The ipc namespace the binderfs mount belongs to.
> > > + * @control_dentry: This records the dentry of this binderfs mount
> > > + * binder-control device.
> > > + * @root_uid: uid that needs to be used when a new binder device is
> > > + * created.
> > > + * @root_gid: gid that needs to be used when a new binder device is
> > > + * created.
> > > + * @mount_opts: The mount options in use.
> > > + * @device_count: The current number of allocated binder devices.
> > > + * @proc_log_dir: Pointer to the directory dentry containing process-specific
> > > + * logs.
> > > + */
> > > +struct binderfs_info {
> > > + struct ipc_namespace *ipc_ns;
> > > + struct dentry *control_dentry;
> > > + kuid_t root_uid;
> > > + kgid_t root_gid;
> > > + struct binderfs_mount_opts mount_opts;
> > > + int device_count;
> > > + struct dentry *proc_log_dir;
> > > +};
> > > +
> > > extern const struct file_operations binder_fops;
> > >
> > > extern char *binder_devices_param;
> > >
> > > #ifdef CONFIG_ANDROID_BINDERFS
> > > extern bool is_binderfs_device(const struct inode *inode);
> > > +extern struct dentry *binderfs_create_file(struct dentry *dir, const char *name,
> > > + const struct file_operations *fops,
> > > + void *data);
> > > +extern void binderfs_remove_file(struct dentry *dentry);
> > > #else
> > > static inline bool is_binderfs_device(const struct inode *inode)
> > > {
> > > return false;
> > > }
> > > +static inline struct dentry *binderfs_create_file(struct dentry *dir,
> > > + const char *name,
> > > + const struct file_operations *fops,
> > > + void *data)
> > > +{
> > > + return NULL;
> > > +}
> > > +static inline void binderfs_remove_file(struct dentry *dentry) {}
> > > #endif
> > >
> > > #ifdef CONFIG_ANDROID_BINDERFS
> > > diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
> > > index dc25a7d759c9..c386a3738290 100644
> > > --- a/drivers/android/binderfs.c
> > > +++ b/drivers/android/binderfs.c
> > > @@ -48,16 +48,6 @@ static dev_t binderfs_dev;
> > > static DEFINE_MUTEX(binderfs_minors_mutex);
> > > static DEFINE_IDA(binderfs_minors);
> > >
> > > -/**
> > > - * binderfs_mount_opts - mount options for binderfs
> > > - * @max: maximum number of allocatable binderfs binder devices
> > > - * @stats_mode: enable binder stats in binderfs.
> > > - */
> > > -struct binderfs_mount_opts {
> > > - int max;
> > > - int stats_mode;
> > > -};
> > > -
> > > enum {
> > > Opt_max,
> > > Opt_stats_mode,
> > > @@ -75,27 +65,6 @@ static const match_table_t tokens = {
> > > { Opt_err, NULL }
> > > };
> > >
> > > -/**
> > > - * binderfs_info - information about a binderfs mount
> > > - * @ipc_ns: The ipc namespace the binderfs mount belongs to.
> > > - * @control_dentry: This records the dentry of this binderfs mount
> > > - * binder-control device.
> > > - * @root_uid: uid that needs to be used when a new binder device is
> > > - * created.
> > > - * @root_gid: gid that needs to be used when a new binder device is
> > > - * created.
> > > - * @mount_opts: The mount options in use.
> > > - * @device_count: The current number of allocated binder devices.
> > > - */
> > > -struct binderfs_info {
> > > - struct ipc_namespace *ipc_ns;
> > > - struct dentry *control_dentry;
> > > - kuid_t root_uid;
> > > - kgid_t root_gid;
> > > - struct binderfs_mount_opts mount_opts;
> > > - int device_count;
> > > -};
> > > -
> > > static inline struct binderfs_info *BINDERFS_I(const struct inode *inode)
> > > {
> > > return inode->i_sb->s_fs_info;
> > > @@ -535,7 +504,22 @@ static struct dentry *binderfs_create_dentry(struct dentry *dir,
> > > return dentry;
> > > }
> > >
> > > -static struct dentry *binderfs_create_file(struct dentry *dir, const char *name,
> > > +void binderfs_remove_file(struct dentry *dentry)
> > > +{
> > > + struct inode *dir;
> > > +
> > > + dir = d_inode(dentry->d_parent);
> > > + inode_lock(dir);
> > > + if (simple_positive(dentry)) {
> > > + dget(dentry);
> > > + simple_unlink(dir, dentry);
> > > + d_delete(dentry);
> > > + dput(dentry);
> > > + }
> > > + inode_unlock(dir);
> > > +}
> > > +
> > > +struct dentry *binderfs_create_file(struct dentry *dir, const char *name,
> > > const struct file_operations *fops,
> > > void *data)
> > > {
> > > @@ -604,7 +588,8 @@ static struct dentry *binderfs_create_dir(struct dentry *parent,
> > >
> > > static int init_binder_logs(struct super_block *sb)
> > > {
> > > - struct dentry *binder_logs_root_dir, *file_dentry;
> > > + struct dentry *binder_logs_root_dir, *file_dentry, *proc_log_dir;
> > > + struct binderfs_info *info;
> > > int ret = 0;
> > >
> > > binder_logs_root_dir = binderfs_create_dir(sb->s_root,
> > > @@ -648,8 +633,18 @@ static int init_binder_logs(struct super_block *sb)
> > > "failed_transaction_log",
> > > &binder_transaction_log_fops,
> > > &binder_transaction_log_failed);
> > > - if (IS_ERR(file_dentry))
> > > + if (IS_ERR(file_dentry)) {
> > > ret = PTR_ERR(file_dentry);
> > > + goto out;
> > > + }
> > > +
> > > + proc_log_dir = binderfs_create_dir(binder_logs_root_dir, "proc");
> > > + if (IS_ERR(proc_log_dir)) {
> > > + ret = PTR_ERR(proc_log_dir);
> > > + goto out;
> > > + }
> > > + info = sb->s_fs_info;
> > > + info->proc_log_dir = proc_log_dir;
> > >
> > > out:
> > > return ret;
> > > --
> > > 2.23.0.187.g17f5b7556c-goog
> > >
> >
> > --
> > To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@android.com.
> >
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2019-08-28 20:42 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-27 20:41 [PATCH 0/4] Add binder state and statistics to binderfs Hridya Valsaraju
2019-08-27 20:41 ` [PATCH 1/4] binder: add a mount option to show global stats Hridya Valsaraju
2019-08-28 9:22 ` Greg Kroah-Hartman
2019-08-28 12:39 ` Christian Brauner
2019-08-28 17:21 ` Hridya Valsaraju
2019-08-27 20:41 ` [PATCH 2/4] binder: Add stats, state and transactions files Hridya Valsaraju
2019-08-28 12:58 ` Christian Brauner
2019-08-28 20:19 ` Hridya Valsaraju
2019-08-27 20:41 ` [PATCH 3/4] binder: Make transaction_log available in binderfs Hridya Valsaraju
2019-08-28 12:59 ` Christian Brauner
2019-08-28 16:05 ` Todd Kjos
2019-08-27 20:41 ` [PATCH 4/4] binder: Add binder_proc logging to binderfs Hridya Valsaraju
2019-08-28 13:08 ` Christian Brauner
2019-08-28 16:21 ` Todd Kjos
2019-08-28 20:42 ` Hridya Valsaraju
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).