DriverDev-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v3 0/4] Add binder state and statistics to binderfs
@ 2019-09-03 16:16 Hridya Valsaraju
  2019-09-03 16:16 ` [PATCH v3 1/4] binder: add a mount option to show global stats Hridya Valsaraju
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Hridya Valsaraju @ 2019-09-03 16:16 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos,
	Martijn Coenen, Joel Fernandes, Christian Brauner, devel,
	linux-kernel
  Cc: Hridya Valsaraju, kernel-team

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

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v3 1/4] binder: add a mount option to show global stats
  2019-09-03 16:16 [PATCH v3 0/4] Add binder state and statistics to binderfs Hridya Valsaraju
@ 2019-09-03 16:16 ` Hridya Valsaraju
  2019-09-03 16:16 ` [PATCH v3 2/4] binder: Add stats, state and transactions files Hridya Valsaraju
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Hridya Valsaraju @ 2019-09-03 16:16 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos,
	Martijn Coenen, Joel Fernandes, Christian Brauner, devel,
	linux-kernel
  Cc: Hridya Valsaraju, kernel-team

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>
---

 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

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v3 2/4] binder: Add stats, state and transactions files
  2019-09-03 16:16 [PATCH v3 0/4] Add binder state and statistics to binderfs Hridya Valsaraju
  2019-09-03 16:16 ` [PATCH v3 1/4] binder: add a mount option to show global stats Hridya Valsaraju
@ 2019-09-03 16:16 ` Hridya Valsaraju
  2019-09-03 16:16 ` [PATCH v3 3/4] binder: Make transaction_log available in binderfs Hridya Valsaraju
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Hridya Valsaraju @ 2019-09-03 16:16 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos,
	Martijn Coenen, Joel Fernandes, Christian Brauner, devel,
	linux-kernel
  Cc: Hridya Valsaraju, Christian Brauner, kernel-team

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 <christian.brauner@ubuntu.com>
Signed-off-by: Hridya Valsaraju <hridya@google.com>
---

 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

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v3 3/4] binder: Make transaction_log available in binderfs
  2019-09-03 16:16 [PATCH v3 0/4] Add binder state and statistics to binderfs Hridya Valsaraju
  2019-09-03 16:16 ` [PATCH v3 1/4] binder: add a mount option to show global stats Hridya Valsaraju
  2019-09-03 16:16 ` [PATCH v3 2/4] binder: Add stats, state and transactions files Hridya Valsaraju
@ 2019-09-03 16:16 ` Hridya Valsaraju
  2019-09-03 16:16 ` [PATCH v3 4/4] binder: Add binder_proc logging to binderfs Hridya Valsaraju
  2019-09-04 11:19 ` [PATCH v3 0/4] Add binder state and statistics " Christian Brauner
  4 siblings, 0 replies; 11+ messages in thread
From: Hridya Valsaraju @ 2019-09-03 16:16 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos,
	Martijn Coenen, Joel Fernandes, Christian Brauner, devel,
	linux-kernel
  Cc: Hridya Valsaraju, Christian Brauner, kernel-team

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 <christian.brauner@ubuntu.com>
Signed-off-by: Hridya Valsaraju <hridya@google.com>
---

 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

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v3 4/4] binder: Add binder_proc logging to binderfs
  2019-09-03 16:16 [PATCH v3 0/4] Add binder state and statistics to binderfs Hridya Valsaraju
                   ` (2 preceding siblings ...)
  2019-09-03 16:16 ` [PATCH v3 3/4] binder: Make transaction_log available in binderfs Hridya Valsaraju
@ 2019-09-03 16:16 ` Hridya Valsaraju
  2019-09-04 11:19 ` [PATCH v3 0/4] Add binder state and statistics " Christian Brauner
  4 siblings, 0 replies; 11+ messages in thread
From: Hridya Valsaraju @ 2019-09-03 16:16 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos,
	Martijn Coenen, Joel Fernandes, Christian Brauner, devel,
	linux-kernel
  Cc: Hridya Valsaraju, Christian Brauner, kernel-team

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 <christian.brauner@ubuntu.com>
Signed-off-by: Hridya Valsaraju <hridya@google.com>
---

 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

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 0/4] Add binder state and statistics to binderfs
  2019-09-03 16:16 [PATCH v3 0/4] Add binder state and statistics to binderfs Hridya Valsaraju
                   ` (3 preceding siblings ...)
  2019-09-03 16:16 ` [PATCH v3 4/4] binder: Add binder_proc logging to binderfs Hridya Valsaraju
@ 2019-09-04 11:19 ` " Christian Brauner
  2019-09-04 14:20   ` Joel Fernandes
  4 siblings, 1 reply; 11+ messages in thread
From: Christian Brauner @ 2019-09-04 11:19 UTC (permalink / raw)
  To: Hridya Valsaraju
  Cc: devel, kernel-team, Todd Kjos, Greg Kroah-Hartman, linux-kernel,
	Arve Hjønnevåg, Joel Fernandes, Martijn Coenen

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 <christian.brauner@ubuntu.com>

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
> 
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 0/4] Add binder state and statistics to binderfs
  2019-09-04 11:19 ` [PATCH v3 0/4] Add binder state and statistics " Christian Brauner
