Re: [RFC PATCH 1/7] core/metricfs: Create metricfs, standardized files under debugfs.

[Greg KH <gregkh@linuxfoundation.org>]

debugfs interaction nits:

On Fri, Aug 07, 2020 at 02:29:10PM -0700, Jonathan Adams wrote:
> +static struct dentry *metricfs_init_dentry(void)
> +{
> +	static int once;
> +
> +	if (d_metricfs)
> +		return d_metricfs;
> +
> +	if (!debugfs_initialized())
> +		return NULL;
> +
> +	d_metricfs = debugfs_create_dir("metricfs", NULL);
> +
> +	if (!d_metricfs && !once) {

As it is impossible for d_metricfs to ever be NULL, why are you checking
it?

> +		once = 1;
> +		pr_warn("Could not create debugfs directory 'metricfs'\n");

There is a pr_warn_once I think, but again, how can this ever trigger?

> +		return NULL;
> +	}
> +
> +	return d_metricfs;
> +}
> +
> +/* We always cast in and out to struct dentry. */
> +struct metricfs_subsys {
> +	struct dentry dentry;
> +};
> +
> +static struct dentry *metricfs_create_file(const char *name,
> +					   mode_t mode,
> +					   struct dentry *parent,
> +					   void *data,
> +					   const struct file_operations *fops)
> +{
> +	struct dentry *ret;
> +
> +	ret = debugfs_create_file(name, mode, parent, data, fops);
> +	if (!ret)
> +		pr_warn("Could not create debugfs '%s' entry\n", name);

As ret can never be NULL, why check?

There is no need to ever check debugfs return values, just keep on
going, that's the design of it.

thanks,

greg k-h