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