* [PATCH 1/3] DEBUGFS: Automatically create parents for debugfs files
2011-12-02 18:43 Add simple way to add statistic counters to debugfs Andi Kleen
@ 2011-12-02 18:43 ` Andi Kleen
2011-12-02 19:17 ` Greg KH
2011-12-02 18:43 ` [PATCH 2/3] DEBUGFS: Add per cpu counters Andi Kleen
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Andi Kleen @ 2011-12-02 18:43 UTC (permalink / raw)
To: greg; +Cc: linux-kernel, linux-fsdevel, fengguang.wu, 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.
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
fs/debugfs/inode.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 51 insertions(+), 0 deletions(-)
diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index f3a257d..7ea9a6c 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -146,6 +146,52 @@ 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) {
+ /* ignore error here */
+ 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 +199,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 +208,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,6 +233,9 @@ 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;
}
--
1.7.4.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] DEBUGFS: Automatically create parents for debugfs files
2011-12-02 18:43 ` [PATCH 1/3] DEBUGFS: Automatically create parents for debugfs files Andi Kleen
@ 2011-12-02 19:17 ` Greg KH
2011-12-02 19:42 ` Andi Kleen
0 siblings, 1 reply; 11+ messages in thread
From: Greg KH @ 2011-12-02 19:17 UTC (permalink / raw)
To: Andi Kleen; +Cc: linux-kernel, linux-fsdevel, fengguang.wu, Andi Kleen
On Fri, Dec 02, 2011 at 10:43:25AM -0800, Andi Kleen wrote:
> 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.
I like this, but some documentation will probably need to be added
somwhere saying that it's now legal to create a debugfs file with
"this/is/a/tree" and have it all be expanded out.
And yes, cleaning up the directories shouldn't be a big issue, but what
if the directory is already there and it tries to be created again?
Shouldn't you do a lookup first and use that dentry if it's there?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] DEBUGFS: Automatically create parents for debugfs files
2011-12-02 19:17 ` Greg KH
@ 2011-12-02 19:42 ` Andi Kleen
2011-12-02 19:58 ` Greg KH
0 siblings, 1 reply; 11+ messages in thread
From: Andi Kleen @ 2011-12-02 19:42 UTC (permalink / raw)
To: Greg KH; +Cc: Andi Kleen, linux-kernel, linux-fsdevel, fengguang.wu, Andi Kleen
On Fri, Dec 02, 2011 at 11:17:24AM -0800, Greg KH wrote:
> I like this, but some documentation will probably need to be added
> somwhere saying that it's now legal to create a debugfs file with
> "this/is/a/tree" and have it all be expanded out.
Ok. Updating the kerneldoc entries.
>
> And yes, cleaning up the directories shouldn't be a big issue, but what
> if the directory is already there and it tries to be created again?
> Shouldn't you do a lookup first and use that dentry if it's there?
In this case debugfs_create_dir() errors out, the caller ignores
the error and just looks it up. So yes should work.
-Andi
--
ak@linux.intel.com -- Speaking for myself only.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] DEBUGFS: Automatically create parents for debugfs files
2011-12-02 19:42 ` Andi Kleen
@ 2011-12-02 19:58 ` Greg KH
0 siblings, 0 replies; 11+ messages in thread
From: Greg KH @ 2011-12-02 19:58 UTC (permalink / raw)
To: Andi Kleen; +Cc: linux-kernel, linux-fsdevel, fengguang.wu, Andi Kleen
On Fri, Dec 02, 2011 at 08:42:19PM +0100, Andi Kleen wrote:
> On Fri, Dec 02, 2011 at 11:17:24AM -0800, Greg KH wrote:
> > I like this, but some documentation will probably need to be added
> > somwhere saying that it's now legal to create a debugfs file with
> > "this/is/a/tree" and have it all be expanded out.
>
> Ok. Updating the kerneldoc entries.
>
> >
> > And yes, cleaning up the directories shouldn't be a big issue, but what
> > if the directory is already there and it tries to be created again?
> > Shouldn't you do a lookup first and use that dentry if it's there?
>
> In this case debugfs_create_dir() errors out, the caller ignores
> the error and just looks it up. So yes should work.
Ah, ok, nevermind then :)
thanks,
greg k-h
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/3] DEBUGFS: Add per cpu counters
2011-12-02 18:43 Add simple way to add statistic counters to debugfs Andi Kleen
2011-12-02 18:43 ` [PATCH 1/3] DEBUGFS: Automatically create parents for debugfs files Andi Kleen
@ 2011-12-02 18:43 ` Andi Kleen
2011-12-06 11:44 ` Wu Fengguang
2011-12-02 18:43 ` [PATCH 3/3] VFS: Add event counting to dcache Andi Kleen
2011-12-02 19:18 ` Add simple way to add statistic counters to debugfs Greg KH
3 siblings, 1 reply; 11+ messages in thread
From: Andi Kleen @ 2011-12-02 18:43 UTC (permalink / raw)
To: greg; +Cc: linux-kernel, linux-fsdevel, fengguang.wu, 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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] DEBUGFS: Add per cpu counters
2011-12-02 18:43 ` [PATCH 2/3] DEBUGFS: Add per cpu counters Andi Kleen
@ 2011-12-06 11:44 ` Wu Fengguang
2011-12-06 17:17 ` Andi Kleen
0 siblings, 1 reply; 11+ messages in thread
From: Wu Fengguang @ 2011-12-06 11:44 UTC (permalink / raw)
To: Andi Kleen
Cc: greg@kroah.com, linux-kernel@vger.kernel.org,
linux-fsdevel@vger.kernel.org, Andi Kleen
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] DEBUGFS: Add per cpu counters
2011-12-06 11:44 ` Wu Fengguang
@ 2011-12-06 17:17 ` Andi Kleen
2011-12-09 3:00 ` Wu Fengguang
0 siblings, 1 reply; 11+ messages in thread
From: Andi Kleen @ 2011-12-06 17:17 UTC (permalink / raw)
To: Wu Fengguang
Cc: Andi Kleen, greg@kroah.com, linux-kernel@vger.kernel.org,
linux-fsdevel@vger.kernel.org
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] DEBUGFS: Add per cpu counters
2011-12-06 17:17 ` Andi Kleen
@ 2011-12-09 3:00 ` Wu Fengguang
0 siblings, 0 replies; 11+ messages in thread
From: Wu Fengguang @ 2011-12-09 3:00 UTC (permalink / raw)
To: Andi Kleen
Cc: greg@kroah.com, linux-kernel@vger.kernel.org,
linux-fsdevel@vger.kernel.org
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/3] VFS: Add event counting to dcache
2011-12-02 18:43 Add simple way to add statistic counters to debugfs Andi Kleen
2011-12-02 18:43 ` [PATCH 1/3] DEBUGFS: Automatically create parents for debugfs files Andi Kleen
2011-12-02 18:43 ` [PATCH 2/3] DEBUGFS: Add per cpu counters Andi Kleen
@ 2011-12-02 18:43 ` Andi Kleen
2011-12-02 19:18 ` Add simple way to add statistic counters to debugfs Greg KH
3 siblings, 0 replies; 11+ messages in thread
From: Andi Kleen @ 2011-12-02 18:43 UTC (permalink / raw)
To: greg; +Cc: linux-kernel, linux-fsdevel, fengguang.wu, Andi Kleen
From: Andi Kleen <ak@linux.intel.com>
Most self respecting subsystems -- like networking or MM -- have
own counter infrastructure these days. This is useful to understand
the behaviour of a running system. Counters are low enough
overhead that they can be always enabled.
This patch adds event counts to the dcache.
Instead of developing an own counter infrastructure for the VFS
I'm using generic counters implemented in debugfs.
Since we had problems with this recently I instrumented the dcache
RCU code. This is rather tricky code which is difficult to tune,
and some indication on why aborts happen is quite useful.
I'm especially interested in feedback on the placement of the event counters.
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
fs/namei.c | 42 ++++++++++++++++++++++++++++++++++++------
1 files changed, 36 insertions(+), 6 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index 5008f01..bfbe36a 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -33,10 +33,22 @@
#include <linux/device_cgroup.h>
#include <linux/fs_struct.h>
#include <linux/posix_acl.h>
+#include <linux/debugfs.h>
#include <asm/uaccess.h>
#include "internal.h"
+const char dname[] = "vfs/dcache";
+DEFINE_DEBUGFS_COUNTER(dcache_rcu_root_changed_abort, dname);
+DEFINE_DEBUGFS_COUNTER(dcache_rcu_dir_changed_abort, dname);
+DEFINE_DEBUGFS_COUNTER(dcache_rcu_entry_changed_abort, dname);
+DEFINE_DEBUGFS_COUNTER(dcache_rcu_permission_abort, dname);
+DEFINE_DEBUGFS_COUNTER(dcache_rcu_revalidate_abort, dname);
+DEFINE_DEBUGFS_COUNTER(dcache_ref_walks, dname);
+DEFINE_DEBUGFS_COUNTER(dcache_rcu_walks, dname);
+DEFINE_DEBUGFS_COUNTER(dcache_reval_walks, dname);
+
+
/* [Feb-1997 T. Schoebel-Theuer]
* Fundamental changes in the pathname lookup mechanisms (namei)
* were necessary because of omirr. The reason is that omirr needs
@@ -430,20 +442,26 @@ static int unlazy_walk(struct nameidata *nd, struct dentry *dentry)
want_root = 1;
spin_lock(&fs->lock);
if (nd->root.mnt != fs->root.mnt ||
- nd->root.dentry != fs->root.dentry)
+ nd->root.dentry != fs->root.dentry) {
+ debugfs_counter_inc(dcache_rcu_root_changed_abort);
goto err_root;
+ }
}
spin_lock(&parent->d_lock);
if (!dentry) {
- if (!__d_rcu_to_refcount(parent, nd->seq))
+ if (!__d_rcu_to_refcount(parent, nd->seq)) {
+ debugfs_counter_inc(dcache_rcu_dir_changed_abort);
goto err_parent;
+ }
BUG_ON(nd->inode != parent->d_inode);
} else {
if (dentry->d_parent != parent)
goto err_parent;
spin_lock_nested(&dentry->d_lock, DENTRY_D_LOCK_NESTED);
- if (!__d_rcu_to_refcount(dentry, nd->seq))
+ if (!__d_rcu_to_refcount(dentry, nd->seq)) {
+ debugfs_counter_inc(dcache_rcu_entry_changed_abort);
goto err_child;
+ }
/*
* If the sequence check on the child dentry passed, then
* the child has not been removed from its parent. This
@@ -474,6 +492,7 @@ err_parent:
err_root:
if (want_root)
spin_unlock(&fs->lock);
+ debugfs_counter_inc(dcache_rcu_root_changed_abort);
return -ECHILD;
}
@@ -522,6 +541,7 @@ static int complete_walk(struct nameidata *nd)
spin_unlock(&dentry->d_lock);
rcu_read_unlock();
br_read_unlock(vfsmount_lock);
+ debugfs_counter_inc(dcache_rcu_entry_changed_abort);
return -ECHILD;
}
BUG_ON(nd->inode != dentry->d_inode);
@@ -960,6 +980,7 @@ failed:
nd->root.mnt = NULL;
rcu_read_unlock();
br_read_unlock(vfsmount_lock);
+ debugfs_counter_inc(dcache_rcu_entry_changed_abort);
return -ECHILD;
}
@@ -1132,8 +1153,10 @@ static int do_lookup(struct nameidata *nd, struct qstr *name,
goto unlazy;
/* Memory barrier in read_seqcount_begin of child is enough */
- if (__read_seqcount_retry(&parent->d_seq, nd->seq))
+ if (__read_seqcount_retry(&parent->d_seq, nd->seq)) {
+ debugfs_counter_inc(dcache_rcu_dir_changed_abort);
return -ECHILD;
+ }
nd->seq = seq;
if (unlikely(dentry->d_flags & DCACHE_OP_REVALIDATE)) {
@@ -1141,6 +1164,7 @@ static int do_lookup(struct nameidata *nd, struct qstr *name,
if (unlikely(status <= 0)) {
if (status != -ECHILD)
need_reval = 0;
+ debugfs_counter_inc(dcache_rcu_revalidate_abort);
goto unlazy;
}
}
@@ -1226,6 +1250,7 @@ static inline int may_lookup(struct nameidata *nd)
int err = inode_permission(nd->inode, MAY_EXEC|MAY_NOT_BLOCK);
if (err != -ECHILD)
return err;
+ debugfs_counter_inc(dcache_rcu_permission_abort);
if (unlazy_walk(nd, NULL))
return -ECHILD;
}
@@ -1643,10 +1668,15 @@ static int do_path_lookup(int dfd, const char *name,
unsigned int flags, struct nameidata *nd)
{
int retval = path_lookupat(dfd, name, flags | LOOKUP_RCU, nd);
- if (unlikely(retval == -ECHILD))
+ debugfs_counter_inc(dcache_rcu_walks);
+ if (unlikely(retval == -ECHILD)) {
+ debugfs_counter_inc(dcache_ref_walks);
retval = path_lookupat(dfd, name, flags, nd);
- if (unlikely(retval == -ESTALE))
+ }
+ if (unlikely(retval == -ESTALE)) {
+ debugfs_counter_inc(dcache_reval_walks);
retval = path_lookupat(dfd, name, flags | LOOKUP_REVAL, nd);
+ }
if (likely(!retval)) {
if (unlikely(!audit_dummy_context())) {
--
1.7.4.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: Add simple way to add statistic counters to debugfs
2011-12-02 18:43 Add simple way to add statistic counters to debugfs Andi Kleen
` (2 preceding siblings ...)
2011-12-02 18:43 ` [PATCH 3/3] VFS: Add event counting to dcache Andi Kleen
@ 2011-12-02 19:18 ` Greg KH
3 siblings, 0 replies; 11+ messages in thread
From: Greg KH @ 2011-12-02 19:18 UTC (permalink / raw)
To: Andi Kleen; +Cc: linux-kernel, linux-fsdevel, fengguang.wu
On Fri, Dec 02, 2011 at 10:43:24AM -0800, Andi Kleen wrote:
> This extends debugfs to make it very simple to add per cpu counters
> statistics that are exported there. This is useful for all kind
> of debugging situations. The counters have very little overhead
> and are simpler to use than trace points.
>
> This is the infrastructure for debugfs and a demo application
> to instrument the dcache RCU which I developed some time ago
> to track down some problems there.
>
> Posting here also so that Fengguang can use it.
>
> Module support is still missing, but could be added without too
> much work.
I like this a lot, very nice job. I just had one comment with the first
patch.
And yes, I would like module support as well, but others can add that if
they really need it :)
Care to fix up the first patch and I'll be glad to take the two debugfs
patches and any others if you want me to.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 11+ messages in thread