[PATCH 2/3] DEBUGFS: Add per cpu counters

[PATCH 2/3] DEBUGFS: Add per cpu counters

[Andi Kleen]

From: Andi Kleen <ak@linux.intel.com>

Add a simple way to export per cpu counters in debugfs.

This is done using a section, so that they can be declared using
a simple macro with minimal typing.

OPEN: modules are not supported yet.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 fs/debugfs/Makefile               |    2 +-
 fs/debugfs/counter.c              |   73 +++++++++++++++++++++++++++++++++++++
 include/asm-generic/vmlinux.lds.h |    6 +++
 include/linux/debugfs.h           |   27 ++++++++++++++
 4 files changed, 107 insertions(+), 1 deletions(-)
 create mode 100644 fs/debugfs/counter.c

diff --git a/fs/debugfs/Makefile b/fs/debugfs/Makefile
index 840c456..21be8d3 100644
--- a/fs/debugfs/Makefile
+++ b/fs/debugfs/Makefile
@@ -1,4 +1,4 @@
-debugfs-objs	:= inode.o file.o
+debugfs-objs	:= inode.o file.o counter.o
 
 obj-$(CONFIG_DEBUG_FS)	+= debugfs.o
 
diff --git a/fs/debugfs/counter.c b/fs/debugfs/counter.c
new file mode 100644
index 0000000..2372faf
--- /dev/null
+++ b/fs/debugfs/counter.c
@@ -0,0 +1,73 @@
+#include <linux/debugfs.h>
+#include <linux/seq_file.h>
+#include <linux/percpu.h>
+#include <linux/kernel.h>
+
+/* OPEN: implement module support */
+
+extern struct debugfs_counter __start___debugfs[], __stop___debugfs[];
+
+static unsigned sum_counter(unsigned __percpu *counter)
+{
+	int cpu;
+	unsigned sum = 0;
+
+	for_each_online_cpu (cpu)
+		sum += per_cpu(*counter, cpu);
+	return sum;
+}
+
+static int
+dump_counters(struct seq_file *m,
+	      struct debugfs_counter *cnt, struct debugfs_counter *stop,
+	      char *fn)
+{
+	int n = 0;
+	for (; cnt < stop; cnt++)
+		if (fn[0] == cnt->fn[0] && !strcmp(cnt->fn, fn))
+			n += seq_printf(m, "%s: %u\n",
+					cnt->name, sum_counter(cnt->ptr));
+	return n;
+}
+
+static int show_debugfs_counter(struct seq_file *m, void *arg)
+{
+	int n;
+	n = dump_counters(m, __start___debugfs, __stop___debugfs, m->private);
+	return n;
+}
+
+static int debugfs_counter_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, show_debugfs_counter, inode->i_private);
+}
+
+static const struct file_operations debugfs_counter_fops = {
+	.open = debugfs_counter_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = single_release
+};
+
+static void
+init_counters(struct debugfs_counter *cnt, struct debugfs_counter *stop)
+{
+	const char *last = NULL;
+
+	for (; cnt < stop; cnt++) {
+		if (cnt->fn == last)
+			continue;
+		last = cnt->fn;
+
+		/* Ignore error. Nothing we can do. */
+		debugfs_create_file(cnt->fn, 0444, NULL, (void *)cnt->fn,
+				    &debugfs_counter_fops);
+	}
+}
+
+static int init_debugfs_counters(void)
+{
+	init_counters(__start___debugfs, __stop___debugfs);
+	return 0;
+}
+module_init(init_debugfs_counters);
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index b5e2e4c..fbfb95b 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -272,6 +272,12 @@
 									\
 	TRACEDATA							\
 									\