@ 2019-09-04 14:20   ` Joel Fernandes
  2019-09-04 14:49     ` Greg Kroah-Hartman
  2019-09-04 17:05     ` Hridya Valsaraju
  0 siblings, 2 replies; 11+ messages in thread
From: Joel Fernandes @ 2019-09-04 14:20 UTC (permalink / raw)
  To: Christian Brauner, Hridya Valsaraju
  Cc: open list:ANDROID DRIVERS, kernel-team, Todd Kjos,
	Greg Kroah-Hartman, LKML, Arve Hjønnevåg,
	Martijn Coenen

On September 4, 2019 7:19:35 AM EDT, Christian Brauner
<christian.brauner@ubuntu.com> 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 <christian.brauner@ubuntu.com>
>
>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
>>
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 0/4] Add binder state and statistics to binderfs
  2019-09-04 14:20   ` Joel Fernandes
@ 2019-09-04 14:49     ` Greg Kroah-Hartman
  2019-09-04 15:12       ` Christian Brauner
  2019-09-04 17:05     ` Hridya Valsaraju
  1 sibling, 1 reply; 11+ messages in thread
From: Greg Kroah-Hartman @ 2019-09-04 14:49 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: open list:ANDROID DRIVERS, Todd Kjos, Martijn Coenen, LKML,
	Arve Hjønnevåg, Hridya Valsaraju, Christian Brauner,
	kernel-team

On Wed, Sep 04, 2019 at 10:20:32AM -0400, Joel Fernandes wrote:
> On September 4, 2019 7:19:35 AM EDT, Christian Brauner
> <christian.brauner@ubuntu.com> 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 <christian.brauner@ubuntu.com>
> >
> >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
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 0/4] Add binder state and statistics to binderfs
  2019-09-04 14:49     ` Greg Kroah-Hartman
@ 2019-09-04 15:12       ` Christian Brauner
  2019-09-04 16:51         ` Hridya Valsaraju
  0 siblings, 1 reply; 11+ messages in thread
From: Christian Brauner @ 2019-09-04 15:12 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: open list:ANDROID DRIVERS, Todd Kjos, Martijn Coenen, LKML,
	Hridya Valsaraju, Arve Hjønnevåg, Joel Fernandes,
	kernel-team

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
> > <christian.brauner@ubuntu.com> 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 <christian.brauner@ubuntu.com>
> > >
> > >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
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 0/4] Add binder state and statistics to binderfs
  2019-09-04 15:12       ` Christian Brauner
@ 2019-09-04 16:51         ` Hridya Valsaraju
  0 siblings, 0 replies; 11+ messages in thread
From: Hridya Valsaraju @ 2019-09-04 16:51 UTC (permalink / raw)
  To: Christian Brauner
  Cc: open list:ANDROID DRIVERS, Todd Kjos, Greg Kroah-Hartman, LKML,
	Arve Hjønnevåg, Martijn Coenen, Joel Fernandes,
	kernel-team

On Wed, Sep 4, 2019 at 8:12 AM Christian Brauner
<christian.brauner@ubuntu.com> 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
> > > <christian.brauner@ubuntu.com> 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 <christian.brauner@ubuntu.com>
> > > >
> > > >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
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 0/4] Add binder state and statistics to binderfs
  2019-09-04 14:20   ` Joel Fernandes
  2019-09-04 14:49     ` Greg Kroah-Hartman
@ 2019-09-04 17:05     ` Hridya Valsaraju
  1 sibling, 0 replies; 11+ messages in thread
From: Hridya Valsaraju @ 2019-09-04 17:05 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: open list:ANDROID DRIVERS, kernel-team, Todd Kjos,
	Greg Kroah-Hartman, LKML, Arve Hjønnevåg,
	Christian Brauner, Martijn Coenen

On Wed, Sep 4, 2019 at 7:20 AM Joel Fernandes <joel@joelfernandes.org> wrote:
>
> On September 4, 2019 7:19:35 AM EDT, Christian Brauner
> <christian.brauner@ubuntu.com> 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 <christian.brauner@ubuntu.com>
> >
> >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
> >>
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, back to index

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-03 16:16 [PATCH v3 0/4] Add binder state and statistics to binderfs Hridya Valsaraju
2019-09-03 16:16 ` [PATCH v3 1/4] binder: add a mount option to show global stats Hridya Valsaraju
2019-09-03 16:16 ` [PATCH v3 2/4] binder: Add stats, state and transactions files Hridya Valsaraju
2019-09-03 16:16 ` [PATCH v3 3/4] binder: Make transaction_log available in binderfs Hridya Valsaraju
2019-09-03 16:16 ` [PATCH v3 4/4] binder: Add binder_proc logging to binderfs Hridya Valsaraju
2019-09-04 11:19 ` [PATCH v3 0/4] Add binder state and statistics " Christian Brauner
2019-09-04 14:20   ` Joel Fernandes
2019-09-04 14:49     ` Greg Kroah-Hartman
2019-09-04 15:12       ` Christian Brauner
2019-09-04 16:51         ` Hridya Valsaraju
2019-09-04 17:05     ` Hridya Valsaraju

DriverDev-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/driverdev-devel/0 driverdev-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 driverdev-devel driverdev-devel/ https://lore.kernel.org/driverdev-devel \
		driverdev-devel@linuxdriverproject.org devel@driverdev.osuosl.org driverdev-devel@archiver.kernel.org
	public-inbox-index driverdev-devel


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.linuxdriverproject.driverdev-devel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox