2019-09-03 16:18:15

by Hridya Valsaraju

[permalink] [raw]
Subject: [PATCH v3 0/4] Add binder state and statistics to binderfs

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 | 95 ++++++-----
drivers/android/binder_internal.h | 84 ++++++++++
drivers/android/binderfs.c | 255 ++++++++++++++++++++++++++----
3 files changed, 362 insertions(+), 72 deletions(-)

--
2.23.0.187.g17f5b7556c-goog


2019-09-03 16:18:23

by Hridya Valsaraju

[permalink] [raw]
Subject: [PATCH v3 1/4] binder: add a mount option to show global stats

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 <[email protected]>
---

Changes in v2:
- Improve error check in binderfs_parse_mount_opts() as per Greg
Kroah-Hartman.
- Change pr_info() log before failure to pr_err() as per Greg
Kroah-Hartman.

drivers/android/binderfs.c | 45 ++++++++++++++++++++++++++++++++++++--
1 file changed, 43 insertions(+), 2 deletions(-)

diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
index cc2e71576396..7045bfe5b52b 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,22 @@ static int binderfs_parse_mount_opts(char *data,

opts->max = max_devices;
break;
+ case Opt_stats_mode:
+ if (!capable(CAP_SYS_ADMIN))
+ return -EINVAL;
+
+ stats = match_strdup(&args[0]);
+ if (!stats)
+ return -ENOMEM;
+
+ if (strcmp(stats, "global") != 0) {
+ kfree(stats);
+ return -EINVAL;
+ }
+
+ opts->stats_mode = STATS_GLOBAL;
+ kfree(stats);
+ break;
default:
pr_err("Invalid mount options\n");
return -EINVAL;
@@ -322,8 +348,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_err("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 +372,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

2019-09-03 16:18:52

by Hridya Valsaraju

[permalink] [raw]
Subject: [PATCH v3 2/4] binder: Add stats, state and transactions files

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.

Acked-by: Christian Brauner <[email protected]>
Signed-off-by: Hridya Valsaraju <[email protected]>
---

Changes in v3:
- Use set_nlink() instead of inc_nlink() in binderfs_create_dir() as
per Christian Brauner.
- Replace parent->d_inode usage with d_inode(parent) in
binderfs_create_file() for consistency as per Christian Brauner.

Changes in v2:
- Consistently name variables across functions as per Christian
Brauner.
- Improve check for binderfs device in binderfs_evict_inode()
as per Christian Brauner.

drivers/android/binder.c | 15 ++--
drivers/android/binder_internal.h | 8 ++
drivers/android/binderfs.c | 140 +++++++++++++++++++++++++++++-
3 files changed, 153 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 7045bfe5b52b..01c1db463053 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 (!S_ISCHR(inode->i_mode) || !device)
return;

mutex_lock(&binderfs_minors_mutex);
@@ -502,6 +502,141 @@ 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 *parent,
+ const char *name)
+{
+ struct dentry *dentry;
+
+ dentry = lookup_one_len(name, parent, 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 *parent,
+ const char *name,
+ const struct file_operations *fops,
+ void *data)
+{
+ 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_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(parent_inode, dentry);
+
+out:
+ inode_unlock(parent_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;
+
+ set_nlink(new_inode, 2);
+ 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, *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;
+ }
+
+ dentry = binderfs_create_file(binder_logs_root_dir, "stats",
+ &binder_stats_fops, NULL);
+ if (IS_ERR(dentry)) {
+ ret = PTR_ERR(dentry);
+ goto out;
+ }
+
+ dentry = binderfs_create_file(binder_logs_root_dir, "state",
+ &binder_state_fops, NULL);
+ if (IS_ERR(dentry)) {
+ ret = PTR_ERR(dentry);
+ goto out;
+ }
+
+ dentry = binderfs_create_file(binder_logs_root_dir, "transactions",
+ &binder_transactions_fops, NULL);
+ if (IS_ERR(dentry))
+ ret = PTR_ERR(dentry);
+
+out:
+ return ret;
+}
+
static int binderfs_fill_super(struct super_block *sb, void *data, int silent)
{
int ret;
@@ -580,6 +715,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

2019-09-03 16:19:25

by Hridya Valsaraju

[permalink] [raw]
Subject: [PATCH v3 4/4] binder: Add binder_proc logging to binderfs

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.

Acked-by: Christian Brauner <[email protected]>
Signed-off-by: Hridya Valsaraju <[email protected]>
---

Changes in v2:
- Consistent variable naming across functions as per Christian
Brauner.
- As suggested by Todd Kjos, log a failure to create a
process-specific binderfs log file if the error code is not EEXIST
since an error code of EEXIST is expected if
multiple contexts of the same process try to create the file during
binder_open().

drivers/android/binder.c | 46 ++++++++++++++++++++-
drivers/android/binder_internal.h | 46 +++++++++++++++++++++
drivers/android/binderfs.c | 68 ++++++++++++++-----------------
3 files changed, 121 insertions(+), 39 deletions(-)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index bed217310197..ee610ea48309 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,35 @@ 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 with error code EEXIST 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;
+ } else {
+ int error;
+
+ error = PTR_ERR(binderfs_entry);
+ if (error != -EEXIST) {
+ pr_warn("Unable to create file %s in binderfs (error %d)\n",
+ strbuf, error);
+ }
+ }
+ }
+
return 0;
}