+	__debugfs  	: AT(ADDR(__debugfs) - LOAD_OFFSET) {		\
+		VMLINUX_SYMBOL(__start___debugfs) = .;			\
+		*(__debugfs)						\
+		VMLINUX_SYMBOL(__stop___debugfs) = .;			\
+	}								\
+									\
 	/* Kernel symbol table: Normal symbols */			\
 	__ksymtab         : AT(ADDR(__ksymtab) - LOAD_OFFSET) {		\
 		VMLINUX_SYMBOL(__start___ksymtab) = .;			\
diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h
index e7d9b20..a243b05 100644
--- a/include/linux/debugfs.h
+++ b/include/linux/debugfs.h
@@ -18,6 +18,7 @@
 #include <linux/fs.h>
 
 #include <linux/types.h>
+#include <linux/percpu.h>
 
 struct file_operations;
 
@@ -76,6 +77,24 @@ struct dentry *debugfs_create_blob(const char *name, mode_t mode,
 
 bool debugfs_initialized(void);
 
+struct debugfs_counter {
+	unsigned __percpu *ptr;
+	const char *fn;
+	const char *name;
+} __attribute__((aligned(sizeof(char *))));
+
+/* Note: static doesn't work unlike DEFINE_PERCPU. Sorry. */
+#define DEFINE_DEBUGFS_COUNTER(name_, file)				\
+	DEFINE_PER_CPU(unsigned, name_ ## _counter);			\
+	struct debugfs_counter name_ ## _pcpu_counter __used		\
+	__attribute__((aligned(sizeof(char *)),section("__debugfs"),unused)) \
+	= { .ptr = &name_ ## _counter, .fn = file, .name = #name_ }; \
+
+#define DECLARE_DEBUGFS_COUNTER(name)			\
+	struct debugfs_counter name ## _pcpu_counter
+
+#define debugfs_counter_inc(name) this_cpu_inc(name ## _counter)
+
 #else
 
 #include <linux/err.h>
@@ -193,6 +212,14 @@ static inline bool debugfs_initialized(void)
 	return false;
 }
 
+
+struct debugfs_counter {
+};
+
+#define DEFINE_DEBUGFS_COUNTER(name, file)
+#define DECLARE_DEBUGFS_COUNTER(name)
+#define debugfs_counter_inc(name) do {} while(0)
+
 #endif
 
 #endif
-- 
1.7.4.4

[PATCH 2/3] DEBUGFS: Add per cpu counters

[Andi Kleen]

From: Andi Kleen <ak@linux.intel.com>

Add a simple way to export per cpu counters in debugfs.

This is done using a section, so that they can be declared using
a simple macro with minimal typing.

OPEN: modules are not supported yet.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 fs/debugfs/Makefile               |    2 +-
 fs/debugfs/counter.c              |   73 +++++++++++++++++++++++++++++++++++++
 include/asm-generic/vmlinux.lds.h |    6 +++
 include/linux/debugfs.h           |   27 ++++++++++++++
 4 files changed, 107 insertions(+), 1 deletions(-)
 create mode 100644 fs/debugfs/counter.c

diff --git a/fs/debugfs/Makefile b/fs/debugfs/Makefile
index 840c456..21be8d3 100644
--- a/fs/debugfs/Makefile
+++ b/fs/debugfs/Makefile
@@ -1,4 +1,4 @@
-debugfs-objs	:= inode.o file.o
+debugfs-objs	:= inode.o file.o counter.o
 
 obj-$(CONFIG_DEBUG_FS)	+= debugfs.o
 
diff --git a/fs/debugfs/counter.c b/fs/debugfs/counter.c
new file mode 100644
index 0000000..2372faf
--- /dev/null
+++ b/fs/debugfs/counter.c
@@ -0,0 +1,73 @@
+#include <linux/debugfs.h>
+#include <linux/seq_file.h>
+#include <linux/percpu.h>
+#include <linux/kernel.h>
+
+/* OPEN: implement module support */
+
+extern struct debugfs_counter __start___debugfs[], __stop___debugfs[];
+
+static unsigned sum_counter(unsigned __percpu *counter)
+{
+	int cpu;
+	unsigned sum = 0;
+
+	for_each_online_cpu (cpu)
+		sum += per_cpu(*counter, cpu);
+	return sum;
+}
+
+static int
+dump_counters(struct seq_file *m,
+	      struct debugfs_counter *cnt, struct debugfs_counter *stop,
+	      char *fn)
+{
+	int n = 0;
+	for (; cnt < stop; cnt++)
+		if (fn[0] == cnt->fn[0] && !strcmp(cnt->fn, fn))
+			n += seq_printf(m, "%s: %u\n",
+					cnt->name, sum_counter(cnt->ptr));
+	return n;
+}
+
+static int show_debugfs_counter(struct seq_file *m, void *arg)
+{
+	int n;
+	n = dump_counters(m, __start___debugfs, __stop___debugfs, m->private);
+	return n;
+}
+
+static int debugfs_counter_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, show_debugfs_counter, inode->i_private);
+}
+
+static const struct file_operations debugfs_counter_fops = {
+	.open = debugfs_counter_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = single_release
+};
+
+static void
+init_counters(struct debugfs_counter *cnt, struct debugfs_counter *stop)
+{
+	const char *last = NULL;
+
+	for (; cnt < stop; cnt++) {
+		if (cnt->fn == last)
+			continue;
+		last = cnt->fn;
+
+		/* Ignore error. Nothing we can do. */
+		debugfs_create_file(cnt->fn, 0444, NULL, (void *)cnt->fn,
+				    &debugfs_counter_fops);
+	}
+}
+
+static int init_debugfs_counters(void)
+{
+	init_counters(__start___debugfs, __stop___debugfs);
+	return 0;
+}
+module_init(init_debugfs_counters);
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index b5e2e4c..fbfb95b 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -272,6 +272,12 @@
 									\
 	TRACEDATA							\
 									\
+	__debugfs  	: AT(ADDR(__debugfs) - LOAD_OFFSET) {		\
+		VMLINUX_SYMBOL(__start___debugfs) = .;			\
+		*(__debugfs)						\
+		VMLINUX_SYMBOL(__stop___debugfs) = .;			\
+	}								\
+									\
 	/* Kernel symbol table: Normal symbols */			\
 	__ksymtab         : AT(ADDR(__ksymtab) - LOAD_OFFSET) {		\
 		VMLINUX_SYMBOL(__start___ksymtab) = .;			\
diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h
index e7d9b20..a243b05 100644
--- a/include/linux/debugfs.h
+++ b/include/linux/debugfs.h
@@ -18,6 +18,7 @@
 #include <linux/fs.h>
 
 #include <linux/types.h>
+#include <linux/percpu.h>
 
 struct file_operations;
 
@@ -76,6 +77,24 @@ struct dentry *debugfs_create_blob(const char *name, mode_t mode,
 
 bool debugfs_initialized(void);
 
+struct debugfs_counter {
+	unsigned __percpu *ptr;
+	const char *fn;
+	const char *name;
+} __attribute__((aligned(sizeof(char *))));
+
+/* Note: static doesn't work unlike DEFINE_PERCPU. Sorry. */
+#define DEFINE_DEBUGFS_COUNTER(name_, file)				\
+	DEFINE_PER_CPU(unsigned, name_ ## _counter);			\
+	struct debugfs_counter name_ ## _pcpu_counter __used		\
+	__attribute__((aligned(sizeof(char *)),section("__debugfs"),unused)) \
+	= { .ptr = &name_ ## _counter, .fn = file, .name = #name_ }; \
+
+#define DECLARE_DEBUGFS_COUNTER(name)			\
+	struct debugfs_counter name ## _pcpu_counter
+
+#define debugfs_counter_inc(name) this_cpu_inc(name ## _counter)
+
 #else
 
 #include <linux/err.h>
@@ -193,6 +212,14 @@ static inline bool debugfs_initialized(void)
 	return false;
 }
 
+
+struct debugfs_counter {
+};
+
+#define DEFINE_DEBUGFS_COUNTER(name, file)
+#define DECLARE_DEBUGFS_COUNTER(name)
+#define debugfs_counter_inc(name) do {} while(0)
+
 #endif
 
 #endif
-- 
1.7.4.4

Re: [PATCH 2/3] DEBUGFS: Add per cpu counters

[Wu Fengguang]

On Sat, Dec 03, 2011 at 02:43:26AM +0800, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> Add a simple way to export per cpu counters in debugfs.
> 
> This is done using a section, so that they can be declared using
> a simple macro with minimal typing.
> 
> OPEN: modules are not supported yet.
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> ---
>  fs/debugfs/Makefile               |    2 +-
>  fs/debugfs/counter.c              |   73 +++++++++++++++++++++++++++++++++++++
>  include/asm-generic/vmlinux.lds.h |    6 +++
>  include/linux/debugfs.h           |   27 ++++++++++++++
>  4 files changed, 107 insertions(+), 1 deletions(-)
>  create mode 100644 fs/debugfs/counter.c
> 
> diff --git a/fs/debugfs/Makefile b/fs/debugfs/Makefile
> index 840c456..21be8d3 100644
> --- a/fs/debugfs/Makefile
> +++ b/fs/debugfs/Makefile
> @@ -1,4 +1,4 @@
> -debugfs-objs	:= inode.o file.o
> +debugfs-objs	:= inode.o file.o counter.o
>  
>  obj-$(CONFIG_DEBUG_FS)	+= debugfs.o
>  
> diff --git a/fs/debugfs/counter.c b/fs/debugfs/counter.c
> new file mode 100644
> index 0000000..2372faf
> --- /dev/null
> +++ b/fs/debugfs/counter.c
> @@ -0,0 +1,73 @@
> +#include <linux/debugfs.h>
> +#include <linux/seq_file.h>
> +#include <linux/percpu.h>
> +#include <linux/kernel.h>
> +
> +/* OPEN: implement module support */

Yeah, I think the module support would benefit my case as well.

To support module users, init_counters() will be exported with the
__start___debugfs/__stop___debugfs hard coding removed. Then I'll be
call it from the readahead initilization code:

DEFINE_PER_CPU(unsigned long[RA_PATTERN_MAX][RA_ACCOUNT_MAX], ra_counter);
struct debugfs_counter ra_pcpu_counter[RA_PATTERN_MAX][RA_ACCOUNT_MAX];

// init ra_pcpu_counter in a loop
for each pattern
        init_counters(ra_pcpu_counter[pattern], ra_pcpu_counter[pattern+1]);

Does this usage sound reasonable?

> +extern struct debugfs_counter __start___debugfs[], __stop___debugfs[];
> +
> +static unsigned sum_counter(unsigned __percpu *counter)
> +{
> +	int cpu;
> +	unsigned sum = 0;
> +
> +	for_each_online_cpu (cpu)
> +		sum += per_cpu(*counter, cpu);
> +	return sum;
> +}
> +
> +static int
> +dump_counters(struct seq_file *m,
> +	      struct debugfs_counter *cnt, struct debugfs_counter *stop,
> +	      char *fn)
> +{
> +	int n = 0;
> +	for (; cnt < stop; cnt++)
> +		if (fn[0] == cnt->fn[0] && !strcmp(cnt->fn, fn))
> +			n += seq_printf(m, "%s: %u\n",
> +					cnt->name, sum_counter(cnt->ptr));
> +	return n;
> +}
> +
> +static int show_debugfs_counter(struct seq_file *m, void *arg)
> +{
> +	int n;
> +	n = dump_counters(m, __start___debugfs, __stop___debugfs, m->private);

That hard coded __start___debugfs/__stop___debugfs is OK for POC, and
will need to be improved to work with multiple users in kernel.

Thanks,
Fengguang

Re: [PATCH 2/3] DEBUGFS: Add per cpu counters

[Andi Kleen]

On Tue, Dec 06, 2011 at 07:44:38PM +0800, Wu Fengguang wrote:
> > +#include <linux/seq_file.h>
> > +#include <linux/percpu.h>
> > +#include <linux/kernel.h>
> > +
> > +/* OPEN: implement module support */
> 
> Yeah, I think the module support would benefit my case as well.
> 
> To support module users, init_counters() will be exported with the
> __start___debugfs/__stop___debugfs hard coding removed. Then I'll be
> call it from the readahead initilization code:

No, the module loader should take care of this: look for the magic
section, register it. This is already done for other magic
sections and not too difficult, i just didn't write the code
so far.

Then the DEFINE_* macros would work seamlessly in modules too.

Do you really need it for the readahead code? I thought that
was builtin.


> 
> DEFINE_PER_CPU(unsigned long[RA_PATTERN_MAX][RA_ACCOUNT_MAX], ra_counter);
> struct debugfs_counter ra_pcpu_counter[RA_PATTERN_MAX][RA_ACCOUNT_MAX];
> 
> // init ra_pcpu_counter in a loop
> for each pattern
>         init_counters(ra_pcpu_counter[pattern], ra_pcpu_counter[pattern+1]);

Ok so you need arrays.

The idea is to not call some init function, but just put it into section
and let the init code walk it.

Should probably have a macro that handles arrays nicely.

> > +static int show_debugfs_counter(struct seq_file *m, void *arg)
> > +{
> > +	int n;
> > +	n = dump_counters(m, __start___debugfs, __stop___debugfs, m->private);
> 
> That hard coded __start___debugfs/__stop___debugfs is OK for POC, and
> will need to be improved to work with multiple users in kernel.

For Modules it obviously has to walk a list. It's a straight forward extension.

I don't think it should allow everyone to register their own lists,
that doesn't make sense because they could as well create the debug 
files themselves.

-Andi

Re: [PATCH 2/3] DEBUGFS: Add per cpu counters

[Wu Fengguang]

On Wed, Dec 07, 2011 at 01:17:51AM +0800, Andi Kleen wrote:
> On Tue, Dec 06, 2011 at 07:44:38PM +0800, Wu Fengguang wrote:
> > > +#include <linux/seq_file.h>
> > > +#include <linux/percpu.h>
> > > +#include <linux/kernel.h>
> > > +
> > > +/* OPEN: implement module support */
> > 
> > Yeah, I think the module support would benefit my case as well.
> > 
> > To support module users, init_counters() will be exported with the
> > __start___debugfs/__stop___debugfs hard coding removed. Then I'll be
> > call it from the readahead initilization code:
> 
> No, the module loader should take care of this: look for the magic
> section, register it. This is already done for other magic
> sections and not too difficult, i just didn't write the code
> so far.
>
> Then the DEFINE_* macros would work seamlessly in modules too.
 
OK.

> Do you really need it for the readahead code? I thought that
> was builtin.

Yeah because in the current form I'm initializing an array of counters
at runtime in the readahead call site. 

> > DEFINE_PER_CPU(unsigned long[RA_PATTERN_MAX][RA_ACCOUNT_MAX], ra_counter);
> > struct debugfs_counter ra_pcpu_counter[RA_PATTERN_MAX][RA_ACCOUNT_MAX];
> > 
> > // init ra_pcpu_counter in a loop
> > for each pattern
> >         init_counters(ra_pcpu_counter[pattern], ra_pcpu_counter[pattern+1]);
> 
> Ok so you need arrays.
> 
> The idea is to not call some init function, but just put it into section
> and let the init code walk it.
> 
> Should probably have a macro that handles arrays nicely.

That would be nice!

> > > +static int show_debugfs_counter(struct seq_file *m, void *arg)
> > > +{
> > > +	int n;
> > > +	n = dump_counters(m, __start___debugfs, __stop___debugfs, m->private);
> > 
> > That hard coded __start___debugfs/__stop___debugfs is OK for POC, and
> > will need to be improved to work with multiple users in kernel.
> 
> For Modules it obviously has to walk a list. It's a straight forward extension.
> 
> I don't think it should allow everyone to register their own lists,
> that doesn't make sense because they could as well create the debug 
> files themselves.

OK. It just looks a bit inefficient when (this becomes so popular that)
the list grows large.

Thanks,
Fengguang

Re: [PATCH 2/3] DEBUGFS: Add per cpu counters

[Greg KH]

On Fri, Dec 02, 2011 at 12:02:47PM -0800, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> Add a simple way to export per cpu counters in debugfs.
> 
> This is done using a section, so that they can be declared using
> a simple macro with minimal typing.
> 
> OPEN: modules are not supported yet.
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>

After applying the previous patch, this one still needs "fuzz" which
makes git not happy, care to redo this one as well when you redo the 1/3
patch?

And I'm guessing that I don't need 3/3?

thanks,

greg k-h

[PATCH 1/3] DEBUGFS: Automatically create parents for debugfs files v2

[Andi Kleen]

From: Andi Kleen <ak@linux.intel.com>

Allow passing path names to debugfs_create* and automatically create
the parents. This makes it much simpler for the caller to create hierarchies.

The way the reference counts are handled is admittedly a bit ugly.

There is no way to clean them up currently other than to delete the tree,
but that doesn't seem like a big problem for debugfs to leave
behind a few empty directories.

v2: Comment fixes based on review comments.
v3: satisfy the kernel community's white space obsession.
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 fs/debugfs/inode.c |   62 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index f3a257d..0081ac0 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -146,6 +146,55 @@ static struct file_system_type debug_fs_type = {
 	.kill_sb =	kill_litter_super,
 };
 
+/*
+ * Created parents stay around later. Sorry, but it shouldn't be a big issue.
+ */
+static const char *create_parents(const char *name, struct dentry *parent,
+				  struct dentry **pp, int *ref)
+{
+	char fn[strlen(name) + 1];
+	char *tail;
+	char *s, *p;
+	struct dentry *new_parent, *dentry, *old_parent = NULL;
+
+	strcpy(fn, name);
+	tail = strrchr(fn, '/');
+	if (!tail)
+		return name;
+	tail++;
+
+	s = fn;
+	while ((p = strsep(&s, "/")) != NULL && p != tail) {
+		/*
+		 * When the parent already exists, this will error out.
+		 * Ignore that error. We'll just look the dentry up below.
+		 */
+		dentry = debugfs_create_dir(p, parent);
+
+		/* Is it there now? */
+		mutex_lock(&parent->d_inode->i_mutex);
+		new_parent = lookup_one_len(p, parent, strlen(p));
+		mutex_unlock(&parent->d_inode->i_mutex);
+
+		if (!PTR_ERR(dentry))
+			dput(dentry);
+		if (IS_ERR(new_parent))
+			return NULL;
+		if (old_parent)
+			dput(old_parent);
+		if (parent != *pp)
+			old_parent = parent;
+		parent = new_parent;
+	}
+	if (parent) {
+		*ref = 1;
+		*pp = parent;
+	}
+	if (old_parent)
+		dput(old_parent);
+	return strrchr(name, '/') + 1;
+}
+
 static int debugfs_create_by_name(const char *name, mode_t mode,
 				  struct dentry *parent,
 				  struct dentry **dentry,
@@ -153,6 +202,7 @@ static int debugfs_create_by_name(const char *name, mode_t mode,
 				  const struct file_operations *fops)
 {
 	int error = 0;
+	int ref = 0;
 
 	/* If the parent is not specified, we create it in the root.
 	 * We need the root dentry to do this, which is in the super 
@@ -161,6 +211,7 @@ static int debugfs_create_by_name(const char *name, mode_t mode,
 	 */
 	if (!parent)
 		parent = debugfs_mount->mnt_sb->s_root;
+	name = create_parents(name, parent, &parent, &ref);
 
 	*dentry = NULL;
 	mutex_lock(&parent->d_inode->i_mutex);
@@ -185,12 +236,17 @@ static int debugfs_create_by_name(const char *name, mode_t mode,
 		error = PTR_ERR(*dentry);
 	mutex_unlock(&parent->d_inode->i_mutex);
 
+	if (ref)
+		dput(parent);
+
 	return error;
 }
 
 /**
  * debugfs_create_file - create a file in the debugfs filesystem
  * @name: a pointer to a string containing the name of the file to create.
+ *        This can also contain directories (dir/file), in this case missing
+ *        directories will be automatically created.
  * @mode: the permission that the file should have.
  * @parent: a pointer to the parent dentry for this file.  This should be a
  *          directory dentry if set.  If this paramater is NULL, then the
@@ -243,7 +299,8 @@ EXPORT_SYMBOL_GPL(debugfs_create_file);
 /**
  * debugfs_create_dir - create a directory in the debugfs filesystem
  * @name: a pointer to a string containing the name of the directory to
- *        create.
+ *        create. This can also contain directories (dir/file), in this case
+ *        missing directories will be automatically created.
  * @parent: a pointer to the parent dentry for this file.  This should be a
  *          directory dentry if set.  If this paramater is NULL, then the
  *          directory will be created in the root of the debugfs filesystem.
@@ -269,7 +326,8 @@ EXPORT_SYMBOL_GPL(debugfs_create_dir);
 /**
  * debugfs_create_symlink- create a symbolic link in the debugfs filesystem
  * @name: a pointer to a string containing the name of the symbolic link to
- *        create.
+ *        create.  This can also contain directories (dir/file), in this case
+ *        missing directories will be automatically created
  * @parent: a pointer to the parent dentry for this symbolic link.  This
  *          should be a directory dentry if set.  If this paramater is NULL,
  *          then the symbolic link will be created in the root of the debugfs
-- 
1.7.7.3

[PATCH 2/3] DEBUGFS: Add per cpu counters

[Andi Kleen]

From: Andi Kleen <ak@linux.intel.com>

Add a simple way to export per cpu counters in debugfs.

This is done using a section, so that they can be declared using
a simple macro with minimal typing.

OPEN: modules are not supported yet.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 fs/debugfs/Makefile               |    2 +-
 fs/debugfs/counter.c              |   73 +++++++++++++++++++++++++++++++++++++
 include/asm-generic/vmlinux.lds.h |    6 +++
 include/linux/debugfs.h           |   27 ++++++++++++++
 4 files changed, 107 insertions(+), 1 deletions(-)
 create mode 100644 fs/debugfs/counter.c

diff --git a/fs/debugfs/Makefile b/fs/debugfs/Makefile
index 840c456..21be8d3 100644
--- a/fs/debugfs/Makefile
+++ b/fs/debugfs/Makefile
@@ -1,4 +1,4 @@
-debugfs-objs	:= inode.o file.o
+debugfs-objs	:= inode.o file.o counter.o
 
 obj-$(CONFIG_DEBUG_FS)	+= debugfs.o
 
diff --git a/fs/debugfs/counter.c b/fs/debugfs/counter.c
new file mode 100644
index 0000000..2372faf
--- /dev/null
+++ b/fs/debugfs/counter.c
@@ -0,0 +1,73 @@
+#include <linux/debugfs.h>
+#include <linux/seq_file.h>
+#include <linux/percpu.h>
+#include <linux/kernel.h>
+
+/* OPEN: implement module support */
+
+extern struct debugfs_counter __start___debugfs[], __stop___debugfs[];
+
+static unsigned sum_counter(unsigned __percpu *counter)
+{
+	int cpu;
+	unsigned sum = 0;
+
+	for_each_online_cpu (cpu)
+		sum += per_cpu(*counter, cpu);
+	return sum;
+}
+
+static int
+dump_counters(struct seq_file *m,
+	      struct debugfs_counter *cnt, struct debugfs_counter *stop,
+	      char *fn)
+{
+	int n = 0;
+	for (; cnt < stop; cnt++)
+		if (fn[0] == cnt->fn[0] && !strcmp(cnt->fn, fn))
+			n += seq_printf(m, "%s: %u\n",
+					cnt->name, sum_counter(cnt->ptr));
+	return n;
+}
+
+static int show_debugfs_counter(struct seq_file *m, void *arg)
+{
+	int n;
+	n = dump_counters(m, __start___debugfs, __stop___debugfs, m->private);
+	return n;
+}
+
+static int debugfs_counter_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, show_debugfs_counter, inode->i_private);
+}
+
+static const struct file_operations debugfs_counter_fops = {
+	.open = debugfs_counter_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = single_release
+};
+
+static void
+init_counters(struct debugfs_counter *cnt, struct debugfs_counter *stop)
+{
+	const char *last = NULL;
+
+	for (; cnt < stop; cnt++) {
+		if (cnt->fn == last)
+			continue;
+		last = cnt->fn;
+
+		/* Ignore error. Nothing we can do. */
+		debugfs_create_file(cnt->fn, 0444, NULL, (void *)cnt->fn,
+				    &debugfs_counter_fops);
+	}
+}
+
+static int init_debugfs_counters(void)
+{
+	init_counters(__start___debugfs, __stop___debugfs);
+	return 0;
+}
+module_init(init_debugfs_counters);
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index b5e2e4c..7a54092 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -272,6 +272,12 @@
 									\
 	TRACEDATA							\
 									\
+	__debugfs	: AT(ADDR(__debugfs) - LOAD_OFFSET) {		\
+		VMLINUX_SYMBOL(__start___debugfs) = .;			\
+		*(__debugfs)						\
+		VMLINUX_SYMBOL(__stop___debugfs) = .;			\
+	}								\
+									\
 	/* Kernel symbol table: Normal symbols */			\
 	__ksymtab         : AT(ADDR(__ksymtab) - LOAD_OFFSET) {		\
 		VMLINUX_SYMBOL(__start___ksymtab) = .;			\
diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h
index e7d9b20..a243b05 100644
--- a/include/linux/debugfs.h
+++ b/include/linux/debugfs.h
@@ -18,6 +18,7 @@
 #include <linux/fs.h>
 
 #include <linux/types.h>
+#include <linux/percpu.h>
 
 struct file_operations;
 
@@ -76,6 +77,24 @@ struct dentry *debugfs_create_blob(const char *name, mode_t mode,
 
 bool debugfs_initialized(void);
 
+struct debugfs_counter {
+	unsigned __percpu *ptr;
+	const char *fn;
+	const char *name;
+} __attribute__((aligned(sizeof(char *))));
+
+/* Note: static doesn't work unlike DEFINE_PERCPU. Sorry. */
+#define DEFINE_DEBUGFS_COUNTER(name_, file)				\
+	DEFINE_PER_CPU(unsigned, name_ ## _counter);			\
+	struct debugfs_counter name_ ## _pcpu_counter __used		\
+	__attribute__((aligned(sizeof(char *)),section("__debugfs"),unused)) \
+	= { .ptr = &name_ ## _counter, .fn = file, .name = #name_ }; \
+
+#define DECLARE_DEBUGFS_COUNTER(name)			\
+	struct debugfs_counter name ## _pcpu_counter
+
+#define debugfs_counter_inc(name) this_cpu_inc(name ## _counter)
+
 #else
 
 #include <linux/err.h>
@@ -193,6 +212,14 @@ static inline bool debugfs_initialized(void)
 	return false;
 }
 
+
+struct debugfs_counter {
+};
+
+#define DEFINE_DEBUGFS_COUNTER(name, file)
+#define DECLARE_DEBUGFS_COUNTER(name)
+#define debugfs_counter_inc(name) do {} while(0)
+
 #endif
 
 #endif
-- 
1.7.7.3

Re: [PATCH 1/3] DEBUGFS: Automatically create parents for debugfs files v2

[Cyrill Gorcunov]

On Tue, Dec 13, 2011 at 01:45:32PM -0800, Andi Kleen wrote:
>  
> +/*
> + * Created parents stay around later. Sorry, but it shouldn't be a big issue.
> + */
> +static const char *create_parents(const char *name, struct dentry *parent,
> +				  struct dentry **pp, int *ref)
> +{
> +	char fn[strlen(name) + 1];
> +	char *tail;
> +	char *s, *p;
> +	struct dentry *new_parent, *dentry, *old_parent = NULL;
> +
> +	strcpy(fn, name);
> +	tail = strrchr(fn, '/');
> +	if (!tail)
> +		return name;
> +	tail++;
> +

Hi Andi, is it possible to do say kstrdup or something instead of
variable-size array in fn[strlen(name) + 1] ? As far as I remember
we already had some warnings with them at least in perf land (or
trace land). Or I miss something?

	Cyrill

Re: [PATCH 1/3] DEBUGFS: Automatically create parents for debugfs files v2

[Andi Kleen]

> Hi Andi, is it possible to do say kstrdup or something instead of

It's possible, just would add complexity with an additional
error path.

> variable-size array in fn[strlen(name) + 1] ? As far as I remember
> we already had some warnings with them at least in perf land (or
> trace land). Or I miss something?

Not aware of any problems with tracing or perf. I like them at least.

-Andi
-- 
ak@linux.intel.com -- Speaking for myself only

Re: [PATCH 2/3] DEBUGFS: Add per cpu counters

[Thomas Gleixner]

On Tue, 13 Dec 2011, Andi Kleen wrote:
>  									\
> +	__debugfs	: AT(ADDR(__debugfs) - LOAD_OFFSET) {		\
> +		VMLINUX_SYMBOL(__start___debugfs) = .;			\
> +		*(__debugfs)						\
> +		VMLINUX_SYMBOL(__stop___debugfs) = .;			\
> +	}								\
> +									\

....
  
> +struct debugfs_counter {
> +	unsigned __percpu *ptr;
> +	const char *fn;
> +	const char *name;
> +} __attribute__((aligned(sizeof(char *))));
> +
> +/* Note: static doesn't work unlike DEFINE_PERCPU. Sorry. */
> +#define DEFINE_DEBUGFS_COUNTER(name_, file)				\
> +	DEFINE_PER_CPU(unsigned, name_ ## _counter);			\
> +	struct debugfs_counter name_ ## _pcpu_counter __used		\
> +	__attribute__((aligned(sizeof(char *)),section("__debugfs"),unused)) \
> +	= { .ptr = &name_ ## _counter, .fn = file, .name = #name_ }; \

Sigh, we had that section forms an array problem more than once
already. Why do you invent another variant and think that it will not
explode?

Your alignment magic does not guarantee at all that the structs will
form an array. The "aligned" attribute guarantees only the _MINIMUM_
alignment for a structure, but the compiler and the linker are free to
align on larger multiples.

See commit 654986462 for details. That has been discussed to death and
your solution wont work either across all the various compiler
incarnations we have and will have.

Thanks,

	tglx

Re: [PATCH 2/3] DEBUGFS: Add per cpu counters

[Andi Kleen]

On Tue, Dec 13, 2011 at 11:43:16PM +0100, Thomas Gleixner wrote:
> On Tue, 13 Dec 2011, Andi Kleen wrote:
> >  									\
> > +	__debugfs	: AT(ADDR(__debugfs) - LOAD_OFFSET) {		\
> > +		VMLINUX_SYMBOL(__start___debugfs) = .;			\
> > +		*(__debugfs)						\
> > +		VMLINUX_SYMBOL(__stop___debugfs) = .;			\
> > +	}								\
> > +									\
> 
> ....
>   
> > +struct debugfs_counter {
> > +	unsigned __percpu *ptr;
> > +	const char *fn;
> > +	const char *name;
> > +} __attribute__((aligned(sizeof(char *))));
> > +
> > +/* Note: static doesn't work unlike DEFINE_PERCPU. Sorry. */
> > +#define DEFINE_DEBUGFS_COUNTER(name_, file)				\
> > +	DEFINE_PER_CPU(unsigned, name_ ## _counter);			\
> > +	struct debugfs_counter name_ ## _pcpu_counter __used		\
> > +	__attribute__((aligned(sizeof(char *)),section("__debugfs"),unused)) \
> > +	= { .ptr = &name_ ## _counter, .fn = file, .name = #name_ }; \
> 
> Sigh, we had that section forms an array problem more than once
> already. Why do you invent another variant and think that it will not
> explode?

I did three or four different sections like this in the past and as far
as I know none of them has exploded so far in production use.

Can you be more specific? Where exactly do you think this will
not work?

> Your alignment magic does not guarantee at all that the structs will
> form an array. The "aligned" attribute guarantees only the _MINIMUM_
> alignment for a structure, but the compiler and the linker are free to
> align on larger multiples.

> 
> See commit 654986462 for details.

Doesn't give a lot of details actually. Which target?

Note that my structure only has pointers, so there is not a lot 
of potential for "evil" alignment.

-Andi

Re: [PATCH 1/3] DEBUGFS: Automatically create parents for debugfs files v2

[Thomas Gleixner]

On Tue, 13 Dec 2011, Andi Kleen wrote:

> > Hi Andi, is it possible to do say kstrdup or something instead of
> 
> It's possible, just would add complexity with an additional
> error path.
> 
> > variable-size array in fn[strlen(name) + 1] ? As far as I remember
> > we already had some warnings with them at least in perf land (or
> > trace land). Or I miss something?
> 
> Not aware of any problems with tracing or perf. I like them at least.

That does not make them better. We had wreckage before and you can
just consult commit a84a79e4d3 and then find a good explanation why
your usage is safe.

Thanks,

	tglx

Re: [PATCH 2/3] DEBUGFS: Add per cpu counters

[Steven Rostedt]

On Tue, 2011-12-13 at 14:56 -0800, Andi Kleen wrote:
>  
> > > +struct debugfs_counter {
> > > +	unsigned __percpu *ptr;
> > > +	const char *fn;
> > > +	const char *name;
> > > +} __attribute__((aligned(sizeof(char *))));
> > > +

> > 
> > See commit 654986462 for details.
> 
> Doesn't give a lot of details actually. Which target?
> 
> Note that my structure only has pointers, so there is not a lot 
> of potential for "evil" alignment.

We hit this with trace events as well, and your structure does have an
evil alignment, it's 3 pointers, which is not a base 2 number. Thus,
there were some compilers that liked to add padding to make the
alignment a power of 2. That is, between two sections of files we ended
up with something like:


.section file1
.ptra1
.ptra2
.ptra3
.ptrb1
.ptrb2
.ptrb3
.ptrc1
.ptrc2
.ptrc3
.ptrd1
.ptrd2
.ptrd3
.ptre1
.ptre2
.ptre3
<space>
.section file2
.ptrf1
.ptrf2
.ptrf3
[...]



It didn't happen often, heck, trace events and tracepoints were like
this for sometime before this blew up in our faces.

>From what I've seen is that 1 or 2 longs will pack nicely, but anything
else (except for maybe 4, 8, 16, etc) will run a risk of crashing.

-- Steve

Re: [PATCH 1/3] DEBUGFS: Automatically create parents for debugfs files v2

[Andi Kleen]

On Wed, Dec 14, 2011 at 12:03:03AM +0100, Thomas Gleixner wrote:
> On Tue, 13 Dec 2011, Andi Kleen wrote:
> 
> > > Hi Andi, is it possible to do say kstrdup or something instead of
> > 
> > It's possible, just would add complexity with an additional
> > error path.
> > 
> > > variable-size array in fn[strlen(name) + 1] ? As far as I remember
> > > we already had some warnings with them at least in perf land (or
> > > trace land). Or I miss something?
> > 
> > Not aware of any problems with tracing or perf. I like them at least.
> 
> That does not make them better. We had wreckage before and you can
> just consult commit a84a79e4d3 and then find a good explanation why
> your usage is safe.

Ok. I must admit I sometimes forget with what substandard toolchains
some of the ports are afflicted with.

But fair enough. I'll just make the code longer, more complex,
more error prone and slower.

Should probably find some way to let gcc warn about them, I'm sure
I'm not the only one who likes them.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only

Re: [PATCH 2/3] DEBUGFS: Add per cpu counters

[Mathieu Desnoyers]

* Steven Rostedt (rostedt@goodmis.org) wrote:
> On Tue, 2011-12-13 at 14:56 -0800, Andi Kleen wrote:
> >  
> > > > +struct debugfs_counter {
> > > > +	unsigned __percpu *ptr;
> > > > +	const char *fn;
> > > > +	const char *name;
> > > > +} __attribute__((aligned(sizeof(char *))));
> > > > +
> 
> > > 
> > > See commit 654986462 for details.
> > 
> > Doesn't give a lot of details actually. Which target?
> > 
> > Note that my structure only has pointers, so there is not a lot 
> > of potential for "evil" alignment.
> 
> We hit this with trace events as well, and your structure does have an
> evil alignment, it's 3 pointers, which is not a base 2 number. Thus,
> there were some compilers that liked to add padding to make the
> alignment a power of 2. That is, between two sections of files we ended
> up with something like:
> 
> 
> .section file1
> .ptra1
> .ptra2
> .ptra3
> .ptrb1
> .ptrb2
> .ptrb3
> .ptrc1
> .ptrc2
> .ptrc3
> .ptrd1
> .ptrd2
> .ptrd3
> .ptre1
> .ptre2
> .ptre3
> <space>
> .section file2
> .ptrf1
> .ptrf2
> .ptrf3
> [...]
> 
> 
> 
> It didn't happen often, heck, trace events and tracepoints were like
> this for sometime before this blew up in our faces.
> 
> From what I've seen is that 1 or 2 longs will pack nicely, but anything
> else (except for maybe 4, 8, 16, etc) will run a risk of crashing.

Yep. We changed our implementations to use an array of pointers to the
elements rather than creating a section of these elements per se to make
sure we would not trigger this problem anymore. Unless my memory fails
me, it started blowing up in our faces with new gcc versions. The thing
is that gcc is within its right to happily over-align those structures.
__attribute__((aligned())) is just a hint providing a minimum alignment
to respect, but the compiler can choose a higher alignment value as it
sees fit.

Mathieu

> 
> -- Steve
> 
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

Re: [PATCH 2/3] DEBUGFS: Add per cpu counters

[Steven Rostedt]

On Tue, 2011-12-13 at 18:08 -0500, Steven Rostedt wrote:

> We hit this with trace events as well, 

Here's the commit if you were interested:

e4a9ea5ee7c8812a7bf0c3fb725ceeaa3d4c2fcc

-- Steve

Re: [PATCH 1/3] DEBUGFS: Automatically create parents for debugfs files v2

[Thomas Gleixner]

On Tue, 13 Dec 2011, Andi Kleen wrote:

> On Wed, Dec 14, 2011 at 12:03:03AM +0100, Thomas Gleixner wrote:
> > On Tue, 13 Dec 2011, Andi Kleen wrote:
> > 
> > > > Hi Andi, is it possible to do say kstrdup or something instead of
> > > 
> > > It's possible, just would add complexity with an additional
> > > error path.
> > > 
> > > > variable-size array in fn[strlen(name) + 1] ? As far as I remember
> > > > we already had some warnings with them at least in perf land (or
> > > > trace land). Or I miss something?
> > > 
> > > Not aware of any problems with tracing or perf. I like them at least.
> > 
> > That does not make them better. We had wreckage before and you can
> > just consult commit a84a79e4d3 and then find a good explanation why
> > your usage is safe.
> 
> Ok. I must admit I sometimes forget with what substandard toolchains
> some of the ports are afflicted with.
> 
> But fair enough. I'll just make the code longer, more complex,
> more error prone and slower.

Oh, yes it's so complex to implement an error path correctly. And of
course it's so fcking relevant whether that code takes a few more
cycles more or not.
 
Thanks,

	tglx

Re: [PATCH 2/3] DEBUGFS: Add per cpu counters

[Thomas Gleixner]

On Tue, 13 Dec 2011, Andi Kleen wrote:
> On Tue, Dec 13, 2011 at 11:43:16PM +0100, Thomas Gleixner wrote:
> > On Tue, 13 Dec 2011, Andi Kleen wrote:
> > >  									\
> > > +	__debugfs	: AT(ADDR(__debugfs) - LOAD_OFFSET) {		\
> > > +		VMLINUX_SYMBOL(__start___debugfs) = .;			\
> > > +		*(__debugfs)						\
> > > +		VMLINUX_SYMBOL(__stop___debugfs) = .;			\
> > > +	}								\
> > > +									\
> > 
> > ....
> >   
> > > +struct debugfs_counter {
> > > +	unsigned __percpu *ptr;
> > > +	const char *fn;
> > > +	const char *name;
> > > +} __attribute__((aligned(sizeof(char *))));
> > > +
> > > +/* Note: static doesn't work unlike DEFINE_PERCPU. Sorry. */
> > > +#define DEFINE_DEBUGFS_COUNTER(name_, file)				\
> > > +	DEFINE_PER_CPU(unsigned, name_ ## _counter);			\
> > > +	struct debugfs_counter name_ ## _pcpu_counter __used		\
> > > +	__attribute__((aligned(sizeof(char *)),section("__debugfs"),unused)) \
> > > +	= { .ptr = &name_ ## _counter, .fn = file, .name = #name_ }; \
> > 
> > Sigh, we had that section forms an array problem more than once
> > already. Why do you invent another variant and think that it will not
> > explode?
> 
> I did three or four different sections like this in the past and as far
> as I know none of them has exploded so far in production use.
> 
> Can you be more specific? Where exactly do you think this will
> not work?

Can you actually read and understand what I wrote below?
 
> > Your alignment magic does not guarantee at all that the structs will
> > form an array. The "aligned" attribute guarantees only the _MINIMUM_
> > alignment for a structure, but the compiler and the linker are free to
> > align on larger multiples.
> 
> > 
> > See commit 654986462 for details.
> 
> Doesn't give a lot of details actually.

There are enough details to figure it out. http://www.jfgit.com might
help.

>  Which target?

The target is irrelevant.

> Note that my structure only has pointers, so there is not a lot 
> of potential for "evil" alignment.

That does NOT matter at ALL. Also whether you did that in the past or
not is completely irrelevant. Its also totally irrelevant whether your
structure only has pointers or not.

There is NO GUARANTEE that this will end up in a linear array with a
stride of the struct size. Period.

Just get it.

Thanks,

	tglx