lkml.org 
[lkml]   [2004]   [Dec]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC PATCH] debugfs - yet another in-kernel file system
Two-word summary: cool stuff!  Thanks!

On Thu, 9 December 2004 16:50:56 -0800, Greg KH wrote:
>
> Thus debugfs was born (yes, I know there's a userspace program called
> debugfs, this is an in-kernel filesystem and has nothing to do with
> that.) debugfs is ment for putting stuff that kernel developers need to
> see exported to userspace, yet don't always want hanging around.

In principle, it is the same as /proc, just with the explicit
information that binary compatibility will never be a goal, right?

Details differ, sure.

> diff -Nru a/fs/debugfs/debugfs_test.c b/fs/debugfs/debugfs_test.c

Nice example code. But I'd vote for either killing it or renaming it
to debugfs_example.c. Just to document that anyone actually compiling
it in is stupid.

> +static ssize_t default_read_file(struct file *file, char __user *user_buf,
> + size_t count, loff_t *ppos)

For a similar reason, I'd call this example_read_file(). You actually
fooled me for a moment and I was wondering why the heck this should be
part of debugfs. ;)

> +#define simple_type(type, format, temptype, strtolfn) \
> +static ssize_t read_file_##type(struct file *file, char __user *user_buf, size_t count, loff_t *ppos) \

Long lines. Taste varies, but...

> +{ \
> + char buf[32]; \
> + type *val = file->private_data; \
> + \
> + snprintf(buf, sizeof(buf), format, *val); \
> + return simple_read_from_buffer(user_buf, count, ppos, buf, strlen(buf)); \
> +} \
> +static ssize_t write_file_##type(struct file *file, const char __user *user_buf, size_t count, loff_t *ppos) \
> +{ \
> + char *endp; \
> + char buf[32]; \
> + int buf_size; \
> + type *val = file->private_data; \
> + temptype tmp; \
> + \
> + memset(buf, 0x00, sizeof(buf)); \
> + buf_size = min(count, (sizeof(buf)-1)); \
> + if (copy_from_user(buf, user_buf, buf_size)) \
> + return -EFAULT; \
> + \
> + tmp = strtolfn(buf, &endp, 0); \
> + if ((endp == buf) || ((type)tmp != tmp)) \
> + return -EINVAL; \
> + *val = tmp; \
> + return count; \
> +} \
> +static struct file_operations fops_##type = { \
> + .read = read_file_##type, \
> + .write = write_file_##type, \
> + .open = default_open, \
> + .llseek = default_file_lseek, \
> +}; \
> +struct dentry *debugfs_create_##type(const char *name, mode_t mode, struct dentry *parent, type *value) \
> +{ \
> + return debugfs_create_file(name, mode, parent, value, &fops_##type); \
> +}
> +
> +simple_type(u8, "%c", unsigned long, simple_strtoul);
> +simple_type(u16, "%hi", unsigned long, simple_strtoul);
> +simple_type(u32, "%i", unsigned long, simple_strtoul);
> +EXPORT_SYMBOL_GPL(debugfs_create_u8);
> +EXPORT_SYMBOL_GPL(debugfs_create_u16);
> +EXPORT_SYMBOL_GPL(debugfs_create_u32);

Move above three lines into the macro? Or do you prefer to me the
export move obvious?

> +#include <linux/config.h>
> +#include <linux/module.h>
> +#include <linux/fs.h>
> +#include <linux/mount.h>
> +#include <linux/pagemap.h>
> +#include <linux/init.h>
> +#include <linux/namei.h>

I like to sort the above alphabetically. Shouldn't matter, but it
looks neat and since there is no other natural order...

> +static struct inode *debugfs_get_inode(struct super_block *sb, int mode, dev_t dev)
> +{
> + struct inode *inode = new_inode(sb);
> +
> + if (inode) {
> + inode->i_mode = mode;
> + inode->i_uid = 0;
> + inode->i_gid = 0;
> + inode->i_blksize = PAGE_CACHE_SIZE;
> + inode->i_blocks = 0;
> + inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME;
> + switch (mode & S_IFMT) {
> + default:
> + init_special_inode(inode, mode, dev);

Just out of curiosity: why would anyone want special nodes under
/debug?

> + break;
> + case S_IFREG:
> + inode->i_fop = &debugfs_file_operations;
> + break;
> + case S_IFDIR:
> + inode->i_op = &simple_dir_inode_operations;
> + inode->i_fop = &simple_dir_operations;
> +
> + /* directory inodes start off with i_nlink == 2 (for "." entry) */
> + inode->i_nlink++;
> + break;
> + }
> + }
> + return inode;
> +}


> +static inline struct dentry *debugfs_create_file(const char *name, mode_t mode, struct dentry *parent, void *data, struct file_operations *fops)
> +{ return ERR_PTR(-ENODEV); }

Nice. Gotta remember that one.

Jörn

--
To recognize individual spam features you have to try to get into the
mind of the spammer, and frankly I want to spend as little time inside
the minds of spammers as possible.
-- Paul Graham
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 14:08    [from the cache]
©2003-2011 Jasper Spaans