@@ -5442,6 +5478,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 9bb54fd3a48a..aee0b8aeff94 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;
@@ -533,10 +502,24 @@ static struct dentry *binderfs_create_dentry(struct dentry *parent,
return dentry;
}

-static struct dentry *binderfs_create_file(struct dentry *parent,
- const char *name,
- const struct file_operations *fops,
- void *data)
+void binderfs_remove_file(struct dentry *dentry)
+{
+ struct inode *parent_inode;
+
+ parent_inode = d_inode(dentry->d_parent);
+ inode_lock(parent_inode);
+ if (simple_positive(dentry)) {
+ dget(dentry);
+ simple_unlink(parent_inode, dentry);
+ d_delete(dentry);
+ dput(dentry);
+ }
+ inode_unlock(parent_inode);
+}
+
+struct dentry *binderfs_create_file(struct dentry *parent, const char *name,
+ const struct file_operations *fops,
+ void *data)
{
struct dentry *dentry;
struct inode *new_inode, *parent_inode;
@@ -604,7 +587,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, *dentry;
+ struct dentry *binder_logs_root_dir, *dentry, *proc_log_dir;
+ struct binderfs_info *info;
int ret = 0;

binder_logs_root_dir = binderfs_create_dir(sb->s_root,
@@ -648,8 +632,18 @@ static int init_binder_logs(struct super_block *sb)
"failed_transaction_log",
&binder_transaction_log_fops,
&binder_transaction_log_failed);
- if (IS_ERR(dentry))
+ if (IS_ERR(dentry)) {
ret = PTR_ERR(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

2019-09-03 16:20:23

by Hridya Valsaraju

[permalink] [raw]
Subject: [PATCH v3 3/4] binder: Make transaction_log available in binderfs

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.

Acked-by: Christian Brauner <[email protected]>
Signed-off-by: Hridya Valsaraju <[email protected]>
---

Changes in v2:
-Consistent variable naming accross functions as per Christian Brauner.

drivers/android/binder.c | 34 +++++--------------------------
drivers/android/binder_internal.h | 30 +++++++++++++++++++++++++++
drivers/android/binderfs.c | 18 ++++++++++++++++
3 files changed, 53 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 01c1db463053..9bb54fd3a48a 100644
--- a/drivers/android/binderfs.c
+++ b/drivers/android/binderfs.c
@@ -630,6 +630,24 @@ static int init_binder_logs(struct super_block *sb)

dentry = binderfs_create_file(binder_logs_root_dir, "transactions",
&binder_transactions_fops, NULL);
+ if (IS_ERR(dentry)) {
+ ret = PTR_ERR(dentry);
+ goto out;
+ }
+
+ dentry = binderfs_create_file(binder_logs_root_dir,
+ "transaction_log",
+ &binder_transaction_log_fops,
+ &binder_transaction_log);
+ if (IS_ERR(dentry)) {
+ ret = PTR_ERR(dentry);
+ goto out;
+ }
+
+ dentry = binderfs_create_file(binder_logs_root_dir,
+ "failed_transaction_log",
+ &binder_transaction_log_fops,
+ &binder_transaction_log_failed);
if (IS_ERR(dentry))
ret = PTR_ERR(dentry);

--
2.23.0.187.g17f5b7556c-goog

2019-09-04 11:21:57

by Christian Brauner

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] Add binder state and statistics to binderfs

On Tue, Sep 03, 2019 at 09:16:51AM -0700, Hridya Valsaraju wrote:
> 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

Acked-by: Christian Brauner <[email protected]>

Btw, I think your counting is off-by-one. :) We usually count the
initial send of a series as 0 and the first rework of that series as v1.
I think you counted your initial send as v1 and the first rework as v2. :)

Christian

>
> 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 | 95 ++++++-----
> drivers/android/binder_internal.h | 84 ++++++++++
> drivers/android/binderfs.c | 255 ++++++++++++++++++++++++++----
> 3 files changed, 362 insertions(+), 72 deletions(-)
>
> --
> 2.23.0.187.g17f5b7556c-goog
>

