2007-12-19 00:33:04

by Stefano Brivio

[permalink] [raw]
Subject: [PATCH v3 7/8] debugfs: allow access to signed values

Add debugfs_create_s{8,16,32,64}. For these to work properly, we need to remove
a cast in libfs.

Cc: Johannes Berg <[email protected]>
To: Greg Kroah-Hartman <[email protected]>
Signed-off-by: Stefano Brivio <[email protected]>
---
Greg,

here comes an implementation of debugfs_create_s{8,16,32,64} which avoids code
duplication, suggested by Johannes Berg. We would need this to be merged to
2.6.25, as we need those functions to be available for rc80211-pid, the new
mac80211 rate control algorithm.

---
fs/debugfs/file.c | 166 ++++++++++++++++++++++++++++++++++++++++++++++++
fs/libfs.c | 4 -
include/linux/debugfs.h | 30 ++++++++
3 files changed, 197 insertions(+), 3 deletions(-)

Index: wireless-2.6/fs/debugfs/file.c
===================================================================
--- wireless-2.6.orig/fs/debugfs/file.c
+++ wireless-2.6/fs/debugfs/file.c
@@ -221,6 +221,172 @@ struct dentry *debugfs_create_u64(const
}
EXPORT_SYMBOL_GPL(debugfs_create_u64);

+
+static void debugfs_s8_set(void *data, u64 val)
+{
+ *(s8 *)data = val;
+}
+static u64 debugfs_s8_get(void *data)
+{
+ return *(s8 *)data;
+}
+DEFINE_SIMPLE_ATTRIBUTE(fops_s8, debugfs_s8_get, debugfs_s8_set, "%lld\n");
+
+/**
+ * debugfs_create_s8 - create a debugfs file that is used to read and write a signed 8-bit 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.
+ *
+ * This function creates a file in debugfs with the given name that
+ * contains the value of the variable @value. If the @mode variable is so
+ * set, it can be read from, and written to.
+ *
+ * This function will return a pointer to a dentry if it succeeds. This
+ * pointer must be passed to the debugfs_remove() function when the file is
+ * to be removed (no automatic cleanup happens if your module is unloaded,
+ * you are responsible here.) If an error occurs, %NULL will be returned.
+ *
+ * If debugfs is not enabled in the kernel, the value -%ENODEV will be
+ * returned. It is not wise to check for this value, but rather, check for
+ * %NULL or !%NULL instead as to eliminate the need for #ifdef in the calling
+ * code.
+ */
+struct dentry *debugfs_create_s8(const char *name, mode_t mode,
+ struct dentry *parent, s8 *value)
+{
+ return debugfs_create_file(name, mode, parent, value, &fops_s8);
+}
+EXPORT_SYMBOL_GPL(debugfs_create_s8);
+
+static void debugfs_s16_set(void *data, u64 val)
+{
+ *(s16 *)data = val;
+}
+static u64 debugfs_s16_get(void *data)
+{
+ return *(s16 *)data;
+}
+DEFINE_SIMPLE_ATTRIBUTE(fops_s16, debugfs_s16_get, debugfs_s16_set, "%lld\n");
+
+/**
+ * debugfs_create_s16 - create a debugfs file that is used to read and write a signed 16-bit 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.
+ *
+ * This function creates a file in debugfs with the given name that
+ * contains the value of the variable @value. If the @mode variable is so
+ * set, it can be read from, and written to.
+ *
+ * This function will return a pointer to a dentry if it succeeds. This
+ * pointer must be passed to the debugfs_remove() function when the file is
+ * to be removed (no automatic cleanup happens if your module is unloaded,
+ * you are responsible here.) If an error occurs, %NULL will be returned.
+ *
+ * If debugfs is not enabled in the kernel, the value -%ENODEV will be
+ * returned. It is not wise to check for this value, but rather, check for
+ * %NULL or !%NULL instead as to eliminate the need for #ifdef in the calling
+ * code.
+ */
+struct dentry *debugfs_create_s16(const char *name, mode_t mode,
+ struct dentry *parent, s16 *value)
+{
+ return debugfs_create_file(name, mode, parent, value, &fops_s16);
+}
+EXPORT_SYMBOL_GPL(debugfs_create_s16);
+
+static void debugfs_s32_set(void *data, u64 val)
+{
+ *(s32 *)data = val;
+}
+static u64 debugfs_s32_get(void *data)
+{
+ return *(s32 *)data;
+}
+DEFINE_SIMPLE_ATTRIBUTE(fops_s32, debugfs_s32_get, debugfs_s32_set, "%lld\n");
+
+/**
+ * debugfs_create_s32 - create a debugfs file that is used to read and write a signed 32-bit 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.
+ *
+ * This function creates a file in debugfs with the given name that
+ * contains the value of the variable @value. If the @mode variable is so
+ * set, it can be read from, and written to.
+ *
+ * This function will return a pointer to a dentry if it succeeds. This
+ * pointer must be passed to the debugfs_remove() function when the file is
+ * to be removed (no automatic cleanup happens if your module is unloaded,
+ * you are responsible here.) If an error occurs, %NULL will be returned.
+ *
+ * If debugfs is not enabled in the kernel, the value -%ENODEV will be
+ * returned. It is not wise to check for this value, but rather, check for
+ * %NULL or !%NULL instead as to eliminate the need for #ifdef in the calling
+ * code.
+ */
+struct dentry *debugfs_create_s32(const char *name, mode_t mode,
+ struct dentry *parent, s32 *value)
+{
+ return debugfs_create_file(name, mode, parent, value, &fops_s32);
+}
+EXPORT_SYMBOL_GPL(debugfs_create_s32);
+
+static void debugfs_s64_set(void *data, u64 val)
+{
+ *(s64 *)data = val;
+}
+
+static u64 debugfs_s64_get(void *data)
+{
+ return *(s64 *)data;
+}
+DEFINE_SIMPLE_ATTRIBUTE(fops_s64, debugfs_s64_get, debugfs_s64_set, "%lld\n");
+
+/**
+ * debugfs_create_s64 - create a debugfs file that is used to read and write a signed 64-bit 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.
+ *
+ * This function creates a file in debugfs with the given name that
+ * contains the value of the variable @value. If the @mode variable is so
+ * set, it can be read from, and written to.
+ *
+ * This function will return a pointer to a dentry if it succeeds. This
+ * pointer must be passed to the debugfs_remove() function when the file is
+ * to be removed (no automatic cleanup happens if your module is unloaded,
+ * you are responsible here.) If an error occurs, %NULL will be returned.
+ *
+ * If debugfs is not enabled in the kernel, the value -%ENODEV will be
+ * returned. It is not wise to check for this value, but rather, check for
+ * %NULL or !%NULL instead as to eliminate the need for #ifdef in the calling
+ * code.
+ */
+struct dentry *debugfs_create_s64(const char *name, mode_t mode,
+ struct dentry *parent, u64 *value)
+{
+ return debugfs_create_file(name, mode, parent, value, &fops_s64);
+}
+EXPORT_SYMBOL_GPL(debugfs_create_s64);
+
DEFINE_SIMPLE_ATTRIBUTE(fops_x8, debugfs_u8_get, debugfs_u8_set, "0x%02llx\n");

DEFINE_SIMPLE_ATTRIBUTE(fops_x16, debugfs_u16_get, debugfs_u16_set, "0x%04llx\n");

