help converting zcache from sysfs to debugfs?

help converting zcache from sysfs to debugfs?

[Dan Magenheimer]

Zcache (in staging) has a large number of read-only counters that
are primarily of interest to developers.  These counters are currently
visible from sysfs.  However sysfs is not really appropriate and
zcache will need to switch to debugfs before it can be promoted
out of staging.

For some of the counters, it is critical that they remain accurate so
an atomic_t must be used.  But AFAICT there is no way for debugfs
to work with atomic_t.

Is that correct?  Or am I missing something?

Assuming it is correct, I have a workaround but it is ugly:

static unsigned long counterX;
static atomic_t atomic_counterX;

	counterX = atomic_*_return(atomic_counterX)

and use atomic_counter in normal code and counter for debugfs.

This works but requires each counter to be stored twice AND
makes the code look ugly.

Is there a better way?  I can probably bury the ugliness in
macros but that doesn't solve the duplicate storage.  (Though
since there are only about a dozen, maybe it doesn't matter?)

Thanks,
Dan

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: help converting zcache from sysfs to debugfs?

[Konrad Rzeszutek Wilk]

[-- Attachment #1: Type: text/plain, Size: 1530 bytes --]

On Jun 19, 2012 8:30 PM, "Dan Magenheimer" <dan.magenheimer@oracle.com>
wrote:
>
> Zcache (in staging) has a large number of read-only counters that
> are primarily of interest to developers.  These counters are currently
> visible from sysfs.  However sysfs is not really appropriate and
> zcache will need to switch to debugfs before it can be promoted
> out of staging.
>
> For some of the counters, it is critical that they remain accurate so
> an atomic_t must be used.  But AFAICT there is no way for debugfs
> to work with atomic_t.

Which ones must be atomic? Do they really need to be atomic if they are for
diagnostics/developers?
>
> Is that correct?  Or am I missing something?
>
> Assuming it is correct, I have a workaround but it is ugly:
>
> static unsigned long counterX;
> static atomic_t atomic_counterX;
>
>        counterX = atomic_*_return(atomic_counterX)
>
> and use atomic_counter in normal code and counter for debugfs.
>
> This works but requires each counter to be stored twice AND
> makes the code look ugly.

But only for those counters that truly must be atomic.
>
> Is there a better way?  I can probably bury the ugliness in
> macros but that doesn't solve the duplicate storage.  (Though
> since there are only about a dozen, maybe it doesn't matter?)

A dozen that _MUST_ be atomic?

>
> Thanks,
> Dan
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href
>

[-- Attachment #2: Type: text/html, Size: 2091 bytes --]

RE: help converting zcache from sysfs to debugfs?

[Dan Magenheimer]

> From: Konrad Rzeszutek Wilk [mailto:konrad@darnok.org]
> Subject: Re: help converting zcache from sysfs to debugfs?
> 
> On Jun 19, 2012 8:30 PM, "Dan Magenheimer" <dan.magenheimer@oracle.com> wrote:
> >
> > Zcache (in staging) has a large number of read-only counters that
> > are primarily of interest to developers.  These counters are currently
> > visible from sysfs.  However sysfs is not really appropriate and
> > zcache will need to switch to debugfs before it can be promoted
> > out of staging.
> >
> > For some of the counters, it is critical that they remain accurate so
> > an atomic_t must be used.  But AFAICT there is no way for debugfs
> > to work with atomic_t.
> 
> Which ones must be atomic? Do they really need to be atomic if they are for diagnostics/developers?
>  :
> A dozen that _MUST_ be atomic?

Hi Konrad --

On second look, there are only eight.  They are unsigned, go
up and down, and the code needs to know if/when they are zero
(or BUG if they go below zero).  When viewed by a developer
(or very savvy administrator), they can be viewed directly or
feed into analysis tools that would likely get very confused
by "huge" ("negative unsigned") values.  Recently, Seth changed
one zcache counter from unsigned to atomic_t for this reason.

Thanks,
Dan

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: help converting zcache from sysfs to debugfs?

[Seth Jennings]

On 06/19/2012 07:29 PM, Dan Magenheimer wrote:

> Zcache (in staging) has a large number of read-only counters that
> are primarily of interest to developers.  These counters are currently
> visible from sysfs.  However sysfs is not really appropriate and
> zcache will need to switch to debugfs before it can be promoted
> out of staging.
> 
> For some of the counters, it is critical that they remain accurate so
> an atomic_t must be used.  But AFAICT there is no way for debugfs
> to work with atomic_t.


Yes, there doesn't seem to be an existing interface.

You could add support for it to fs/debugfs/file.c.  It doesn't look too
complicated.

--
Seth

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: help converting zcache from sysfs to debugfs?

[Seth Jennings]

Something like this (untested):

diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index 2340f69..652b882 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -21,6 +21,7 @@
 #include <linux/debugfs.h>
 #include <linux/io.h>
 #include <linux/slab.h>
+#include <linux/atomic.h>
 
 static ssize_t default_read_file(struct file *file, char __user *buf,
 				 size_t count, loff_t *ppos)
@@ -403,6 +404,45 @@ struct dentry *debugfs_create_size_t(const char *name, umode_t mode,
 }
 EXPORT_SYMBOL_GPL(debugfs_create_size_t);
 
+static int debugfs_atomic_t_set(void *data, u64 val)
+{
+	atomic_set((atomic_t *)data, val);
+	return 0;
+}
+static int debugfs_atomic_t_get(void *data, u64 *val)
+{
+	*val = atomic_read((atomic_t *)data);
+	return 0;
+}
+DEFINE_SIMPLE_ATTRIBUTE(fops_atomic_t, debugfs_atomic_t_get,
+			debugfs_atomic_t_set, "%llu\n");
+DEFINE_SIMPLE_ATTRIBUTE(fops_atomic_t_ro, debugfs_atomic_t_get, NULL, "%llu\n");
+DEFINE_SIMPLE_ATTRIBUTE(fops_atomic_t_wo, NULL, debugfs_atomic_t_set, "%llu\n");
+
+/**
+ * debugfs_create_atomic_t - create a debugfs file that is used to read and
+ * write an atomic_t value
+ * @name: a pointer to a string containing the name of the file to create.
+ * @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 parameter is %NULL, then the
+ *          file will be created in the root of the debugfs filesystem.
+ * @value: a pointer to the variable that the file should read to and write
+ *         from.
+ */
+struct dentry *debugfs_create_atomic_t(const char *name, umode_t mode,
+				 struct dentry *parent, atomic_t *value)
+{
+	/* if there are no write bits set, make read only */
+	if (!(mode & S_IWUGO))
+		return debugfs_create_file(name, mode, parent, value, &fops_atomic_t_ro);
+	/* if there are no read bits set, make write only */
+	if (!(mode & S_IRUGO))
+		return debugfs_create_file(name, mode, parent, value, &fops_atomic_t_wo);
+
+	return debugfs_create_file(name, mode, parent, value, &fops_atomic_t);
+}
+EXPORT_SYMBOL_GPL(debugfs_create_atomic_t);
 
 static ssize_t read_file_bool(struct file *file, char __user *user_buf,
 			      size_t count, loff_t *ppos)
diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h
index 66c434f..51fea70 100644
--- a/include/linux/debugfs.h
+++ b/include/linux/debugfs.h
@@ -79,6 +79,8 @@ struct dentry *debugfs_create_x64(const char *name, umode_t mode,
 				  struct dentry *parent, u64 *value);
 struct dentry *debugfs_create_size_t(const char *name, umode_t mode,
 				     struct dentry *parent, size_t *value);
+struct dentry *debugfs_create_atomic_t(const char *name, umode_t mode,
+				     struct dentry *parent, atomic_t *value);
 struct dentry *debugfs_create_bool(const char *name, umode_t mode,
 				  struct dentry *parent, u32 *value);
 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

RE: help converting zcache from sysfs to debugfs?

[Dan Magenheimer]

> From: Seth Jennings [mailto:sjenning@linux.vnet.ibm.com]
> Subject: Re: help converting zcache from sysfs to debugfs?
> 
> Something like this (untested):

Nice!  I also need a set for atomic_long_t.

But forgive me if I nearly have a heart attack as I
contemplate another chicken-and-egg scenario trying
to get debugfs-support-for-atomics upstream before
zcache code that depends on it.

Maybe I'm a leetle bit over-sensitized to dependencies...
or maybe not enough ;-)

Anyway, I will probably use the ugly code and add a
comment that says the code can be made cleaner when
debugfs supports atomics.

Thanks!
Dan

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: help converting zcache from sysfs to debugfs?

[Seth Jennings]

On 06/20/2012 10:30 AM, Dan Magenheimer wrote:

> But forgive me if I nearly have a heart attack as I
> contemplate another chicken-and-egg scenario trying
> to get debugfs-support-for-atomics upstream before
> zcache code that depends on it.


I don't think this is the same situation as most of our
other chicken and egg situations.  This is a very generic
addition to debugfs (i.e. someone anyone can use, not just
zcache).  Especially as more people use debugfs or convert
some of their sysfs to debufs. In all honesty, I'm surprised
it hasn't been added sooner.

--
Seth

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: help converting zcache from sysfs to debugfs?

[Konrad Rzeszutek Wilk]

On Wed, Jun 20, 2012 at 08:30:35AM -0700, Dan Magenheimer wrote:
> > From: Seth Jennings [mailto:sjenning@linux.vnet.ibm.com]
> > Subject: Re: help converting zcache from sysfs to debugfs?
> > 
> > Something like this (untested):
> 
> Nice!  I also need a set for atomic_long_t.
> 
> But forgive me if I nearly have a heart attack as I
> contemplate another chicken-and-egg scenario trying
> to get debugfs-support-for-atomics upstream before
> zcache code that depends on it.
> 
> Maybe I'm a leetle bit over-sensitized to dependencies...
> or maybe not enough ;-)

I wouldn't that much. Especially as Greg KH is the maintainer
of debugfs.
> 
> Anyway, I will probably use the ugly code and add a
> comment that says the code can be made cleaner when
> debugfs supports atomics.
> 
> Thanks!
> Dan
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>