2019-09-04 14:22:37

by Joel Fernandes

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] Add binder state and statistics to binderfs

On September 4, 2019 7:19:35 AM EDT, Christian Brauner
<[email protected]> wrote:
>On Tue, Sep 03, 2019 at 09:16:51AM -0700, Hridya Valsaraju wrote:
>> 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
>
>Acked-by: Christian Brauner <[email protected]>
>
>Btw, I think your counting is off-by-one. :) We usually count the
>initial send of a series as 0 and the first rework of that series as
>v1.
>I think you counted your initial send as v1 and the first rework as v2.

Which is fine. I have done it both ways. Is this a rule written somewhere?

>:)
>

If I am not mistaken, this is Hridya's first set of kernel patches.
Congrats on landing it upstream and to everyone for reviews! (assuming
nothing falls apart on the way to Linus tree).

thanks,

- Joel

[TLDR]
My first kernel patch was 10 years ago to a WiFi driver when I was an
intern at University. I was thrilled to have fixed a bug in network
bridging code in the 802.11s stack. This is always a special moment so
congrats again! ;-)





>Christian
>
>>
>> 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 | 95 ++++++-----
>> drivers/android/binder_internal.h | 84 ++++++++++
>> drivers/android/binderfs.c | 255
>++++++++++++++++++++++++++----
>> 3 files changed, 362 insertions(+), 72 deletions(-)
>>
>> --
>> 2.23.0.187.g17f5b7556c-goog
>>

2019-09-04 14:50:40

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] Add binder state and statistics to binderfs

On Wed, Sep 04, 2019 at 10:20:32AM -0400, Joel Fernandes wrote:
> On September 4, 2019 7:19:35 AM EDT, Christian Brauner
> <[email protected]> wrote:
> >On Tue, Sep 03, 2019 at 09:16:51AM -0700, Hridya Valsaraju wrote:
> >> 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
> >
> >Acked-by: Christian Brauner <[email protected]>
> >
> >Btw, I think your counting is off-by-one. :) We usually count the
> >initial send of a series as 0 and the first rework of that series as
> >v1.
> >I think you counted your initial send as v1 and the first rework as v2.
>
> Which is fine. I have done it both ways. Is this a rule written somewhere?

No where, I can count both ways, it's not a big deal :)

greg k-h

2019-09-04 15:13:33

by Christian Brauner

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] Add binder state and statistics to binderfs

On Wed, Sep 04, 2019 at 04:49:03PM +0200, Greg Kroah-Hartman wrote:
> On Wed, Sep 04, 2019 at 10:20:32AM -0400, Joel Fernandes wrote:
> > On September 4, 2019 7:19:35 AM EDT, Christian Brauner
> > <[email protected]> wrote:
> > >On Tue, Sep 03, 2019 at 09:16:51AM -0700, Hridya Valsaraju wrote:
> > >> 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
> > >
> > >Acked-by: Christian Brauner <[email protected]>
> > >
> > >Btw, I think your counting is off-by-one. :) We usually count the
> > >initial send of a series as 0 and the first rework of that series as
> > >v1.
> > >I think you counted your initial send as v1 and the first rework as v2.
> >
> > Which is fine. I have done it both ways. Is this a rule written somewhere?
>
> No where, I can count both ways, it's not a big deal :)

It isn't documented (as many things we still do are) and it's not a big
deal. But most people seem to be counting revisions starting from 0 it
seems. I went looking for previous version to link to in the patch cover
letter and was confused because I was missing a v1. :)

Anyway, I'm happy that Hridya landed this! It was fun helping her the
last couple of weeks on- and off-list. Thanks for getting this done! I
hope we'll see even more patches in the future. :)

Christian

2019-09-04 16:53:51

by Hridya Valsaraju

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] Add binder state and statistics to binderfs

On Wed, Sep 4, 2019 at 8:12 AM Christian Brauner
<[email protected]> wrote:
>
> On Wed, Sep 04, 2019 at 04:49:03PM +0200, Greg Kroah-Hartman wrote:
> > On Wed, Sep 04, 2019 at 10:20:32AM -0400, Joel Fernandes wrote:
> > > On September 4, 2019 7:19:35 AM EDT, Christian Brauner
> > > <[email protected]> wrote:
> > > >On Tue, Sep 03, 2019 at 09:16:51AM -0700, Hridya Valsaraju wrote:
> > > >> 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
> > > >
> > > >Acked-by: Christian Brauner <[email protected]>
> > > >
> > > >Btw, I think your counting is off-by-one. :) We usually count the
> > > >initial send of a series as 0 and the first rework of that series as
> > > >v1.
> > > >I think you counted your initial send as v1 and the first rework as v2.
> > >
> > > Which is fine. I have done it both ways. Is this a rule written somewhere?
> >
> > No where, I can count both ways, it's not a big deal :)
>
> It isn't documented (as many things we still do are) and it's not a big
> deal. But most people seem to be counting revisions starting from 0 it
> seems. I went looking for previous version to link to in the patch cover
> letter and was confused because I was missing a v1. :)
>
> Anyway, I'm happy that Hridya landed this! It was fun helping her the
> last couple of weeks on- and off-list. Thanks for getting this done! I
> hope we'll see even more patches in the future. :)

Thank you so much Christian and Todd for the guidance and thorough
reviews on the many patches I sent your way both on and off-list :) I
really appreciate it!

>
> Christian

2019-09-04 17:09:41

by Hridya Valsaraju

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] Add binder state and statistics to binderfs

On Wed, Sep 4, 2019 at 7:20 AM Joel Fernandes <[email protected]> wrote:
>
> On September 4, 2019 7:19:35 AM EDT, Christian Brauner
> <[email protected]> wrote:
> >On Tue, Sep 03, 2019 at 09:16:51AM -0700, Hridya Valsaraju wrote:
> >> 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
> >
> >Acked-by: Christian Brauner <[email protected]>
> >
> >Btw, I think your counting is off-by-one. :) We usually count the
> >initial send of a series as 0 and the first rework of that series as
> >v1.
> >I think you counted your initial send as v1 and the first rework as v2.
>
> Which is fine. I have done it both ways. Is this a rule written somewhere?
>
> >:)
> >
>
> If I am not mistaken, this is Hridya's first set of kernel patches.
> Congrats on landing it upstream and to everyone for reviews! (assuming
> nothing falls apart on the way to Linus tree).

I really hope so! Thank you Joel and everyone else for the reviews !


>
> thanks,
>
> - Joel
>
> [TLDR]
> My first kernel patch was 10 years ago to a WiFi driver when I was an
> intern at University. I was thrilled to have fixed a bug in network
> bridging code in the 802.11s stack. This is always a special moment so
> congrats again! ;-)
>
>
>
>
>
> >Christian
> >
> >>
> >> 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 | 95 ++++++-----
> >> drivers/android/binder_internal.h | 84 ++++++++++
> >> drivers/android/binderfs.c | 255
> >++++++++++++++++++++++++++----
> >> 3 files changed, 362 insertions(+), 72 deletions(-)
> >>
> >> --
> >> 2.23.0.187.g17f5b7556c-goog
> >>

2019-09-27 13:22:11

by Christian Brauner

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] Add binder state and statistics to binderfs

On Tue, Sep 03, 2019 at 09:16:51AM -0700, Hridya Valsaraju wrote:
> 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

I'm sitting in a talk from Jonathan about kernel documentation and what
I realized is that we forgot to update the documentation I wrote for
binderfs in Documentation/admin-guide/binderfs.rst to reflect the new
stats=global mount option. Would be great if we could add that after rc1
is out. Would you have time to do that, Hridya?

Should just be a new entry under:

Options
-------
max
binderfs instances can be mounted with a limit on the number of binder
devices that can be allocated. The ``max=<count>`` mount option serves as
a per-instance limit. If ``max=<count>`` is set then only ``<count>`` number
of binder devices can be allocated in this binderfs instance.
stats
<description>

Thanks!
Christian

2019-09-27 18:13:24

by Hridya Valsaraju

[permalink] [raw]
Subject: Re: [PATCH v3 0/4] Add binder state and statistics to binderfs

On Fri, Sep 27, 2019 at 6:19 AM Christian Brauner
<[email protected]> wrote:
>
> On Tue, Sep 03, 2019 at 09:16:51AM -0700, Hridya Valsaraju wrote:
> > 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
>
> I'm sitting in a talk from Jonathan about kernel documentation and what
> I realized is that we forgot to update the documentation I wrote for
> binderfs in Documentation/admin-guide/binderfs.rst to reflect the new
> stats=global mount option. Would be great if we could add that after rc1
> is out. Would you have time to do that, Hridya?

Thanks for catching that Christian, sorry about the miss! I will send
out a patch ASAP to add the documentation.

Regards,
Hridya

>
> Should just be a new entry under:
>
> Options
> -------
> max
> binderfs instances can be mounted with a limit on the number of binder
> devices that can be allocated. The ``max=<count>`` mount option serves as
> a per-instance limit. If ``max=<count>`` is set then only ``<count>`` number
> of binder devices can be allocated in this binderfs instance.
> stats
> <description>
>
> Thanks!
> Christian