linux-kernel - Re: [PATCH v2] component: add debugfs support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20171115140131.GB21840@kroah.com>
Date:   Wed, 15 Nov 2017 15:01:31 +0100
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Maciej Purski <m.purski@...sung.com>
Cc:     linux-kernel@...r.kernel.org,
        Russell King <rmk+kernel@...linux.org.uk>
Subject: Re: [PATCH v2] component: add debugfs support

On Wed, Nov 15, 2017 at 01:05:26PM +0100, Maciej Purski wrote:
> Add 'component' directory to debugfs. Create a new file for each master,
> when a master is added. Remove it on a master deletion.
> 
> Show a list of devices matched with master and indicate if
> master's components were successfully added and if the whole master is
> bound.
> 
> Signed-off-by: Maciej Purski <m.purski@...sung.com>
> ---
> Changes in v2:
> - use seq_printf() instead of seq_puts() when printing headers
> - move whole debugfs code to the file beginning in order to avoid
>   forward declarations or using multiple ifdefs
> ---
>  drivers/base/component.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 83 insertions(+)
> 
> diff --git a/drivers/base/component.c b/drivers/base/component.c
> index 89b032f..8745ad9 100644
> --- a/drivers/base/component.c
> +++ b/drivers/base/component.c
> @@ -17,6 +17,7 @@
>  #include <linux/module.h>
>  #include <linux/mutex.h>
>  #include <linux/slab.h>
> +#include <linux/debugfs.h>
>  
>  struct component;
>  
> @@ -41,6 +42,7 @@ struct master {
>  	const struct component_master_ops *ops;
>  	struct device *dev;
>  	struct component_match *match;
> +	struct dentry *dentry;
>  };
>  
>  struct component {
> @@ -56,6 +58,85 @@ static DEFINE_MUTEX(component_mutex);
>  static LIST_HEAD(component_list);
>  static LIST_HEAD(masters);
>  
> +#ifdef CONFIG_DEBUG_FS
> +
> +static struct dentry *component_debugfs_dir;
> +
> +static int component_devices_show(struct seq_file *s, void *data)
> +{
> +	struct master *m = s->private;
> +	struct component_match *match = m->match;
> +	size_t i;
> +
> +	mutex_lock(&component_mutex);
> +	seq_printf(s, "%-40s %20s\n", "master name", "status");
> +	seq_puts(s, "-------------------------------------------------------------\n");
> +	seq_printf(s, "%-40s %20s\n\n",
> +		   dev_name(m->dev), m->bound ? "bound" : "not bound");
> +
> +	seq_printf(s, "%-40s %20s\n", "device name", "status");
> +	seq_puts(s, "-------------------------------------------------------------\n");
> +	for (i = 0; i < match->num; i++) {
> +		struct device *d = (struct device *)match->compare[i].data;
> +
> +		seq_printf(s, "%-40s %20s\n", dev_name(d),
> +			   match->compare[i].component ?
> +			   "registered" : "not registered");
> +	}
> +	mutex_unlock(&component_mutex);
> +
> +	return 0;
> +}
> +
> +static int component_devices_open(struct inode *inode, struct file *file)
> +{
> +	return single_open(file, component_devices_show, inode->i_private);
> +}
> +
> +static const struct file_operations component_devices_fops = {
> +	.open = component_devices_open,
> +	.read = seq_read,
> +	.llseek = seq_lseek,
> +	.release = single_release,
> +};
> +
> +static int __init component_debug_init(void)
> +{
> +	component_debugfs_dir = debugfs_create_dir("component", NULL);
> +
> +	if (!component_debugfs_dir)
> +		return -ENOMEM;

No need to test this at all, you should never fail anything if debugfs
is not working properly, just continue and move on.  The result of any
debugfs call can be fed back into any other debugfs call without any
problems.

> +
> +	return 0;
> +}
> +
> +core_initcall(component_debug_init);
> +
> +static void component_master_debugfs_add(struct master *m)
> +{
> +	m->dentry = debugfs_create_file(dev_name(m->dev), 0444,
> +					component_debugfs_dir,
> +					m, &component_devices_fops);

See, you do it well here, do the same thing when you create the initial
debugfs directory.

Also, "component" is very vague, can you think of a better term for
this?  "device_component"?  "dev_component"?  Something else?  But I
don't care, if you really like "component", that's fine.

thanks,

greg k-h