debugfs: Revamp debugfs_create_{u,x,s}{8,16,32,64} to support signed integers
PATCH 1/1 debugfs: Revamp debugfs_create_{u,x,s}{8,16,32,64} to support signed integers
This makes debugfs use its own file_operations for the value accessor files created by debugfs_create_XXX. Having that, we can also have proper versions for signed integers.
---
When writing some debugfs code for mac80211 I wanted to have access to some s32 variables. I thought it's better to make debugfs support signed integers instead of adding the functionality locally. I assume generic versions will be useful for other people as well.
Please CC me for replies.
fs/debugfs/file.c | 291 +++++++++++++++++++++++++++++++++++++++-------- include/linux/debugfs.h | 45 +++++++ 2 files changed, 287 insertions(+), 49 deletions(-)
fs/debugfs/file.c +242 -49
@@ -56,18 +56,121 @@ const struct inode_operations debugfs_link_operations = { .follow_link = debugfs_follow_link, }; -static void debugfs_u8_set (void *data,u64 val) +/* Simple numeric attributes */ +struct debugfs_num_attr { - *(u8 *)data = val; +void (*get)(void *,char *); +void (*set)(void *,char *); +char get_buf[24 ]; +char set_buf[24 ]; +void *pnum; +struct mutex mutex; +}; + +static int debugfs_num_attr_open (struct inode *inode,struct file *file, +void (*get)(void *,char *), +void (*set)(void *,char *)) +{ +struct debugfs_num_attr *attr; + + attr =kmalloc (sizeof (*attr),GFP_KERNEL ); +if (!attr) +return -ENOMEM ; + + attr->get = get; + attr->set = set; + attr->pnum = inode->i_private; +mutex_init (&attr->mutex); + + file->private_data = attr; + +return nonseekable_open (inode, file); +} + +static int debugfs_num_attr_close (struct inode *inode,struct file *file) +{ +kfree (file->private_data); + +return 0 ; +} + +static ssize_t debugfs_num_attr_read (struct file *file,char __user *buf, +size_t len,loff_t *ppos) +{ +struct debugfs_num_attr *attr; +size_t size; +ssize_t ret; + + attr = file->private_data; + +mutex_lock (&attr->mutex); +if (!*ppos) { +/* first read */ + attr->get (attr->pnum, attr->get_buf); + attr->get_buf[sizeof (attr->get_buf) -1 ] ='\0' ; + } + + size =strlen (attr->get_buf); + ret =simple_read_from_buffer (buf, len, ppos, attr->get_buf, size); +mutex_unlock (&attr->mutex); + +return ret; } -static u64 debugfs_u8_get (void *data) + +static ssize_t debugfs_num_attr_write (struct file *file,const char __user *buf, +size_t len,loff_t *ppos) { -return *(u8 *)data; +struct debugfs_num_attr *attr; +size_t size; +ssize_t ret; + + attr = file->private_data; + +mutex_lock (&attr->mutex); + ret = -EFAULT ; + size =min (sizeof (attr->set_buf) -1 , len); +if (copy_from_user (attr->set_buf, buf, size)) + goto out; + + ret = len;/* claim we got the whole input */ + attr->set_buf[size] ='\0' ; + attr->set (attr->pnum, attr->set_buf); +out: +mutex_unlock (&attr->mutex); +return ret; } -DEFINE_SIMPLE_ATTRIBUTE (fops_u8, debugfs_u8_get, debugfs_u8_set,"%llu\n" ); + +#define DEFINE_NUM_ATTR (__fops, __type, __format) \ +static void __fops ## _get(void *data, char *buf) \ +{ \ + scnprintf(buf, 24, __format, *(__type *) data); \ +} \ +static void __fops ## _set(void *data, char *buf) \ +{ \ + sscanf(buf, __format, (__type *) data); \ +} \ +static int __fops ## _open(struct inode *inode, struct file *file) \ +{ \ + return debugfs_num_attr_open(inode, file, __fops ## _get, \ + __fops ## _set); \ +} \ +static struct file_operations __fops = { \ + .owner = THIS_MODULE, \ + .open = __fops ## _open, \ + .release = debugfs_num_attr_close, \ + .read = debugfs_num_attr_read, \ + .write = debugfs_num_attr_write, \ +}; + +DEFINE_NUM_ATTR(fops_u8, u8, "%hhu\n") +DEFINE_NUM_ATTR(fops_u16, u16, "%hu\n") +DEFINE_NUM_ATTR(fops_u32, u32, "%u\n") +DEFINE_NUM_ATTR (fops_u64, u64,"%llu\n" )/** -* debugfs_create_u8 - create a debugfs file that is used to read and write an unsigned 8-bit value +* debugfs_create_u8 - create a debugfs file that is used to read and write an +* unsigned 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
@@ -97,18 +200,10 @@ struct dentry *debugfs_create_u8(const char *name, mode_t mode, }EXPORT_SYMBOL_GPL (debugfs_create_u8); -static void debugfs_u16_set (void *data,u64 val) -{ - *(u16 *)data = val; -} -static u64 debugfs_u16_get (void *data) -{ -return *(u16 *)data; -} -DEFINE_SIMPLE_ATTRIBUTE (fops_u16, debugfs_u16_get, debugfs_u16_set,"%llu\n" ); -/** -* debugfs_create_u16 - create a debugfs file that is used to read and write an unsigned 16-bit value +* debugfs_create_u16 - create a debugfs file that is used to read and write an +* unsigned 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
@@ -138,18 +233,10 @@ struct dentry *debugfs_create_u16(const char *name, mode_t mode, }EXPORT_SYMBOL_GPL (debugfs_create_u16); -static void debugfs_u32_set (void *data,u64 val) -{ - *(u32 *)data = val; -} -static u64 debugfs_u32_get (void *data) -{ -return *(u32 *)data; -} -DEFINE_SIMPLE_ATTRIBUTE (fops_u32, debugfs_u32_get, debugfs_u32_set,"%llu\n" ); -/** -* debugfs_create_u32 - create a debugfs file that is used to read and write an unsigned 32-bit value +* debugfs_create_u32 - create a debugfs file that is used to read and write an +* unsigned 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
@@ -179,19 +266,10 @@ struct dentry *debugfs_create_u32(const char *name, mode_t mode, }EXPORT_SYMBOL_GPL (debugfs_create_u32); -static void debugfs_u64_set (void *data,u64 val) -{ - *(u64 *)data = val; -} - -static u64 debugfs_u64_get (void *data) -{ -return *(u64 *)data; -} -DEFINE_SIMPLE_ATTRIBUTE (fops_u64, debugfs_u64_get, debugfs_u64_set,"%llu\n" ); -/** -* debugfs_create_u64 - create a debugfs file that is used to read and write an unsigned 64-bit value +* debugfs_create_u64 - create a debugfs file that is used to read and write an +* unsigned 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
@@ -221,14 +299,104 @@ struct dentry *debugfs_create_u64(const char *name, mode_t mode, }EXPORT_SYMBOL_GPL (debugfs_create_u64); -DEFINE_SIMPLE_ATTRIBUTE (fops_x8, debugfs_u8_get, debugfs_u8_set,"0x%02llx\n" ); +DEFINE_NUM_ATTR(fops_s8, s8, "%hhd\n") +DEFINE_NUM_ATTR(fops_s16, s16, "%hd\n") +DEFINE_NUM_ATTR(fops_s32, s32, "%d\n") +DEFINE_NUM_ATTR (fops_s64, s64,"%lld\n" ) + +/* +* debugfs_create_s{8,16,32,64} - create a debugfs file that is used to read +* and write a signed {8,16,32,64}-bit value +* +* These functions are exactly the same as the above functions (but use a hex +* output for the decimal challenged). For details look at the above unsigned +* decimal functions. +*/ + +/** +* 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. + */ +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); + +/** +* 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. +*/ +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); + +/** +* 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. +*/ +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); -DEFINE_SIMPLE_ATTRIBUTE (fops_x16, debugfs_u16_get, debugfs_u16_set,"0x%04llx\n" ); +/** +* debugfs_create_s64 - 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. +*/ +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_x32, debugfs_u32_get, debugfs_u32_set,"0x%08llx\n" ); +DEFINE_NUM_ATTR(fops_x8, u8, "%hhx\n") +DEFINE_NUM_ATTR(fops_x16, u16, "%hx\n") +DEFINE_NUM_ATTR(fops_x32, u32, "%x\n") +DEFINE_NUM_ATTR (fops_x64, u64,"%llx\n" )/* -* debugfs_create_x{8,16,32} - create a debugfs file that is used to read and write an unsigned {8,16,32}-bit value +* debugfs_create_x{8,16,32,64} - create a debugfs file that is used to read +* and write an unsigned {8,16,32,64}-bit value * * These functions are exactly the same as the above functions (but use a hex * output for the decimal challenged). For details look at the above unsigned
@@ -236,7 +404,9 @@ DEFINE_SIMPLE_ATTRIBUTE(fops_x32, debugfs_u32_get, debugfs_u32_set, "0x%08llx\n"*/ /** -* debugfs_create_x8 - create a debugfs file that is used to read and write an unsigned 8-bit value +* debugfs_create_x8 - create a debugfs file that is used to read and write an +* unsigned 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
@@ -253,7 +423,9 @@ struct dentry *debugfs_create_x8(const char *name, mode_t mode,EXPORT_SYMBOL_GPL (debugfs_create_x8);/** -* debugfs_create_x16 - create a debugfs file that is used to read and write an unsigned 16-bit value +* debugfs_create_x16 - create a debugfs file that is used to read and write an +* unsigned 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
@@ -263,14 +435,16 @@ EXPORT_SYMBOL_GPL(debugfs_create_x8);* from. */ struct dentry *debugfs_create_x16 (const char *name,mode_t mode, -struct dentry *parent,u16 *value) +struct dentry *parent,u16 *value) {return debugfs_create_file (name, mode, parent, value, &fops_x16); }EXPORT_SYMBOL_GPL (debugfs_create_x16);/** -* debugfs_create_x32 - create a debugfs file that is used to read and write an unsigned 32-bit value +* debugfs_create_x32 - create a debugfs file that is used to read and write an +* unsigned 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
@@ -280,12 +454,31 @@ EXPORT_SYMBOL_GPL(debugfs_create_x16);* from. */ struct dentry *debugfs_create_x32 (const char *name,mode_t mode, -struct dentry *parent,u32 *value) +struct dentry *parent,u32 *value) {return debugfs_create_file (name, mode, parent, value, &fops_x32); }EXPORT_SYMBOL_GPL (debugfs_create_x32); +/** +* debugfs_create_x64 - create a debugfs file that is used to read and write an +* unsigned 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. +*/ +struct dentry *debugfs_create_x64 (const char *name,mode_t mode, +struct dentry *parent,u64 *value) +{ +return debugfs_create_file (name, mode, parent, value, &fops_x64); +} +EXPORT_SYMBOL_GPL (debugfs_create_x64); +static ssize_t read_file_bool (struct file *file,char __user *user_buf,size_t count,loff_t *ppos) {
include/linux/debugfs.h +45 -0
69 lines not on this page · raw message
Signature · 1 line
1.5.3.7
Quoted 3 lines · Mattias Nissler
This makes debugfs use its own
file_operationsfor the value accessor files created bydebugfs_create_XXX. Having that, we can also have proper versions for signed integers.
Why not tweak the "SIMPLE_ATTRIBUTE" code to support this instead? That way debugfs and all other filesystems could also use these attributes?
thanks,
greg k-h
Quoted 7 lines · Greg KH
Quoted 3 lines · Mattias Nissler
This makes debugfs use its own
file_operationsfor the value accessor files created bydebugfs_create_XXX. Having that, we can also have proper versions for signed integers.Why not tweak the "
SIMPLE_ATTRIBUTE" code to support this instead? That way debugfs and all other filesystems could also use these attributes?
I expected that question ;-) Actually, I had a version that did this. But it ended up being ugly, cause SIMPLE_ATTRIBUTEs expect to access the values via get/set functions using u64. Here is what I did and why I didn't like it:
* I made the set/get function interface more generic (as it is in the debugfs patch), i.e. getting passed the buf, so they have to decode/encode whatever they like into the buf instead of working with u64
* This means either all code using SIMPLE_ATTRIBUTES must be changed, or I need to add another wrapper macro that produces suitable get/set functions from the u64 versions it gets passed.
* Changing all SIMPLE_ATTRIBUTE users is not what I want, cause moving the csnprintf()/strtoull() calls out of the SIMPLE_ATTRIBUTE code means it becomes less simple to use it, right?
* The wrapper is actually possible, but it suffers from cases where somebody passes NULL for a get or set function (which is actually expected, compare the code in libfs.c) It is possible to hack around this, but it's ugly.
Then I thought, well, SIMPLE_ATTRIBUTEs are made to work with a set/get function pair. But debugfs expects a pointer to an actual variable anyway, so just bypass the SIMPLE_ATTRIBUTE code and make debugfs use it's own file_operations (most of them shamelessly stolen from the SIMPLE_ATTRIBUTE code, I admit).
Mattias