2012-05-02 12:48:38

by Sasikantha Babu

[permalink] [raw]
Subject: [PATCH] debugfs: New debugfs interface for creation of files, directory and symlinks

As we know the current debugfs file or directory or symlink creation
doesn't return proper error codes to the caller on failure. Because
of this caller and user could not able to find the exact reason of
the failure.

As Andrew Morton suggested (http://www.spinics.net/lists/linux-mm/msg33617.html)
introduced new debugfs interface to create debugfs entries. Newer APIs
returns proper error codes(ERR_PTR) on failure. Since fixing older
APIs will break the existing debugfs layer (because the API's used
in 100s of places, first we must address all those caller code before
fixing the debugfs creation API's. Instead of that, creating newer interface
and migrating to newer interface is much simpler).

And also added read only and write only support for x64 and u64.

New Debug FS API
-----------------

debugfs_file_create
debugfs_dir_create
debugfs_symlink_create
debugfs_u8_create
debugfs_u16_create
debugfs_u32_create
debugfs_u64_create
debugfs_x8_create
debugfs_x16_create
debugfs_x32_create
debugfs_x64_create
debugfs_size_t_create
debugfs_bool_create
debugfs_blob_create
debugfs_regset32_create

All new debug fs APIs returns pointer to an error (ERR_PTR). Once
everyone migrated to new ones we can remove the older ones.

Note : 1) old debugfs API names "debugfs_X_Y" changed to "debugfs_Y_X".
2) No change in the arguments passed

For Example :
OLD API: debugfs_create_file
NEW API: debugfs_file_create

I had tested and verified all new APIs and it is working fine.


Signed-off-by: Sasikantha babu <[email protected]>
---
fs/debugfs/file.c | 440 +++++++++++++++++++++++++++++++++++++++++++++++
fs/debugfs/inode.c | 125 +++++++++++++
include/linux/debugfs.h | 120 +++++++++++++
3 files changed, 685 insertions(+), 0 deletions(-)

diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index 5dfafdd..12ffe3e 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -21,6 +21,20 @@
#include <linux/debugfs.h>
#include <linux/io.h>

+enum debugfs_u8_type {
+ DEBUGFS_U8 = 0x1,
+ DEBUGFS_U16,
+ DEBUGFS_U32,
+ DEBUGFS_U64
+};
+
+enum debugfs_x8_type {
+ DEBUGFS_X8 = 0x1,
+ DEBUGFS_X16,
+ DEBUGFS_X32,
+ DEBUGFS_X64
+};
+
static ssize_t default_read_file(struct file *file, char __user *buf,
size_t count, loff_t *ppos)
{
@@ -273,6 +287,8 @@ DEFINE_SIMPLE_ATTRIBUTE(fops_x32_ro, debugfs_u32_get, NULL, "0x%08llx\n");
DEFINE_SIMPLE_ATTRIBUTE(fops_x32_wo, NULL, debugfs_u32_set, "0x%08llx\n");

DEFINE_SIMPLE_ATTRIBUTE(fops_x64, debugfs_u64_get, debugfs_u64_set, "0x%016llx\n");
+DEFINE_SIMPLE_ATTRIBUTE(fops_x64_ro, debugfs_u64_get, NULL, "0x%016llx\n");
+DEFINE_SIMPLE_ATTRIBUTE(fops_x64_wo, NULL, debugfs_u64_set, "0x%016llx\n");

/*
* 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
@@ -611,4 +627,428 @@ struct dentry *debugfs_create_regset32(const char *name, umode_t mode,
}
EXPORT_SYMBOL_GPL(debugfs_create_regset32);

+/**
+ * debugfs_regset32_create - create a debugfs file that returns register values
+ * @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.
+ * @regset: a pointer to a struct debugfs_regset32, which contains a pointer
+ * to an array of register definitions, the array size and the base
+ * address where the register bank is to be found.
+ *
+ * This function creates a file in debugfs with the given name that reports
+ * the names and values of a set of 32-bit registers. If the @mode variable
+ * is so set it can be read from. Writing is not supported.
+ *
+ * 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, error pointer 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_regset32_create(const char *name, umode_t mode,
+ struct dentry *parent,
+ struct debugfs_regset32 *regset)
+{
+ return debugfs_file_create(name, mode, parent, regset, &fops_regset32);
+}
+EXPORT_SYMBOL_GPL(debugfs_regset32_create);
+
#endif /* CONFIG_HAS_IOMEM */
+
+/*
+ * debugfs_create_u{8,16,32,64} - create a debugfs file that is used to read and
+ * write an unsigned {8,16,32,64}-bit value
+ */
+
+static struct dentry *debugfs_create_uX(const char *name, umode_t mode,
+ struct dentry *parent, void *value,
+ enum debugfs_u8_type type)
+{
+ const struct file_operations *fops = NULL;
+
+ switch (type) {
+ case DEBUGFS_U8:
+ if (!(mode & S_IWUGO))
+ fops = &fops_u8_ro;
+ else if (!(mode & S_IRUGO))
+ fops = &fops_u8_wo;
+ else
+ fops = &fops_u8;
+ break;
+ case DEBUGFS_U16:
+ if (!(mode & S_IWUGO))
+ fops = &fops_u16_ro;
+ else if (!(mode & S_IRUGO))
+ fops = &fops_u16_wo;
+ else
+ fops = &fops_u16;
+ break;
+ case DEBUGFS_U32:
+ if (!(mode & S_IWUGO))
+ fops = &fops_u32_ro;
+ else if (!(mode & S_IRUGO))
+ fops = &fops_u32_wo;
+ else
+ fops = &fops_u32;
+ break;
+ case DEBUGFS_U64:
+ if (!(mode & S_IWUGO))
+ fops = &fops_u64_ro;
+ else if (!(mode & S_IRUGO))
+ fops = &fops_u64_wo;
+ else
+ fops = &fops_u64;
+ break;
+ }
+
+ return debugfs_file_create(name, mode, parent, value, fops);
+}
+
+/**
+ * debugfs_u8_create - 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
+ * 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, error pointer 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_u8_create(const char *name, umode_t mode,
+ struct dentry *parent, u8 *value)
+{
+ return debugfs_create_uX(name, mode, parent, value, DEBUGFS_U8);
+}
+EXPORT_SYMBOL_GPL(debugfs_u8_create);
+
+/**
+ * debugfs_u16_create - 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
+ * 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, error pointer 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_u16_create(const char *name, umode_t mode,
+ struct dentry *parent, u16 *value)
+{
+ return debugfs_create_uX(name, mode, parent, value, DEBUGFS_U16);
+}
+EXPORT_SYMBOL_GPL(debugfs_u16_create);
+
+/**
+ * debugfs_u32_create - 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.
+ *
+ * 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_u32_create(const char *name, umode_t mode,
+ struct dentry *parent, u32 *value)
+{
+ return debugfs_create_uX(name, mode, parent, value, DEBUGFS_U32);
+}
+EXPORT_SYMBOL_GPL(debugfs_u32_create);
+
+/**
+ * debugfs_u64_create - 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
+ * 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_u64_create(const char *name, umode_t mode,
+ struct dentry *parent, u64 *value)
+{
+ return debugfs_create_uX(name, mode, parent, value, DEBUGFS_U64);
+}
+EXPORT_SYMBOL_GPL(debugfs_u64_create);
+
+/*
+ * 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
+ * decimal functions.
+ */
+
+static struct dentry *debugfs_create_xX(const char *name, umode_t mode,
+ struct dentry *parent, void *value,
+ enum debugfs_x8_type type)
+{
+ const struct file_operations *fops = NULL;
+
+ switch (type) {
+ case DEBUGFS_X8:
+ if (!(mode & S_IWUGO))
+ fops = &fops_x8_ro;
+ else if (!(mode & S_IRUGO))
+ fops = &fops_x8_wo;
+ else
+ fops = &fops_x8;
+ break;
+ case DEBUGFS_X16:
+ if (!(mode & S_IWUGO))
+ fops = &fops_x16_ro;
+ else if (!(mode & S_IRUGO))
+ fops = &fops_x16_wo;
+ else
+ fops = &fops_x16;
+ break;
+ case DEBUGFS_X32:
+ if (!(mode & S_IWUGO))
+ fops = &fops_x32_ro;
+ else if (!(mode & S_IRUGO))
+ fops = &fops_x32_wo;
+ else
+ fops = &fops_x32;
+ break;
+ case DEBUGFS_X64:
+ if (!(mode & S_IWUGO))
+ fops = &fops_x64_ro;
+ else if (!(mode & S_IRUGO))
+ fops = &fops_x64_wo;
+ else
+ fops = &fops_x64;
+ break;
+ }
+
+ return debugfs_file_create(name, mode, parent, value, fops);
+}
+
+/**
+ * debugfs_x8_create - 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
+ * 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_x8_create(const char *name, umode_t mode,
+ struct dentry *parent, u8 *value)
+{
+ return debugfs_create_xX(name, mode, parent, value, DEBUGFS_X8);
+}
+EXPORT_SYMBOL_GPL(debugfs_x8_create);
+
+/**
+ * debugfs_x16_create - 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
+ * 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_x16_create(const char *name, umode_t mode,
+ struct dentry *parent, u16 *value)
+{
+ return debugfs_create_xX(name, mode, parent, value, DEBUGFS_X16);
+}
+EXPORT_SYMBOL_GPL(debugfs_x16_create);
+
+/**
+ * debugfs_x32_create - 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_x32_create(const char *name, umode_t mode,
+ struct dentry *parent, u32 *value)
+{
+ return debugfs_create_xX(name, mode, parent, value, DEBUGFS_X32);
+}
+EXPORT_SYMBOL_GPL(debugfs_x32_create);
+
+/**
+ * debugfs_x64_create - 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
+ * 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_x64_create(const char *name, umode_t mode,
+ struct dentry *parent, u64 *value)
+{
+ return debugfs_create_xX(name, mode, parent, value, DEBUGFS_X64);
+}
+EXPORT_SYMBOL_GPL(debugfs_x64_create);
+
+/**
+ * debugfs_size_t_create - create a debugfs file that is used to read and write
+ * an size_t value
+ * @name: a pointer to a string containing the name of the file to create.
+ * @mode: the permission that the file should have
+ * @parent: a pointer to the parent dentry for this file. This should be a
+ * directory dentry if set. If this parameter is %NULL, then the
+ * file will be created in the root of the debugfs filesystem.
+ * @value: a pointer to the variable that the file should read to and write
+ * from.
+ */
+struct dentry *debugfs_size_t_create(const char *name, umode_t mode,
+ struct dentry *parent, size_t *value)
+{
+ return debugfs_file_create(name, mode, parent, value, &fops_size_t);
+}
+EXPORT_SYMBOL_GPL(debugfs_size_t_create);
+/**
+ * debugfs_bool_create - create a debugfs file that is used to read and
+ * write a boolean 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, error pointer 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_bool_create(const char *name, umode_t mode,
+ struct dentry *parent, u32 *value)
+{
+ return debugfs_file_create(name, mode, parent, value, &fops_bool);
+}
+EXPORT_SYMBOL_GPL(debugfs_bool_create);
+
+/**
+ * debugfs_blob_create - create a debugfs file that is used to read a binary
+ * blob
+ * @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.
+ * @blob: a pointer to a struct debugfs_blob_wrapper which contains a pointer
+ * to the blob data and the size of the data.
+ *
+ * This function creates a file in debugfs with the given name that exports
+ * @blob->data as a binary blob. If the @mode variable is so set it can be
+ * read from. Writing is not supported.
+ *
+ * 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, error pointer 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_blob_create(const char *name, umode_t mode,
+ struct dentry *parent,
+ struct debugfs_blob_wrapper *blob)
+{
+ return debugfs_file_create(name, mode, parent, blob, &fops_blob);
+}
+EXPORT_SYMBOL_GPL(debugfs_blob_create);
diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index b80bc84..a8e91f5 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -660,6 +660,131 @@ exit:
EXPORT_SYMBOL_GPL(debugfs_rename);

/**
+ * debugfs_file_create - create a file in the debugfs filesystem
+ * @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 paramater is NULL, then the
+ * file will be created in the root of the debugfs filesystem.
+ * @data: a pointer to something that the caller will want to get to later
+ * on. The inode.i_private pointer will point to this value on
+ * the open() call.
+ * @fops: a pointer to a struct file_operations that should be used for
+ * this file.
+ *
+ * This is the basic "create a file" function for debugfs. It allows for a
+ * wide range of flexibility in creating a file, or a directory (if you want
+ * to create a directory, the debugfs_dir_create() function is
+ * recommended to be used instead.)
+ *
+ * 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, error pointer will be
+ * returned.
+ *
+ * If debugfs is not enabled in the kernel, the value -%ENODEV will be
+ * returned.
+ */
+struct dentry *debugfs_file_create(const char *name, umode_t mode,
+ struct dentry *parent, void *data,
+ const struct file_operations *fops)
+{
+ struct dentry *dentry = NULL;
+ int error;
+
+ pr_debug("debugfs: creating file '%s'\n", name);
+
+ error = simple_pin_fs(&debug_fs_type, &debugfs_mount,
+ &debugfs_mount_count);
+ if (error)
+ return ERR_PTR(error);
+
+ error = debugfs_create_by_name(name, mode, parent, &dentry,
+ data, fops);
+ if (error) {
+ dentry = ERR_PTR(error);
+ simple_release_fs(&debugfs_mount, &debugfs_mount_count);
+ }
+
+ return dentry;
+}
+EXPORT_SYMBOL_GPL(debugfs_file_create);
+
+/**
+ * debugfs_dir_create - create a directory in the debugfs filesystem
+ * @name: a pointer to a string containing the name of the directory to
+ * create.
+ * @parent: a pointer to the parent dentry for this file. This should be a
+ * directory dentry if set. If this paramater is NULL, then the
+ * directory will be created in the root of the debugfs filesystem.
+ *
+ * This function creates a directory in debugfs with the given name.
+ *
+ * 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, error pointer will be
+ * returned.
+ *
+ * If debugfs is not enabled in the kernel, the value -%ENODEV will be
+ * returned.
+ */
+struct dentry *debugfs_dir_create(const char *name, struct dentry *parent)
+{
+ return debugfs_file_create(name,
+ S_IFDIR | S_IRWXU | S_IRUGO | S_IXUGO,
+ parent, NULL, NULL);
+}
+EXPORT_SYMBOL_GPL(debugfs_dir_create);
+
+/**
+ * debugfs_symlink_create- create a symbolic link in the debugfs filesystem
+ * @name: a pointer to a string containing the name of the symbolic link to
+ * create.
+ * @parent: a pointer to the parent dentry for this symbolic link. This
+ * should be a directory dentry if set. If this paramater is NULL,
+ * then the symbolic link will be created in the root of the debugfs
+ * filesystem.
+ * @target: a pointer to a string containing the path to the target of the
+ * symbolic link.
+ *
+ * This function creates a symbolic link with the given name in debugfs that
+ * links to the given target path.
+ *
+ * This function will return a pointer to a dentry if it succeeds. This
+ * pointer must be passed to the debugfs_remove() function when the symbolic
+ * link is to be removed (no automatic cleanup happens if your module is
+ * unloaded, you are responsible here.) If an error occurs, error pointer
+ * will be returned.
+ *
+ * If debugfs is not enabled in the kernel, the value -%ENODEV will be
+ * returned.
+ */
+struct dentry *debugfs_symlink_create(const char *name, struct dentry *parent,
+ const char *target)
+{
+ struct dentry *result;
+ char *link;
+
+ if (!target)
+ /*XXX: is this return value correct one*/
+ return ERR_PTR(-ENODEV);
+
+ link = kstrdup(target, GFP_KERNEL);
+ if (!link)
+ return ERR_PTR(-ENOMEM);
+
+ result = debugfs_file_create(name, S_IFLNK | S_IRWXUGO, parent, link,
+ NULL);
+ if (IS_ERR(result))
+ kfree(link);
+
+ return result;
+}
+EXPORT_SYMBOL_GPL(debugfs_symlink_create);
+
+/**
* debugfs_initialized - Tells whether debugfs has been registered
*/
bool debugfs_initialized(void)
diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h
index ae36b72..8271c2e 100644
--- a/include/linux/debugfs.h
+++ b/include/linux/debugfs.h
@@ -95,6 +95,45 @@ int debugfs_print_regs32(struct seq_file *s, const struct debugfs_reg32 *regs,

bool debugfs_initialized(void);

+/*Debug Fs New interface prototypes*/
+struct dentry *debugfs_file_create(const char *name, umode_t mode,
+ struct dentry *parent, void *data,
+ const struct file_operations *fops);
+
+struct dentry *debugfs_dir_create(const char *name, struct dentry *parent);
+
+struct dentry *debugfs_symlink_create(const char *name, struct dentry *parent,
+ const char *dest);
+struct dentry *debugfs_u8_create(const char *name, umode_t mode,
+ struct dentry *parent, u8 *value);
+struct dentry *debugfs_u16_create(const char *name, umode_t mode,
+ struct dentry *parent, u16 *value);
+struct dentry *debugfs_u32_create(const char *name, umode_t mode,
+ struct dentry *parent, u32 *value);
+struct dentry *debugfs_u64_create(const char *name, umode_t mode,
+ struct dentry *parent, u64 *value);
+struct dentry *debugfs_x8_create(const char *name, umode_t mode,
+ struct dentry *parent, u8 *value);
+struct dentry *debugfs_x16_create(const char *name, umode_t mode,
+ struct dentry *parent, u16 *value);
+struct dentry *debugfs_x32_create(const char *name, umode_t mode,
+ struct dentry *parent, u32 *value);
+struct dentry *debugfs_x64_create(const char *name, umode_t mode,
+ struct dentry *parent, u64 *value);
+struct dentry *debugfs_size_t_create(const char *name, umode_t mode,
+ struct dentry *parent, size_t *value);
+struct dentry *debugfs_bool_create(const char *name, umode_t mode,
+ struct dentry *parent, u32 *value);
+
+struct dentry *debugfs_blob_create(const char *name, umode_t mode,
+ struct dentry *parent,
+ struct debugfs_blob_wrapper *blob);
+
+struct dentry *debugfs_regset32_create(const char *name, umode_t mode,
+ struct dentry *parent,
+ struct debugfs_regset32 *regset);
+
+
#else

#include <linux/err.h>
@@ -219,6 +258,87 @@ static inline bool debugfs_initialized(void)
return false;
}

+struct dentry *debugfs_file_create(const char *name, umode_t mode,
+ struct dentry *parent, void *data,
+ const struct file_operations *fops)
+{
+ return ERR_PTR(-ENODEV);
+}
+
+struct dentry *debugfs_dir_create(const char *name, struct dentry *parent)
+{
+ return ERR_PTR(-ENODEV);
+}
+
+struct dentry *debugfs_symlink_create(const char *name, struct dentry *parent,
+ const char *dest)
+{
+ return ERR_PTR(-ENODEV);
+}
+struct dentry *debugfs_u8_create(const char *name, umode_t mode,
+ struct dentry *parent, u8 *value)
+{
+ return ERR_PTR(-ENODEV);
+}
+struct dentry *debugfs_u16_create(const char *name, umode_t mode,
+ struct dentry *parent, u16 *value)
+{
+ return ERR_PTR(-ENODEV);
+}
+struct dentry *debugfs_u32_create(const char *name, umode_t mode,
+ struct dentry *parent, u32 *value)
+{
+ return ERR_PTR(-ENODEV);
+}
+struct dentry *debugfs_u64_create(const char *name, umode_t mode,
+ struct dentry *parent, u64 *value)
+{
+ return ERR_PTR(-ENODEV);
+}
+struct dentry *debugfs_x8_create(const char *name, umode_t mode,
+ struct dentry *parent, u8 *value)
+{
+ return ERR_PTR(-ENODEV);
+}
+struct dentry *debugfs_x16_create(const char *name, umode_t mode,
+ struct dentry *parent, u16 *value)
+{
+ return ERR_PTR(-ENODEV);
+}
+struct dentry *debugfs_x32_create(const char *name, umode_t mode,
+ struct dentry *parent, u32 *value)
+{
+ return ERR_PTR(-ENODEV);
+}
+struct dentry *debugfs_x64_create(const char *name, umode_t mode,
+ struct dentry *parent, u64 *value)
+{
+ return ERR_PTR(-ENODEV);
+}
+struct dentry *debugfs_size_t_create(const char *name, umode_t mode,
+ struct dentry *parent, size_t *value)
+{
+ return ERR_PTR(-ENODEV);
+}
+struct dentry *debugfs_bool_create(const char *name, umode_t mode,
+ struct dentry *parent, u32 *value)
+{
+ return ERR_PTR(-ENODEV);
+}
+
+struct dentry *debugfs_blob_create(const char *name, umode_t mode,
+ struct dentry *parent,
+ struct debugfs_blob_wrapper *blob)
+{
+ return ERR_PTR(-ENODEV);
+}
+
+struct dentry *debugfs_regset32_create(const char *name, umode_t mode,
+ struct dentry *parent,
+ struct debugfs_regset32 *regset)
+{
+ return ERR_PTR(-ENODEV);
+}
#endif

#endif
--
1.7.3.4


2012-05-02 15:31:07

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] debugfs: New debugfs interface for creation of files, directory and symlinks

On Wed, May 02, 2012 at 06:20:54PM +0530, Sasikantha babu wrote:
> As we know the current debugfs file or directory or symlink creation
> doesn't return proper error codes to the caller on failure. Because
> of this caller and user could not able to find the exact reason of
> the failure.

And what is the problem with this? Either the file is created or not,
you really shouldn't care anymore than that. It's not like you are
going to retry the creation, or are you?

Who really cares if the file is failed to be created?

> As Andrew Morton suggested (http://www.spinics.net/lists/linux-mm/msg33617.html)
> introduced new debugfs interface to create debugfs entries. Newer APIs
> returns proper error codes(ERR_PTR) on failure.

Again, why? What root problem are you trying to solve here?

thanks,

greg k-h

2012-05-02 21:58:19

by Sasikantha Babu

[permalink] [raw]
Subject: Re: [PATCH] debugfs: New debugfs interface for creation of files, directory and symlinks

On Wed, May 2, 2012 at 9:01 PM, Greg Kroah-Hartman
<[email protected]> wrote:
> On Wed, May 02, 2012 at 06:20:54PM +0530, Sasikantha babu wrote:
>> As we know the current debugfs file or directory or symlink creation
>> doesn't return proper error codes to the caller on failure. Because
>> of this caller and user could not able to find the exact reason of
>> the failure.
>
> And what is the problem with this? ?Either the file is created or not,
> you really shouldn't care anymore than that. ?It's not like you are
> going to retry the creation, or are you?
>
> Who really cares if the file is failed to be created?

In most of cases I had observed caller of debufs_create_file or
debufs_create_dir always returns -ENOMEM on failure, which is not true.
I felt returning proper error code will help in figuring out the actual
reason of the failure (for eg: it can be -EEXISTS based on the error
caller can change the name of the file or dir)

>
>> As Andrew Morton suggested (http://www.spinics.net/lists/linux-mm/msg33617.html)
>> introduced new debugfs interface to create debugfs entries. Newer APIs
>> returns proper error codes(ERR_PTR) on failure.
>
> Again, why? ?What root problem are you trying to solve here?

The usage of debugfs throughout the kernel is not uniform especially the
error handling scenarios. Some place it is IS_ERR validations , other place
against NULL and some place ignoring the return value. Just tried to make it
uniform.
>
> thanks,
>
> greg k-h

2012-05-02 22:04:22

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] debugfs: New debugfs interface for creation of files, directory and symlinks

On Thu, May 03, 2012 at 03:28:17AM +0530, Sasikanth babu wrote:
> On Wed, May 2, 2012 at 9:01 PM, Greg Kroah-Hartman
> <[email protected]> wrote:
> > On Wed, May 02, 2012 at 06:20:54PM +0530, Sasikantha babu wrote:
> >> As we know the current debugfs file or directory or symlink creation
> >> doesn't return proper error codes to the caller on failure. Because
> >> of this caller and user could not able to find the exact reason of
> >> the failure.
> >
> > And what is the problem with this? ?Either the file is created or not,
> > you really shouldn't care anymore than that. ?It's not like you are
> > going to retry the creation, or are you?
> >
> > Who really cares if the file is failed to be created?
>
> In most of cases I had observed caller of debufs_create_file or
> debufs_create_dir always returns -ENOMEM on failure, which is not true.

Where does that happen? And why would the creation of a debugfs file
fail?

> I felt returning proper error code will help in figuring out the actual
> reason of the failure (for eg: it can be -EEXISTS based on the error
> caller can change the name of the file or dir)

Why would it ever conflict in the first place?

What files in the kernel have this problem today that they would create
conflicting files/dirs?

> >> As Andrew Morton suggested (http://www.spinics.net/lists/linux-mm/msg33617.html)
> >> introduced new debugfs interface to create debugfs entries. Newer APIs
> >> returns proper error codes(ERR_PTR) on failure.
> >
> > Again, why? ?What root problem are you trying to solve here?
>
> The usage of debugfs throughout the kernel is not uniform especially the
> error handling scenarios. Some place it is IS_ERR validations , other place
> against NULL and some place ignoring the return value. Just tried to make it
> uniform.

You can fixup the callers to make it uniform, the api is uniform in what
it returns today, right?

Again, I see no real benifit for returning the "true" error as no one
really cares about that, all that matters is if it worked or not, and
even then, no one should really care about that either, as remember,
this is debugfs, whose one rule is, "there is no rules."

thanks,

greg k-h

2012-05-02 22:20:07

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] debugfs: New debugfs interface for creation of files, directory and symlinks

On Wed, 2 May 2012 15:04:17 -0700
Greg Kroah-Hartman <[email protected]> wrote:

> On Thu, May 03, 2012 at 03:28:17AM +0530, Sasikanth babu wrote:
> > On Wed, May 2, 2012 at 9:01 PM, Greg Kroah-Hartman
> > <[email protected]> wrote:
> > > On Wed, May 02, 2012 at 06:20:54PM +0530, Sasikantha babu wrote:
> > >> As we know the current debugfs file or directory or symlink creation
> > >> doesn't return proper error codes to the caller on failure. Because
> > >> of this caller and user could not able to find the exact reason of
> > >> the failure.
> > >
> > > And what is the problem with this? __Either the file is created or not,
> > > you really shouldn't care anymore than that. __It's not like you are
> > > going to retry the creation, or are you?
> > >
> > > Who really cares if the file is failed to be created?
> >
> > In most of cases I had observed caller of debufs_create_file or
> > debufs_create_dir always returns -ENOMEM on failure, which is not true.
>
> Where does that happen? And why would the creation of a debugfs file
> fail?

How's he supposed to know, when the kernel cannot return the correct
diagnostic?

That's the whole reason we have errnos: to report on what went wrong,
so operators can understand *why* it failed and so that programmers can
diagnose and fix bugs.

There's nothing special about debugfs and there is no reason why it
should deviate from well-established practice.

If well-written code checks the return value (as it should) and then
propagates an error code back to its caller (as it should), the stupid
debugfs interface forces that caller to invent an errno from thin air.
And if that guessed errno is wrong, it is actively misleading!

> You can fixup the callers to make it uniform, the api is uniform in what
> it returns today, right?

The API is stupid and wrong, actually. There is no *advantage* to
having done it this way - none at all.

> Again, I see no real benifit for returning the "true" error as no one
> really cares about that, all that matters is if it worked or not, and
> even then, no one should really care about that either, as remember,
> this is debugfs, whose one rule is, "there is no rules."

No. If something has gone wrong then something needs to be fixed.
Either the system needs reconfiguration or kernel programmers have
repairs to do. Either way, information is needed to make those fixes.
debugfs designers don't get to tell debugfs users what is and is not
important to those users.


All that being said, it is unobvious that fixing this mistake is worth
all the churn.

2012-05-02 22:36:10

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] debugfs: New debugfs interface for creation of files, directory and symlinks

On Wed, May 02, 2012 at 03:20:03PM -0700, Andrew Morton wrote:
> On Wed, 2 May 2012 15:04:17 -0700
> Greg Kroah-Hartman <[email protected]> wrote:
>
> > On Thu, May 03, 2012 at 03:28:17AM +0530, Sasikanth babu wrote:
> > > On Wed, May 2, 2012 at 9:01 PM, Greg Kroah-Hartman
> > > <[email protected]> wrote:
> > > > On Wed, May 02, 2012 at 06:20:54PM +0530, Sasikantha babu wrote:
> > > >> As we know the current debugfs file or directory or symlink creation
> > > >> doesn't return proper error codes to the caller on failure. Because
> > > >> of this caller and user could not able to find the exact reason of
> > > >> the failure.
> > > >
> > > > And what is the problem with this? __Either the file is created or not,
> > > > you really shouldn't care anymore than that. __It's not like you are
> > > > going to retry the creation, or are you?
> > > >
> > > > Who really cares if the file is failed to be created?
> > >
> > > In most of cases I had observed caller of debufs_create_file or
> > > debufs_create_dir always returns -ENOMEM on failure, which is not true.
> >
> > Where does that happen? And why would the creation of a debugfs file
> > fail?
>
> How's he supposed to know, when the kernel cannot return the correct
> diagnostic?
>
> That's the whole reason we have errnos: to report on what went wrong,
> so operators can understand *why* it failed and so that programmers can
> diagnose and fix bugs.
>
> There's nothing special about debugfs and there is no reason why it
> should deviate from well-established practice.
>
> If well-written code checks the return value (as it should) and then
> propagates an error code back to its caller (as it should), the stupid
> debugfs interface forces that caller to invent an errno from thin air.
> And if that guessed errno is wrong, it is actively misleading!
>
> > You can fixup the callers to make it uniform, the api is uniform in what
> > it returns today, right?
>
> The API is stupid and wrong, actually. There is no *advantage* to
> having done it this way - none at all.

Yes there is, it makes the caller logic trivial, which was the main goal
here in getting people to use it over creating new proc files all the
time for no good reason.

No one cares about the return value when you create a proc file, either
it succeeds or not, and you handle things from there, you never change
the name to try it again.

Same thing with debugfs, you only care if it works or not, and really,
you don't even care if it works, as the api lets you continue on if it
didn't just fine.

These are debugging files, with no set rules on what they contain. Yes,
people have grown to get used to them over the years, but the namespace
in which they work has worked out for itself, and I have yet to ever
hear of any two people wanting to create the same file/directory
anywhere, and have anything fail.

Or am I missing some subsystem that is having problems like this with
debugfs?

thanks,

greg k-h

2012-05-02 22:44:41

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] debugfs: New debugfs interface for creation of files, directory and symlinks

On Wed, 2 May 2012 15:36:03 -0700
Greg Kroah-Hartman <[email protected]> wrote:

> >
> > The API is stupid and wrong, actually. There is no *advantage* to
> > having done it this way - none at all.
>
> Yes there is, it makes the caller logic trivial, which was the main goal
> here in getting people to use it over creating new proc files all the
> time for no good reason.
>
> No one cares about the return value when you create a proc file, either
> it succeeds or not, and you handle things from there, you never change
> the name to try it again.
>
> Same thing with debugfs, you only care if it works or not, and really,
> you don't even care if it works, as the api lets you continue on if it
> didn't just fine.
>
> These are debugging files, with no set rules on what they contain. Yes,
> people have grown to get used to them over the years, but the namespace
> in which they work has worked out for itself, and I have yet to ever
> hear of any two people wanting to create the same file/directory
> anywhere, and have anything fail.
>
> Or am I missing some subsystem that is having problems like this with
> debugfs?

grr, you appear to have ignored everything I wrote. Here it is again:

> > That's the whole reason we have errnos: to report on what went wrong,
> > so operators can understand *why* it failed and so that programmers can
> > diagnose and fix bugs.

and

> > If well-written code checks the return value (as it should) and then
> > propagates an error code back to its caller (as it should), the stupid
> > debugfs interface forces that caller to invent an errno from thin air.
> > And if that guessed errno is wrong, it is actively misleading!

I would add that an interface which encourages callers to silently
ignore programming and configuration errors is not a good one.

2012-05-02 23:07:35

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] debugfs: New debugfs interface for creation of files, directory and symlinks

On Wed, May 02, 2012 at 03:44:38PM -0700, Andrew Morton wrote:
> On Wed, 2 May 2012 15:36:03 -0700
> Greg Kroah-Hartman <[email protected]> wrote:
>
> > >
> > > The API is stupid and wrong, actually. There is no *advantage* to
> > > having done it this way - none at all.
> >
> > Yes there is, it makes the caller logic trivial, which was the main goal
> > here in getting people to use it over creating new proc files all the
> > time for no good reason.
> >
> > No one cares about the return value when you create a proc file, either
> > it succeeds or not, and you handle things from there, you never change
> > the name to try it again.
> >
> > Same thing with debugfs, you only care if it works or not, and really,
> > you don't even care if it works, as the api lets you continue on if it
> > didn't just fine.
> >
> > These are debugging files, with no set rules on what they contain. Yes,
> > people have grown to get used to them over the years, but the namespace
> > in which they work has worked out for itself, and I have yet to ever
> > hear of any two people wanting to create the same file/directory
> > anywhere, and have anything fail.
> >
> > Or am I missing some subsystem that is having problems like this with
> > debugfs?
>
> grr, you appear to have ignored everything I wrote. Here it is again:

No, sorry, I didn't ignore it.

> > > That's the whole reason we have errnos: to report on what went wrong,
> > > so operators can understand *why* it failed and so that programmers can
> > > diagnose and fix bugs.
>
> and
>
> > > If well-written code checks the return value (as it should) and then
> > > propagates an error code back to its caller (as it should), the stupid
> > > debugfs interface forces that caller to invent an errno from thin air.
> > > And if that guessed errno is wrong, it is actively misleading!
>
> I would add that an interface which encourages callers to silently
> ignore programming and configuration errors is not a good one.

I would not disagree, but I wrote this interface to be as simple as
possible. Perhaps I messed up, but I don't think so as it was a strong
point to get people to use it in the first place. It was less lines of
code to use debugfs instead of proc files, and you can do error
detection, it's just a binary "did this work or not" type of detection,
not a "this failed in this manner" type of error.

And for one-off debugging files, I still think "did this work or not" is
as good as is needed. The only errors that could come back really would
be:
-EEXISTS
-ENOMEM

If we have no more memory, things are messed up and recovering is going
to be hard anyway.

If the file/dir already exists, it was hard-coded in the first place, so
changing it requires changing the code. So returning NULL for -EEXISTS
is just as good, as the code will not work, it's "obvious" that the
existing file is not the one that you just tried to create, so something
needs to be changed.

A rich error reporting system is good for things that matter. I didn't
feel that debugfs files really "matter" when I created it. The fact
that this hasn't come up as a real problem in the 8+ years that debugfs
has been around, either means I was right, or that no one cared until
now :)

thanks,

greg k-h