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

Reply via email to