Index: wireless-2.6/fs/libfs.c
===================================================================
--- wireless-2.6.orig/fs/libfs.c
+++ wireless-2.6/fs/libfs.c
@@ -586,7 +586,7 @@ int simple_transaction_release(struct in
/* Simple attribute files */

struct simple_attr {
- u64 (*get)(void *);
+ unsigned long long (*get)(void *);
void (*set)(void *, u64);
char get_buf[24]; /* enough to store a u64 and "\n\0" */
char set_buf[24];
@@ -643,7 +643,7 @@ ssize_t simple_attr_read(struct file *fi
else /* first read */
size = scnprintf(attr->get_buf, sizeof(attr->get_buf),
attr->fmt,
- (unsigned long long)attr->get(attr->data));
+ attr->get(attr->data));

ret = simple_read_from_buffer(buf, len, ppos, attr->get_buf, size);
mutex_unlock(&attr->mutex);
Index: wireless-2.6/include/linux/debugfs.h
===================================================================
--- wireless-2.6.orig/include/linux/debugfs.h
+++ wireless-2.6/include/linux/debugfs.h
@@ -65,7 +65,7 @@ struct dentry *debugfs_create_blob(const

#include <linux/err.h>

-/*
+/*
* We do not return NULL from these functions if CONFIG_DEBUG_FS is not enabled
* so users have a chance to detect if there was a real error or not. We don't
* want to duplicate the design decision mistakes of procfs and devfs again.
@@ -128,6 +128,34 @@ static inline struct dentry *debugfs_cre
return ERR_PTR(-ENODEV);
}

+static inline struct dentry *debugfs_create_s8(const char *name, mode_t mode,
+ struct dentry *parent,
+ u8 *value)
+{
+ return ERR_PTR(-ENODEV);
+}
+
+static inline struct dentry *debugfs_create_s16(const char *name, mode_t mode,
+ struct dentry *parent,
+ s16 *value)
+{
+ return ERR_PTR(-ENODEV);
+}
+
+static inline struct dentry *debugfs_create_s32(const char *name, mode_t mode,
+ struct dentry *parent,
+ s32 *value)
+{
+ return ERR_PTR(-ENODEV);
+}
+
+static inline struct dentry *debugfs_create_s64(const char *name, mode_t mode,
+ struct dentry *parent,
+ s64 *value)
+{
+ return ERR_PTR(-ENODEV);
+}
+
static inline struct dentry *debugfs_create_x8(const char *name, mode_t mode,
struct dentry *parent,
u8 *value)

--
Ciao
Stefano


2007-12-19 06:43:10

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH v3 7/8] debugfs: allow access to signed values

On Wed, Dec 19, 2007 at 01:27:39AM +0100, Stefano Brivio wrote:
> Add debugfs_create_s{8,16,32,64}. For these to work properly, we need to remove
> a cast in libfs.
>
> Cc: Johannes Berg <[email protected]>
> To: Greg Kroah-Hartman <[email protected]>
> Signed-off-by: Stefano Brivio <[email protected]>
> ---
> Greg,
>
> here comes an implementation of debugfs_create_s{8,16,32,64} which avoids code
> duplication, suggested by Johannes Berg. We would need this to be merged to
> 2.6.25, as we need those functions to be available for rc80211-pid, the new
> mac80211 rate control algorithm.

Looks good to me, do you want me to add this to my tree and send it to
Linus when 2.6.25 opens up, or do you want this to go through the
wireless tree as you have patches relying on it?

Whatever works the best for you is fine for me. If the wireless tree,
feel free to add:
Signed-off-by: Greg Kroah-Hartman <[email protected]>
to the patch.

thanks,

greg k-h

2007-12-19 09:25:38

by Stefano Brivio

[permalink] [raw]
Subject: Re: [PATCH v3 7/8] debugfs: allow access to signed values

On Tue, 18 Dec 2007 22:45:10 -0800
Greg KH <[email protected]> wrote:

> On Wed, Dec 19, 2007 at 01:27:39AM +0100, Stefano Brivio wrote:
> > Add debugfs_create_s{8,16,32,64}. For these to work properly, we need to remove
> > a cast in libfs.
> >
> > Cc: Johannes Berg <[email protected]>
> > To: Greg Kroah-Hartman <[email protected]>
> > Signed-off-by: Stefano Brivio <[email protected]>
> > ---
> > Greg,
> >
> > here comes an implementation of debugfs_create_s{8,16,32,64} which avoids code
> > duplication, suggested by Johannes Berg. We would need this to be merged to
> > 2.6.25, as we need those functions to be available for rc80211-pid, the new
> > mac80211 rate control algorithm.
>
> Looks good to me, do you want me to add this to my tree and send it to
> Linus when 2.6.25 opens up, or do you want this to go through the
> wireless tree as you have patches relying on it?

I have one patch (rate control rework, 8/8) relying on it. Let's ask John. John,
what's the best for you?

> Whatever works the best for you is fine for me. If the wireless tree,
> feel free to add:
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
> to the patch.

Thanks. I'm going to resend the patch to John with your Signed-off-by -- in case.


--
Ciao
Stefano

2007-12-19 09:33:52

by Stefano Brivio

[permalink] [raw]
Subject: [PATCH v4 7/8] debugfs: allow access to signed values

Add debugfs_create_s{8,16,32,64}. For these to work properly, we need to remove
a cast in libfs.

Cc: Johannes Berg <[email protected]>
Signed-off-by: Stefano Brivio <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>

---
fs/debugfs/file.c | 166 ++++++++++++++++++++++++++++++++++++++++++++++++
fs/libfs.c | 4 -
include/linux/debugfs.h | 30 ++++++++
3 files changed, 197 insertions(+), 3 deletions(-)

Index: wireless-2.6/fs/debugfs/file.c
===================================================================
--- wireless-2.6.orig/fs/debugfs/file.c
+++ wireless-2.6/fs/debugfs/file.c
@@ -221,6 +221,172 @@ struct dentry *debugfs_create_u64(const
}
EXPORT_SYMBOL_GPL(debugfs_create_u64);

+
+static void debugfs_s8_set(void *data, u64 val)
+{
+ *(s8 *)data = val;
+}
+static u64 debugfs_s8_get(void *data)
+{
+ return *(s8 *)data;
+}
+DEFINE_SIMPLE_ATTRIBUTE(fops_s8, debugfs_s8_get, debugfs_s8_set, "%lld\n");
+
+/**
+ * debugfs_create_s8 - create a debugfs file that is used to read and write a signed 8-bit 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.
+ *
+ * This function creates a file in debugfs with the given name that
+ * contains the value of the variable @value. If the @mode variable is so
+ * set, it can be read from, and written to.
+ *
+ * This function will return a pointer to a dentry if it succeeds. This
+ * pointer must be passed to the debugfs_remove() function when the file is
+ * to be removed (no automatic cleanup happens if your module is unloaded,
+ * you are responsible here.) If an error occurs, %NULL will be returned.
+ *
+ * If debugfs is not enabled in the kernel, the value -%ENODEV will be
+ * returned. It is not wise to check for this value, but rather, check for
+ * %NULL or !%NULL instead as to eliminate the need for #ifdef in the calling
+ * code.
+ */
+struct dentry *debugfs_create_s8(const char *name, mode_t mode,
+ struct dentry *parent, s8 *value)
+{
+ return debugfs_create_file(name, mode, parent, value, &fops_s8);
+}
+EXPORT_SYMBOL_GPL(debugfs_create_s8);
+
+static void debugfs_s16_set(void *data, u64 val)
+{
+ *(s16 *)data = val;
+}
+static u64 debugfs_s16_get(void *data)
+{
+ return *(s16 *)data;
+}
+DEFINE_SIMPLE_ATTRIBUTE(fops_s16, debugfs_s16_get, debugfs_s16_set, "%lld\n");
+
+/**
+ * debugfs_create_s16 - create a debugfs file that is used to read and write a signed 16-bit 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.
+ *
+ * This function creates a file in debugfs with the given name that
+ * contains the value of the variable @value. If the @mode variable is so
+ * set, it can be read from, and written to.
+ *
+ * This function will return a pointer to a dentry if it succeeds. This
+ * pointer must be passed to the debugfs_remove() function when the file is
+ * to be removed (no automatic cleanup happens if your module is unloaded,
+ * you are responsible here.) If an error occurs, %NULL will be returned.
+ *
+ * If debugfs is not enabled in the kernel, the value -%ENODEV will be
+ * returned. It is not wise to check for this value, but rather, check for
+ * %NULL or !%NULL instead as to eliminate the need for #ifdef in the calling
+ * code.
+ */
+struct dentry *debugfs_create_s16(const char *name, mode_t mode,
+ struct dentry *parent, s16 *value)
+{
+ return debugfs_create_file(name, mode, parent, value, &fops_s16);
+}
+EXPORT_SYMBOL_GPL(debugfs_create_s16);
+
+static void debugfs_s32_set(void *data, u64 val)
+{
+ *(s32 *)data = val;
+}
+static u64 debugfs_s32_get(void *data)
+{
+ return *(s32 *)data;
+}
+DEFINE_SIMPLE_ATTRIBUTE(fops_s32, debugfs_s32_get, debugfs_s32_set, "%lld\n");
+
+/**
+ * debugfs_create_s32 - create a debugfs file that is used to read and write a signed 32-bit 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.
+ *
+ * This function creates a file in debugfs with the given name that
+ * contains the value of the variable @value. If the @mode variable is so
+ * set, it can be read from, and written to.
+ *
+ * This function will return a pointer to a dentry if it succeeds. This
+ * pointer must be passed to the debugfs_remove() function when the file is
+ * to be removed (no automatic cleanup happens if your module is unloaded,
+ * you are responsible here.) If an error occurs, %NULL will be returned.
+ *
+ * If debugfs is not enabled in the kernel, the value -%ENODEV will be
+ * returned. It is not wise to check for this value, but rather, check for
+ * %NULL or !%NULL instead as to eliminate the need for #ifdef in the calling
+ * code.
+ */
+struct dentry *debugfs_create_s32(const char *name, mode_t mode,
+ struct dentry *parent, s32 *value)
+{
+ return debugfs_create_file(name, mode, parent, value, &fops_s32);
+}
+EXPORT_SYMBOL_GPL(debugfs_create_s32);
+
+static void debugfs_s64_set(void *data, u64 val)
+{
+ *(s64 *)data = val;
+}
+
+static u64 debugfs_s64_get(void *data)
+{
+ return *(s64 *)data;
+}
+DEFINE_SIMPLE_ATTRIBUTE(fops_s64, debugfs_s64_get, debugfs_s64_set, "%lld\n");
+
+/**
+ * debugfs_create_s64 - create a debugfs file that is used to read and write a signed 64-bit 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.
+ *
+ * This function creates a file in debugfs with the given name that
+ * contains the value of the variable @value. If the @mode variable is so
+ * set, it can be read from, and written to.
+ *
+ * This function will return a pointer to a dentry if it succeeds. This
+ * pointer must be passed to the debugfs_remove() function when the file is
+ * to be removed (no automatic cleanup happens if your module is unloaded,
+ * you are responsible here.) If an error occurs, %NULL will be returned.
+ *
+ * If debugfs is not enabled in the kernel, the value -%ENODEV will be
+ * returned. It is not wise to check for this value, but rather, check for
+ * %NULL or !%NULL instead as to eliminate the need for #ifdef in the calling
+ * code.
+ */
+struct dentry *debugfs_create_s64(const char *name, mode_t mode,
+ struct dentry *parent, u64 *value)
+{
+ return debugfs_create_file(name, mode, parent, value, &fops_s64);
+}
+EXPORT_SYMBOL_GPL(debugfs_create_s64);
+
DEFINE_SIMPLE_ATTRIBUTE(fops_x8, debugfs_u8_get, debugfs_u8_set, "0x%02llx\n");

DEFINE_SIMPLE_ATTRIBUTE(fops_x16, debugfs_u16_get, debugfs_u16_set, "0x%04llx\n");

Index: wireless-2.6/fs/libfs.c
===================================================================
--- wireless-2.6.orig/fs/libfs.c
+++ wireless-2.6/fs/libfs.c
@@ -586,7 +586,7 @@ int simple_transaction_release(struct in
/* Simple attribute files */

struct simple_attr {
- u64 (*get)(void *);
+ unsigned long long (*get)(void *);
void (*set)(void *, u64);
char get_buf[24]; /* enough to store a u64 and "\n\0" */
char set_buf[24];
@@ -643,7 +643,7 @@ ssize_t simple_attr_read(struct file *fi
else /* first read */
size = scnprintf(attr->get_buf, sizeof(attr->get_buf),
attr->fmt,
- (unsigned long long)attr->get(attr->data));
+ attr->get(attr->data));

ret = simple_read_from_buffer(buf, len, ppos, attr->get_buf, size);
mutex_unlock(&attr->mutex);
Index: wireless-2.6/include/linux/debugfs.h
===================================================================
--- wireless-2.6.orig/include/linux/debugfs.h
+++ wireless-2.6/include/linux/debugfs.h
@@ -65,7 +65,7 @@ struct dentry *debugfs_create_blob(const

#include <linux/err.h>

-/*
+/*
* We do not return NULL from these functions if CONFIG_DEBUG_FS is not enabled
* so users have a chance to detect if there was a real error or not. We don't
* want to duplicate the design decision mistakes of procfs and devfs again.
@@ -128,6 +128,34 @@ static inline struct dentry *debugfs_cre
return ERR_PTR(-ENODEV);
}

+static inline struct dentry *debugfs_create_s8(const char *name, mode_t mode,
+ struct dentry *parent,
+ u8 *value)
+{
+ return ERR_PTR(-ENODEV);
+}
+
+static inline struct dentry *debugfs_create_s16(const char *name, mode_t mode,
+ struct dentry *parent,
+ s16 *value)
+{
+ return ERR_PTR(-ENODEV);
+}
+
+static inline struct dentry *debugfs_create_s32(const char *name, mode_t mode,
+ struct dentry *parent,
+ s32 *value)
+{
+ return ERR_PTR(-ENODEV);
+}
+
+static inline struct dentry *debugfs_create_s64(const char *name, mode_t mode,
+ struct dentry *parent,
+ s64 *value)
+{
+ return ERR_PTR(-ENODEV);
+}
+
static inline struct dentry *debugfs_create_x8(const char *name, mode_t mode,
struct dentry *parent,
u8 *value)


--
Ciao
Stefano

2007-12-19 15:00:17

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH v3 7/8] debugfs: allow access to signed values

On Wed, Dec 19, 2007 at 10:20:16AM +0100, Stefano Brivio wrote:
> On Tue, 18 Dec 2007 22:45:10 -0800
> Greg KH <[email protected]> wrote:
>
> > On Wed, Dec 19, 2007 at 01:27:39AM +0100, Stefano Brivio wrote:

> > > here comes an implementation of debugfs_create_s{8,16,32,64} which avoids code
> > > duplication, suggested by Johannes Berg. We would need this to be merged to
> > > 2.6.25, as we need those functions to be available for rc80211-pid, the new
> > > mac80211 rate control algorithm.
> >
> > Looks good to me, do you want me to add this to my tree and send it to
> > Linus when 2.6.25 opens up, or do you want this to go through the
> > wireless tree as you have patches relying on it?
>
> I have one patch (rate control rework, 8/8) relying on it. Let's ask John. John,
> what's the best for you?

Since Greg approves, I'll take it through wireless-2.6 to avoid any
staging problems for patch 8/8.

Thanks,

John
--
John W. Linville
[email protected]

2007-12-19 16:00:51

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v3 7/8] debugfs: allow access to signed values


> +static u64 debugfs_s8_get(void *data)


> struct simple_attr {
> - u64 (*get)(void *);
> + unsigned long long (*get)(void *);

That seems wrong. Wouldn't you have to declare all the _get functions
with unsigned long long too now to avoid trouble on systems where u64 !=
unsigned long long?

johannes


Attachments:
signature.asc (828.00 B)
This is a digitally signed message part

2007-12-20 12:22:28

by Stefano Brivio

[permalink] [raw]
Subject: [PATCH v4 7/8] debugfs: allow access to signed values

debugfs: allow access to signed values

Add debugfs_create_s{8,16,32,64}. For these to work properly, we need to remove
a cast in libfs, change the simple_attr_open prototype and thus fix the users as
well.

Cc: Johannes Berg <[email protected]>
Cc: Mattias Nissler <[email protected]>
To: Greg Kroah-Hartman <[email protected]>
To: Arnd Bergmann <[email protected]>
To: Akinobu Mita <[email protected]>
Signed-off-by: Stefano Brivio <[email protected]>
---

This version addresses last Johannes' concerns.

---
arch/powerpc/platforms/cell/spufs/file.c | 48 ++++----
fs/debugfs/file.c | 186 +++++++++++++++++++++++++++++--
fs/libfs.c | 23 ++-
include/linux/debugfs.h | 38 ++++++
include/linux/fs.h | 25 ++--
lib/fault-inject.c | 11 +
6 files changed, 268 insertions(+), 63 deletions(-)

Index: wireless-2.6/fs/debugfs/file.c
===================================================================
--- wireless-2.6.orig/fs/debugfs/file.c
+++ wireless-2.6/fs/debugfs/file.c
@@ -56,11 +56,11 @@ const struct inode_operations debugfs_li
.follow_link = debugfs_follow_link,
};

-static void debugfs_u8_set(void *data, u64 val)
+static void debugfs_u8_set(void *data, unsigned long long val)
{
*(u8 *)data = val;
}
-static u64 debugfs_u8_get(void *data)
+static unsigned long long debugfs_u8_get(void *data)
{
return *(u8 *)data;
}
@@ -97,11 +97,11 @@ struct dentry *debugfs_create_u8(const c
}
EXPORT_SYMBOL_GPL(debugfs_create_u8);

-static void debugfs_u16_set(void *data, u64 val)
+static void debugfs_u16_set(void *data, unsigned long long val)
{
*(u16 *)data = val;
}
-static u64 debugfs_u16_get(void *data)
+static unsigned long long debugfs_u16_get(void *data)
{
return *(u16 *)data;
}
@@ -138,11 +138,11 @@ struct dentry *debugfs_create_u16(const
}
EXPORT_SYMBOL_GPL(debugfs_create_u16);

-static void debugfs_u32_set(void *data, u64 val)
+static void debugfs_u32_set(void *data, unsigned long long val)
{
*(u32 *)data = val;
}
-static u64 debugfs_u32_get(void *data)
+static unsigned long long debugfs_u32_get(void *data)
{
return *(u32 *)data;
}
@@ -179,12 +179,12 @@ struct dentry *debugfs_create_u32(const
}
EXPORT_SYMBOL_GPL(debugfs_create_u32);

-static void debugfs_u64_set(void *data, u64 val)
+static void debugfs_u64_set(void *data, unsigned long long val)
{
*(u64 *)data = val;
}

-static u64 debugfs_u64_get(void *data)
+static unsigned long long debugfs_u64_get(void *data)
{
return *(u64 *)data;
}
@@ -221,6 +221,172 @@ struct dentry *debugfs_create_u64(const
}
EXPORT_SYMBOL_GPL(debugfs_create_u64);

+
+static void debugfs_s8_set(void *data, unsigned long long val)
+{
+ *(s8 *)data = val;
+}
+static unsigned long long debugfs_s8_get(void *data)
+{
+ return *(s8 *)data;
+}
+DEFINE_SIMPLE_ATTRIBUTE(fops_s8, debugfs_s8_get, debugfs_s8_set, "%lld\n");
+
+/**
+ * debugfs_create_s8 - create a debugfs file that is used to read and write a signed 8-bit 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.
+ *
+ * This function creates a file in debugfs with the given name that
+ * contains the value of the variable @value. If the @mode variable is so
+ * set, it can be read from, and written to.
+ *
+ * This function will return a pointer to a dentry if it succeeds. This
+ * pointer must be passed to the debugfs_remove() function when the file is
+ * to be removed (no automatic cleanup happens if your module is unloaded,
+ * you are responsible here.) If an error occurs, %NULL will be returned.
+ *
+ * If debugfs is not enabled in the kernel, the value -%ENODEV will be
+ * returned. It is not wise to check for this value, but rather, check for
+ * %NULL or !%NULL instead as to eliminate the need for #ifdef in the calling
+ * code.
+ */
+struct dentry *debugfs_create_s8(const char *name, mode_t mode,
+ struct dentry *parent, s8 *value)
+{
+ return debugfs_create_file(name, mode, parent, value, &fops_s8);
+}
+EXPORT_SYMBOL_GPL(debugfs_create_s8);
+
+static void debugfs_s16_set(void *data, unsigned long long val)
+{
+ *(s16 *)data = val;
+}
+static unsigned long long debugfs_s16_get(void *data)
+{
+ return *(s16 *)data;
+}
+DEFINE_SIMPLE_ATTRIBUTE(fops_s16, debugfs_s16_get, debugfs_s16_set, "%lld\n");
+
+/**
+ * debugfs_create_s16 - create a debugfs file that is used to read and write a signed 16-bit 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.
+ *
+ * This function creates a file in debugfs with the given name that
+ * contains the value of the variable @value. If the @mode variable is so
+ * set, it can be read from, and written to.
+ *
+ * This function will return a pointer to a dentry if it succeeds. This
+ * pointer must be passed to the debugfs_remove() function when the file is
+ * to be removed (no automatic cleanup happens if your module is unloaded,
+ * you are responsible here.) If an error occurs, %NULL will be returned.
+ *
+ * If debugfs is not enabled in the kernel, the value -%ENODEV will be
+ * returned. It is not wise to check for this value, but rather, check for
+ * %NULL or !%NULL instead as to eliminate the need for #ifdef in the calling
+ * code.
+ */
+struct dentry *debugfs_create_s16(const char *name, mode_t mode,
+ struct dentry *parent, s16 *value)
+{
+ return debugfs_create_file(name, mode, parent, value, &fops_s16);
+}
+EXPORT_SYMBOL_GPL(debugfs_create_s16);
+
+static void debugfs_s32_set(void *data, unsigned long long val)
+{
+ *(s32 *)data = val;
+}
+static unsigned long long debugfs_s32_get(void *data)
+{
+ return *(s32 *)data;
+}
+DEFINE_SIMPLE_ATTRIBUTE(fops_s32, debugfs_s32_get, debugfs_s32_set, "%lld\n");
+
+/**
+ * debugfs_create_s32 - create a debugfs file that is used to read and write a signed 32-bit 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.
+ *
+ * This function creates a file in debugfs with the given name that
+ * contains the value of the variable @value. If the @mode variable is so
+ * set, it can be read from, and written to.
+ *
+ * This function will return a pointer to a dentry if it succeeds. This
+ * pointer must be passed to the debugfs_remove() function when the file is
+ * to be removed (no automatic cleanup happens if your module is unloaded,
+ * you are responsible here.) If an error occurs, %NULL will be returned.
+ *
+ * If debugfs is not enabled in the kernel, the value -%ENODEV will be
+ * returned. It is not wise to check for this value, but rather, check for
+ * %NULL or !%NULL instead as to eliminate the need for #ifdef in the calling
+ * code.
+ */
+struct dentry *debugfs_create_s32(const char *name, mode_t mode,
+ struct dentry *parent, s32 *value)
+{
+ return debugfs_create_file(name, mode, parent, value, &fops_s32);
+}
+EXPORT_SYMBOL_GPL(debugfs_create_s32);
+
+static void debugfs_s64_set(void *data, unsigned long long val)
+{
+ *(s64 *)data = val;
+}
+
+static unsigned long long debugfs_s64_get(void *data)
+{
+ return *(s64 *)data;
+}
+DEFINE_SIMPLE_ATTRIBUTE(fops_s64, debugfs_s64_get, debugfs_s64_set, "%lld\n");
+
+/**
+ * debugfs_create_s64 - create a debugfs file that is used to read and write a signed 64-bit 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.
+ *
+ * This function creates a file in debugfs with the given name that
+ * contains the value of the variable @value. If the @mode variable is so
+ * set, it can be read from, and written to.
+ *
+ * This function will return a pointer to a dentry if it succeeds. This
+ * pointer must be passed to the debugfs_remove() function when the file is
+ * to be removed (no automatic cleanup happens if your module is unloaded,
+ * you are responsible here.) If an error occurs, %NULL will be returned.
+ *
+ * If debugfs is not enabled in the kernel, the value -%ENODEV will be
+ * returned. It is not wise to check for this value, but rather, check for
+ * %NULL or !%NULL instead as to eliminate the need for #ifdef in the calling
+ * code.
+ */
+struct dentry *debugfs_create_s64(const char *name, mode_t mode,
+ struct dentry *parent, s64 *value)
+{
+ return debugfs_create_file(name, mode, parent, value, &fops_s64);
+}
+EXPORT_SYMBOL_GPL(debugfs_create_s64);
+
DEFINE_SIMPLE_ATTRIBUTE(fops_x8, debugfs_u8_get, debugfs_u8_set, "0x%02llx\n");

DEFINE_SIMPLE_ATTRIBUTE(fops_x16, debugfs_u16_get, debugfs_u16_set, "0x%04llx\n");
@@ -291,7 +457,7 @@ static ssize_t read_file_bool(struct fil
{
char buf[3];
u32 *val = file->private_data;
-
+
if (*val)
buf[0] = 'Y';
else
@@ -324,7 +490,7 @@ static ssize_t write_file_bool(struct fi
*val = 0;
break;
}
-
+
return count;
}

Index: wireless-2.6/fs/libfs.c
===================================================================
--- wireless-2.6.orig/fs/libfs.c
+++ wireless-2.6/fs/libfs.c
@@ -59,7 +59,7 @@ int simple_sync_file(struct file * file,
{
return 0;
}
-
+
int dcache_dir_open(struct inode *inode, struct file *file)
{
static struct qstr cursor_name = {.len = 1, .name = "."};
@@ -160,9 +160,9 @@ int dcache_readdir(struct file * filp, v
continue;

spin_unlock(&dcache_lock);
- if (filldir(dirent, next->d_name.name,
- next->d_name.len, filp->f_pos,
- next->d_inode->i_ino,
+ if (filldir(dirent, next->d_name.name,
+ next->d_name.len, filp->f_pos,
+ next->d_inode->i_ino,
dt_type(next->d_inode)) < 0)
return 0;
spin_lock(&dcache_lock);
@@ -586,10 +586,10 @@ int simple_transaction_release(struct in
/* Simple attribute files */

struct simple_attr {
- u64 (*get)(void *);
- void (*set)(void *, u64);
- char get_buf[24]; /* enough to store a u64 and "\n\0" */
- char set_buf[24];
+ unsigned long long (*get)(void *);
+ void (*set)(void *, unsigned long long);
+ char get_buf[sizeof(unsigned long long) + 2]; /* ULL + "\n\0" */
+ char set_buf[sizeof(unsigned long long) + 2];
void *data;
const char *fmt; /* format for read operation */
struct mutex mutex; /* protects access to these buffers */
@@ -598,7 +598,8 @@ struct simple_attr {
/* simple_attr_open is called by an actual attribute open file operation
* to set the attribute specific access operations. */
int simple_attr_open(struct inode *inode, struct file *file,
- u64 (*get)(void *), void (*set)(void *, u64),
+ unsigned long long (*get)(void *),
+ void (*set)(void *, unsigned long long),
const char *fmt)
{
struct simple_attr *attr;
@@ -643,7 +644,7 @@ ssize_t simple_attr_read(struct file *fi
else /* first read */
size = scnprintf(attr->get_buf, sizeof(attr->get_buf),
attr->fmt,
- (unsigned long long)attr->get(attr->data));
+ attr->get(attr->data));

ret = simple_read_from_buffer(buf, len, ppos, attr->get_buf, size);
mutex_unlock(&attr->mutex);
@@ -655,7 +656,7 @@ ssize_t simple_attr_write(struct file *f
size_t len, loff_t *ppos)
{
struct simple_attr *attr;
- u64 val;
+ unsigned long long val;
size_t size;
ssize_t ret;

Index: wireless-2.6/include/linux/debugfs.h
===================================================================
--- wireless-2.6.orig/include/linux/debugfs.h
+++ wireless-2.6/include/linux/debugfs.h
@@ -49,6 +49,14 @@ struct dentry *debugfs_create_u32(const
struct dentry *parent, u32 *value);
struct dentry *debugfs_create_u64(const char *name, mode_t mode,
struct dentry *parent, u64 *value);
+struct dentry *debugfs_create_s8(const char *name, mode_t mode,
+ struct dentry *parent, s8 *value);
+struct dentry *debugfs_create_s16(const char *name, mode_t mode,
+ struct dentry *parent, s16 *value);
+struct dentry *debugfs_create_s32(const char *name, mode_t mode,
+ struct dentry *parent, s32 *value);
+struct dentry *debugfs_create_s64(const char *name, mode_t mode,
+ struct dentry *parent, s64 *value);
struct dentry *debugfs_create_x8(const char *name, mode_t mode,
struct dentry *parent, u8 *value);
struct dentry *debugfs_create_x16(const char *name, mode_t mode,
@@ -65,7 +73,7 @@ struct dentry *debugfs_create_blob(const

#include <linux/err.h>

-/*
+/*
* We do not return NULL from these functions if CONFIG_DEBUG_FS is not enabled
* so users have a chance to detect if there was a real error or not. We don't
* want to duplicate the design decision mistakes of procfs and devfs again.
@@ -128,6 +136,34 @@ static inline struct dentry *debugfs_cre
return ERR_PTR(-ENODEV);
}

+static inline struct dentry *debugfs_create_s8(const char *name, mode_t mode,
+ struct dentry *parent,
+ u8 *value)
+{
+ return ERR_PTR(-ENODEV);
+}
+
+static inline struct dentry *debugfs_create_s16(const char *name, mode_t mode,
+ struct dentry *parent,
+ s16 *value)
+{
+ return ERR_PTR(-ENODEV);
+}
+
+static inline struct dentry *debugfs_create_s32(const char *name, mode_t mode,
+ struct dentry *parent,
+ s32 *value)
+{
+ return ERR_PTR(-ENODEV);
+}
+
+static inline struct dentry *debugfs_create_s64(const char *name, mode_t mode,
+ struct dentry *parent,
+ s64 *value)
+{
+ return ERR_PTR(-ENODEV);
+}
+
static inline struct dentry *debugfs_create_x8(const char *name, mode_t mode,
struct dentry *parent,
u8 *value)
Index: wireless-2.6/arch/powerpc/platforms/cell/spufs/file.c
===================================================================
--- wireless-2.6.orig/arch/powerpc/platforms/cell/spufs/file.c
+++ wireless-2.6/arch/powerpc/platforms/cell/spufs/file.c
@@ -286,10 +286,10 @@ static int spufs_cntl_mmap(struct file *
#define spufs_cntl_mmap NULL
#endif /* !SPUFS_MMAP_4K */

-static u64 spufs_cntl_get(void *data)
+static unsigned long long spufs_cntl_get(void *data)
{
struct spu_context *ctx = data;
- u64 val;
+ unsigned long long val;

spu_acquire(ctx);
val = ctx->ops->status_read(ctx);
@@ -298,7 +298,7 @@ static u64 spufs_cntl_get(void *data)
return val;
}

-static void spufs_cntl_set(void *data, u64 val)
+static void spufs_cntl_set(void *data, unsigned long long val)
{
struct spu_context *ctx = data;

@@ -1086,10 +1086,10 @@ static const struct file_operations spuf
#define SPU_ATTR_ACQUIRE_SAVED 2

#define DEFINE_SPUFS_ATTRIBUTE(__name, __get, __set, __fmt, __acquire) \
-static u64 __##__get(void *data) \
+static unsigned long long __##__get(void *data) \
{ \
struct spu_context *ctx = data; \
- u64 ret; \
+ unsigned long long ret; \
\
if (__acquire == SPU_ATTR_ACQUIRE) { \
spu_acquire(ctx); \
@@ -1106,7 +1106,7 @@ static u64 __##__get(void *data) \
} \
DEFINE_SIMPLE_ATTRIBUTE(__name, __##__get, __set, __fmt);

-static void spufs_signal1_type_set(void *data, u64 val)
+static void spufs_signal1_type_set(void *data, unsigned long long val)
{
struct spu_context *ctx = data;

@@ -1115,7 +1115,7 @@ static void spufs_signal1_type_set(void
spu_release(ctx);
}

-static u64 spufs_signal1_type_get(struct spu_context *ctx)
+static unsigned long long spufs_signal1_type_get(struct spu_context *ctx)
{
return ctx->ops->signal1_type_get(ctx);
}
@@ -1123,7 +1123,7 @@ DEFINE_SPUFS_ATTRIBUTE(spufs_signal1_typ
spufs_signal1_type_set, "%llu", SPU_ATTR_ACQUIRE);


-static void spufs_signal2_type_set(void *data, u64 val)
+static void spufs_signal2_type_set(void *data, unsigned long long val)
{
struct spu_context *ctx = data;

@@ -1132,7 +1132,7 @@ static void spufs_signal2_type_set(void
spu_release(ctx);
}

-static u64 spufs_signal2_type_get(struct spu_context *ctx)
+static unsigned long long spufs_signal2_type_get(struct spu_context *ctx)
{
return ctx->ops->signal2_type_get(ctx);
}
@@ -1605,7 +1605,7 @@ static const struct file_operations spuf
.mmap = spufs_mfc_mmap,
};

-static void spufs_npc_set(void *data, u64 val)
+static void spufs_npc_set(void *data, unsigned long long val)
{
struct spu_context *ctx = data;
spu_acquire(ctx);
@@ -1613,14 +1613,14 @@ static void spufs_npc_set(void *data, u6
spu_release(ctx);
}

-static u64 spufs_npc_get(struct spu_context *ctx)
+static unsigned long long spufs_npc_get(struct spu_context *ctx)
{
return ctx->ops->npc_read(ctx);
}
DEFINE_SPUFS_ATTRIBUTE(spufs_npc_ops, spufs_npc_get, spufs_npc_set,
"0x%llx\n", SPU_ATTR_ACQUIRE);

-static void spufs_decr_set(void *data, u64 val)
+static void spufs_decr_set(void *data, unsigned long long val)
{
struct spu_context *ctx = data;
struct spu_lscsa *lscsa = ctx->csa.lscsa;
@@ -1629,7 +1629,7 @@ static void spufs_decr_set(void *data, u
spu_release_saved(ctx);
}

-static u64 spufs_decr_get(struct spu_context *ctx)
+static unsigned long long spufs_decr_get(struct spu_context *ctx)
{
struct spu_lscsa *lscsa = ctx->csa.lscsa;
return lscsa->decr.slot[0];
@@ -1637,7 +1637,7 @@ static u64 spufs_decr_get(struct spu_con
DEFINE_SPUFS_ATTRIBUTE(spufs_decr_ops, spufs_decr_get, spufs_decr_set,
"0x%llx\n", SPU_ATTR_ACQUIRE_SAVED);

-static void spufs_decr_status_set(void *data, u64 val)
+static void spufs_decr_status_set(void *data, unsigned long long val)
{
struct spu_context *ctx = data;
spu_acquire_saved(ctx);
@@ -1648,7 +1648,7 @@ static void spufs_decr_status_set(void *
spu_release_saved(ctx);
}

-static u64 spufs_decr_status_get(struct spu_context *ctx)
+static unsigned long long spufs_decr_status_get(struct spu_context *ctx)
{
if (ctx->csa.priv2.mfc_control_RW & MFC_CNTL_DECREMENTER_RUNNING)
return SPU_DECR_STATUS_RUNNING;
@@ -1659,7 +1659,7 @@ DEFINE_SPUFS_ATTRIBUTE(spufs_decr_status
spufs_decr_status_set, "0x%llx\n",
SPU_ATTR_ACQUIRE_SAVED);

-static void spufs_event_mask_set(void *data, u64 val)
+static void spufs_event_mask_set(void *data, unsigned long long val)
{
struct spu_context *ctx = data;
struct spu_lscsa *lscsa = ctx->csa.lscsa;
@@ -1668,7 +1668,7 @@ static void spufs_event_mask_set(void *d
spu_release_saved(ctx);
}

-static u64 spufs_event_mask_get(struct spu_context *ctx)
+static unsigned long long spufs_event_mask_get(struct spu_context *ctx)
{
struct spu_lscsa *lscsa = ctx->csa.lscsa;
return lscsa->event_mask.slot[0];
@@ -1678,7 +1678,7 @@ DEFINE_SPUFS_ATTRIBUTE(spufs_event_mask_
spufs_event_mask_set, "0x%llx\n",
SPU_ATTR_ACQUIRE_SAVED);

-static u64 spufs_event_status_get(struct spu_context *ctx)
+static unsigned long long spufs_event_status_get(struct spu_context *ctx)
{
struct spu_state *state = &ctx->csa;
u64 stat;
@@ -1690,7 +1690,7 @@ static u64 spufs_event_status_get(struct
DEFINE_SPUFS_ATTRIBUTE(spufs_event_status_ops, spufs_event_status_get,
NULL, "0x%llx\n", SPU_ATTR_ACQUIRE_SAVED)

-static void spufs_srr0_set(void *data, u64 val)
+static void spufs_srr0_set(void *data, unsigned long long val)
{
struct spu_context *ctx = data;
struct spu_lscsa *lscsa = ctx->csa.lscsa;
@@ -1699,7 +1699,7 @@ static void spufs_srr0_set(void *data, u
spu_release_saved(ctx);
}

-static u64 spufs_srr0_get(struct spu_context *ctx)
+static unsigned long long spufs_srr0_get(struct spu_context *ctx)
{
struct spu_lscsa *lscsa = ctx->csa.lscsa;
return lscsa->srr0.slot[0];
@@ -1707,7 +1707,7 @@ static u64 spufs_srr0_get(struct spu_con
DEFINE_SPUFS_ATTRIBUTE(spufs_srr0_ops, spufs_srr0_get, spufs_srr0_set,
"0x%llx\n", SPU_ATTR_ACQUIRE_SAVED)

-static u64 spufs_id_get(struct spu_context *ctx)
+static unsigned long long spufs_id_get(struct spu_context *ctx)
{
u64 num;

@@ -1721,13 +1721,13 @@ static u64 spufs_id_get(struct spu_conte
DEFINE_SPUFS_ATTRIBUTE(spufs_id_ops, spufs_id_get, NULL, "0x%llx\n",
SPU_ATTR_ACQUIRE)

-static u64 spufs_object_id_get(struct spu_context *ctx)
+static unsigned long long spufs_object_id_get(struct spu_context *ctx)
{
/* FIXME: Should there really be no locking here? */
return ctx->object_id;
}

-static void spufs_object_id_set(void *data, u64 id)
+static void spufs_object_id_set(void *data, unsigned long long id)
{
struct spu_context *ctx = data;
ctx->object_id = id;
@@ -1736,7 +1736,7 @@ static void spufs_object_id_set(void *da
DEFINE_SPUFS_ATTRIBUTE(spufs_object_id_ops, spufs_object_id_get,
spufs_object_id_set, "0x%llx\n", SPU_ATTR_NOACQUIRE);

-static u64 spufs_lslr_get(struct spu_context *ctx)
+static unsigned long long spufs_lslr_get(struct spu_context *ctx)
{
return ctx->csa.priv2.spu_lslr_RW;
}
Index: wireless-2.6/include/linux/fs.h
===================================================================
--- wireless-2.6.orig/include/linux/fs.h
+++ wireless-2.6/include/linux/fs.h
@@ -15,8 +15,8 @@
* nr_file rlimit, so it's safe to set up a ridiculously high absolute
* upper limit on files-per-process.
*
- * Some programs (notably those using select()) may have to be
- * recompiled to take full advantage of the new limits..
+ * Some programs (notably those using select()) may have to be
+ * recompiled to take full advantage of the new limits..
*/

/* Fixed constants first: */
@@ -90,7 +90,7 @@ extern int dir_notify_enable;
#define SEL_EX 4

/* public flags for file_system_type */
-#define FS_REQUIRES_DEV 1
+#define FS_REQUIRES_DEV 1
#define FS_BINARY_MOUNTDATA 2
#define FS_HAS_SUBTYPE 4
#define FS_REVAL_DOT 16384 /* Check the paths ".", ".." for staleness */
@@ -365,7 +365,7 @@ struct iattr {
*/
#include <linux/quota.h>

-/**
+/**
* enum positive_aop_returns - aop return codes with specific semantics
*
* @AOP_WRITEPAGE_ACTIVATE: Informs the caller that page writeback has
@@ -375,7 +375,7 @@ struct iattr {
* be a candidate for writeback again in the near
* future. Other callers must be careful to unlock
* the page if they get this return. Returned by
- * writepage();
+ * writepage();
*
* @AOP_TRUNCATED_PAGE: The AOP method that was handed a locked page has
* unlocked it and the page might have been truncated.
@@ -818,10 +818,10 @@ extern spinlock_t files_lock;

#define MAX_NON_LFS ((1UL<<31) - 1)

-/* Page cache limit. The filesystems should put that into their s_maxbytes
- limits, otherwise bad things can happen in VM. */
+/* Page cache limit. The filesystems should put that into their s_maxbytes
+ limits, otherwise bad things can happen in VM. */
#if BITS_PER_LONG==32
-#define MAX_LFS_FILESIZE (((u64)PAGE_CACHE_SIZE << (BITS_PER_LONG-1))-1)
+#define MAX_LFS_FILESIZE (((u64)PAGE_CACHE_SIZE << (BITS_PER_LONG-1))-1)
#elif BITS_PER_LONG==64
#define MAX_LFS_FILESIZE 0x7fffffffffffffffUL
#endif
@@ -1240,7 +1240,7 @@ struct super_operations {
void (*destroy_inode)(struct inode *);

void (*read_inode) (struct inode *);
-
+
void (*dirty_inode) (struct inode *);
int (*write_inode) (struct inode *, int);
void (*put_inode) (struct inode *);
@@ -1719,7 +1719,7 @@ extern int may_open(struct nameidata *,

extern int kernel_read(struct file *, unsigned long, char *, unsigned long);
extern struct file * open_exec(const char *);
-
+
/* fs/dcache.c -- generic fs support functions */
extern int is_subdir(struct dentry *, struct dentry *);
extern ino_t find_inode_number(struct dentry *, struct qstr *);
@@ -1753,7 +1753,7 @@ extern void unlock_new_inode(struct inod
static inline struct inode *iget(struct super_block *sb, unsigned long ino)
{
struct inode *inode = iget_locked(sb, ino);
-
+
if (inode && (inode->i_state & I_NEW)) {
sb->s_op->read_inode(inode);
unlock_new_inode(inode);
@@ -2062,7 +2062,8 @@ __simple_attr_check_format(const char *f
}

int simple_attr_open(struct inode *inode, struct file *file,
- u64 (*get)(void *), void (*set)(void *, u64),
+ unsigned long long (*get)(void *),
+ void (*set)(void *, unsigned long long),
const char *fmt);
int simple_attr_close(struct inode *inode, struct file *file);
ssize_t simple_attr_read(struct file *file, char __user *buf,
Index: wireless-2.6/lib/fault-inject.c
===================================================================
--- wireless-2.6.orig/lib/fault-inject.c
+++ wireless-2.6/lib/fault-inject.c
@@ -134,13 +134,14 @@ bool should_fail(struct fault_attr *attr

#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS

-static void debugfs_ul_set(void *data, u64 val)
+static void debugfs_ul_set(void *data, unsigned long long val)
{
*(unsigned long *)data = val;
}

#ifdef CONFIG_FAULT_INJECTION_STACKTRACE_FILTER
-static void debugfs_ul_set_MAX_STACK_TRACE_DEPTH(void *data, u64 val)
+static void debugfs_ul_set_MAX_STACK_TRACE_DEPTH(void *data,
+ unsigned long long val)
{
*(unsigned long *)data =
val < MAX_STACK_TRACE_DEPTH ?
@@ -148,7 +149,7 @@ static void debugfs_ul_set_MAX_STACK_TRA
}
#endif /* CONFIG_FAULT_INJECTION_STACKTRACE_FILTER */

-static u64 debugfs_ul_get(void *data)
+static unsigned long long debugfs_ul_get(void *data)
{
return *(unsigned long *)data;
}
@@ -174,12 +175,12 @@ static struct dentry *debugfs_create_ul_
}
#endif /* CONFIG_FAULT_INJECTION_STACKTRACE_FILTER */

-static void debugfs_atomic_t_set(void *data, u64 val)
+static void debugfs_atomic_t_set(void *data, unsigned long long val)
{
atomic_set((atomic_t *)data, val);
}

-static u64 debugfs_atomic_t_get(void *data)
+static unsigned long long debugfs_atomic_t_get(void *data)
{
return atomic_read((atomic_t *)data);
}



--
Ciao
Stefano

2007-12-20 12:46:56

by Stefano Brivio

[permalink] [raw]
Subject: [PATCH v5 7/8] debugfs: allow access to signed values

debugfs: allow access to signed values

Add debugfs_create_s{8,16,32,64}. For these to work properly, we need to remove
a cast in libfs, change the simple_attr_open prototype and thus fix the users as
well.

Cc: Johannes Berg <[email protected]>
Cc: Mattias Nissler <[email protected]>
To: Greg Kroah-Hartman <[email protected]>
To: Arnd Bergmann <[email protected]>
To: Akinobu Mita <[email protected]>
Signed-off-by: Stefano Brivio <[email protected]>
---

This version addresses last Johannes' concerns. Please just discard v4.

---
arch/powerpc/platforms/cell/spufs/file.c | 48 ++++----
fs/debugfs/file.c | 186 +++++++++++++++++++++++++++++--
fs/libfs.c | 21 +--
include/linux/debugfs.h | 38 ++++++
include/linux/fs.h | 25 ++--
lib/fault-inject.c | 11 +
6 files changed, 267 insertions(+), 62 deletions(-)

Index: wireless-2.6/fs/debugfs/file.c
===================================================================
--- wireless-2.6.orig/fs/debugfs/file.c
+++ wireless-2.6/fs/debugfs/file.c
@@ -56,11 +56,11 @@ const struct inode_operations debugfs_li
.follow_link = debugfs_follow_link,
};

-static void debugfs_u8_set(void *data, u64 val)
+static void debugfs_u8_set(void *data, unsigned long long val)
{
*(u8 *)data = val;
}
-static u64 debugfs_u8_get(void *data)
+static unsigned long long debugfs_u8_get(void *data)
{
return *(u8 *)data;
}
@@ -97,11 +97,11 @@ struct dentry *debugfs_create_u8(const c
}
EXPORT_SYMBOL_GPL(debugfs_create_u8);

-static void debugfs_u16_set(void *data, u64 val)
+static void debugfs_u16_set(void *data, unsigned long long val)
{
*(u16 *)data = val;
}
-static u64 debugfs_u16_get(void *data)
+static unsigned long long debugfs_u16_get(void *data)
{
return *(u16 *)data;
}
@@ -138,11 +138,11 @@ struct dentry *debugfs_create_u16(const
}
EXPORT_SYMBOL_GPL(debugfs_create_u16);

-static void debugfs_u32_set(void *data, u64 val)
+static void debugfs_u32_set(void *data, unsigned long long val)
{
*(u32 *)data = val;
}
-static u64 debugfs_u32_get(void *data)
+static unsigned long long debugfs_u32_get(void *data)
{
return *(u32 *)data;
}
@@ -179,12 +179,12 @@ struct dentry *debugfs_create_u32(const
}
EXPORT_SYMBOL_GPL(debugfs_create_u32);

-static void debugfs_u64_set(void *data, u64 val)
+static void debugfs_u64_set(void *data, unsigned long long val)
{
*(u64 *)data = val;
}

-static u64 debugfs_u64_get(void *data)
+static unsigned long long debugfs_u64_get(void *data)
{
return *(u64 *)data;
}
@@ -221,6 +221,172 @@ struct dentry *debugfs_create_u64(const
}
EXPORT_SYMBOL_GPL(debugfs_create_u64);

+
+static void debugfs_s8_set(void *data, unsigned long long val)
+{
+ *(s8 *)data = val;
+}
+static unsigned long long debugfs_s8_get(void *data)
+{
+ return *(s8 *)data;
+}
+DEFINE_SIMPLE_ATTRIBUTE(fops_s8, debugfs_s8_get, debugfs_s8_set, "%lld\n");
+
+/**
+ * debugfs_create_s8 - create a debugfs file that is used to read and write a signed 8-bit 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.
+ *
+ * This function creates a file in debugfs with the given name that
+ * contains the value of the variable @value. If the @mode variable is so
+ * set, it can be read from, and written to.
+ *
+ * This function will return a pointer to a dentry if it succeeds. This
+ * pointer must be passed to the debugfs_remove() function when the file is
+ * to be removed (no automatic cleanup happens if your module is unloaded,
+ * you are responsible here.) If an error occurs, %NULL will be returned.
+ *
+ * If debugfs is not enabled in the kernel, the value -%ENODEV will be
+ * returned. It is not wise to check for this value, but rather, check for
+ * %NULL or !%NULL instead as to eliminate the need for #ifdef in the calling
+ * code.
+ */
+struct dentry *debugfs_create_s8(const char *name, mode_t mode,
+ struct dentry *parent, s8 *value)
+{
+ return debugfs_create_file(name, mode, parent, value, &fops_s8);
+}
+EXPORT_SYMBOL_GPL(debugfs_create_s8);
+
+static void debugfs_s16_set(void *data, unsigned long long val)
+{
+ *(s16 *)data = val;
+}
+static unsigned long long debugfs_s16_get(void *data)
+{
+ return *(s16 *)data;
+}
+DEFINE_SIMPLE_ATTRIBUTE(fops_s16, debugfs_s16_get, debugfs_s16_set, "%lld\n");
+
+/**
+ * debugfs_create_s16 - create a debugfs file that is used to read and write a signed 16-bit 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.
+ *
+ * This function creates a file in debugfs with the given name that
+ * contains the value of the variable @value. If the @mode variable is so
+ * set, it can be read from, and written to.
+ *
+ * This function will return a pointer to a dentry if it succeeds. This
+ * pointer must be passed to the debugfs_remove() function when the file is
+ * to be removed (no automatic cleanup happens if your module is unloaded,
+ * you are responsible here.) If an error occurs, %NULL will be returned.
+ *
+ * If debugfs is not enabled in the kernel, the value -%ENODEV will be
+ * returned. It is not wise to check for this value, but rather, check for
+ * %NULL or !%NULL instead as to eliminate the need for #ifdef in the calling
+ * code.
+ */
+struct dentry *debugfs_create_s16(const char *name, mode_t mode,
+ struct dentry *parent, s16 *value)
+{
+ return debugfs_create_file(name, mode, parent, value, &fops_s16);
+}
+EXPORT_SYMBOL_GPL(debugfs_create_s16);
+
+static void debugfs_s32_set(void *data, unsigned long long val)
+{
+ *(s32 *)data = val;
+}
+static unsigned long long debugfs_s32_get(void *data)
+{
+ return *(s32 *)data;
+}
+DEFINE_SIMPLE_ATTRIBUTE(fops_s32, debugfs_s32_get, debugfs_s32_set, "%lld\n");
+
+/**
+ * debugfs_create_s32 - create a debugfs file that is used to read and write a signed 32-bit 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.
+ *
+ * This function creates a file in debugfs with the given name that
+ * contains the value of the variable @value. If the @mode variable is so
+ * set, it can be read from, and written to.
+ *
+ * This function will return a pointer to a dentry if it succeeds. This
+ * pointer must be passed to the debugfs_remove() function when the file is
+ * to be removed (no automatic cleanup happens if your module is unloaded,
+ * you are responsible here.) If an error occurs, %NULL will be returned.
+ *
+ * If debugfs is not enabled in the kernel, the value -%ENODEV will be
+ * returned. It is not wise to check for this value, but rather, check for
+ * %NULL or !%NULL instead as to eliminate the need for #ifdef in the calling
+ * code.
+ */
+struct dentry *debugfs_create_s32(const char *name, mode_t mode,
+ struct dentry *parent, s32 *value)
+{
+ return debugfs_create_file(name, mode, parent, value, &fops_s32);
+}
+EXPORT_SYMBOL_GPL(debugfs_create_s32);
+
+static void debugfs_s64_set(void *data, unsigned long long val)
+{
+ *(s64 *)data = val;
+}
+
+static unsigned long long debugfs_s64_get(void *data)
+{
+ return *(s64 *)data;
+}
+DEFINE_SIMPLE_ATTRIBUTE(fops_s64, debugfs_s64_get, debugfs_s64_set, "%lld\n");
+
+/**
+ * debugfs_create_s64 - create a debugfs file that is used to read and write a signed 64-bit 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.
+ *
+ * This function creates a file in debugfs with the given name that
+ * contains the value of the variable @value. If the @mode variable is so
+ * set, it can be read from, and written to.
+ *
+ * This function will return a pointer to a dentry if it succeeds. This
+ * pointer must be passed to the debugfs_remove() function when the file is
+ * to be removed (no automatic cleanup happens if your module is unloaded,
+ * you are responsible here.) If an error occurs, %NULL will be returned.
+ *
+ * If debugfs is not enabled in the kernel, the value -%ENODEV will be
+ * returned. It is not wise to check for this value, but rather, check for
+ * %NULL or !%NULL instead as to eliminate the need for #ifdef in the calling
+ * code.
+ */
+struct dentry *debugfs_create_s64(const char *name, mode_t mode,
+ struct dentry *parent, s64 *value)
+{
+ return debugfs_create_file(name, mode, parent, value, &fops_s64);
+}
+EXPORT_SYMBOL_GPL(debugfs_create_s64);
+
DEFINE_SIMPLE_ATTRIBUTE(fops_x8, debugfs_u8_get, debugfs_u8_set, "0x%02llx\n");

DEFINE_SIMPLE_ATTRIBUTE(fops_x16, debugfs_u16_get, debugfs_u16_set, "0x%04llx\n");
@@ -291,7 +457,7 @@ static ssize_t read_file_bool(struct fil
{
char buf[3];
u32 *val = file->private_data;
-
+
if (*val)
buf[0] = 'Y';
else
@@ -324,7 +490,7 @@ static ssize_t write_file_bool(struct fi
*val = 0;
break;
}
-
+
return count;
}

Index: wireless-2.6/fs/libfs.c
===================================================================
--- wireless-2.6.orig/fs/libfs.c
+++ wireless-2.6/fs/libfs.c
@@ -59,7 +59,7 @@ int simple_sync_file(struct file * file,
{
return 0;
}
-
+
int dcache_dir_open(struct inode *inode, struct file *file)
{
static struct qstr cursor_name = {.len = 1, .name = "."};
@@ -160,9 +160,9 @@ int dcache_readdir(struct file * filp, v
continue;

spin_unlock(&dcache_lock);
- if (filldir(dirent, next->d_name.name,
- next->d_name.len, filp->f_pos,
- next->d_inode->i_ino,
+ if (filldir(dirent, next->d_name.name,
+ next->d_name.len, filp->f_pos,
+ next->d_inode->i_ino,
dt_type(next->d_inode)) < 0)
return 0;
spin_lock(&dcache_lock);
@@ -586,9 +586,9 @@ int simple_transaction_release(struct in
/* Simple attribute files */

struct simple_attr {
- u64 (*get)(void *);
- void (*set)(void *, u64);
- char get_buf[24]; /* enough to store a u64 and "\n\0" */
+ unsigned long long (*get)(void *);
+ void (*set)(void *, unsigned long long);
+ char get_buf[24]; /* enough to store an ULL and "\n\0" */
char set_buf[24];
void *data;
const char *fmt; /* format for read operation */
@@ -598,7 +598,8 @@ struct simple_attr {
/* simple_attr_open is called by an actual attribute open file operation
* to set the attribute specific access operations. */
int simple_attr_open(struct inode *inode, struct file *file,
- u64 (*get)(void *), void (*set)(void *, u64),
+ unsigned long long (*get)(void *),
+ void (*set)(void *, unsigned long long),
const char *fmt)
{
struct simple_attr *attr;
@@ -643,7 +644,7 @@ ssize_t simple_attr_read(struct file *fi
else /* first read */
size = scnprintf(attr->get_buf, sizeof(attr->get_buf),
attr->fmt,
- (unsigned long long)attr->get(attr->data));
+ attr->get(attr->data));

ret = simple_read_from_buffer(buf, len, ppos, attr->get_buf, size);
mutex_unlock(&attr->mutex);
@@ -655,7 +656,7 @@ ssize_t simple_attr_write(struct file *f
size_t len, loff_t *ppos)
{
struct simple_attr *attr;
- u64 val;
+ unsigned long long val;
size_t size;
ssize_t ret;

Index: wireless-2.6/include/linux/debugfs.h
===================================================================
--- wireless-2.6.orig/include/linux/debugfs.h
+++ wireless-2.6/include/linux/debugfs.h
@@ -49,6 +49,14 @@ struct dentry *debugfs_create_u32(const
struct dentry *parent, u32 *value);
struct dentry *debugfs_create_u64(const char *name, mode_t mode,
struct dentry *parent, u64 *value);
+struct dentry *debugfs_create_s8(const char *name, mode_t mode,
+ struct dentry *parent, s8 *value);
+struct dentry *debugfs_create_s16(const char *name, mode_t mode,
+ struct dentry *parent, s16 *value);
+struct dentry *debugfs_create_s32(const char *name, mode_t mode,
+ struct dentry *parent, s32 *value);
+struct dentry *debugfs_create_s64(const char *name, mode_t mode,
+ struct dentry *parent, s64 *value);
struct dentry *debugfs_create_x8(const char *name, mode_t mode,
struct dentry *parent, u8 *value);
struct dentry *debugfs_create_x16(const char *name, mode_t mode,
@@ -65,7 +73,7 @@ struct dentry *debugfs_create_blob(const

#include <linux/err.h>

-/*
+/*
* We do not return NULL from these functions if CONFIG_DEBUG_FS is not enabled
* so users have a chance to detect if there was a real error or not. We don't
* want to duplicate the design decision mistakes of procfs and devfs again.
@@ -128,6 +136,34 @@ static inline struct dentry *debugfs_cre
return ERR_PTR(-ENODEV);
}

+static inline struct dentry *debugfs_create_s8(const char *name, mode_t mode,
+ struct dentry *parent,
+ u8 *value)
+{
+ return ERR_PTR(-ENODEV);
+}
+
+static inline struct dentry *debugfs_create_s16(const char *name, mode_t mode,
+ struct dentry *parent,
+ s16 *value)
+{
+ return ERR_PTR(-ENODEV);
+}
+
+static inline struct dentry *debugfs_create_s32(const char *name, mode_t mode,
+ struct dentry *parent,
+ s32 *value)
+{
+ return ERR_PTR(-ENODEV);
+}
+
+static inline struct dentry *debugfs_create_s64(const char *name, mode_t mode,
+ struct dentry *parent,
+ s64 *value)
+{
+ return ERR_PTR(-ENODEV);
+}
+
static inline struct dentry *debugfs_create_x8(const char *name, mode_t mode,
struct dentry *parent,
u8 *value)
Index: wireless-2.6/arch/powerpc/platforms/cell/spufs/file.c
===================================================================
--- wireless-2.6.orig/arch/powerpc/platforms/cell/spufs/file.c
+++ wireless-2.6/arch/powerpc/platforms/cell/spufs/file.c
@@ -286,10 +286,10 @@ static int spufs_cntl_mmap(struct file *
#define spufs_cntl_mmap NULL
#endif /* !SPUFS_MMAP_4K */

-static u64 spufs_cntl_get(void *data)
+static unsigned long long spufs_cntl_get(void *data)
{
struct spu_context *ctx = data;
- u64 val;
+ unsigned long long val;

spu_acquire(ctx);
val = ctx->ops->status_read(ctx);
@@ -298,7 +298,7 @@ static u64 spufs_cntl_get(void *data)
return val;
}

-static void spufs_cntl_set(void *data, u64 val)
+static void spufs_cntl_set(void *data, unsigned long long val)
{
struct spu_context *ctx = data;

@@ -1086,10 +1086,10 @@ static const struct file_operations spuf
#define SPU_ATTR_ACQUIRE_SAVED 2

#define DEFINE_SPUFS_ATTRIBUTE(__name, __get, __set, __fmt, __acquire) \
-static u64 __##__get(void *data) \
+static unsigned long long __##__get(void *data) \
{ \
struct spu_context *ctx = data; \
- u64 ret; \
+ unsigned long long ret; \
\
if (__acquire == SPU_ATTR_ACQUIRE) { \
spu_acquire(ctx); \
@@ -1106,7 +1106,7 @@ static u64 __##__get(void *data) \
} \
DEFINE_SIMPLE_ATTRIBUTE(__name, __##__get, __set, __fmt);

-static void spufs_signal1_type_set(void *data, u64 val)
+static void spufs_signal1_type_set(void *data, unsigned long long val)
{
struct spu_context *ctx = data;

@@ -1115,7 +1115,7 @@ static void spufs_signal1_type_set(void
spu_release(ctx);
}

-static u64 spufs_signal1_type_get(struct spu_context *ctx)
+static unsigned long long spufs_signal1_type_get(struct spu_context *ctx)
{
return ctx->ops->signal1_type_get(ctx);
}
@@ -1123,7 +1123,7 @@ DEFINE_SPUFS_ATTRIBUTE(spufs_signal1_typ
spufs_signal1_type_set, "%llu", SPU_ATTR_ACQUIRE);


-static void spufs_signal2_type_set(void *data, u64 val)
+static void spufs_signal2_type_set(void *data, unsigned long long val)
{
struct spu_context *ctx = data;

@@ -1132,7 +1132,7 @@ static void spufs_signal2_type_set(void
spu_release(ctx);
}

-static u64 spufs_signal2_type_get(struct spu_context *ctx)
+static unsigned long long spufs_signal2_type_get(struct spu_context *ctx)
{
return ctx->ops->signal2_type_get(ctx);
}
@@ -1605,7 +1605,7 @@ static const struct file_operations spuf
.mmap = spufs_mfc_mmap,
};

-static void spufs_npc_set(void *data, u64 val)
+static void spufs_npc_set(void *data, unsigned long long val)
{
struct spu_context *ctx = data;
spu_acquire(ctx);
@@ -1613,14 +1613,14 @@ static void spufs_npc_set(void *data, u6
spu_release(ctx);
}

-static u64 spufs_npc_get(struct spu_context *ctx)
+static unsigned long long spufs_npc_get(struct spu_context *ctx)
{
return ctx->ops->npc_read(ctx);
}
DEFINE_SPUFS_ATTRIBUTE(spufs_npc_ops, spufs_npc_get, spufs_npc_set,
"0x%llx\n", SPU_ATTR_ACQUIRE);

-static void spufs_decr_set(void *data, u64 val)
+static void spufs_decr_set(void *data, unsigned long long val)
{
struct spu_context *ctx = data;
struct spu_lscsa *lscsa = ctx->csa.lscsa;
@@ -1629,7 +1629,7 @@ static void spufs_decr_set(void *data, u
spu_release_saved(ctx);
}

-static u64 spufs_decr_get(struct spu_context *ctx)
+static unsigned long long spufs_decr_get(struct spu_context *ctx)
{
struct spu_lscsa *lscsa = ctx->csa.lscsa;
return lscsa->decr.slot[0];
@@ -1637,7 +1637,7 @@ static u64 spufs_decr_get(struct spu_con
DEFINE_SPUFS_ATTRIBUTE(spufs_decr_ops, spufs_decr_get, spufs_decr_set,
"0x%llx\n", SPU_ATTR_ACQUIRE_SAVED);

-static void spufs_decr_status_set(void *data, u64 val)
+static void spufs_decr_status_set(void *data, unsigned long long val)
{
struct spu_context *ctx = data;
spu_acquire_saved(ctx);
@@ -1648,7 +1648,7 @@ static void spufs_decr_status_set(void *
spu_release_saved(ctx);
}

-static u64 spufs_decr_status_get(struct spu_context *ctx)
+static unsigned long long spufs_decr_status_get(struct spu_context *ctx)
{
if (ctx->csa.priv2.mfc_control_RW & MFC_CNTL_DECREMENTER_RUNNING)
return SPU_DECR_STATUS_RUNNING;
@@ -1659,7 +1659,7 @@ DEFINE_SPUFS_ATTRIBUTE(spufs_decr_status
spufs_decr_status_set, "0x%llx\n",
SPU_ATTR_ACQUIRE_SAVED);

-static void spufs_event_mask_set(void *data, u64 val)
+static void spufs_event_mask_set(void *data, unsigned long long val)
{
struct spu_context *ctx = data;
struct spu_lscsa *lscsa = ctx->csa.lscsa;
@@ -1668,7 +1668,7 @@ static void spufs_event_mask_set(void *d
spu_release_saved(ctx);
}

-static u64 spufs_event_mask_get(struct spu_context *ctx)
+static unsigned long long spufs_event_mask_get(struct spu_context *ctx)
{
struct spu_lscsa *lscsa = ctx->csa.lscsa;
return lscsa->event_mask.slot[0];
@@ -1678,7 +1678,7 @@ DEFINE_SPUFS_ATTRIBUTE(spufs_event_mask_
spufs_event_mask_set, "0x%llx\n",
SPU_ATTR_ACQUIRE_SAVED);

-static u64 spufs_event_status_get(struct spu_context *ctx)
+static unsigned long long spufs_event_status_get(struct spu_context *ctx)
{
struct spu_state *state = &ctx->csa;
u64 stat;
@@ -1690,7 +1690,7 @@ static u64 spufs_event_status_get(struct
DEFINE_SPUFS_ATTRIBUTE(spufs_event_status_ops, spufs_event_status_get,
NULL, "0x%llx\n", SPU_ATTR_ACQUIRE_SAVED)

-static void spufs_srr0_set(void *data, u64 val)
+static void spufs_srr0_set(void *data, unsigned long long val)
{
struct spu_context *ctx = data;
struct spu_lscsa *lscsa = ctx->csa.lscsa;
@@ -1699,7 +1699,7 @@ static void spufs_srr0_set(void *data, u
spu_release_saved(ctx);
}

-static u64 spufs_srr0_get(struct spu_context *ctx)
+static unsigned long long spufs_srr0_get(struct spu_context *ctx)
{
struct spu_lscsa *lscsa = ctx->csa.lscsa;
return lscsa->srr0.slot[0];
@@ -1707,7 +1707,7 @@ static u64 spufs_srr0_get(struct spu_con
DEFINE_SPUFS_ATTRIBUTE(spufs_srr0_ops, spufs_srr0_get, spufs_srr0_set,
"0x%llx\n", SPU_ATTR_ACQUIRE_SAVED)

-static u64 spufs_id_get(struct spu_context *ctx)
+static unsigned long long spufs_id_get(struct spu_context *ctx)
{
u64 num;

@@ -1721,13 +1721,13 @@ static u64 spufs_id_get(struct spu_conte
DEFINE_SPUFS_ATTRIBUTE(spufs_id_ops, spufs_id_get, NULL, "0x%llx\n",
SPU_ATTR_ACQUIRE)

-static u64 spufs_object_id_get(struct spu_context *ctx)
+static unsigned long long spufs_object_id_get(struct spu_context *ctx)
{
/* FIXME: Should there really be no locking here? */
return ctx->object_id;
}

-static void spufs_object_id_set(void *data, u64 id)
+static void spufs_object_id_set(void *data, unsigned long long id)
{
struct spu_context *ctx = data;
ctx->object_id = id;
@@ -1736,7 +1736,7 @@ static void spufs_object_id_set(void *da
DEFINE_SPUFS_ATTRIBUTE(spufs_object_id_ops, spufs_object_id_get,
spufs_object_id_set, "0x%llx\n", SPU_ATTR_NOACQUIRE);

-static u64 spufs_lslr_get(struct spu_context *ctx)
+static unsigned long long spufs_lslr_get(struct spu_context *ctx)
{
return ctx->csa.priv2.spu_lslr_RW;
}
Index: wireless-2.6/include/linux/fs.h
===================================================================
--- wireless-2.6.orig/include/linux/fs.h
+++ wireless-2.6/include/linux/fs.h
@@ -15,8 +15,8 @@
* nr_file rlimit, so it's safe to set up a ridiculously high absolute
* upper limit on files-per-process.
*
- * Some programs (notably those using select()) may have to be
- * recompiled to take full advantage of the new limits..
+ * Some programs (notably those using select()) may have to be
+ * recompiled to take full advantage of the new limits..
*/

/* Fixed constants first: */
@@ -90,7 +90,7 @@ extern int dir_notify_enable;
#define SEL_EX 4

/* public flags for file_system_type */
-#define FS_REQUIRES_DEV 1
+#define FS_REQUIRES_DEV 1
#define FS_BINARY_MOUNTDATA 2
#define FS_HAS_SUBTYPE 4
#define FS_REVAL_DOT 16384 /* Check the paths ".", ".." for staleness */
@@ -365,7 +365,7 @@ struct iattr {
*/
#include <linux/quota.h>

-/**
+/**
* enum positive_aop_returns - aop return codes with specific semantics
*
* @AOP_WRITEPAGE_ACTIVATE: Informs the caller that page writeback has
@@ -375,7 +375,7 @@ struct iattr {
* be a candidate for writeback again in the near
* future. Other callers must be careful to unlock
* the page if they get this return. Returned by
- * writepage();
+ * writepage();
*
* @AOP_TRUNCATED_PAGE: The AOP method that was handed a locked page has
* unlocked it and the page might have been truncated.
@@ -818,10 +818,10 @@ extern spinlock_t files_lock;

#define MAX_NON_LFS ((1UL<<31) - 1)

-/* Page cache limit. The filesystems should put that into their s_maxbytes
- limits, otherwise bad things can happen in VM. */
+/* Page cache limit. The filesystems should put that into their s_maxbytes
+ limits, otherwise bad things can happen in VM. */
#if BITS_PER_LONG==32
-#define MAX_LFS_FILESIZE (((u64)PAGE_CACHE_SIZE << (BITS_PER_LONG-1))-1)
+#define MAX_LFS_FILESIZE (((u64)PAGE_CACHE_SIZE << (BITS_PER_LONG-1))-1)
#elif BITS_PER_LONG==64
#define MAX_LFS_FILESIZE 0x7fffffffffffffffUL
#endif
@@ -1240,7 +1240,7 @@ struct super_operations {
void (*destroy_inode)(struct inode *);

void (*read_inode) (struct inode *);
-
+
void (*dirty_inode) (struct inode *);
int (*write_inode) (struct inode *, int);
void (*put_inode) (struct inode *);
@@ -1719,7 +1719,7 @@ extern int may_open(struct nameidata *,

extern int kernel_read(struct file *, unsigned long, char *, unsigned long);
extern struct file * open_exec(const char *);
-
+
/* fs/dcache.c -- generic fs support functions */
extern int is_subdir(struct dentry *, struct dentry *);
extern ino_t find_inode_number(struct dentry *, struct qstr *);
@@ -1753,7 +1753,7 @@ extern void unlock_new_inode(struct inod
static inline struct inode *iget(struct super_block *sb, unsigned long ino)
{
struct inode *inode = iget_locked(sb, ino);
-
+
if (inode && (inode->i_state & I_NEW)) {
sb->s_op->read_inode(inode);
unlock_new_inode(inode);
@@ -2062,7 +2062,8 @@ __simple_attr_check_format(const char *f
}

int simple_attr_open(struct inode *inode, struct file *file,
- u64 (*get)(void *), void (*set)(void *, u64),
+ unsigned long long (*get)(void *),
+ void (*set)(void *, unsigned long long),
const char *fmt);
int simple_attr_close(struct inode *inode, struct file *file);
ssize_t simple_attr_read(struct file *file, char __user *buf,
Index: wireless-2.6/lib/fault-inject.c
===================================================================
--- wireless-2.6.orig/lib/fault-inject.c
+++ wireless-2.6/lib/fault-inject.c
@@ -134,13 +134,14 @@ bool should_fail(struct fault_attr *attr

#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS

-static void debugfs_ul_set(void *data, u64 val)
+static void debugfs_ul_set(void *data, unsigned long long val)
{
*(unsigned long *)data = val;
}

#ifdef CONFIG_FAULT_INJECTION_STACKTRACE_FILTER
-static void debugfs_ul_set_MAX_STACK_TRACE_DEPTH(void *data, u64 val)
+static void debugfs_ul_set_MAX_STACK_TRACE_DEPTH(void *data,
+ unsigned long long val)
{
*(unsigned long *)data =
val < MAX_STACK_TRACE_DEPTH ?
@@ -148,7 +149,7 @@ static void debugfs_ul_set_MAX_STACK_TRA
}
#endif /* CONFIG_FAULT_INJECTION_STACKTRACE_FILTER */

-static u64 debugfs_ul_get(void *data)
+static unsigned long long debugfs_ul_get(void *data)
{
return *(unsigned long *)data;
}
@@ -174,12 +175,12 @@ static struct dentry *debugfs_create_ul_
}
#endif /* CONFIG_FAULT_INJECTION_STACKTRACE_FILTER */

-static void debugfs_atomic_t_set(void *data, u64 val)
+static void debugfs_atomic_t_set(void *data, unsigned long long val)
{
atomic_set((atomic_t *)data, val);
}

-static u64 debugfs_atomic_t_get(void *data)
+static unsigned long long debugfs_atomic_t_get(void *data)
{
return atomic_read((atomic_t *)data);
}



--
Ciao
Stefano

2007-12-20 12:47:31

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v4 7/8] debugfs: allow access to signed values

On Thursday 20 December 2007, Stefano Brivio wrote:
> debugfs: allow access to signed values
>
> Add debugfs_create_s{8,16,32,64}. For these to work properly, we need to remove
> a cast in libfs, change the simple_attr_open prototype and thus fix the users as
> well.
>
> Cc: Johannes Berg <[email protected]>
> Cc: Mattias Nissler <[email protected]>
> To: Greg Kroah-Hartman <[email protected]>
> To: Arnd Bergmann <[email protected]>
> To: Akinobu Mita <[email protected]>
> Signed-off-by: Stefano Brivio <[email protected]>

Have you checked that spufs still builds? I would guess that you need
to do the same interface changes there.

Also, Christoph has recently posted a suggestion for how to improve
the interface to allow the 'get' operation to return an error:
http://patchwork.ozlabs.org/cbe-oss-dev/patch?id=14962

I'd suggest consolidating the two changes in order to avoid merge
conflicts.

Arnd <><

2007-12-20 15:00:30

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH v5 7/8] debugfs: allow access to signed values

On Thu, Dec 20, 2007 at 01:40:22PM +0100, Stefano Brivio wrote:
> debugfs: allow access to signed values
>
> Add debugfs_create_s{8,16,32,64}. For these to work properly, we need to remove
> a cast in libfs, change the simple_attr_open prototype and thus fix the users as
> well.

Looks like Stefano is going to cooperate w/ hch on this, so I'm
dropping this from wireless-2.6 for now...FYI

John
--
John W. Linville
[email protected]

2007-12-28 18:59:57

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH v4 7/8] debugfs: allow access to signed values

On Thu, Dec 20, 2007 at 01:47:13PM +0100, Arnd Bergmann wrote:
> Also, Christoph has recently posted a suggestion for how to improve
> the interface to allow the 'get' operation to return an error:
> http://patchwork.ozlabs.org/cbe-oss-dev/patch?id=14962
>
> I'd suggest consolidating the two changes in order to avoid merge
> conflicts.

Stefano, I couldn't find your complete patch series. Where are the
other 7 patches for that series? Anyway, I'll post my series with
the simple attribute chanegs ASAP and I'll Cc you. If you send me the
latest version of your patches I'll port it ontop of my patches.

2007-12-28 19:15:41

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v4 7/8] debugfs: allow access to signed values


On Fri, 2007-12-28 at 19:58 +0100, Christoph Hellwig wrote:
> On Thu, Dec 20, 2007 at 01:47:13PM +0100, Arnd Bergmann wrote:
> > Also, Christoph has recently posted a suggestion for how to improve
> > the interface to allow the 'get' operation to return an error:
> > http://patchwork.ozlabs.org/cbe-oss-dev/patch?id=14962
> >
> > I'd suggest consolidating the two changes in order to avoid merge
> > conflicts.
>
> Stefano, I couldn't find your complete patch series. Where are the
> other 7 patches for that series? Anyway, I'll post my series with
> the simple attribute chanegs ASAP and I'll Cc you. If you send me the
> latest version of your patches I'll port it ontop of my patches.

There was only one, the seven other patches are plain wireless patches.

johannes


Attachments:
signature.asc (828.00 B)
This is a digitally signed message part

2007-12-29 18:20:22

by Stefano Brivio

[permalink] [raw]
Subject: Re: [PATCH v4 7/8] debugfs: allow access to signed values

On Fri, 28 Dec 2007 19:58:32 +0100
Christoph Hellwig <[email protected]> wrote:

> On Thu, Dec 20, 2007 at 01:47:13PM +0100, Arnd Bergmann wrote:
> > Also, Christoph has recently posted a suggestion for how to improve
> > the interface to allow the 'get' operation to return an error:
> > http://patchwork.ozlabs.org/cbe-oss-dev/patch?id=14962
> >
> > I'd suggest consolidating the two changes in order to avoid merge
> > conflicts.
>
> Stefano, I couldn't find your complete patch series. Where are the
> other 7 patches for that series? Anyway, I'll post my series with
> the simple attribute chanegs ASAP and I'll Cc you. If you send me the
> latest version of your patches I'll port it ontop of my patches.

Sorry for the late reply. As Johannes said, the other patches have nothing
to do with this anyway. I had to put this patch within my patchset because I
originally wanted to send a patch which would export signed values to
debugfs, but then I solved the issue in a different fashion, and I'm not
that interested in it at the moment.

BTW, the patch I sent was just meant to allow exporting signed values
through debugfs, but in order to do that I had to change some users as well.
And being spufs an user, I don't feel that much like sending a patch
without testing on ppc64 in advance.


--
Ciao
Stefano