Messages in this thread | | | Date | Sat, 8 Aug 2020 07:41:43 +0200 | From | Greg KH <> | Subject | Re: [RFC PATCH 1/7] core/metricfs: Create metricfs, standardized files under debugfs. |
| |
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
| |